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
Received on 2018-03-24 20:47:45 CET