[PATCH] transaction: move changelog finalizer to be before bookmark finalizer

Pierre-Yves David pierre-yves.david at ens-lyon.org
Sat Mar 19 15:30:59 EDT 2016

On 03/19/2016 10:04 AM, Durham Goode wrote:
> On 3/18/16 6:07 PM, Pierre-Yves David wrote:
>> I would like to have a clear idea of where we are walking there and
>> what tradeoff we are performing
> K, given that file generators are "bookmarks, dirstate, and phases" and
> finalizers are "revlogs, revbranchcache, fncache", the old race
> conditions are:
> - bookmarks written before revlogs - causes bookmarks to appear missing
> during read
> - dirstate written before revlogs - causes missing working copy parent

I'm not too worried about this one (well, it is annoying and I'm 
suspecting my thg is getting hit by that from time to time). However, 
server usually does not have a working copy and clients have no way to 
access it anyway.

> - phases written before revlogs - should be benign, since any phase on a
> commit that is missing only affects commits that are also missing

We could be seeing changeset as draft while they should be public. This 
can have some annoying consequence but the next pull should fix it 
silently, so it is not too bad.

(This impact people who have hooks to enforce constraint on there 
repository state, the race could breaks this guarantee)

> - bookmarks/dirstate/phases written before revbranch cache - no issues
> - bookmarks/dirstate/phases written before fncache - no issues
> and the new race conditions are:
> - revlogs written before bookmarks - may see commits where the bookmark
> hasn't moved on top of them yet; odd but not an invalid state

Less bad than the other one but still annoying:

   (This impact people who have hooks to enforce constraint on there 
repository state, the race could breaks this guarantee)

> - revlogs written before dirstate - no issues
> - revlogs written before phases - some commits may show up with the
> wrong phase (potentially public)

Yep, this pretty bad, having changeset being published "by chance" is 
pretty bad as this is not really recoverable. Especially there is people 
using non-publishing repository in production with strict permission for 
publishing (including, to some extend the mercurial project). We cannot 
regress this.

> - revbranch cache written before bookmarks/dirstate/phases - no issues
> - fncache written before bookmarks/dirstate/phases - no issues
> Race conditions that existed before (within the two categories):
> - fncache before revlog - fncache could point to file that doesn't exist
> - revlog before fncache - fncache could be missing a file
> So in summary, my change removes the invalid bookmarks and invalid
> dirstate case, and replaces it with an out-of-date phase case.  The out
> of date phase case could potentially be bad, since if we send public
> commits to the client, it may be hard to make them draft later. On the
> flip side, most central-server setups are publishing servers, so this
> wouldn't actually be an issue for them. This could be fixed by moving
> the phase writes to be before the revlogs

Can you poke at what it would look like to have phases written before 
and bookmark written after as mitigation?

> (or by redoing the entire
> storage system to be more widely transactional).

Pierre-Yves David

More information about the Mercurial-devel mailing list