Code review comment for lp:~jameinel/bzr/2.1-categorize-requests-819604

On 8 October 2011 23:01, John A Meinel <email address hidden> wrote:
> John A Meinel has proposed merging lp:~jameinel/bzr/2.1-categorize-requests-819604 into lp:bzr/2.1 with lp:~jameinel/bzr/2.1-client-reconnect-819604 as a prerequisite.
>
> Requested reviews:
>  bzr-core (bzr-core)
>
> For more details, see:
> https://code.launchpad.net/~jameinel/bzr/2.1-categorize-requests-819604/+merge/78708
>
> 1) I analyzed all of the requests that are currently registered as to whether they are safe or not to retry. I ended up with 4 categories:
>
> #   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.

[tweak] "consume the stream" or "start sending the stream"?

The classification looks ok to me too.

You might want to ask spiv too but

  vote approve