Daniel Shahaf wrote:
Julian Foad wrote on Fri, Mar 23, 2018 at 17:30:14 +0000:
Nathan Hartman wrote:
It just occurred to me that because correct operation of the rep-
cache is so crucial to the reliability of the system, perhaps it
would be a good idea to make sure the rep-cache does not contain too-
new revs as a sanity check during normal commits?
Discussed before: thread "FSFS rep-cache validation", 2014-01-22,
https://svn.haxx.se/dev/archive-2014-01/0086.shtml
https://lists.apache.org/thread.html/832c0367adfee2208e609dc54048447651ab66cb12876ddbaa0c8fbd@1390425594@%3Cdev.subversion.apache.org%3E
No conclusion that time. Seems like a good idea. Want to make it happen? :-)
We already check rep-cache references before using them:
[...]
subversion/libsvn_fs_fs/rep-cache.c:314: (apr_err=SVN_ERR_FS_CORRUPT)
svnmucc: E160004: Checksum '005ed0b0c9e3ea5d84be64386aadc786164013d8' in
rep-cache is beyond HEAD
Detecting when a cache hit is beyond HEAD is not the same, and not
sufficient.
While we're at it, do we check, before using a rep-cache reference, that the
revision:offset coordinates we obtained do in fact refer to a representation?
I.e., do we seek() to that position and ensure that we see, at the very least,
either "DELTA" or "PLAIN"?
That's not sufficient either, as you admit ("at the very least"), so why
choose that particular level of probability-increasing rather than
definite methods such as those mentioned in the discussion linked above?
- Julian
Perhaps the call to svn_fs_fs__check_rep_reference()
(in transaction.c:2328) does that, but I don't fully understand that function.
(For starters, why does it take a different codepath for logical addressing
than for physical addressing?)