Re: [PATCH] Implement svnadmin verify --force

Thank you so much Stefan and Daniel.

I learnt so much from you guys during the course of this patch. I
attached a new patch and a log message with this mail. Please let me
know if I missed out on something.

Thanks and regards
Prabhu

On 10/27/2012 09:59 PM, Daniel Shahaf wrote:
> 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