Stefan Sperling wrote on Sat, Oct 27, 2012 at 10:46:59 +0200:
> I don't think this is a problem at all. If revision A has a broken
> rep and revision B refers to that rep, then both A and B are broken.
> Even if the source of the corruption is in A only I want to know that
> B is affected by it. Because B is essentially just as broken as A from
> the user's point of view (somebody who wants to use the repository and
> who doesn't understand FSFS internals).
>
> I don't really care about detailed info such as which revision is
> actually causing the problem. That would be nice but isn't required.
>
I see: your underlying use-case is "What revisions cannot be checked
out", not "Where inside the FS implementation are the corruptions".
I agree that for that use-case, Prabhu's patch is pretty much the sanest
implementation.
The patch could be improved still by adding a keep_going parameter to
svn_fs_verify() as well, or at least trapping the returned error when
svn_repos_fs_verify3().keep_going==TRUE. That seems like a low-hanging
fruit.
Pushing the verification logic into a tree crawl (in either libsvn_repos
or dag.c) would be significantly more work (both code-wise and
research-wise -- why did we change the 'verify' implementation in 1.4?),
so I'll consider it a potential future improvement - not a blocker for
this patch.
> > > A proper 'svnadmin verify' loop will also collect errors reported and
> > > show a summary at the end. That's additional overhead if coding this as
> > > a loop on the command line. The repos library can easily collect this
> > > information internally and pass it as the final notification.
> >
> > I am not sure that I understand this argument.
>
> Output of a quickly written loop on the command line is usually not
> going to be as nice and comprehensive as that of a well implemented
> core feature. Makes sense?
>
You mean,
for i in {0..$(<db/current)} ; svnadmin verify -r$i 2>(sed s/^/r$i:\ /) ./
?
But sure, the library implementation is written just once so it can take
the time to go to more effort in presenting the information.
> > Sounds like you're evaluating Prabhu's patch against a list of features
> > you'd like 'svnadmin verify' to grow someday. I think it would be
> > helpful if you shared that list --- it would let people share their
> > thoughts on the whole context/plan, not just on this patch.
>
> The only item on that list would be the summary report already mentioned.
>
> I just want to know how bad the overall impact of corruption is,
> i.e. which revisions are affected by one or more instances of
> actual corruption that have occurred in the repository.
In other words, your use case is different from mine, and the patch
addresses your use-case. +1 (concept) to apply.
Thanks,
Daniel
Received on 2012-10-27 18:29:53 CEST