[tahoe-lafs-trac-stream] [tahoe-lafs] #1641: fix regressions in convergent uncoordinated write detection

[tahoe-lafs]

#1641: fix regressions in convergent uncoordinated write detection
---------------------+---------------------------
 Reporter:  kevan    |          Owner:  nobody
     Type:  defect   |         Status:  new
 Priority:  major    |      Milestone:  undecided
Component:  unknown  |        Version:  1.9.0
 Keywords:           |  Launchpad Bug:
---------------------+---------------------------
 Comment 4 in ticket 546 describes a fix made to the mutable publisher to
 allow it to tolerate unknown shares in certain circumstances without
 raising an UncoordinatedWriteError. In the pre-1.9 publisher, this is
 implemented by comparing the checkstring (seqnum, roothash, salt) of the
 unexpected share with the checkstring of the file being published. If
 they're the same, then the unexpected share is assumed to be either a
 share that the publish operation placed earlier or an uncoordinated
 convergent write, and tolerated without an uncoordinated write error. The
 1.9 publisher changes this behavior in two ways.

 The first change is a bug. The checkstring that the check examines is set
 on lines 296, 496, and 828 of publish.py:

 {{{
         self._checkstring = self.writers.values()[0].get_checkstring()
 }}}

 {{{self.writes.values()[0]}}} can be an instance of either
 {{{MDMFSlotWriteProxy}}} or {{{SDMFSlotWriteProxy}}}.
 {{{MDMFSlotWriteProxy}}} returns a different checkstring than
 {{{SDMFSlotWriteProxy}}}; specifically, {{{MDMFSlotWriteProxy}}} returns
 the checkstring associated with the file version we're writing, while
 {{{SDMFSlotWriteProxy}}} returns the checkstring associated with the
 existing share (if any). Only {{{MDMFSlotWriteProxy}}} returns the
 checkstring associated with the current version of the mutable file, which
 is necessary in order for the #546 check to behave the same as in the
 pre-1.9 publisher. The fix for this issue is to change
 {{{SDMFSlotWriteProxy}}} to return the same checkstring as
 {{{MDMFSlotWriteProxy}}}.

 The second change is a design flaw. On line 987, I added the following:

 {{{
         # We need to remove from surprise_shares any shares that we are
         # knowingly also writing to that server from other writers.

         # TODO: Precompute this.
         known_shnums = [x.shnum for x in self.writers.values()
                         if x.server == server]
         surprise_shares -= set(known_shnums)
         self.log("found the following surprise shares: %s" %
                  str(surprise_shares))
 }}}

 which essentially exempts any surprise share that we know we're supposed
 to be writing during the publish operation from the #546 check.  The 1.9
 publisher offers no guarantees that all writes to a particular server will
 return before {{{_got_write_answer}}} is called to handle the results for
 a particular write. So a surprise share that is associated with a
 convergent and concurrent write might have either the checkstring of the
 current publish operation or the checkstring of the version associated
 with the existing share. The #546 check only accepts the share in the
 first case, which is probably why I added the exemption. It would be
 better to modify the #546 check to be specific about the second case
 instead of exempting all shares whose numbers match those we're writing.
 Alternatively, the #546 check could be retained as-is if we alter the
 publisher's control flow so that {{{_got_write_answer}}} is only executed
 for a response from a particular server after all writes to that server
 have completed. Since the publisher is designed to follow the existing
 share placement when placing a new version of a mutable file, it is likely
 that uncoordinated writers would try to place the same shares in the same
 places as one another. The exemption that is there now hurts the
 publisher's ability to detect this situation.

 The practical impact of the first regression is that SDMF publish
 operations are less able to figure out when they need to abort a publish
 and try again after another map update. The practical impact of the second
 regression is that the publisher might not detect uncoordinated writes
 that it would have been able to detect before 1.9, and that it might take
 longer to detect uncoordinated writes than before 1.9.

-- 
Ticket URL: <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/1641>
tahoe-lafs <https://tahoe-lafs.org>
secure decentralized storage