Comment 14 for bug 819604

Some thoughts about idempotent requests.

1) Any read-only request should be idempotent.

2) This leaves requests like put(_bytes) and locking calls. Also, append. Note that RemoteTransport.open_write_stream uses an AppendBasedFileStream structure. So if we are using VFS to write out a remote pack file, we will call 'append $PATH content' repeatedly. 'insert_stream_1.19' is also a concern here.

2a) put_bytes should be an all-or-nothing call. If we repeat it, the final content is going to be the same, we may just be uploading the same content twice.

2b) append seems to be the one that can cause the most problems. bzrlib.pack.ContainerWriter does have start and end records that are potentially written as different append calls. It has a '.begin()' which calls write_func then a '.add_bytes_record()' that creates a string with headers and then calls 'write_func', then a final '.end()' that adds the trailer.

So imagine that any one of those got interrupted via a real network disconnect after the server had gotten the message and completed the request, before the client got the 'ok'. You would end up writing 'begin' two times before an actual record, etc.

To make matters even worse, ContainerWriter tracks 'current_offset'. Which means that if any 'write()' call ended up double sent, the index would no longer line up with the real content on disk.

I had originally thought we might be ok, because the indexes only reference the actual content in the pack file. So we could have ended up with bogus data in the middle, but it wouldn't be referenced. However:
 i) Pack files really do want to be readable without their associated index. They aren't 100% ok today, but leaving garbage in the middle wouldn't be a good way forward.
 ii) Our 'write_func' doesn't have any concept of number-of-bytes written and offset-they-were-written to. At least not at the level of ContainerWriter. We *do* have that concept as part of 'Transport.append()' but it isn't exposed to the other locations such that we could make use of it.

3) So I'm thinking to just figure out a way to flag specific remote 'method' calls as being illegal to retry.

I'm thinking to try and hook it into bzrlib.smart.request.request_handlers. That is a server-side registry, but it is the only place that we know about what methods might be available. And even if a server implements different apis, a client won't ever try a request it doesn't know about. And I don't think a given request will change from being safe to unsafe.