subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Julian Foad <julianf...@apache.org>
Subject Re: verify_as_revision_before_current_plus_plus() on a production repo?
Date Tue, 31 Jan 2017 12:00:48 GMT
Daniel Shahaf wrote:
> I agree with your last sentence, but I think you misunderstood me.
> I was simply arguing that the "root->rev is younger than value in
> 'current' on disk" is a situation that can happen in normal API usage,
> and therefore calling svn_fs_verify_root() in that situation is *not*
> undefined behaviour.  That is: the call may either succeed or error out,
> but may not assert or segfault (or grow the proverbial toaster's arm).

OK. To run this in production, it also must not error out in a normal 
commit. I haven't fully traced the verify code to see whether it might 
call svn_fs_fs__read_current().

> Julian Foad wrote:
>> Interesting observation. It would be good the verify could use a read-only
>> FS instance, but I don't think we have such a mechanism.
>
> Not today, but it would be easy enough to add to fs_fs_data_t a boolean
> that tells with_some_lock() to just error out, and to have rep-cache.c
> respect that boolean as well.  (See also the next bullet)

Maybe we should propose this as an enhancement.

>> I meant the opposite direction: a successful commit does some things
>> post-commit (e.g. remove the txn directory, update the fulltext cache,
>> update the rep cache) and this verify must not assume any of those things
>> have been done already.
>
> Taking these one by one:
[...]
> So that's these three.  Is there anything else that's updated between
> bumping 'current' and returning control to svn_fs_commit_txn()'s caller?

No.

> Or some kind of process-global or machine-global cache shared between
> the two FS handles, despite the different cache namespaces?

Not sure.

Nevertheless everything we discussed here gives me some confidence. Thanks.

- Julian


Mime
View raw message