#312 closed defect (fixed)

mutable file: survive encoding variations

Reported by: warner Owned by: warner
Priority: major Milestone: 0.9.0 (Allmydata 3.0 final)
Component: code-encoding Version: 0.7.0
Keywords: mutable Cc:
Launchpad Bug:

Description

The current mutable.py has a nasty bug lurking: since the encoding parameters (k and N) are not included in the URI, a copy is put in each share. The Retrieve code latches on to the first version it sees, and ignores the values from all subsequently-fetched shares. If (for whatever reason) some clients have uploaded the file with different parameters (specifically different values of k, say 3-of-10 vs 2-of-6), then we could wind up feeding 3-of-10 shares into a zfec decoder configured for 2-of-6, which would cause silent data corruption.

The first fix for this is to reject shares that have encoding parameters that differ from the values that we pulled from the first share, rejecting them with a CorruptShareError. That will at least prevent the possible data corruption.

The longer-term fix is to refactor Retrieve to treat k and N as part of the 'verinfo' index, along with seqnum and roothash and the salt. This refactoring also calls for building up a table of available versions, and then deciding which one (or ones) to decode on the basis of available shares and highest seqnum. The new Retrieve class should be able to return multiple versions, or indicate the presence of newer versions (that might not be recoverable).

Change History (9)

comment:1 Changed at 2008-02-13T20:11:29Z by warner

I've pushed the first fix for this. We still need to come up with a unit testing scheme for this stuff, addressed in #207.

comment:2 Changed at 2008-02-13T20:12:24Z by warner

  • Milestone changed from 0.8.0 (Allmydata 3.0 Beta) to 0.9.0 (Allmydata 3.0 final)
  • Priority changed from critical to major

Having that first fix in place addresses the immediate problem, so I'm lowering the severity and pushing the rest of this ticket out a release

comment:3 Changed at 2008-03-08T04:13:31Z by zooko

  • Milestone changed from 0.9.0 (Allmydata 3.0 final) to undecided

comment:4 Changed at 2008-03-08T07:11:12Z by warner

If we want #332 to go into the 0.9.0 release, then we also need to fix #312. Do you agree? My concern is that existing dirnodes will wind up with multiple encodings, but maybe I'm wrong.

comment:5 Changed at 2008-03-08T14:31:17Z by zooko

  • Milestone changed from undecided to 0.9.0 (Allmydata 3.0 final)

Hm... yes it would be good to fix this, so that dirnodes produced by v0.8.0 can survive into v0.9.0 and get converted into K=1 dirnodes.

This is our first backwards compatibility decision. :-)

comment:6 Changed at 2008-03-10T19:37:42Z by zooko

  • Owner set to warner

comment:7 Changed at 2008-03-11T08:48:56Z by warner

  • Resolution set to fixed
  • Status changed from new to closed

Fixed, in 10d3ea504540ae2f. This retains the property that Retrieve will return with whatever version was recoverable first: it classifies all shares that it sees into buckets indexed by their full "verinfo" tuple: seqnum, roothash, encoding parameters. Whichever bucket gets enough valid shares to decode first will win.

The rest of the refactoring (to actually fetch and return multiple versions, and handle the "epsilon" anti-rollback parameter, etc) is left for ticket #205.

comment:8 Changed at 2008-03-11T08:51:50Z by warner

Oh, also note that this change does nothing whatsoever about "rebalancing" mutable files to use more shares upon each successive update. In fact the code retain the behavior that shares are always updated in place rather than moving them, so if you upload 10 shares when there are only three peers on the network, then those shares will remain bunched up on those three peers even after more peers have been added. I don't know if we have an enhancement ticket to rebalance bunched-up shares when we find enough peers to do so.

comment:9 Changed at 2008-04-14T16:32:51Z by zooko

This was fixed in 791482cf8de84a91 (the trac changeset now known as 791482cf8de84a91 was formerly known as 10d3ea504540ae2f -- there were two patches listed in the Trac timeline until now that have been obliterated from our trunk).

Note: See TracTickets for help on using tickets.