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... ;)]
