[PATCH 3 of 5] branchmap: acquires lock before writting the rev branch cache
Pierre-Yves David
pierre-yves.david at ens-lyon.org
Mon Aug 8 10:53:41 EDT 2016
On 08/08/2016 04:50 PM, Mads Kiilerich wrote:
> On Sun, Aug 7, 2016 at 1:21 PM, Yuya Nishihara <yuya at tcha.org
> <mailto:yuya at tcha.org>> wrote:
>
> On Sat, 06 Aug 2016 01:02:55 +0200, Pierre-Yves David wrote:
> > # HG changeset patch
> > # User Pierre-Yves David <pierre-yves.david at ens-lyon.org
> <mailto:pierre-yves.david at ens-lyon.org>>
> > # Date 1470401836 -7200
> > # Fri Aug 05 14:57:16 2016 +0200
> > # Node ID 84885aeec2e442d18dcb70fcce2d65dd9fafbf91
> > # Parent fa3f05a4219c4ea6b9bc9682f258e4418b36c265
> > # EXP-Topic vfsward
> > branchmap: acquires lock before writting the rev branch cache
> >
> > We now attempt to acquire a lock and write the branch cache within
> that lock.
> > This would prevent cache corruption when multiple processes try to
> write the cache
> > at the same time.
>
> (+CC Durham, Mads)
>
> I wonder if revbranchcache is designed to not take a lock.
> 9347c15d8136 says
> corruption can be recovered.
>
>
> Yes, I think it was given as an initial design constraint for acceptance
> upstream that the cache had to be updated on the fly by read only
> operations without locking. Hindsight perspective might have changed
> mine and others memory and opinion ;-)
>
> In my opinion, the revision branch cache should be treated exactly like
> the branch head cache and updated the same way. It might be expensive to
> update initially, but not doing it will end up being more expensive.
>
> Proper transaction support would however also require that the design
> constraint of doing in-place (non-atomic non-append) updates got lifted.
>
> The existing algorithm is not perfect lockfree, but I doubt it was
> possible to come up with a test case where simultaneous writes would
> give a corruption that not would be detected and recovered automatically.
>
> This change looks like a fine small improvement that will reduce the
> risk. But still, like most other write locks in Mercurial, it has the
> problem that data is read and checked before taking the lock and then
> "leaks" into the locked zone.
Yeah, it is not perfect.
> I wonder but haven't investigated: How will this handle the case where
> the repo can't be locked because it is on a read only filesystem? Will
> it get LockHeld (which would be a misleading exception name) and skip
> the writing?
It will raise LockUnavailable, yuya fixed it in flight
Cheers,
--
Pierre-Yves David
More information about the Mercurial-devel
mailing list