Re: verify_as_revision_before_current_plus_plus() on a production repo?

Julian Foad wrote on Mon, Mar 13, 2017 at 11:27:32 +0000:
> We seem to have consensus that enabling
>
> verify_as_revision_before_current_plus_plus()
>
> in production should be safe. It seems like it should be a useful extra
> protection against bug-induced repository corruption.

I think we're as confident about this code as we can be about code that
hasn't been used in production.

> For users who have experienced repository corruption and suspect it is
> bug-induced, or who are more concerned about this than about
> maximizing commit throughput, enabling such code would seem to be
> prudent.

+1

> should our policy be to provide administrative choice?

> Thoughts, please?

A knob sounds good to me: both because this is a new codepath (the FS
layer should be conservative) and because we don't know everyone's use
patterns.

The usual problem with knobs is that they mandate maintaining multiple
codepaths; this consideration does not apply to a knob that simply
enables/disables a sanity check.

If we enable this outside maintainer mode, we should look into the
points raised upthread about ensuring that 'verify' is read-only.
We don't have to fix everything, but we should at least add roadsign
comments in the code to reduce the chance that future changes to verify
will interact badly with the unusual callsite in verify_as_revision_before_current_plus_plus().

> - Julian

By the way, are you planning to enable verify_as_revision_before_current_plus_plus()
in alpha3? I would suggest enabling it unconditionally before alpha3,
then reverting it [or adding the knob under discussion] at some point
before branching 1.10.x.

Cheers,

Daniel
[We should probably give that function a shorter name... ;)]
Received on 2017-03-13 17:11:43 CET