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: [[[ % svnmucc put -mm r/README.txt file://$PWD/r/$RANDOM r1 committed by daniel at 2018-03-24T19:45:29.328228Z % sqlite3 r/db/rep-cache.db 'update rep_cache set revision=2' % svnmucc put -mm r/README.txt file://$PWD/r/$RANDOM /home/daniel/src/svn/t1/./subversion/svnmucc/svnmucc.c:235, /home/daniel/src/svn/t1/./subversion/libsvn_client/mtcc.c:1485, /home/daniel/src/svn/t1/./subversion/libsvn_client/mtcc.c:1256, /home/daniel/src/svn/t1/./subversion/libsvn_client/mtcc.c:1162, /home/daniel/src/svn/t1/./subversion/libsvn_delta/text_delta.c:916, /home/daniel/src/svn/t1/./subversion/libsvn_fs_fs/tree.c:2985, /home/daniel/src/svn/t1/./subversion/libsvn_subr/stream.c:279, /home/daniel/src/svn/t1/./subversion/libsvn_fs_fs/transaction.c:2545, /home/daniel/src/svn/t1/./subversion/libsvn_fs_fs/transaction.c:2341, /home/daniel/src/svn/t1/./subversion/libsvn_fs_fs/rep-cache.c:314: (apr_err=SVN_ERR_FS_CORRUPT) svnmucc: E160004: Checksum '005ed0b0c9e3ea5d84be64386aadc786164013d8' in rep-cache is beyond HEAD /home/daniel/src/svn/t1/./subversion/libsvn_fs_fs/fs_fs.c:1505: (apr_err=SVN_ERR_FS_NO_SUCH_REVISION) svnmucc: E160006: No such revision 2 zsh: exit 1 svnmucc put -mm r/README.txt file://$PWD/r/$RANDOM ]]] 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"? 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?) Cheers, Daniel