You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <da...@elego.de> on 2012/07/25 09:53:43 UTC
Re: [Issue 4129] predecessors links on root node-revision skip
revisions
Should we mark the issue as FIXED ?
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.
Re: [Issue 4129] predecessors links on root node-revision skip
revisions
Posted by Daniel Shahaf <da...@elego.de>.
I think we definitely fixed one bug that could have explained this
corruption. (Specifically the valgrind bug which you isolated no the
ticket.)
Let's close the ticket then, and reopen it if someone can reproduce it
with ≥{1.6.18,1.7.5}.
Philip Martin wrote on Wed, Jul 25, 2012 at 10:10:03 +0100:
> I was going to ask you!
>
> Daniel Shahaf <da...@elego.de> writes:
>
> > Should we mark the issue as FIXED ?
> >
> > 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 Philip Martin <ph...@wandisco.com>.
I was going to ask you!
Daniel Shahaf <da...@elego.de> writes:
> Should we mark the issue as FIXED ?
>
> 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
Re: [Issue 4129] predecessors links on root node-revision skip revisions
Posted by Stefan Fuhrmann <st...@wandisco.com>.
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