From: Brian Warner Date: Sat, 6 Dec 2008 05:49:23 +0000 (-0700) Subject: mutable.modify(): after UCWE, publish even if the second invocation of the modifier... X-Git-Url: https://git.rkrishnan.org/%5B/%5D%20/file/URI:LIT:krugkidfnzsc4/index.php?a=commitdiff_plain;h=ffb598514656e7b2f771349c2be15340e3e6e73d;p=tahoe-lafs%2Ftahoe-lafs.git mutable.modify(): after UCWE, publish even if the second invocation of the modifier didn't modify anything. For #551. --- diff --git a/src/allmydata/mutable/node.py b/src/allmydata/mutable/node.py index 3e6338e6..cfa823d8 100644 --- a/src/allmydata/mutable/node.py +++ b/src/allmydata/mutable/node.py @@ -387,7 +387,13 @@ class MutableFileNode: new_contents = modifier(old_contents, servermap, first_time) if new_contents is None or new_contents == old_contents: # no changes need to be made - return + if first_time: + return + # However, since Publish is not automatically doing a + # recovery when it observes UCWE, we need to do a second + # publish. See #551 for details. We'll basically loop until + # we managed an uncontested publish. + new_contents = old_contents precondition(isinstance(new_contents, str), "Modifier function must return a string or None") return self._upload(new_contents, servermap) diff --git a/src/allmydata/test/test_dirnode.py b/src/allmydata/test/test_dirnode.py index 0447e9c1..657a8b58 100644 --- a/src/allmydata/test/test_dirnode.py +++ b/src/allmydata/test/test_dirnode.py @@ -769,7 +769,10 @@ class UCWEingMutableFileNode(MutableFileNode): def _upload(self, new_contents, servermap): d = MutableFileNode._upload(self, new_contents, servermap) def _ucwe(res): - raise UncoordinatedWriteError() + if self.please_ucwe_after_next_upload: + self.please_ucwe_after_next_upload = False + raise UncoordinatedWriteError() + return res d.addCallback(_ucwe) return d class UCWEingNewDirectoryNode(dirnode.NewDirectoryNode): @@ -799,6 +802,8 @@ class Deleter(SystemTestMixin, unittest.TestCase): d.addCallback(_created_dir) def _do_delete(ignored): n = UCWEingNewDirectoryNode(self.clients[0]).init_from_uri(self.root_uri) + assert n._node.please_ucwe_after_next_upload == False + n._node.please_ucwe_after_next_upload = True # This should succeed, not raise an exception return n.delete(u"file") d.addCallback(_do_delete) diff --git a/src/allmydata/test/test_mutable.py b/src/allmydata/test/test_mutable.py index ad368280..055b4649 100644 --- a/src/allmydata/test/test_mutable.py +++ b/src/allmydata/test/test_mutable.py @@ -374,11 +374,11 @@ class Filenode(unittest.TestCase, testutil.ShouldFailMixin): d.addCallback(_created) return d - def failUnlessCurrentSeqnumIs(self, n, expected_seqnum): + def failUnlessCurrentSeqnumIs(self, n, expected_seqnum, which): d = n.get_servermap(MODE_READ) d.addCallback(lambda servermap: servermap.best_recoverable_version()) d.addCallback(lambda verinfo: - self.failUnlessEqual(verinfo[0], expected_seqnum)) + self.failUnlessEqual(verinfo[0], expected_seqnum, which)) return d def test_modify(self): @@ -399,30 +399,37 @@ class Filenode(unittest.TestCase, testutil.ShouldFailMixin): if len(calls) <= 1: raise UncoordinatedWriteError("simulated") return old_contents + "line3" + def _ucw_error_non_modifier(old_contents, servermap, first_time): + # simulate an UncoordinatedWriteError once, and don't actually + # modify the contents on subsequent invocations + calls.append(1) + if len(calls) <= 1: + raise UncoordinatedWriteError("simulated") + return old_contents d = self.client.create_mutable_file("line1") def _created(n): d = n.modify(_modifier) d.addCallback(lambda res: n.download_best_version()) d.addCallback(lambda res: self.failUnlessEqual(res, "line1line2")) - d.addCallback(lambda res: self.failUnlessCurrentSeqnumIs(n, 2)) + d.addCallback(lambda res: self.failUnlessCurrentSeqnumIs(n, 2, "m")) d.addCallback(lambda res: n.modify(_non_modifier)) d.addCallback(lambda res: n.download_best_version()) d.addCallback(lambda res: self.failUnlessEqual(res, "line1line2")) - d.addCallback(lambda res: self.failUnlessCurrentSeqnumIs(n, 2)) + d.addCallback(lambda res: self.failUnlessCurrentSeqnumIs(n, 2, "non")) d.addCallback(lambda res: n.modify(_none_modifier)) d.addCallback(lambda res: n.download_best_version()) d.addCallback(lambda res: self.failUnlessEqual(res, "line1line2")) - d.addCallback(lambda res: self.failUnlessCurrentSeqnumIs(n, 2)) + d.addCallback(lambda res: self.failUnlessCurrentSeqnumIs(n, 2, "none")) d.addCallback(lambda res: self.shouldFail(ValueError, "error_modifier", None, n.modify, _error_modifier)) d.addCallback(lambda res: n.download_best_version()) d.addCallback(lambda res: self.failUnlessEqual(res, "line1line2")) - d.addCallback(lambda res: self.failUnlessCurrentSeqnumIs(n, 2)) + d.addCallback(lambda res: self.failUnlessCurrentSeqnumIs(n, 2, "err")) d.addCallback(lambda res: self.shouldFail(FileTooLargeError, "toobig_modifier", @@ -430,14 +437,32 @@ class Filenode(unittest.TestCase, testutil.ShouldFailMixin): n.modify, _toobig_modifier)) d.addCallback(lambda res: n.download_best_version()) d.addCallback(lambda res: self.failUnlessEqual(res, "line1line2")) - d.addCallback(lambda res: self.failUnlessCurrentSeqnumIs(n, 2)) + d.addCallback(lambda res: self.failUnlessCurrentSeqnumIs(n, 2, "big")) d.addCallback(lambda res: n.modify(_ucw_error_modifier)) d.addCallback(lambda res: self.failUnlessEqual(len(calls), 2)) d.addCallback(lambda res: n.download_best_version()) d.addCallback(lambda res: self.failUnlessEqual(res, "line1line2line3")) - d.addCallback(lambda res: self.failUnlessCurrentSeqnumIs(n, 3)) + d.addCallback(lambda res: self.failUnlessCurrentSeqnumIs(n, 3, "ucw")) + + def _reset_ucw_error_modifier(res): + calls[:] = [] + return res + d.addCallback(_reset_ucw_error_modifier) + + # in practice, this n.modify call should publish twice: the first + # one gets a UCWE, the second does not. But our test jig (in + # which the modifier raises the UCWE) skips over the first one, + # so in this test there will be only one publish, and the seqnum + # will only be one larger than the previous test, not two (i.e. 4 + # instead of 5). + d.addCallback(lambda res: n.modify(_ucw_error_non_modifier)) + d.addCallback(lambda res: self.failUnlessEqual(len(calls), 2)) + d.addCallback(lambda res: n.download_best_version()) + d.addCallback(lambda res: self.failUnlessEqual(res, + "line1line2line3")) + d.addCallback(lambda res: self.failUnlessCurrentSeqnumIs(n, 4, "ucw")) return d d.addCallback(_created) @@ -472,7 +497,7 @@ class Filenode(unittest.TestCase, testutil.ShouldFailMixin): d = n.modify(_modifier) d.addCallback(lambda res: n.download_best_version()) d.addCallback(lambda res: self.failUnlessEqual(res, "line1line2")) - d.addCallback(lambda res: self.failUnlessCurrentSeqnumIs(n, 2)) + d.addCallback(lambda res: self.failUnlessCurrentSeqnumIs(n, 2, "m")) d.addCallback(lambda res: self.shouldFail(UncoordinatedWriteError, @@ -481,7 +506,7 @@ class Filenode(unittest.TestCase, testutil.ShouldFailMixin): _backoff_stopper)) d.addCallback(lambda res: n.download_best_version()) d.addCallback(lambda res: self.failUnlessEqual(res, "line1line2")) - d.addCallback(lambda res: self.failUnlessCurrentSeqnumIs(n, 2)) + d.addCallback(lambda res: self.failUnlessCurrentSeqnumIs(n, 2, "stop")) def _reset_ucw_error_modifier(res): calls[:] = [] @@ -492,7 +517,7 @@ class Filenode(unittest.TestCase, testutil.ShouldFailMixin): d.addCallback(lambda res: n.download_best_version()) d.addCallback(lambda res: self.failUnlessEqual(res, "line1line2line3")) - d.addCallback(lambda res: self.failUnlessCurrentSeqnumIs(n, 3)) + d.addCallback(lambda res: self.failUnlessCurrentSeqnumIs(n, 3, "pause")) d.addCallback(lambda res: self.shouldFail(UncoordinatedWriteError, @@ -502,7 +527,7 @@ class Filenode(unittest.TestCase, testutil.ShouldFailMixin): d.addCallback(lambda res: n.download_best_version()) d.addCallback(lambda res: self.failUnlessEqual(res, "line1line2line3")) - d.addCallback(lambda res: self.failUnlessCurrentSeqnumIs(n, 3)) + d.addCallback(lambda res: self.failUnlessCurrentSeqnumIs(n, 3, "giveup")) return d d.addCallback(_created)