You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Fuhrmann <st...@wandisco.com> on 2012/07/25 11:03:24 UTC
Re: [Issue 4129] predecessors links on root node-revision skip revisions
On Wed, Jul 25, 2012 at 9:53 AM, Daniel Shahaf <da...@elego.de> wrote:
> Should we mark the issue as FIXED ?
>
Could you please fix the UI experience first?
Currently, svnadmin verify will first verify the
root nodes of all revisions of the whole repository
and *then* start verifying all revisions showing
some progress info.
Two issues with that:
(1) The first part takes about as long as the second,
i.e. minutes or hours w/o visible feedback.
(2) Doing that verification as part of the second
stage would be virtually for free as the node caches
are hot.
-- Stefan^2.
> philip@tigris.org wrote on Tue, Jul 24, 2012 at 16:37:56 -0700:
> > http://subversion.tigris.org/issues/show_bug.cgi?id=4129
> >
> >
> >
> >
> >
> >
> > ------- Additional comments from philip@tigris.org Tue Jul 24 16:37:55
> -0700 2012 -------
> > r1302613 was backported in 1.7.5 and 1.6.18.
>
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: [Issue 4129] predecessors links on root node-revision skip
revisions
Posted by Daniel Shahaf <da...@elego.de>.
Stefan Fuhrmann wrote on Tue, Jul 31, 2012 at 10:56:18 +0200:
> On Tue, Jul 31, 2012 at 10:45 AM, Daniel Shahaf <da...@elego.de> wrote:
>
> > Stefan Fuhrmann wrote on Tue, Jul 31, 2012 at 10:35:24 +0200:
> > > On Thu, Jul 26, 2012 at 11:41 AM, Philip Martin
> > > <ph...@wandisco.com>wrote:
> > >
> > > > Stefan Fuhrmann <st...@wandisco.com> writes:
> > > >
> > > > > Yesterday, I debugged the code and found out why r(N-2)
> > > > > would be reported. This was due to is-fresh-txn-root
> > > > > being set on some of the root noderevs. Some of the
> > > > > affected repositories don't use directory deltification.
> > > > > Maybe, I'm able to look deeper into how that might
> > > > > have happened.
> > > > >
> > > > > I think we found another form of corruption. Since the
> > > > > fix is simply to ignore the flag, the question is whether
> > > > > we may ignore it during (de-)serializing noderevs.
> > > >
> > > > One way to set is-fresh-txn-root is to commit an empty rev:
> > > >
> > > > http://subversion.tigris.org/issues/show_bug.cgi?id=4031
> > > >
> > >
> > > That fully explains why I see those flags in my repos.
> > > Now, svnadmin verify will report them as corrupted.
> >
> > Could svnamdin report them via warnings rather than via fatal errors?
> > They are harmless after all so I think they shouldn't mask "real"
> > corruptions.
> >
>
> Feel free to relax your check to support existing repositories ;)
> But I think we should also fix the root cause in trunk for 1.8.
It's not my check...
Re: [Issue 4129] predecessors links on root node-revision skip revisions
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Tue, Jul 31, 2012 at 10:45 AM, Daniel Shahaf <da...@elego.de> wrote:
> Stefan Fuhrmann wrote on Tue, Jul 31, 2012 at 10:35:24 +0200:
> > On Thu, Jul 26, 2012 at 11:41 AM, Philip Martin
> > <ph...@wandisco.com>wrote:
> >
> > > Stefan Fuhrmann <st...@wandisco.com> writes:
> > >
> > > > Yesterday, I debugged the code and found out why r(N-2)
> > > > would be reported. This was due to is-fresh-txn-root
> > > > being set on some of the root noderevs. Some of the
> > > > affected repositories don't use directory deltification.
> > > > Maybe, I'm able to look deeper into how that might
> > > > have happened.
> > > >
> > > > I think we found another form of corruption. Since the
> > > > fix is simply to ignore the flag, the question is whether
> > > > we may ignore it during (de-)serializing noderevs.
> > >
> > > One way to set is-fresh-txn-root is to commit an empty rev:
> > >
> > > http://subversion.tigris.org/issues/show_bug.cgi?id=4031
> > >
> >
> > That fully explains why I see those flags in my repos.
> > Now, svnadmin verify will report them as corrupted.
>
> Could svnamdin report them via warnings rather than via fatal errors?
> They are harmless after all so I think they shouldn't mask "real"
> corruptions.
>
Feel free to relax your check to support existing repositories ;)
But I think we should also fix the root cause in trunk for 1.8.
-- Stefan^2.
--
*Join us this October for Subversion Live
2012<http://www.wandisco.com/svn-live-2012>– 2 full days of training,
networking, live demos and more! 25%
off before Aug. 10th with discount code “earlybird.”
*Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: [Issue 4129] predecessors links on root node-revision skip
revisions
Posted by Daniel Shahaf <da...@elego.de>.
Stefan Fuhrmann wrote on Tue, Jul 31, 2012 at 10:35:24 +0200:
> On Thu, Jul 26, 2012 at 11:41 AM, Philip Martin
> <ph...@wandisco.com>wrote:
>
> > Stefan Fuhrmann <st...@wandisco.com> writes:
> >
> > > Yesterday, I debugged the code and found out why r(N-2)
> > > would be reported. This was due to is-fresh-txn-root
> > > being set on some of the root noderevs. Some of the
> > > affected repositories don't use directory deltification.
> > > Maybe, I'm able to look deeper into how that might
> > > have happened.
> > >
> > > I think we found another form of corruption. Since the
> > > fix is simply to ignore the flag, the question is whether
> > > we may ignore it during (de-)serializing noderevs.
> >
> > One way to set is-fresh-txn-root is to commit an empty rev:
> >
> > http://subversion.tigris.org/issues/show_bug.cgi?id=4031
> >
>
> That fully explains why I see those flags in my repos.
> Now, svnadmin verify will report them as corrupted.
Could svnamdin report them via warnings rather than via fatal errors?
They are harmless after all so I think they shouldn't mask "real"
corruptions.
Re: [Issue 4129] predecessors links on root node-revision skip revisions
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Thu, Jul 26, 2012 at 11:41 AM, Philip Martin
<ph...@wandisco.com>wrote:
> Stefan Fuhrmann <st...@wandisco.com> writes:
>
> > Yesterday, I debugged the code and found out why r(N-2)
> > would be reported. This was due to is-fresh-txn-root
> > being set on some of the root noderevs. Some of the
> > affected repositories don't use directory deltification.
> > Maybe, I'm able to look deeper into how that might
> > have happened.
> >
> > I think we found another form of corruption. Since the
> > fix is simply to ignore the flag, the question is whether
> > we may ignore it during (de-)serializing noderevs.
>
> One way to set is-fresh-txn-root is to commit an empty rev:
>
> http://subversion.tigris.org/issues/show_bug.cgi?id=4031
>
That fully explains why I see those flags in my repos.
Now, svnadmin verify will report them as corrupted.
-- Stefan^2.
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: [Issue 4129] predecessors links on root node-revision skip revisions
Posted by Philip Martin <ph...@wandisco.com>.
Stefan Fuhrmann <st...@wandisco.com> writes:
> Yesterday, I debugged the code and found out why r(N-2)
> would be reported. This was due to is-fresh-txn-root
> being set on some of the root noderevs. Some of the
> affected repositories don't use directory deltification.
> Maybe, I'm able to look deeper into how that might
> have happened.
>
> I think we found another form of corruption. Since the
> fix is simply to ignore the flag, the question is whether
> we may ignore it during (de-)serializing noderevs.
One way to set is-fresh-txn-root is to commit an empty rev:
http://subversion.tigris.org/issues/show_bug.cgi?id=4031
--
Philip
Re: [Issue 4129] predecessors links on root node-revision skip revisions
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Wed, Jul 25, 2012 at 2:09 PM, Daniel Shahaf <da...@elego.de> wrote:
> Philip Martin wrote on Wed, Jul 25, 2012 at 12:06:48 +0100:
> > Daniel Shahaf <da...@elego.de> writes:
> >
> > > Philip Martin wrote on Wed, Jul 25, 2012 at 11:09:54 +0100:
> > >> Daniel Shahaf <da...@elego.de> writes:
> > >>
> > >> > Philip Martin wrote on Wed, Jul 25, 2012 at 10:49:36 +0100:
> > >> >>
> > >> >> I attached to the issue a repository that demonstrates the
> corruption;
> > >> >> verify doesn't report a problem on that repository. What does
> verify
> > >> >> check?
> > >> >
> > >> > validate_root_noderev() catches an instance of the #4129 corruption
> and
> > >> > in its docstring xrefs svn_fs_fs__verify(), so I think the checks
> in the
> > >> > former are done in the latter too.
> > >>
> > >> validate_root_noderev is only called by write_final_rev and not during
> > >> verify. svn_fs_fs__verify calls svn_fs_fs__verify_root and that does:
> > >>
> > >> /* Issue #4129: bogus predecessors. */
> > >> /* Check 1: predecessor must be an earlier revision.
> > >> */
> > >> if (pred_rev >= root->rev)
> > >> return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
> > >> "r%ld's root node's predecessor is
> r%ld"
> > >> " but must be earlier revision",
> > >> root->rev, pred_rev);
> > >
> > > The check was included in svn_fs_fs__verify_root() in r1349313:
> > >
> > > /* Verify explicitly the predecessor of the root. */
> > > {
> > > ...
> > > /* Check the predecessor's revision. */
> > > if (pred_id)
> > > {
> > > SVN_ERR(svn_fs_fs__dag_get_node(&pred, root->fs, pred_id,
> pool));
> > > SVN_ERR(svn_fs_fs__dag_get_revision(&pred_rev, pred, pool));
> > > if (pred_rev+1 != root->rev)
> > > /* Issue #4129. */
> > > return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
> > > "r%ld's root node's predecessor is
> r%ld",
> > > root->rev, pred_rev);
> > > }
> > > {
> >
> > That check was relaxed by:
> >
> > r1358322 | stefan2 | 2012-07-06 19:04:09 +0100 (Fri, 06 Jul 2012) | 5
> lines
> >
> > Relax overly strict consistency check that is not covered by the FSFS
> > format specification.
> >
> > * subversion/libsvn_fs_fs/tree.c
> > (svn_fs_fs__verify_root): fix test and add commentary
> >
>
> Thanks for digging it. I think that change is wrong --- the predecessor
> of the root noderev of rN MUST be the root noderev of r(N-1) --- that's
> how Subversion filesystems behave (not just FSFS).
>
> Stefan2, WDYT?
>
This is why I did that change: I knew that the usual
type of #4129 corruption is a ridiculous large rev number.
Now, I saw a delta of 2 instead of 1 on some of my repos.
My reaction: "Of course! That is must be a side-effect
of directory deltification."
Yesterday, I debugged the code and found out why r(N-2)
would be reported. This was due to is-fresh-txn-root
being set on some of the root noderevs. Some of the
affected repositories don't use directory deltification.
Maybe, I'm able to look deeper into how that might
have happened.
I think we found another form of corruption. Since the
fix is simply to ignore the flag, the question is whether
we may ignore it during (de-)serializing noderevs.
rI365918 reverts the relaxation.
-- Stefan^2.
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: [Issue 4129] predecessors links on root node-revision skip
revisions
Posted by Daniel Shahaf <da...@elego.de>.
Philip Martin wrote on Wed, Jul 25, 2012 at 12:06:48 +0100:
> Daniel Shahaf <da...@elego.de> writes:
>
> > Philip Martin wrote on Wed, Jul 25, 2012 at 11:09:54 +0100:
> >> Daniel Shahaf <da...@elego.de> writes:
> >>
> >> > Philip Martin wrote on Wed, Jul 25, 2012 at 10:49:36 +0100:
> >> >>
> >> >> I attached to the issue a repository that demonstrates the corruption;
> >> >> verify doesn't report a problem on that repository. What does verify
> >> >> check?
> >> >
> >> > validate_root_noderev() catches an instance of the #4129 corruption and
> >> > in its docstring xrefs svn_fs_fs__verify(), so I think the checks in the
> >> > former are done in the latter too.
> >>
> >> validate_root_noderev is only called by write_final_rev and not during
> >> verify. svn_fs_fs__verify calls svn_fs_fs__verify_root and that does:
> >>
> >> /* Issue #4129: bogus predecessors. */
> >> /* Check 1: predecessor must be an earlier revision.
> >> */
> >> if (pred_rev >= root->rev)
> >> return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
> >> "r%ld's root node's predecessor is r%ld"
> >> " but must be earlier revision",
> >> root->rev, pred_rev);
> >
> > The check was included in svn_fs_fs__verify_root() in r1349313:
> >
> > /* Verify explicitly the predecessor of the root. */
> > {
> > ...
> > /* Check the predecessor's revision. */
> > if (pred_id)
> > {
> > SVN_ERR(svn_fs_fs__dag_get_node(&pred, root->fs, pred_id, pool));
> > SVN_ERR(svn_fs_fs__dag_get_revision(&pred_rev, pred, pool));
> > if (pred_rev+1 != root->rev)
> > /* Issue #4129. */
> > return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
> > "r%ld's root node's predecessor is r%ld",
> > root->rev, pred_rev);
> > }
> > {
>
> That check was relaxed by:
>
> r1358322 | stefan2 | 2012-07-06 19:04:09 +0100 (Fri, 06 Jul 2012) | 5 lines
>
> Relax overly strict consistency check that is not covered by the FSFS
> format specification.
>
> * subversion/libsvn_fs_fs/tree.c
> (svn_fs_fs__verify_root): fix test and add commentary
>
Thanks for digging it. I think that change is wrong --- the predecessor
of the root noderev of rN MUST be the root noderev of r(N-1) --- that's
how Subversion filesystems behave (not just FSFS).
Stefan2, WDYT?
>
> --
> Certified & Supported Apache Subversion Downloads:
> http://www.wandisco.com/subversion/download
Re: [Issue 4129] predecessors links on root node-revision skip revisions
Posted by Philip Martin <ph...@wandisco.com>.
Daniel Shahaf <da...@elego.de> writes:
> Philip Martin wrote on Wed, Jul 25, 2012 at 11:09:54 +0100:
>> Daniel Shahaf <da...@elego.de> writes:
>>
>> > Philip Martin wrote on Wed, Jul 25, 2012 at 10:49:36 +0100:
>> >>
>> >> I attached to the issue a repository that demonstrates the corruption;
>> >> verify doesn't report a problem on that repository. What does verify
>> >> check?
>> >
>> > validate_root_noderev() catches an instance of the #4129 corruption and
>> > in its docstring xrefs svn_fs_fs__verify(), so I think the checks in the
>> > former are done in the latter too.
>>
>> validate_root_noderev is only called by write_final_rev and not during
>> verify. svn_fs_fs__verify calls svn_fs_fs__verify_root and that does:
>>
>> /* Issue #4129: bogus predecessors. */
>> /* Check 1: predecessor must be an earlier revision.
>> */
>> if (pred_rev >= root->rev)
>> return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
>> "r%ld's root node's predecessor is r%ld"
>> " but must be earlier revision",
>> root->rev, pred_rev);
>
> The check was included in svn_fs_fs__verify_root() in r1349313:
>
> /* Verify explicitly the predecessor of the root. */
> {
> ...
> /* Check the predecessor's revision. */
> if (pred_id)
> {
> SVN_ERR(svn_fs_fs__dag_get_node(&pred, root->fs, pred_id, pool));
> SVN_ERR(svn_fs_fs__dag_get_revision(&pred_rev, pred, pool));
> if (pred_rev+1 != root->rev)
> /* Issue #4129. */
> return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
> "r%ld's root node's predecessor is r%ld",
> root->rev, pred_rev);
> }
> {
That check was relaxed by:
r1358322 | stefan2 | 2012-07-06 19:04:09 +0100 (Fri, 06 Jul 2012) | 5 lines
Relax overly strict consistency check that is not covered by the FSFS
format specification.
* subversion/libsvn_fs_fs/tree.c
(svn_fs_fs__verify_root): fix test and add commentary
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: [Issue 4129] predecessors links on root node-revision skip
revisions
Posted by Daniel Shahaf <da...@elego.de>.
Philip Martin wrote on Wed, Jul 25, 2012 at 11:09:54 +0100:
> Daniel Shahaf <da...@elego.de> writes:
>
> > Philip Martin wrote on Wed, Jul 25, 2012 at 10:49:36 +0100:
> >>
> >> I attached to the issue a repository that demonstrates the corruption;
> >> verify doesn't report a problem on that repository. What does verify
> >> check?
> >
> > validate_root_noderev() catches an instance of the #4129 corruption and
> > in its docstring xrefs svn_fs_fs__verify(), so I think the checks in the
> > former are done in the latter too.
>
> validate_root_noderev is only called by write_final_rev and not during
> verify. svn_fs_fs__verify calls svn_fs_fs__verify_root and that does:
>
> /* Issue #4129: bogus predecessors. */
> /* Check 1: predecessor must be an earlier revision.
> */
> if (pred_rev >= root->rev)
> return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
> "r%ld's root node's predecessor is r%ld"
> " but must be earlier revision",
> root->rev, pred_rev);
The check was included in svn_fs_fs__verify_root() in r1349313:
/* Verify explicitly the predecessor of the root. */
{
...
/* Check the predecessor's revision. */
if (pred_id)
{
SVN_ERR(svn_fs_fs__dag_get_node(&pred, root->fs, pred_id, pool));
SVN_ERR(svn_fs_fs__dag_get_revision(&pred_rev, pred, pool));
if (pred_rev+1 != root->rev)
/* Issue #4129. */
return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
"r%ld's root node's predecessor is r%ld",
root->rev, pred_rev);
}
{
Re: [Issue 4129] predecessors links on root node-revision skip revisions
Posted by Philip Martin <ph...@wandisco.com>.
Daniel Shahaf <da...@elego.de> writes:
> Philip Martin wrote on Wed, Jul 25, 2012 at 10:49:36 +0100:
>>
>> I attached to the issue a repository that demonstrates the corruption;
>> verify doesn't report a problem on that repository. What does verify
>> check?
>
> validate_root_noderev() catches an instance of the #4129 corruption and
> in its docstring xrefs svn_fs_fs__verify(), so I think the checks in the
> former are done in the latter too.
validate_root_noderev is only called by write_final_rev and not during
verify. svn_fs_fs__verify calls svn_fs_fs__verify_root and that does:
/* Issue #4129: bogus predecessors. */
/* Check 1: predecessor must be an earlier revision.
*/
if (pred_rev >= root->rev)
return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
"r%ld's root node's predecessor is r%ld"
" but must be earlier revision",
root->rev, pred_rev);
/* Check 2: distances must be a power of 2.
* Note that this condition is not defined by the FSFS format but
* merely a byproduct of the current implementation. Therefore,
* it may help to spot corruptions for the time being but might
* need to be removed / relaxed in later versions.
*/
delta = root->rev - pred_rev;
if (delta & (delta - 1))
return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
"r%ld's root node's predecessor is r%ld"
" but the delta must be a power of 2",
root->rev, pred_rev);
which doesn't catch the corruption I introduced namely that /@4 and /A@4
have predecessor /@2 and /A@2 rather than /@3 and /A@3.
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: [Issue 4129] predecessors links on root node-revision skip
revisions
Posted by Daniel Shahaf <da...@elego.de>.
Philip Martin wrote on Wed, Jul 25, 2012 at 10:49:36 +0100:
> Stefan Fuhrmann <st...@wandisco.com> writes:
>
> > Could you please fix the UI experience first?
> > Currently, svnadmin verify will first verify the
> > root nodes of all revisions of the whole repository
> > and *then* start verifying all revisions showing
> > some progress info.
> >
> > Two issues with that:
> > (1) The first part takes about as long as the second,
> > i.e. minutes or hours w/o visible feedback.
> > (2) Doing that verification as part of the second
> > stage would be virtually for free as the node caches
> > are hot.
>
> I attached to the issue a repository that demonstrates the corruption;
> verify doesn't report a problem on that repository. What does verify
> check?
>
validate_root_noderev() catches an instance of the #4129 corruption and
in its docstring xrefs svn_fs_fs__verify(), so I think the checks in the
former are done in the latter too.
Daniel
> valgrind does report a problem with verify:
>
> $ valgrind -q subversion/svnadmin/.libs/lt-svnadmin verify repo
> * Verified revision 0.
> ==9089== Invalid read of size 1
> ==9089== at 0x4C25FF8: memcpy (mc_replace_strmem.c:497)
> ==9089== by 0x54E73F5: svn_stringbuf_appendbytes (string.c:558)
> ==9089== by 0x54ECEDC: svn_temp_serializer__push (temp_serializer.c:245)
> ==9089== by 0x61132C0: serialize_svn_string (temp_serializer.c:130)
> ==9089== by 0x6113BDA: serialize_txdeltawindow (temp_serializer.c:479)
> ==9089== by 0x6113C70: svn_fs_fs__serialize_txdelta_window (temp_serializer.c:505)
> ==9089== by 0x54B010B: membuffer_cache_set (cache-membuffer.c:1241)
> ==9089== by 0x54B0AC2: svn_membuffer_cache_set (cache-membuffer.c:1766)
> ==9089== by 0x54B2486: svn_cache__set (cache.c:101)
> ==9089== by 0x60FCE0B: set_cached_window (fs_fs.c:4417)
> ==9089== by 0x60FD5F5: read_window (fs_fs.c:4629)
> ==9089== by 0x60FD69C: get_combined_window (fs_fs.c:4655)
> ==9089== Address 0x7e7f575 is 0 bytes after a block of size 5 alloc'd
> ==9089== at 0x4C244E8: malloc (vg_replace_malloc.c:236)
> ==9089== by 0x59516D0: pool_alloc (apr_pools.c:1463)
> ==9089== by 0x5289811: svn_txdelta_read_svndiff_window (svndiff.c:977)
> ==9089== by 0x60FD50E: read_window (fs_fs.c:4618)
> ==9089== by 0x60FD69C: get_combined_window (fs_fs.c:4655)
> ==9089== by 0x60FDC57: get_contents (fs_fs.c:4802)
> ==9089== by 0x60FDD22: rep_read_contents (fs_fs.c:4827)
> ==9089== by 0x54E306E: svn_stream_read (stream.c:143)
> ==9089== by 0x54E3CED: svn_stream_copy3 (stream.c:497)
> ==9089== by 0x4E3C9D2: dump_node (dump.c:627)
> ==9089== by 0x4E3CF70: add_file (dump.c:789)
> ==9089== by 0x527B6F9: add_file (cancel.c:167)
>
>
> --
> Certified & Supported Apache Subversion Downloads:
> http://www.wandisco.com/subversion/download
Re: [Issue 4129] predecessors links on root node-revision skip revisions
Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@wandisco.com> writes:
> This has nothing to do with the corrupt repository. I get the same
> error for:
>
> svnadmin create repo
> svn import -mm repo/format file://`pwd`/repo/f
> svnadmin verify repo
>
> I get a different error for:
>
> svnadmin create repo
> svn mkdir -mm file://`pwd`/repo/A
> svnadmin verify repo
These are unrelated to issue 4129 so I've raised
http://subversion.tigris.org/issues/show_bug.cgi?id=4208
http://subversion.tigris.org/issues/show_bug.cgi?id=4209
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: [Issue 4129] predecessors links on root node-revision skip revisions
Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@wandisco.com> writes:
> $ valgrind -q subversion/svnadmin/.libs/lt-svnadmin verify repo
> * Verified revision 0.
> ==9089== Invalid read of size 1
> ==9089== at 0x4C25FF8: memcpy (mc_replace_strmem.c:497)
> ==9089== by 0x54E73F5: svn_stringbuf_appendbytes (string.c:558)
> ==9089== by 0x54ECEDC: svn_temp_serializer__push (temp_serializer.c:245)
> ==9089== by 0x61132C0: serialize_svn_string (temp_serializer.c:130)
> ==9089== by 0x6113BDA: serialize_txdeltawindow (temp_serializer.c:479)
> ==9089== by 0x6113C70: svn_fs_fs__serialize_txdelta_window (temp_serializer.c:505)
> ==9089== by 0x54B010B: membuffer_cache_set (cache-membuffer.c:1241)
> ==9089== by 0x54B0AC2: svn_membuffer_cache_set (cache-membuffer.c:1766)
> ==9089== by 0x54B2486: svn_cache__set (cache.c:101)
> ==9089== by 0x60FCE0B: set_cached_window (fs_fs.c:4417)
> ==9089== by 0x60FD5F5: read_window (fs_fs.c:4629)
> ==9089== by 0x60FD69C: get_combined_window (fs_fs.c:4655)
> ==9089== Address 0x7e7f575 is 0 bytes after a block of size 5 alloc'd
> ==9089== at 0x4C244E8: malloc (vg_replace_malloc.c:236)
> ==9089== by 0x59516D0: pool_alloc (apr_pools.c:1463)
> ==9089== by 0x5289811: svn_txdelta_read_svndiff_window (svndiff.c:977)
> ==9089== by 0x60FD50E: read_window (fs_fs.c:4618)
> ==9089== by 0x60FD69C: get_combined_window (fs_fs.c:4655)
> ==9089== by 0x60FDC57: get_contents (fs_fs.c:4802)
> ==9089== by 0x60FDD22: rep_read_contents (fs_fs.c:4827)
> ==9089== by 0x54E306E: svn_stream_read (stream.c:143)
> ==9089== by 0x54E3CED: svn_stream_copy3 (stream.c:497)
> ==9089== by 0x4E3C9D2: dump_node (dump.c:627)
> ==9089== by 0x4E3CF70: add_file (dump.c:789)
> ==9089== by 0x527B6F9: add_file (cancel.c:167)
This has nothing to do with the corrupt repository. I get the same
error for:
svnadmin create repo
svn import -mm repo/format file://`pwd`/repo/f
svnadmin verify repo
I get a different error for:
svnadmin create repo
svn mkdir -mm file://`pwd`/repo/A
svnadmin verify repo
../src/subversion/libsvn_repos/dump.c:1401: (apr_err=160006)
../src/subversion/libsvn_fs/fs-loader.c:489: (apr_err=160006)
../src/subversion/libsvn_fs_fs/fs_fs.c:9706: (apr_err=160006)
../src/subversion/libsvn_fs_fs/rep-cache.c:161: (apr_err=160006)
../src/subversion/libsvn_fs_fs/fs_fs.c:1804: (apr_err=160006)
../src/subversion/libsvn_fs_fs/fs_fs.c:1779: (apr_err=160006)
svnadmin: E160006: Invalid revision number '-1'
../src/subversion/libsvn_subr/sqlite.c:754: (apr_err=235000)
svn: E235000: In file '../src/subversion/libsvn_subr/sqlite.c' line 754: internal malfunction
Aborted
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: [Issue 4129] predecessors links on root node-revision skip revisions
Posted by Philip Martin <ph...@wandisco.com>.
Stefan Fuhrmann <st...@wandisco.com> writes:
> Could you please fix the UI experience first?
> Currently, svnadmin verify will first verify the
> root nodes of all revisions of the whole repository
> and *then* start verifying all revisions showing
> some progress info.
>
> Two issues with that:
> (1) The first part takes about as long as the second,
> i.e. minutes or hours w/o visible feedback.
> (2) Doing that verification as part of the second
> stage would be virtually for free as the node caches
> are hot.
I attached to the issue a repository that demonstrates the corruption;
verify doesn't report a problem on that repository. What does verify
check?
valgrind does report a problem with verify:
$ valgrind -q subversion/svnadmin/.libs/lt-svnadmin verify repo
* Verified revision 0.
==9089== Invalid read of size 1
==9089== at 0x4C25FF8: memcpy (mc_replace_strmem.c:497)
==9089== by 0x54E73F5: svn_stringbuf_appendbytes (string.c:558)
==9089== by 0x54ECEDC: svn_temp_serializer__push (temp_serializer.c:245)
==9089== by 0x61132C0: serialize_svn_string (temp_serializer.c:130)
==9089== by 0x6113BDA: serialize_txdeltawindow (temp_serializer.c:479)
==9089== by 0x6113C70: svn_fs_fs__serialize_txdelta_window (temp_serializer.c:505)
==9089== by 0x54B010B: membuffer_cache_set (cache-membuffer.c:1241)
==9089== by 0x54B0AC2: svn_membuffer_cache_set (cache-membuffer.c:1766)
==9089== by 0x54B2486: svn_cache__set (cache.c:101)
==9089== by 0x60FCE0B: set_cached_window (fs_fs.c:4417)
==9089== by 0x60FD5F5: read_window (fs_fs.c:4629)
==9089== by 0x60FD69C: get_combined_window (fs_fs.c:4655)
==9089== Address 0x7e7f575 is 0 bytes after a block of size 5 alloc'd
==9089== at 0x4C244E8: malloc (vg_replace_malloc.c:236)
==9089== by 0x59516D0: pool_alloc (apr_pools.c:1463)
==9089== by 0x5289811: svn_txdelta_read_svndiff_window (svndiff.c:977)
==9089== by 0x60FD50E: read_window (fs_fs.c:4618)
==9089== by 0x60FD69C: get_combined_window (fs_fs.c:4655)
==9089== by 0x60FDC57: get_contents (fs_fs.c:4802)
==9089== by 0x60FDD22: rep_read_contents (fs_fs.c:4827)
==9089== by 0x54E306E: svn_stream_read (stream.c:143)
==9089== by 0x54E3CED: svn_stream_copy3 (stream.c:497)
==9089== by 0x4E3C9D2: dump_node (dump.c:627)
==9089== by 0x4E3CF70: add_file (dump.c:789)
==9089== by 0x527B6F9: add_file (cancel.c:167)
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: [Issue 4129] predecessors links on root node-revision skip
revisions
Posted by Daniel Shahaf <da...@elego.de>.
Stefan Fuhrmann wrote on Wed, Jul 25, 2012 at 11:03:24 +0200:
> On Wed, Jul 25, 2012 at 9:53 AM, Daniel Shahaf <da...@elego.de> wrote:
>
> > Should we mark the issue as FIXED ?
> >
>
> Could you please fix the UI experience first?
> Currently, svnadmin verify will first verify the
> root nodes of all revisions of the whole repository
> and *then* start verifying all revisions showing
> some progress info.
>
> Two issues with that:
> (1) The first part takes about as long as the second,
> i.e. minutes or hours w/o visible feedback.
> (2) Doing that verification as part of the second
> stage would be virtually for free as the node caches
> are hot.
>
Could you open a separate issue for that? Feel free to mark it as a
1.8.0 blocker if you think that is merited, but the problems you
describe have nothing to do with #4129, IMO.
And, naturally, if I wrote the revprop verifying code I'll be happy to
look into any issues you may have with it.
> -- Stefan^2.
>
>
> > philip@tigris.org wrote on Tue, Jul 24, 2012 at 16:37:56 -0700:
> > > http://subversion.tigris.org/issues/show_bug.cgi?id=4129
> > >
> > >
> > >
> > >
> > >
> > >
> > > ------- Additional comments from philip@tigris.org Tue Jul 24 16:37:55
> > -0700 2012 -------
> > > r1302613 was backported in 1.7.5 and 1.6.18.
> >
>
>
>
> --
> Certified & Supported Apache Subversion Downloads:
> http://www.wandisco.com/subversion/download
Re: [Issue 4129] predecessors links on root node-revision skip revisions
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Wed, Jul 25, 2012 at 11:14 AM, Philip Martin
<ph...@wandisco.com>wrote:
> Stefan Fuhrmann <st...@wandisco.com> writes:
>
> > On Wed, Jul 25, 2012 at 9:53 AM, Daniel Shahaf <da...@elego.de>
> wrote:
> >
> >> Should we mark the issue as FIXED ?
> >>
> >
> > Could you please fix the UI experience first?
> > Currently, svnadmin verify will first verify the
> > root nodes of all revisions of the whole repository
> > and *then* start verifying all revisions showing
> > some progress info.
> >
> > Two issues with that:
> > (1) The first part takes about as long as the second,
> > i.e. minutes or hours w/o visible feedback.
> > (2) Doing that verification as part of the second
> > stage would be virtually for free as the node caches
> > are hot.
>
> Please update the issue if more work is required.
Done.
-- Stefan^2.
Re: [Issue 4129] predecessors links on root node-revision skip revisions
Posted by Philip Martin <ph...@wandisco.com>.
Stefan Fuhrmann <st...@wandisco.com> writes:
> On Wed, Jul 25, 2012 at 9:53 AM, Daniel Shahaf <da...@elego.de> wrote:
>
>> Should we mark the issue as FIXED ?
>>
>
> Could you please fix the UI experience first?
> Currently, svnadmin verify will first verify the
> root nodes of all revisions of the whole repository
> and *then* start verifying all revisions showing
> some progress info.
>
> Two issues with that:
> (1) The first part takes about as long as the second,
> i.e. minutes or hours w/o visible feedback.
> (2) Doing that verification as part of the second
> stage would be virtually for free as the node caches
> are hot.
Please update the issue if more work is required.
>
> -- Stefan^2.
>
>
>> philip@tigris.org wrote on Tue, Jul 24, 2012 at 16:37:56 -0700:
>> > http://subversion.tigris.org/issues/show_bug.cgi?id=4129
>> >
>> >
>> >
>> >
>> >
>> >
>> > ------- Additional comments from philip@tigris.org Tue Jul 24 16:37:55
>> -0700 2012 -------
>> > r1302613 was backported in 1.7.5 and 1.6.18.
>>
>
>
>
> --
> Certified & Supported Apache Subversion Downloads:
> http://www.wandisco.com/subversion/download
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download