You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@wandisco.com> on 2014/11/21 11:53:51 UTC
Re: svn commit: r1640832 - /subversion/trunk/subversion/libsvn_delta/text_delta.c
philip@apache.org writes:
> Author: philip
> Date: Thu Nov 20 22:36:27 2014
> New Revision: 1640832
>
> URL: http://svn.apache.org/r1640832
> Log:
> Followup to r1618841, the old fast_memcpy() returned target+len but
> the replacement memcpy() returns target.
>
> * subversion/libsvn_delta/text_delta.c
> (patterning_copy): Adjust target after memcpy.
This bug caused checksum failures when reading a BDB repository. As far
as I can tell it only affects the BDB code (the delta combiner used in
the BDB code is the only place that generates svn_txdelta_target) and
this code is not exercised by the test suite.
I don't understand the patterns that cause the repository to contain
svn_txdelta_target so I can't easily write a testcase. I've reduced the
original repository that demonstrates the problem to the 14 revisions of
repos-test.c leading up to r840335, but that still a 2.5MB repository:
$ svnadmin create repo --fs-type bdb
$ for r in `svn log -r839490:840335 ../src/subversion/tests/libsvn_repos/repos-test.c -q | awk '/^r/{print $1}'` ; do svn cat ../src/subversion/tests/libsvn_repos/repos-test.c@$r | svnmucc -mm -- put - file://`pwd`/repo/f ; done
$ subversion/svnadmin/svnadmin verify -rhead repo
* Error verifying revision 14.
svnadmin: E160004: Filesystem is corrupt
svnadmin: E200014: MD5 checksum mismatch on representation 'r':
expected: 5182e8876ed894dc7fe28f6ff5b2fee6
actual: 24281a451adca3181bfa59f932441a5e
svnadmin: E165011: Repository 'repo' failed to verify
--
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*
Re: svn commit: r1640832 - /subversion/trunk/subversion/libsvn_delta/text_delta.c
Posted by Branko Čibej <br...@wandisco.com>.
On 21.11.2014 13:57, Philip Martin wrote:
> Branko Čibej <br...@wandisco.com> writes:
>
>> The combiner only produces target copies if the source delta(s) contain
>> them. We shouldn't be generating any since we started using the xdelta
>> algorithm instead of vdelta; I don't really understand why BDB would be
>> using vdelta instead, not even for compatibility reasons.
> A compressed dump file is only 7K. Loading this into a BDB repo
> generates svn_txdelta_target ops during post-commit deltification:
[...]
Amazing. This is definitely something I'd like to understand a bit better.
> I might use this dump file to create a test.
That would be great. Thanks!
-- Brane
Re: svn commit: r1640832 - /subversion/trunk/subversion/libsvn_delta/text_delta.c
Posted by Philip Martin <ph...@wandisco.com>.
Branko Čibej <br...@wandisco.com> writes:
> The combiner only produces target copies if the source delta(s) contain
> them. We shouldn't be generating any since we started using the xdelta
> algorithm instead of vdelta; I don't really understand why BDB would be
> using vdelta instead, not even for compatibility reasons.
A compressed dump file is only 7K. Loading this into a BDB repo
generates svn_txdelta_target ops during post-commit deltification:
#0 svn_txdelta__insert_op (build_baton=0x7fffffffc340,
opcode=svn_txdelta_target, offset=2321, length=1, new_data=0x0,
pool=0x7ffff5558028) at ../src/subversion/libsvn_delta/text_delta.c:222
#1 0x00007ffff7f8ce6d in svn_txdelta_compose_windows (
window_A=0x7fffffffc490, window_B=0x7ffff5550278, pool=0x7ffff5558028)
at ../src/subversion/libsvn_delta/compose_delta.c:811
#2 0x00007ffff72d8111 in compose_handler (window=0x7fffffffc490,
baton=0x7fffffffd670)
...
#13 0x00007ffff72dac33 in svn_fs_base__rep_deltify (fs=0x7ffff7e2d050,
target=0x7ffff55918a8 "f", source=0x7ffff5591b18 "d",
trail=0x7ffff55970f8, pool=0x7ffff5591028)
at ../src/subversion/libsvn_fs_base/reps-strings.c:1494
#14 0x00007ffff72d1700 in maybe_deltify_mutable_rep (
target_rep_key=0x7ffff55918a8 "f", source_rep_key=0x7ffff5591b18 "d",
txn_id=0x7ffff5599140 "8", trail=0x7ffff55970f8, pool=0x7ffff5591028)
at ../src/subversion/libsvn_fs_base/dag.c:1490
...
#21 0x00007ffff72e352e in svn_fs_base__deltify (fs=0x7ffff7e2d050, revision=8,
pool=0x7ffff55ac028) at ../src/subversion/libsvn_fs_base/tree.c:2956
#22 0x00007ffff7fa6130 in svn_fs_deltify_revision (fs=0x7ffff7e2d050,
revision=8, pool=0x7ffff55ac028)
at ../src/subversion/libsvn_fs/fs-loader.c:1508
I might use this dump file to create a test.
Re: svn commit: r1640832 - /subversion/trunk/subversion/libsvn_delta/text_delta.c
Posted by Branko Čibej <br...@wandisco.com>.
On 21.11.2014 11:53, Philip Martin wrote:
> philip@apache.org writes:
>
>> Author: philip
>> Date: Thu Nov 20 22:36:27 2014
>> New Revision: 1640832
>>
>> URL: http://svn.apache.org/r1640832
>> Log:
>> Followup to r1618841, the old fast_memcpy() returned target+len but
>> the replacement memcpy() returns target.
>>
>> * subversion/libsvn_delta/text_delta.c
>> (patterning_copy): Adjust target after memcpy.
> This bug caused checksum failures when reading a BDB repository. As far
> as I can tell it only affects the BDB code (the delta combiner used in
> the BDB code is the only place that generates svn_txdelta_target) and
> this code is not exercised by the test suite.
>
> I don't understand the patterns that cause the repository to contain
> svn_txdelta_target so I can't easily write a testcase.
The combiner only produces target copies if the source delta(s) contain
them. We shouldn't be generating any since we started using the xdelta
algorithm instead of vdelta; I don't really understand why BDB would be
using vdelta instead, not even for compatibility reasons.
In any case, your fix is correct.
-- Brane
P.S.: The presence of target copies in deltas could, in certain
not-so-rare cases with overlapping target copies, make the delta
combiner grind to a halt. The combiner algorithm could suddenly have a
fourth-order dependency on the number of instructions, which would blow
the CPU for the actual delta application sky-high. That's why dberlin
replaced vdelta with xdelta, way back when.