[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1331883 - /subversion/trunk/subversion/svnadmin/main.c

From: Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com>
Date: Sun, 21 Oct 2012 19:39:41 +0200

On Thu, Oct 18, 2012 at 10:46 PM, Daniel Shahaf <d.s_at_daniel.shahaf.name>wrote:

> Stefan Fuhrmann wrote on Thu, Oct 18, 2012 at 02:26:13 +0200:
> > On Thu, Oct 18, 2012 at 1:57 AM, Daniel Shahaf <d.s_at_daniel.shahaf.name
> >wrote:
> >
> > > Branko ─îibej wrote on Mon, Oct 15, 2012 at 23:51:43 -0400:
> > > > On 15.10.2012 17:14, Stefan Fuhrmann wrote:
> > > > > However, if you have a long-running process like a server, that
> race
> > > > > condition extends now extends over its whole lifetime. I.e. once a
> > > > > revprop got read, any change to its value by a pre-1.8 tool may
> never
> > > > > get detected.
> > > >
> > > > Ouch. This seems wrong. I'd understand a design like this if the
> > > > long-running server were the only process accessing the repository,
> but
> > > > that has never been the case in Subversion. In this case, a cache
> that
> > > > can't detect out-of-band changes to the canonical dataset isn't very
> > > useful.
> > >
> > > Another questionable part of the revprop cache: the
> > > ATOMIC_REVPROP_TIMEOUT mechanism in fs_fs.c. (Basically, the code
> > > assumes that if another process is rewriting a revprops file, it will
> > > always finish within 10 seconds.)
> > >
> >
> > Did you read and *understand* the code?
> > What exactly will happen after the 10s timeout?
> The two processes will have a different idea of what revprop
> generation N is? One process will miss the other's changes?

The key is that the revprop generation is placed in
shared memory and all processes should always
see a consistent value. A process increasing that
value will automatically invalidate *everyones* caches.
So, increasing is always safe.

I admit I wasn't really sure about the concrete kind of badness that
> would happen if the assumption around ATOMIC_REVPROP_TIMEOUT ---
> I changed my mind at least once while drafting the email --- but I'd be
> surprised if no badness at all happens even if the assumption "the other
> process must have crashed" turns out to be wrong. (If that is the case,
> why does the comment mention the assumption at all?)

Thinking really hard, I finally was able to come up with
a very unlikely constellation that's being addressed by
r1400669. There is no problem when the writer gets
blocked (on a simple atomic file operation) and other
processes simply suspect that they might have missed
a revprop change and therefore bump the generation.

In that case, the write would still complete its operation
eventually and *further* bump the generation. The hole
in my logic had been what happens if the file op eventually
gets through but the writer is being terminated / killed /
crashes just before bumping the generation.

This should now be addressed by the other processes
acquiring the write lock before bumping the generation
after the timeout. At that point, we know that the original
writer will not modify the data afterwards (and potentially
terminate before telling us about it). Once we got the lock,
we may or may not need to bump the generation (the
writer or some other reader might have done that before us).

-- Stefan^2.

Join us this October at Subversion Live
 for two days of best practice SVN training, networking, live demos,
committer meet and greet, and more! Space is limited, so get signed up
Received on 2012-10-21 19:40:16 CEST

This is an archived mail posted to the Subversion Dev mailing list.