On Thu, 2011-02-24, Branko Čibej wrote:
> On 24.02.2011 18:03, Julian Foad wrote:
> > On Wed, 2011-02-23, Daniel Shahaf wrote:
> >> julianfoad_at_apache.org wrote on Tue, Feb 22, 2011 at 15:38:35 -0000:
> >>> +/* Set (*BATON->contents) to a readable stream from which the pristine text
> >>> + * identified by BATON->sha1_checksum can be read from the pristine store of
> >>> + * SDB. If that text is not in the pristine store, return an error.
> >>> + *
> >>> + * Allocate the stream in BATON->result_pool.
> >>> + *
> >>> + * This function expects to be executed inside a SQLite txn.
> >>> + *
> >>> + * Implements 'notes/wc-ng/pristine-store' section A-3(d).
> >>> + * Implements svn_sqlite__transaction_callback_t. */
> >>> +static svn_error_t *
> >>> +pristine_read_txn(void *baton,
> >>> + svn_sqlite__db_t *sdb,
> >>> + apr_pool_t *scratch_pool)
> >>> +{
> >> Should document somewhere that the returned stream is only guaranteed to
> >> remain valid if the caller holds a wc lock, etc.?
> >>
> >> (svn_wc__db_pristine_read()'s docs don't mention locks)
> > For now, I've written (on both functions):
> >
> > The stream will remain readable as long as the pristine text remains in
> > the store; if the pristine text is removed from the store, any subsequent
> > read or other operation on the stream may return an error.
Hi Brane. Thanks for helping me out here. I'm *very* glad of your
attention.
A summary of this reply: I think I get you and agree your way is better.
But taking one piece at a time...
> Wait wait wait, I missed something there... You want to make *reads*
> from the (transactionally opened) stream to error out if someone deletes
> the pristine file? Woah. That implies a stat for every read, at least in
> Unix-like environments, and frankly I can't see any good reason to do that.
... you did miss something: I said it *may* return an error. That is:
this API does not promise that you can continue to read from the file
after removing it from the pristine store, but nor does it promise that
any such attempt shall fail (immediately or at all).
The idea was that the caller should ensure the pristine file won't be
deleted while it's reading it. (One way the caller can do so is to hold
a WC lock on a WC subtree that contains a reference to that pristine.
We could define other ways.)
Does that make more sense? I thought that was the kind of non-guarantee
(although not the specific non-guarantee) you were advocating when you
wrote before:
> * Impose constraints on what users of the file handle can expect,
> for example:
> o you are not allowed write access through the file (pristine
> file creation is a magical step)
> o you cannot assume that you will find the file that the
> handle refers to in any directory listing
> o you cannot assume that the file will continue to exist after
> the handle is closed
[...]
> It is not the business of the wc_db+pristine-store to track
> every process that happens to have an open handle to the pristine file.
> A deletion of the pristine file should succeed even if there are open
> handles referring to it.
So you're suggesting we should promise that a reader can continue
reading the file (at least once through to the end, not sure about
rewinding) even if something else deletes the file from the store part
way through. I think you're suggesting those semantics are more
reasonable than "you have to hold some sort of lock while you read it",
which is what my design boiled down to.
OK, I am happy to take that advice. In the design I have, the reader
must hold a WC lock to be sure that the pristine text won't be deleted
from the store. But a WC lock is a write lock. That sounds
undesirable, now I come to state it!
> That's a trivial task on Unix (i.e., "do
> nothing" ... modulo NFS most likely) and a trifle more involved on
> Windows (where the trifle lies in doing the right thing if we need to
> reinstate a file with the same checksum while there are open handles to it).
I guess I'll have to figure out how to implement this "trifle more
involved" part on Windows, now.
> All it takes to make this behaviour valid is to state in the docs that
> there's absolutely no guarantee that the pristine file that you've just
> opened will still represent anything in the working copy by the time you
> manage to read from it, or indeed that it will even continue to exist if
> you close it.
Those conditions are pretty much inevitable, but yes, better to state
them.
- Julian
Received on 2011-02-25 16:53:54 CET