Comment 16 for bug 819604

Revision history for this message
John A Meinel (jameinel) wrote :

Analysis of whether any given RPC is safe to retry. Because of potential
corruption, I'm tempted to whitelist them instead of blacklist them. However,
99% of requests are safe, so whitelisting is a bit of a pain.

I'm thinking to use the 'info' parameter of register_lazy to signal the
retry-semantics of a given command.

    'append', 'bzrlib.smart.vfs', 'AppendRequest')
Not safe. The current location in the file is tracked on the client for some
callers of this function. An append which succeeded but then got retried will
put bad data in the middle of the file.

    'Branch.get_config_file', 'bzrlib.smart.branch',
    'SmartServerBranchGetConfigFile')
safe, read-only

    'Branch.get_parent', 'bzrlib.smart.branch', 'SmartServerBranchGetParent')
safe, read-only

    'Branch.get_tags_bytes', 'bzrlib.smart.branch',
    'SmartServerBranchGetTagsBytes')
safe, read-only

    'Branch.set_tags_bytes', 'bzrlib.smart.branch',
    'SmartServerBranchSetTagsBytes')
safe, this is an all-or-nothing set. so setting it two times to the same thing
still leaves you at the same content in the end.

    'Branch.get_stacked_on_url', 'bzrlib.smart.branch', 'SmartServerBranchRequestGetStackedOnURL')
safe, read-only

    'Branch.last_revision_info', 'bzrlib.smart.branch', 'SmartServerBranchRequestLastRevisionInfo')
safe, read-only

    'Branch.lock_write', 'bzrlib.smart.branch', 'SmartServerBranchRequestLockWrite')
safe, will fail if first succeeded
safe in that it won't cause corruption. not safe in that if the first request
succeeds and the connection dies (so we don't get to see the lock tocken), the
second request will fail because the branch is already locked.
I'm still willing to call this 'safe', since it won't silently corrupt
anything. The worst case is that the action will get interrupted, but we
already had that problem because of ConnectionReset. (Retrying is strictly
better than not retrying.)

    'Branch.revision_history', 'bzrlib.smart.branch', 'SmartServerRequestRevisionHistory')
safe, read-only

    'Branch.set_config_option', 'bzrlib.smart.branch', 'SmartServerBranchRequestSetConfigOption')
I think this is safe. Setting an option to the same value twice should be safe.
The config code already should handle multiple concurrent requests.

    'Branch.set_last_revision', 'bzrlib.smart.branch', 'SmartServerBranchRequestSetLastRevision')
deprecated function, since it uses branch._lefthand_history to determine the
revision number to store. However, still safe, as setting the tip revision to
the same value is safe.

    'Branch.set_last_revision_info', 'bzrlib.smart.branch', 'SmartServerBranchRequestSetLastRevisionInfo')
safe

    'Branch.set_last_revision_ex', 'bzrlib.smart.branch', 'SmartServerBranchRequestSetLastRevisionEx')
safe

    'Branch.set_parent_location', 'bzrlib.smart.branch', 'SmartServerBranchRequestSetParentLocation')
safe

    'Branch.unlock', 'bzrlib.smart.branch', 'SmartServerBranchRequestUnlock')
safe, will fail if first succeeded
same as the lock_write call. If the first one succeeds, the second one will
fail with a 'not locked' and cause us to spuriously interrupt what we are
doing. However, it is still 'safe' in that we would have already interrupted
the process because of ConnectionReset.

    'BzrDir.cloning_metadir', 'bzrlib.smart.bzrdir', 'SmartServerBzrDirRequestCloningMetaDir')
safe, read-only

    'BzrDir.create_branch', 'bzrlib.smart.bzrdir', 'SmartServerRequestCreateBranch')
safe, will fail if first succeeded

    'BzrDir.create_repository', 'bzrlib.smart.bzrdir', 'SmartServerRequestCreateRepository')
safe, will fail if first succeeded

    'BzrDir.find_repository', 'bzrlib.smart.bzrdir', 'SmartServerRequestFindRepositoryV1')
safe, read-only

    'BzrDir.find_repositoryV2', 'bzrlib.smart.bzrdir', 'SmartServerRequestFindRepositoryV2')
safe, read-only

    'BzrDir.find_repositoryV3', 'bzrlib.smart.bzrdir', 'SmartServerRequestFindRepositoryV3')
safe, read-only

    'BzrDir.get_config_file', 'bzrlib.smart.bzrdir', 'SmartServerBzrDirRequestConfigFile')
safe, read-only

    'BzrDirFormat.initialize', 'bzrlib.smart.bzrdir', 'SmartServerRequestInitializeBzrDir')
safe, will fail if first succeeded

    'BzrDirFormat.initialize_ex_1.16', 'bzrlib.smart.bzrdir', 'SmartServerRequestBzrDirInitializeEx')
safe, will fail if first succeeded

    'BzrDir.open', 'bzrlib.smart.bzrdir', 'SmartServerRequestOpenBzrDir')
