On Mon, May 7, 2012 at 5:22 PM, C. Michael Pilato <cmpilato_at_collab.net> wrote:
> On 05/04/2012 05:16 PM, Greg Stein wrote:
>> On Fri, May 4, 2012 at 9:13 AM, <cmpilato_at_apache.org> wrote:
>>> Author: cmpilato
>>> Date: Fri May 4 13:13:00 2012
>>> New Revision: 1333936
>
> Okay. I think I've digested and acted open all of the feedback I've cropped
> out of this response. There's just this one task remaining now:
Nice. Thanks!
>...
>> As a recommendation, I would suggest creating a "lazy open" stream
>> which takes a callback that opens the stream upon first read. We are
>> going to need this lazy-open stream during commits (the RA layer may
>> choose not to deliver contents to the server, so we shouldn't bother
>> opening them).
>
> To be clear, you mean that we need something like:
>
> {{{
> typedef svn_error_t (*svn_stream_lazyopen_func_t)(svn_stream_t **stream,
> void *baton,
> apr_pool_t *result_pool,
> apr_pool_t *scratch_pool);
> svn_error_t *
> svn_stream_lazyopen_create(svn_stream_t **stream,
> svn_stream_lazyopen_func_t open_func,
> void *open_baton,
> apr_pool_t *result_pool,
> apr_pool_t *scratch_pool);
> }}}
Bingo.
Tho you can likely lose the scratch_pool on _create(), and likely just
return the bare stream. I can't foresee any complexity inside that
function.
> In my specific situation, svn_wc__get_pristine_contents_by_checksum() would
> call svn_stream_lazyopen_create(), passing an open_func callback which is
> written to invoke svn_wc__db_pristine_read().
That was my thinking, yes.
> One downside of this is that svn_wc__db_pristine_read() promises that "even
> if the pristine text is removed from the store while it is being read, the
> stream will remain valid and readable until it is closed." That promise
> becomes more valuable the earlier you open the stream to the pristine
> contents; in this delayed open scenario, I anticipate that a "move"
> operation from the server could see reduced benefit, even if the "add" side
> of the move is processed before the "delete"). While parsing the REPORT,
> the RA callback might say, "Yep, I've got those contents!" But by the time
> the serf connection pipelining magic runs to actually read the contents,
> prior delete handling might have blown them away.
I believe that we take out an administrative lock on the working copy
during the update. Pristines will not be cleaned when those are
present (or when workqueue items are present).
In the total edge case of paranoia, the lazyopen could grab its own
administrative lock.
I'd just verify the admin lock during update, which means that the
callback you provide to the update process can work with that
assumption.
(and the ugly fact that we don't automagically clean them out anyways;
but we may figure that out one day, so can't be relied upon)
> Oh well, *if* that's even really what would happen, I suppose it still
> doesn't outweight the current badness of having Mark running out of file
> handles. :-P
Sure thing, boss.
Cheers,
-g
Received on 2012-05-07 23:53:54 CEST