Update for mutable files is sensitive to the number of servers

[markberger]

When the number of servers used in a grid is greater than or equal to N + k, there is a race condition when mutable files are updated. While waiting for responses, ServermapUpdater will hit a threshold after EPSILON servers do not have any shares (currently EPSILON = k). This causes the process to finish and reject any remaining responses.

However, this can occur before the ServermapUpdater has added the server to the servermap, causing the client to reject the server's response. Since the server is not added to the servermap, a writer for that server is not created. This causes a key error when the updater attempts to retrieve the corresponding writer for each share.

This race condition can be replicated by setting the number of servers for allmydata.test.test_mutable.Update to 13.

===============================================================================
[FAIL]
Traceback (most recent call last):
  File "/Users/markberger/Code/tahoe-lafs/src/allmydata/test/test_mutable.py", line 3663, in <lambda>
    self.failUnlessEqual(results, new_data))
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/twisted/trial/_synctest.py", line 356, in assertEqual
    % (msg, pformat(first), pformat(second)))
twisted.trial.unittest.FailTest: not equal:
a = 'test datatest datatest datatest datatest datatest datatest datatest datatest datatest data'
b = 'test datatest datatest datatest datatest datatest datatest datatest datatest datatest dataappended'


allmydata.test.test_mutable.Update.test_update_sdmf
===============================================================================
[ERROR]
Traceback (most recent call last):
Failure: allmydata.mutable.common.NotEnoughServersError: ("Publish ran out of good servers, last failure was: [Failure instance: Traceback: <type 'exceptions.KeyError'>: 0\n/Users/markberger/Code/tahoe-lafs/src/allmydata/mutable/publish.py:642:_push\n/Users/markberger/Code/tahoe-lafs/src/allmydata/mutable/publish.py:662:push_segment\n/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/twisted/internet/defer.py:304:addCallback\n/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/twisted/internet/defer.py:293:addCallbacks\n--- <exception caught here> ---\n/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/twisted/internet/defer.py:575:_runCallbacks\n/Users/markberger/Code/tahoe-lafs/src/allmydata/mutable/publish.py:767:_push_segment\n]", None)

allmydata.test.test_mutable.Update.test_append
allmydata.test.test_mutable.Update.test_append_power_of_two
allmydata.test.test_mutable.Update.test_multiple_segment_replace
allmydata.test.test_mutable.Update.test_replace_beginning
allmydata.test.test_mutable.Update.test_replace_segstart1
===============================================================================
[ERROR]
Traceback (most recent call last):
Failure: allmydata.mutable.common.NotEnoughServersError: ("Publish ran out of good servers, last failure was: [Failure instance: Traceback: <type 'exceptions.KeyError'>: 1\n/Users/markberger/Code/tahoe-lafs/src/allmydata/mutable/publish.py:642:_push\n/Users/markberger/Code/tahoe-lafs/src/allmydata/mutable/publish.py:662:push_segment\n/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/twisted/internet/defer.py:304:addCallback\n/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/twisted/internet/defer.py:293:addCallbacks\n--- <exception caught here> ---\n/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/twisted/internet/defer.py:575:_runCallbacks\n/Users/markberger/Code/tahoe-lafs/src/allmydata/mutable/publish.py:767:_push_segment\n]", None)

allmydata.test.test_mutable.Update.test_replace_and_extend
allmydata.test.test_mutable.Update.test_replace_in_last_segment
allmydata.test.test_mutable.Update.test_replace_locations
allmydata.test.test_mutable.Update.test_replace_middle
allmydata.test.test_mutable.Update.test_replace_zero_length_middle
===============================================================================
[ERROR]
Traceback (most recent call last):
  File "/Users/markberger/Code/tahoe-lafs/src/allmydata/mutable/filenode.py", line 1177, in _build_uploadable_and_finish
    return p.update(u, offset, segments_and_bht[2], self._version)
  File "/Users/markberger/Code/tahoe-lafs/src/allmydata/mutable/publish.py", line 341, in update
    self._push()
  File "/Users/markberger/Code/tahoe-lafs/src/allmydata/mutable/publish.py", line 642, in _push
    return self.push_segment(self._current_segment)
  File "/Users/markberger/Code/tahoe-lafs/src/allmydata/mutable/publish.py", line 659, in push_segment
    return self._push()
  File "/Users/markberger/Code/tahoe-lafs/src/allmydata/mutable/publish.py", line 645, in _push
    return self.push_everything_else()
  File "/Users/markberger/Code/tahoe-lafs/src/allmydata/mutable/publish.py", line 778, in push_everything_else
    self.push_blockhashes()
  File "/Users/markberger/Code/tahoe-lafs/src/allmydata/mutable/publish.py", line 806, in push_blockhashes
    writers = self.writers[shnum]
exceptions.KeyError: 0

allmydata.test.test_mutable.Update.test_replace_zero_length_beginning
allmydata.test.test_mutable.Update.test_replace_zero_length_segstart1
-------------------------------------------------------------------------------
Ran 14 tests in 58.570s

[markberger]

Possible related tickets: #549, #1042

[markberger]

Here is a patch that will solve this problem: ​https://github.com/markberger/tahoe-lafs/tree/2034-mutable-update-race-condition

[markberger]

This patch is necessary in order to have tests for #1057 with a reasonably large number of servers.

[zooko]

I just had a look at ​https://github.com/markberger/tahoe-lafs/commit/08f70b511345c9ae267ae90fe31adf5cee1fd723, but I wasn't sure I understood the issue. I thought maybe looking at a unit test would help me understand what this patch is fixing, but there is no unit test included in the patch. Then I came back and read this ticket's original message and started to understand it better.

However, I still don't really understand it.

However, this can occur before the ServermapUpdater? has added the server to the servermap, causing the client to reject the server's response. Since the server is not added to the servermap, a writer for that server is not created.

When can this happen? Is this if there are > K servers in existence, and K servers have responded to queries, saying that they have no shares (or, perhaps some servers fail instead of responding "I have no shares"), and then… what happens? Is this server that "is not added to the servermap", mentioned above, one of the servers that has not yet been queried? Or it has been queried but it has not yet responded?

Ideas:

• Should we patch the unit tests, as described in the original message::

"This race condition can be replicated by setting the number of servers for allmydata.test.test_mutable.Update to 13."

?

• Should the resulting unit test have a comment giving this ticket # and URL to this ticket?