> -----Original Message-----
> From: Philip Martin [mailto:philip.mar...@wandisco.com]
> Sent: maandag 23 november 2009 4:07
> To: Bert Huijben
> Cc: 'Hyrum K. Wright'; dev@subversion.apache.org
> Subject: Re: svn commit: r882232 - in /subversion/trunk/subversion:
> libsvn_client/revert.c tests/cmdline/schedule_tests.py
>
> "Bert Huijben" <b...@qqmail.nl> writes:
>
> > * The atomic operations. WC-NG operations that can operate without
> outside
> > knowledge learned before the operation.
> >
> > These functions that are just one sqlite transaction by itself, just
> need to
> > make sure nobody else has a write lock. Having a write lock is not
> required
> > for operations like just changing the actual properties on a node. Of
> course
> > nobody else can own a write lock, or it might change the properties
> after
> > sending the commit data, but before moving the data to the base_node
> table.
>
> It's not just commits. WC-NG is supposed to be backwards compatible
> with 1.6 which means that once a process takes a 1.6 write lock no
> other process can modify the wc; it makes no difference whether that
> modification is atomic or not.
>
> You also seem to be implying that some operations don't need to create
> write locks, that atomic operations merely need to ensure no write
> locks exist. At what point does this check occur?
>
> - process A: start atomic transaction
> - process B: take a 1.6 write lock
> - process A: continue atomic transaction
> - process B: use 1.6 write lock
> - process A: continue atomic transaction
> - process B: remove 1.6 write lock
> - process A: complete/commit/finalise atomic transaction
An atomic transaction which checks the existance of a write lock inside a
sqlite transaction can't be interrupted by another process taking the write
lock.. As that would require breaking the sqlite transaction.
> Does the atomic transaction in one process block the 1.6 write lock
> creation in another process? That's just like having the transaction
> take a write lock.
Just checking it inside the transaction is enough. Values that are read
can't change before committing.
Writing it and removing it in the same transaction doesn't add anything.
> Does the atomic transaction check all the way through for a write
> lock? That doesn't even seem possible.
When everything is in a single DB we only have to store the roots of locks
in the database.. So checking if a lock exists for a specific path is
walking up all parents, which is pretty cheap in my eyes. (Not walking all
descendants like pre 1.7)
> Does the transaction do some sort of write lock counting to make sure
> that none have been created during the transaction? I don't see
> anything like that in the code.
See the Sqlite locking documentation
> > Once we have a central DB it will be really easy to perform this
> nobody else
> > has a write lock check. (Checking if we ourselves have a write lock
> ourself
> > is just a memory lookup of course).
>
> So the plan is that locking remains broken until centralised metadata?
Ask Hyrum (who removed the write locks) or Greg (who designed the new meta
data system)...
I objected against removing the 1.6 style write locks until this new system
is in place and well tested. Previous discussion came to the conclusion that
it was OK for trunk to leave out some guarantees for the transition period.
Personally I think we have bigger problems now than this specific locking
issue that can be resolved by introducing these checks in wc_db.c.
I agree that we should be able to take some shortcuts in the transition
period towards WC-NG, because keeping all guarantees up (around loggy as
poor mans transaction, etc.) takes an insane amount of extra work that can
be /dev/null-ed a few weeks later, when the working queue items and pristine
handling replace most of the loggy stuff.
But we have to be in a guaranteed stable/lock safe state at release time.
(And personally I would have preferred to keep the old locks until we are
sure we can remove them in the final state...)
Bert
>
> > * Partial operations
> >
> > These operations rely on data read before the wc_db operation and
> only work
> > correctly if the data didn't change since reading. All entry based
> > operations are in this category and the WC-NG work tries to redesign
> several
> > of these operation to the first class of operations.
> >
> > These operations really need a write lock and we need far more checks
> here.
>
> --
> Philip
