Reusable buckets

1 view
Skip to first unread message

Lieven Govaerts

unread,
Jun 28, 2008, 11:33:48 AM6/28/08
to serf...@googlegroups.com
We're seeing an issue in Subversion using serf when buckets are needed
a second time after they have been read completely. Most typically
this happens with the body of a request that has failed authentication
so the request needs to be resent with updated authorization headers.
Ra_serf has a callback mechanism to create these buckets on the fly,
but that's not always feasible, eg. some request bodies are the result
of crawling the working copy, something we don't want to do twice. See
also the issue report:
http://subversion.tigris.org/issues/show_bug.cgi?id=3212

My proposal is to make it possible to restore buckets to an earlier
state, if needed. This means we need two functions:

/* Set aside the current state of the bucket for later. */
apr_status_t serf_XXX_bucket_snapshot(serf_bucket_t *bucket);

and

/* Reset the bucket to the state at the last snapshot */
apr_status_t serf_XXX_restore_snapshot(serf_bucket_t *bucket);

We'll probably also want a serf_XXX_clear_snapshot, but that we
(Subversion) don't need right now.

Then we would use the snapshot function when the body of the request
is ready but not yet sent, and then later when we need to resend the
request we can restore the request's original body.

Attached is a WIP patch that implements serf_XXX_restore_snapshot for
simple and aggregate buckets. It's not complete, but it shows the
idea. This feature is easy to add to simple buckets, but for eg.
socket buckets might require copying data into memory or to diskcache.
For aggregate buckets there's a disadvantage of keeping the data
longer in memory, where it's freed now as soon as it's read, but that
drawback is only for those we choose to use snapshots.

APR folks: is this what the 'setaside' function is for in APR buckets?

Other remarks and Suggestions are welcome.

Lieven

serf-snapshots.patch

Justin Erenkrantz

unread,
Jun 28, 2008, 4:37:48 PM6/28/08
to serf...@googlegroups.com
On Sat, Jun 28, 2008 at 8:33 AM, Lieven Govaerts
<lieven....@gmail.com> wrote:
> We're seeing an issue in Subversion using serf when buckets are needed
> a second time after they have been read completely. Most typically
> this happens with the body of a request that has failed authentication
> so the request needs to be resent with updated authorization headers.
> Ra_serf has a callback mechanism to create these buckets on the fly,
> but that's not always feasible, eg. some request bodies are the result
> of crawling the working copy, something we don't want to do twice. See
> also the issue report:
> http://subversion.tigris.org/issues/show_bug.cgi?id=3212
>
> My proposal is to make it possible to restore buckets to an earlier
> state, if needed. This means we need two functions:
>
> /* Set aside the current state of the bucket for later. */
> apr_status_t serf_XXX_bucket_snapshot(serf_bucket_t *bucket);
>
> and
>
> /* Reset the bucket to the state at the last snapshot */
> apr_status_t serf_XXX_restore_snapshot(serf_bucket_t *bucket);
>
> We'll probably also want a serf_XXX_clear_snapshot, but that we
> (Subversion) don't need right now.

I'm not quite clear: who would call _snapshot and _restore? Serf
itself, or Subversion/ra_serf?

> Then we would use the snapshot function when the body of the request
> is ready but not yet sent, and then later when we need to resend the
> request we can restore the request's original body.
>
> Attached is a WIP patch that implements serf_XXX_restore_snapshot for
> simple and aggregate buckets. It's not complete, but it shows the
> idea. This feature is easy to add to simple buckets, but for eg.
> socket buckets might require copying data into memory or to diskcache.
> For aggregate buckets there's a disadvantage of keeping the data
> longer in memory, where it's freed now as soon as it's read, but that
> drawback is only for those we choose to use snapshots.
>
> APR folks: is this what the 'setaside' function is for in APR buckets?

Setaside in APR buckets is to move the data from one pool to a
longer-lived pool. An example would be taking a bucket that was
created in the request pool and moving it to the connection pool.

Thanks. -- justin

Lieven Govaerts

unread,
Jun 29, 2008, 3:54:59 PM6/29/08
to serf...@googlegroups.com
On Sat, Jun 28, 2008 at 10:37 PM, Justin Erenkrantz
<jus...@erenkrantz.com> wrote:
>
> On Sat, Jun 28, 2008 at 8:33 AM, Lieven Govaerts
> <lieven....@gmail.com> wrote:
>> We're seeing an issue in Subversion using serf when buckets are needed
>> a second time after they have been read completely. Most typically
>> this happens with the body of a request that has failed authentication
>> so the request needs to be resent with updated authorization headers.
>> Ra_serf has a callback mechanism to create these buckets on the fly,
>> but that's not always feasible, eg. some request bodies are the result
>> of crawling the working copy, something we don't want to do twice. See
>> also the issue report:
>> http://subversion.tigris.org/issues/show_bug.cgi?id=3212
>>
>> My proposal is to make it possible to restore buckets to an earlier
>> state, if needed. This means we need two functions:
>>
>> /* Set aside the current state of the bucket for later. */
>> apr_status_t serf_XXX_bucket_snapshot(serf_bucket_t *bucket);
>>
>> and
>>
>> /* Reset the bucket to the state at the last snapshot */
>> apr_status_t serf_XXX_restore_snapshot(serf_bucket_t *bucket);
>>
>> We'll probably also want a serf_XXX_clear_snapshot, but that we
>> (Subversion) don't need right now.
>
> I'm not quite clear: who would call _snapshot and _restore? Serf
> itself, or Subversion/ra_serf?

Oh, that would be in ra_serf. Serf is not actively doing anything
here, it just provides the framework.

Attached is a test that demonstrates the whole 'reuse request body
bucket' scenario, which hopefully shows what I have in mind.
(ignore the printf statements and the commented-out tests, I didn't
plan to commit this in fact)

>
>> Then we would use the snapshot function when the body of the request
>> is ready but not yet sent, and then later when we need to resend the
>> request we can restore the request's original body.
>>
>> Attached is a WIP patch that implements serf_XXX_restore_snapshot for
>> simple and aggregate buckets. It's not complete, but it shows the
>> idea. This feature is easy to add to simple buckets, but for eg.
>> socket buckets might require copying data into memory or to diskcache.
>> For aggregate buckets there's a disadvantage of keeping the data
>> longer in memory, where it's freed now as soon as it's read, but that
>> drawback is only for those we choose to use snapshots.
>>
>> APR folks: is this what the 'setaside' function is for in APR buckets?
>
> Setaside in APR buckets is to move the data from one pool to a
> longer-lived pool. An example would be taking a bucket that was
> created in the request pool and moving it to the connection pool.
>

Oh ok, not the same thing then.

Lieven

Lieven Govaerts

unread,
Jun 29, 2008, 4:46:38 PM6/29/08
to serf...@googlegroups.com

Er, that was a mistake. The patch I forgot to attach is not really
demonstrating the scenario, as that is based on a how Subversion
implements the setup_request callback. If that's needed to better
understand the need for this feature, I'll make a patch for Subversion
itself.

Lieven

Reply all
Reply to author
Forward
0 new messages