On Fri, Nov 21, 2008 at 11:48 AM, Hyrum K. Wright
<hyrum_wright_at_mail.utexas.edu> wrote:
> Per Dave Glasser's concerns as noted in TODO-1.6, and following the example of
> BDB in r33975, I've added the ability to store both SHA1 (for rep-sharing) and
> MD5 (for backwards compat and integrity checking) to FSFS. The SHA1 stuff
> should be conditional on the FS format number, but I plan on handling that in a
> future commit.
>
> The following patch passes all tests, except for svnadmin_tests 12. This test
> is a test for failure, and with the patch it still fails, just in a different
> way. I'm tempted to tweak the expected error output of the test, but want to
> make sure that I'm not hiding more sinister problems with the patch first.
>
> There's still a few rough edges to be fixed in followup commits, but could a
> FSFS-minded person give this a look over to make sure things are right?
>
> Thanks,
> -Hyrum
>
[snip]
> return apr_psprintf(pool, "%ld %" APR_OFF_T_FMT " %" SVN_FILESIZE_T_FMT
> - " %" SVN_FILESIZE_T_FMT " %s %" APR_INT64_T_FMT,
> + " %" SVN_FILESIZE_T_FMT " %s %s %" APR_INT64_T_FMT,
> rep->revision, rep->offset, rep->size,
> rep->expanded_size,
> - svn_checksum_to_cstring_display(rep->checksum,
> - pool),
> + rep->md5_checksum ?
> + svn_checksum_to_cstring_display(rep->md5_checksum,
> + pool) :
> + "00000000000000000000000000000000",
> + rep->sha1_checksum ?
> + svn_checksum_to_cstring_display(rep->sha1_checksum,
> + pool) :
> + "0000000000000000000000000000000000000000",
> rep->reuse_count);
> }
Most everything looks okay to me. I do have a couple of questions
though. Will the sqlite rep cache be okay with 0s for a value? What
I mean is, we're probably assuming that the SHA1 is unique for a given
rep, and this edge case breaks that, right? The MD5 was never a
problem because it was only used ever as a checksum. Whereas the SHA1
is likely a key (I haven't actually looked at that code). Also, how
does this affect repairing revisions? With the MD5 you could make
some errors go away by zeroing the MD5 field (not recommended, but you
could). I assume we'd have to edit the rep cache in some way too?
Either way, it nice work Hyrum!
-John
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-11-22 11:31:48 CET