Rev 4906: Categorize all of the requests as safe/unsafe/semi/stream for retrying purposes. in http://bazaar.launchpad.net/~jameinel/bzr/2.1-categorize-requests-819604

John Arbash Meinel john at arbash-meinel.com
Sat Oct 8 11:48:31 UTC 2011


At http://bazaar.launchpad.net/~jameinel/bzr/2.1-categorize-requests-819604

------------------------------------------------------------
revno: 4906
revision-id: john at arbash-meinel.com-20111008114800-ldcbe9fr0wqs7dkm
parent: john at arbash-meinel.com-20111008102315-5y0yk3bnxhpa86el
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: 2.1-categorize-requests-819604
timestamp: Sat 2011-10-08 13:48:00 +0200
message:
  Categorize all of the requests as safe/unsafe/semi/stream for retrying purposes.
-------------- next part --------------
=== modified file 'bzrlib/smart/request.py'
--- a/bzrlib/smart/request.py	2010-02-17 17:11:16 +0000
+++ b/bzrlib/smart/request.py	2011-10-08 11:48:00 +0000
@@ -486,152 +486,197 @@
         return SuccessfulSmartServerResponse((answer,))
 
 
+# In the 'info' attribute, we store whether this request is 'safe' to retry if
+# we get a disconnect while reading the response. It can have the values:
+#   safe    The request is strictly idempotent, calling it twice results in
+#           the same result as calling it one time. This includes all read-only
+#           requests, and write requests like 'put' where the end result is
+#           identical content.
+#   unsafe  A request which is unsafe means that state is updated in a way that
+#           replaying that request results in a different state. For example
+#           'append' writes more bytes to a given file. If append succeeds, it
+#           moves the file pointer.
+#   semi    This is a request that isn't strictly idempotent, but doesn't
+#           result in corruption if it is retried. This is for things like
+#           'lock' and 'unlock'. If you call lock, it updates the disk
+#           structure. If you fail to read the response, you won't be able to
+#           use the lock, because you don't have the lock token. Calling lock
+#           again will fail, because the lock is already taken. However, we
+#           can't tell if the server received our request or not. If it didn't,
+#           then retrying the request is fine, as it will actually do what we
+#           want. If it did, we will interrupt the current operation, but we
+#           are no worse off than interrupting the current operation because of
+#           a ConnectionReset.
+#   stream  This is a request that takes a stream that cannot be restarted if
+#           consumed. This request is 'safe' in that if we determine the
+#           connection is closed before we consume the stream, we can try
+#           again.
 request_handlers = registry.Registry()
 request_handlers.register_lazy(
-    'append', 'bzrlib.smart.vfs', 'AppendRequest')
+    'append', 'bzrlib.smart.vfs', 'AppendRequest', info='unsafe')
 request_handlers.register_lazy(
     'Branch.get_config_file', 'bzrlib.smart.branch',
-    'SmartServerBranchGetConfigFile')
+    'SmartServerBranchGetConfigFile', info='safe')
 request_handlers.register_lazy(
-    'Branch.get_parent', 'bzrlib.smart.branch', 'SmartServerBranchGetParent')
+    'Branch.get_parent', 'bzrlib.smart.branch', 'SmartServerBranchGetParent',
+    info='safe')
 request_handlers.register_lazy(
     'Branch.get_tags_bytes', 'bzrlib.smart.branch',
-    'SmartServerBranchGetTagsBytes')
+    'SmartServerBranchGetTagsBytes', info='safe')
 request_handlers.register_lazy(
     'Branch.set_tags_bytes', 'bzrlib.smart.branch',
-    'SmartServerBranchSetTagsBytes')
-request_handlers.register_lazy(
-    'Branch.get_stacked_on_url', 'bzrlib.smart.branch', 'SmartServerBranchRequestGetStackedOnURL')
-request_handlers.register_lazy(
-    'Branch.last_revision_info', 'bzrlib.smart.branch', 'SmartServerBranchRequestLastRevisionInfo')
-request_handlers.register_lazy(
-    'Branch.lock_write', 'bzrlib.smart.branch', 'SmartServerBranchRequestLockWrite')
+    'SmartServerBranchSetTagsBytes', info='safe')
+request_handlers.register_lazy(
+    'Branch.get_stacked_on_url', 'bzrlib.smart.branch',
+    'SmartServerBranchRequestGetStackedOnURL', info='safe')
+request_handlers.register_lazy(
+    'Branch.last_revision_info', 'bzrlib.smart.branch',
+    'SmartServerBranchRequestLastRevisionInfo', info='safe')
+request_handlers.register_lazy(
+    'Branch.lock_write', 'bzrlib.smart.branch',
+    'SmartServerBranchRequestLockWrite', info='semi')
 request_handlers.register_lazy( 'Branch.revision_history',
-    'bzrlib.smart.branch', 'SmartServerRequestRevisionHistory')
+    'bzrlib.smart.branch', 'SmartServerRequestRevisionHistory',
+    info='safe')
 request_handlers.register_lazy( 'Branch.set_config_option',
-    'bzrlib.smart.branch', 'SmartServerBranchRequestSetConfigOption')
+    'bzrlib.smart.branch', 'SmartServerBranchRequestSetConfigOption',
+    info='safe')
 request_handlers.register_lazy( 'Branch.set_last_revision',
-    'bzrlib.smart.branch', 'SmartServerBranchRequestSetLastRevision')
+    'bzrlib.smart.branch', 'SmartServerBranchRequestSetLastRevision',
+    info='safe')
 request_handlers.register_lazy(
     'Branch.set_last_revision_info', 'bzrlib.smart.branch',
-    'SmartServerBranchRequestSetLastRevisionInfo')
+    'SmartServerBranchRequestSetLastRevisionInfo', info='safe')
 request_handlers.register_lazy(
     'Branch.set_last_revision_ex', 'bzrlib.smart.branch',
-    'SmartServerBranchRequestSetLastRevisionEx')
+    'SmartServerBranchRequestSetLastRevisionEx', info='safe')
 request_handlers.register_lazy(
     'Branch.set_parent_location', 'bzrlib.smart.branch',
-    'SmartServerBranchRequestSetParentLocation')
+    'SmartServerBranchRequestSetParentLocation', info='safe')
 request_handlers.register_lazy(
-    'Branch.unlock', 'bzrlib.smart.branch', 'SmartServerBranchRequestUnlock')
+    'Branch.unlock', 'bzrlib.smart.branch', 'SmartServerBranchRequestUnlock',
+    info='safe')
 request_handlers.register_lazy(
     'BzrDir.cloning_metadir', 'bzrlib.smart.bzrdir',
-    'SmartServerBzrDirRequestCloningMetaDir')
+    'SmartServerBzrDirRequestCloningMetaDir', info='safe')
 request_handlers.register_lazy(
     'BzrDir.create_branch', 'bzrlib.smart.bzrdir',
-    'SmartServerRequestCreateBranch')
+    'SmartServerRequestCreateBranch', info='semi')
 request_handlers.register_lazy(
     'BzrDir.create_repository', 'bzrlib.smart.bzrdir',
-    'SmartServerRequestCreateRepository')
+    'SmartServerRequestCreateRepository', info='semi')
 request_handlers.register_lazy(
     'BzrDir.find_repository', 'bzrlib.smart.bzrdir',
-    'SmartServerRequestFindRepositoryV1')
+    'SmartServerRequestFindRepositoryV1', info='safe')
 request_handlers.register_lazy(
     'BzrDir.find_repositoryV2', 'bzrlib.smart.bzrdir',
-    'SmartServerRequestFindRepositoryV2')
+    'SmartServerRequestFindRepositoryV2', info='safe')
 request_handlers.register_lazy(
     'BzrDir.find_repositoryV3', 'bzrlib.smart.bzrdir',
-    'SmartServerRequestFindRepositoryV3')
+    'SmartServerRequestFindRepositoryV3', info='safe')
 request_handlers.register_lazy(
     'BzrDir.get_config_file', 'bzrlib.smart.bzrdir',
-    'SmartServerBzrDirRequestConfigFile')
+    'SmartServerBzrDirRequestConfigFile', info='safe')
 request_handlers.register_lazy(
     'BzrDirFormat.initialize', 'bzrlib.smart.bzrdir',
-    'SmartServerRequestInitializeBzrDir')
+    'SmartServerRequestInitializeBzrDir', info='semi')
 request_handlers.register_lazy(
     'BzrDirFormat.initialize_ex_1.16', 'bzrlib.smart.bzrdir',
-    'SmartServerRequestBzrDirInitializeEx')
-request_handlers.register_lazy(
-    'BzrDir.open', 'bzrlib.smart.bzrdir', 'SmartServerRequestOpenBzrDir')
-request_handlers.register_lazy(
-    'BzrDir.open_2.1', 'bzrlib.smart.bzrdir', 'SmartServerRequestOpenBzrDir_2_1')
+    'SmartServerRequestBzrDirInitializeEx', info='semi')
+request_handlers.register_lazy(
+    'BzrDir.open', 'bzrlib.smart.bzrdir', 'SmartServerRequestOpenBzrDir',
+    info='safe')
+request_handlers.register_lazy(
+    'BzrDir.open_2.1', 'bzrlib.smart.bzrdir',
+    'SmartServerRequestOpenBzrDir_2_1', info='safe')
 request_handlers.register_lazy(
     'BzrDir.open_branch', 'bzrlib.smart.bzrdir',
-    'SmartServerRequestOpenBranch')
+    'SmartServerRequestOpenBranch', info='safe')
 request_handlers.register_lazy(
     'BzrDir.open_branchV2', 'bzrlib.smart.bzrdir',
-    'SmartServerRequestOpenBranchV2')
+    'SmartServerRequestOpenBranchV2', info='safe')
 request_handlers.register_lazy(
     'BzrDir.open_branchV3', 'bzrlib.smart.bzrdir',
-    'SmartServerRequestOpenBranchV3')
-request_handlers.register_lazy(
-    'delete', 'bzrlib.smart.vfs', 'DeleteRequest')
-request_handlers.register_lazy(
-    'get', 'bzrlib.smart.vfs', 'GetRequest')
-request_handlers.register_lazy(
-    'get_bundle', 'bzrlib.smart.request', 'GetBundleRequest')
-request_handlers.register_lazy(
-    'has', 'bzrlib.smart.vfs', 'HasRequest')
-request_handlers.register_lazy(
-    'hello', 'bzrlib.smart.request', 'HelloRequest')
-request_handlers.register_lazy(
-    'iter_files_recursive', 'bzrlib.smart.vfs', 'IterFilesRecursiveRequest')
-request_handlers.register_lazy(
-    'list_dir', 'bzrlib.smart.vfs', 'ListDirRequest')
-request_handlers.register_lazy(
-    'mkdir', 'bzrlib.smart.vfs', 'MkdirRequest')
-request_handlers.register_lazy(
-    'move', 'bzrlib.smart.vfs', 'MoveRequest')
-request_handlers.register_lazy(
-    'put', 'bzrlib.smart.vfs', 'PutRequest')
-request_handlers.register_lazy(
-    'put_non_atomic', 'bzrlib.smart.vfs', 'PutNonAtomicRequest')
-request_handlers.register_lazy(
-    'readv', 'bzrlib.smart.vfs', 'ReadvRequest')
-request_handlers.register_lazy(
-    'rename', 'bzrlib.smart.vfs', 'RenameRequest')
+    'SmartServerRequestOpenBranchV3', info='safe')
+request_handlers.register_lazy(
+    'delete', 'bzrlib.smart.vfs', 'DeleteRequest', info='semi')
+request_handlers.register_lazy(
+    'get', 'bzrlib.smart.vfs', 'GetRequest', info='safe')
+request_handlers.register_lazy(
+    'get_bundle', 'bzrlib.smart.request', 'GetBundleRequest', info='safe')
+request_handlers.register_lazy(
+    'has', 'bzrlib.smart.vfs', 'HasRequest', info='safe')
+request_handlers.register_lazy(
+    'hello', 'bzrlib.smart.request', 'HelloRequest', info='safe')
+request_handlers.register_lazy(
+    'iter_files_recursive', 'bzrlib.smart.vfs', 'IterFilesRecursiveRequest',
+    info='safe')
+request_handlers.register_lazy(
+    'list_dir', 'bzrlib.smart.vfs', 'ListDirRequest', info='safe')
+request_handlers.register_lazy(
+    'mkdir', 'bzrlib.smart.vfs', 'MkdirRequest', info='semi')
+request_handlers.register_lazy(
+    'move', 'bzrlib.smart.vfs', 'MoveRequest', info='semi')
+request_handlers.register_lazy(
+    'put', 'bzrlib.smart.vfs', 'PutRequest', info='safe')
+request_handlers.register_lazy(
+    'put_non_atomic', 'bzrlib.smart.vfs', 'PutNonAtomicRequest', info='safe')
+request_handlers.register_lazy(
+    'readv', 'bzrlib.smart.vfs', 'ReadvRequest', info='safe')
+request_handlers.register_lazy(
+    'rename', 'bzrlib.smart.vfs', 'RenameRequest', info='semi')
 request_handlers.register_lazy(
     'PackRepository.autopack', 'bzrlib.smart.packrepository',
-    'SmartServerPackRepositoryAutopack')
-request_handlers.register_lazy('Repository.gather_stats',
-                               'bzrlib.smart.repository',
-                               'SmartServerRepositoryGatherStats')
-request_handlers.register_lazy('Repository.get_parent_map',
-                               'bzrlib.smart.repository',
-                               'SmartServerRepositoryGetParentMap')
-request_handlers.register_lazy(
-    'Repository.get_revision_graph', 'bzrlib.smart.repository', 'SmartServerRepositoryGetRevisionGraph')
-request_handlers.register_lazy(
-    'Repository.has_revision', 'bzrlib.smart.repository', 'SmartServerRequestHasRevision')
-request_handlers.register_lazy(
-    'Repository.insert_stream', 'bzrlib.smart.repository', 'SmartServerRepositoryInsertStream')
-request_handlers.register_lazy(
-    'Repository.insert_stream_1.19', 'bzrlib.smart.repository', 'SmartServerRepositoryInsertStream_1_19')
-request_handlers.register_lazy(
-    'Repository.insert_stream_locked', 'bzrlib.smart.repository', 'SmartServerRepositoryInsertStreamLocked')
-request_handlers.register_lazy(
-    'Repository.is_shared', 'bzrlib.smart.repository', 'SmartServerRepositoryIsShared')
-request_handlers.register_lazy(
-    'Repository.lock_write', 'bzrlib.smart.repository', 'SmartServerRepositoryLockWrite')
+    'SmartServerPackRepositoryAutopack', info='safe')
+request_handlers.register_lazy(
+    'Repository.gather_stats', 'bzrlib.smart.repository',
+    'SmartServerRepositoryGatherStats', info='safe')
+request_handlers.register_lazy(
+    'Repository.get_parent_map', 'bzrlib.smart.repository',
+    'SmartServerRepositoryGetParentMap', info='safe')
+request_handlers.register_lazy(
+    'Repository.get_revision_graph', 'bzrlib.smart.repository',
+    'SmartServerRepositoryGetRevisionGraph', info='safe')
+request_handlers.register_lazy(
+    'Repository.has_revision', 'bzrlib.smart.repository',
+    'SmartServerRequestHasRevision', info='safe')
+request_handlers.register_lazy(
+    'Repository.insert_stream', 'bzrlib.smart.repository',
+    'SmartServerRepositoryInsertStream', info='stream')
+request_handlers.register_lazy(
+    'Repository.insert_stream_1.19', 'bzrlib.smart.repository',
+    'SmartServerRepositoryInsertStream_1_19', info='stream')
+request_handlers.register_lazy(
+    'Repository.insert_stream_locked', 'bzrlib.smart.repository',
+    'SmartServerRepositoryInsertStreamLocked', info='stream')
+request_handlers.register_lazy(
+    'Repository.is_shared', 'bzrlib.smart.repository',
+    'SmartServerRepositoryIsShared', info='safe')
+request_handlers.register_lazy(
+    'Repository.lock_write', 'bzrlib.smart.repository',
+    'SmartServerRepositoryLockWrite', info='semi')
 request_handlers.register_lazy(
     'Repository.set_make_working_trees', 'bzrlib.smart.repository',
-    'SmartServerRepositorySetMakeWorkingTrees')
+    'SmartServerRepositorySetMakeWorkingTrees', info='safe')
 request_handlers.register_lazy(
-    'Repository.unlock', 'bzrlib.smart.repository', 'SmartServerRepositoryUnlock')
+    'Repository.unlock', 'bzrlib.smart.repository',
+    'SmartServerRepositoryUnlock', info='semi')
 request_handlers.register_lazy(
     'Repository.get_rev_id_for_revno', 'bzrlib.smart.repository',
-    'SmartServerRepositoryGetRevIdForRevno')
+    'SmartServerRepositoryGetRevIdForRevno', info='safe')
 request_handlers.register_lazy(
     'Repository.get_stream', 'bzrlib.smart.repository',
-    'SmartServerRepositoryGetStream')
+    'SmartServerRepositoryGetStream', info='safe')
 request_handlers.register_lazy(
     'Repository.get_stream_1.19', 'bzrlib.smart.repository',
-    'SmartServerRepositoryGetStream_1_19')
+    'SmartServerRepositoryGetStream_1_19', info='safe')
 request_handlers.register_lazy(
     'Repository.tarball', 'bzrlib.smart.repository',
-    'SmartServerRepositoryTarball')
-request_handlers.register_lazy(
-    'rmdir', 'bzrlib.smart.vfs', 'RmdirRequest')
-request_handlers.register_lazy(
-    'stat', 'bzrlib.smart.vfs', 'StatRequest')
-request_handlers.register_lazy(
-    'Transport.is_readonly', 'bzrlib.smart.request', 'SmartServerIsReadonly')
+    'SmartServerRepositoryTarball', info='safe')
+request_handlers.register_lazy(
+    'rmdir', 'bzrlib.smart.vfs', 'RmdirRequest', info='semi')
+request_handlers.register_lazy(
+    'stat', 'bzrlib.smart.vfs', 'StatRequest', info='safe')
+request_handlers.register_lazy(
+    'Transport.is_readonly', 'bzrlib.smart.request', 'SmartServerIsReadonly',
+    info='safe')

=== modified file 'bzrlib/tests/test_smart_request.py'
--- a/bzrlib/tests/test_smart_request.py	2009-07-27 02:11:25 +0000
+++ b/bzrlib/tests/test_smart_request.py	2011-10-08 11:48:00 +0000
@@ -109,6 +109,17 @@
         self.assertEqual(
             [[transport]] * 3, handler._command.jail_transports_log)
 
+    def test_all_registered_requests_are_safety_qualified(self):
+        unclassified_requests = []
+        for key in request.request_handlers.keys():
+            info = request.request_handlers.get_info(key)
+            if info is None or info not in ('safe', 'unsafe', 'semi', 'stream'):
+                unclassified_requests.append(key)
+        if unclassified_requests:
+            self.fail('These requests were not categorized as safe/unsafe'
+                      ' to retry: %s'  % (unclassified_requests,))
+        
+
 
 
 class TestSmartRequestHandlerErrorTranslation(TestCase):



More information about the bazaar-commits mailing list