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
Received on 2017-01-31 13:01:00 CET