safe, read-only

    'BzrDir.open_2.1', 'bzrlib.smart.bzrdir', 'SmartServerRequestOpenBzrDir_2_1')
safe, read-only

    'BzrDir.open_branch', 'bzrlib.smart.bzrdir', 'SmartServerRequestOpenBranch')
safe, read-only

    'BzrDir.open_branchV2', 'bzrlib.smart.bzrdir', 'SmartServerRequestOpenBranchV2')
safe, read-only

    'BzrDir.open_branchV3', 'bzrlib.smart.bzrdir', 'SmartServerRequestOpenBranchV3')
safe, read-only

    'delete', 'bzrlib.smart.vfs', 'DeleteRequest')
safe, will fail if first succeeded

    'get', 'bzrlib.smart.vfs', 'GetRequest')
safe, read-only

    'get_bundle', 'bzrlib.smart.request', 'GetBundleRequest')
safe, read-only

    'has', 'bzrlib.smart.vfs', 'HasRequest')
safe, read-only

    'hello', 'bzrlib.smart.request', 'HelloRequest')
safe, read-only

    'iter_files_recursive', 'bzrlib.smart.vfs', 'IterFilesRecursiveRequest')
safe, read-only

    'list_dir', 'bzrlib.smart.vfs', 'ListDirRequest')
safe, read-only

    'mkdir', 'bzrlib.smart.vfs', 'MkdirRequest')
safe, will fail if first succeeded

    'move', 'bzrlib.smart.vfs', 'MoveRequest')
safe, will fail if first succeeded

    'put', 'bzrlib.smart.vfs', 'PutRequest')
safe, putting the same content twice leaves you with the same content

    'put_non_atomic', 'bzrlib.smart.vfs', 'PutNonAtomicRequest')
safe, putting the same content twice leaves you with the same content

    'readv', 'bzrlib.smart.vfs', 'ReadvRequest')
safe, read-only

    'rename', 'bzrlib.smart.vfs', 'RenameRequest')
safe, will fail if first succeeded

    'PackRepository.autopack', 'bzrlib.smart.packrepository', 'SmartServerPackRepositoryAutopack')
safe, a bit unfortunate if it actually does the work twice, but I think we have
good "oh, i'm already well packed, nothing to do" logic around here already.

    'Repository.gather_stats', 'bzrlib.smart.repository', 'SmartServerRepositoryGatherStats')
safe, read-only

    'Repository.get_parent_map', 'bzrlib.smart.repository', 'SmartServerRepositoryGetParentMap')
safe, read-only

    'Repository.get_revision_graph', 'bzrlib.smart.repository', 'SmartServerRepositoryGetRevisionGraph')
safe, read-only

    'Repository.has_revision', 'bzrlib.smart.repository', 'SmartServerRequestHasRevision')
safe, read-only

    'Repository.insert_stream', 'bzrlib.smart.repository', 'SmartServerRepositoryInsertStream')
    'Repository.insert_stream_1.19', 'bzrlib.smart.repository', 'SmartServerRepositoryInsertStream_1_19')
    'Repository.insert_stream_locked', 'bzrlib.smart.repository', 'SmartServerRepositoryInsertStreamLocked')
safe, in that a repository is allowed to have the same content multiple times.
Note that from the client-side, it isn't possible to restart this trivially,
because the stream is a generator that gets consumed, and can't be trivially
restarted from the beginning.

    'Repository.is_shared', 'bzrlib.smart.repository', 'SmartServerRepositoryIsShared')
safe, read-only

    'Repository.lock_write', 'bzrlib.smart.repository', 'SmartServerRepositoryLockWrite')
safe, will fail if first succeeded

    'Repository.set_make_working_trees', 'bzrlib.smart.repository', 'SmartServerRepositorySetMakeWorkingTrees')
safe, setting the flag to the same thing twice just leaves us in the same place

    'Repository.unlock', 'bzrlib.smart.repository', 'SmartServerRepositoryUnlock')
safe, will fail if first succeeded

    'Repository.get_rev_id_for_revno', 'bzrlib.smart.repository', 'SmartServerRepositoryGetRevIdForRevno')
safe, read-only

    'Repository.get_stream', 'bzrlib.smart.repository', 'SmartServerRepositoryGetStream')
safe, read-only

    'Repository.get_stream_1.19', 'bzrlib.smart.repository', 'SmartServerRepositoryGetStream_1_19')
safe, read-only

    'Repository.tarball', 'bzrlib.smart.repository', 'SmartServerRepositoryTarball')
safe, read-only, do we even still support this verb? I really thought we wanted
this disabled because it adds too much load to the server, and the client no
longer tries it.

    'rmdir', 'bzrlib.smart.vfs', 'RmdirRequest')
safe, will fail if first succeeded

    'stat', 'bzrlib.smart.vfs', 'StatRequest')
safe, read-only

    'Transport.is_readonly', 'bzrlib.smart.request', 'SmartServerIsReadonly')
safe, read-only