[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1555109 - /subversion/trunk/subversion/libsvn_fs_fs/pack.c

From: Evgeny Kotkov <evgeny.kotkov_at_visualsvn.com>
Date: Thu, 27 Feb 2014 15:48:10 +0400

>> Now, in order to distinguish different clusters, parts (1) and (2) remember
>> (like, "lock on") the (PATH, REP_ID) for the first noderev from the current
>> cluster. Iff the PATH changes (this means we have just entered the next
>> cluster), we update the stored (PATH, REP_ID) values. We need to remember
>> the REP_ID in order to reconstruct those delta chains in (2), because they
>> start from this REP_ID.
>> As far as I can tell, the problem lies in the way we handle the first path.
>> For any non-empty path set, the first path always marks the beginning of the
>> corresponding cluster. However, we only remember the PATH part of the (PATH,
>> REP_ID) pair for the first path and omit the REP_ID. In case when the path
>> set consists of *a single cluster* (i.e. all path_order_t objects share the
>> same PATH), phase (2) will end up in an attempt to reconstruct the delta
>> chain starting from the uninitialized REP_ID. There are scenarios that lead
>> to this behavior in fs-fs-pack-test (see create_packed_filesystem() with
>> multiple /iota content tweaks falling into the same cluster) and this can
>> easily happen with real-world repositories.
> Well, chances are that PATH mismatches for the first
> non-NULL iteration in (2), which gets REP_ID set.
> However, if there is only one cluster with more than
> a single change in the pack file, the path PATH does
> match and REP_ID will have some random value.

Aha, for some reason I assumed that you need to carry the REP_ID from
(1) to (2) in order to be able to reconstruct the delta chain for this single
cluster (i.e. single path) scenario. That's clearly incorrect and now I
understand it.

> I committed a fix in r1572512. It removes the unnecessary
> write to REP_ID during (1) and makes sure that (PATH, REP_ID)
> get initialized to the first *relevant* entry at the being of (2).

Thank you for fixing this.

Evgeny Kotkov
Received on 2014-02-27 12:49:03 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.