Avoid gRPC retries for non-idempotent KV operations on gRPC UNAVAILABLE as default

[jcferretti]

Versions

• etcd: N/A
• jetcd: 0.8.3; also impacts earlier versions since 0.6.0
• java: N/A

Describe the bug
Jepsen report https://jepsen.io/analyses/jetcd-0.8.2 explains issues that can occur due to automated retries of non-idempotent operations. The issue has been discussed at length in several etcd github issues (listed below), and some of the conclusions of those discussions captured for etcd clients on etcd issue 18424. 18424 describes problems in detail.

To Reproduce
Refer to previous paragraph

Expected behavior
Non-idempotent operations like writes and transactions should not retry by default when it is not known at the client if the server may have performed the operation. Eg, retrying an arbitrary etcd Put RPC on an auth token expired error is safe, retrying it on a general gRPC UNAVAILABLE error is not

Additional context

• See the description for UNAVAILABLE in the gRPC documentation https://grpc.github.io/grpc/core/md_doc_statuscodes.html stating "Note that it is not always safe to retry non-idempotent operations"
• Jepsen originally created jetcd issue 1072 but the understanding of the root casue of the problem was less clear at that point; the discussions on etcd issues and further jepsen research since have shed more light, clearly identifying the originally reported behavior as being a direct result of automated retries of non-idempotent operations (TXN in the original jetcd issue report 1072).
• The etcd issues filed by jepsen whose discussion led to the conclusions in 18424 are mentioned in the jepsen report, section 4. "Discussion", and are listed below for convenience, note the last one 14890 is the one that generated engagement from the etcd team and led eventually to the creation of 18424:
  14092
  14890

Proposed change
I propose we separate the current implementation of isRetryable

jetcd/jetcd-core/src/main/java/io/etcd/jetcd/support/Errors.java

Line 30 in a240dc6

return Status.UNAVAILABLE.getCode().equals(status.getCode()) || isInvalidTokenError(status)

and create two distinct functions, one for idempotent operations (eg, Get RPC), and another one for non-idempotent operations (eg, Put, Txn RPCs). The current implementation of isRetryable is good for retries of idempotent ops. The non-idempotent version would remove Status.UNAVAILABLE from the or (||) expression to prevent retries on that case.
Locations that reference isRetryable today can be modified to call the appropriate new version, eg:

• For a Get RPC we can call the idempotent version of isRetryable:
  

  jetcd/jetcd-core/src/main/java/io/etcd/jetcd/impl/KVImpl.java

  Line 84 in a240dc6

  Errors::isRetryable);

• For a Put RPC we can call the non-idempotent version of isRetryable:
  

  jetcd/jetcd-core/src/main/java/io/etcd/jetcd/impl/KVImpl.java

  Line 68 in a240dc6

  Errors::isRetryable);

There is value in providing users with the ability to ask for automated retry of operations that due to their use case they know will be safe, eg, ensuring a particular Put RPC goes through may be preferable in some particular case even if it ends up resulting in two writes in the server; we can give users the ability to request automated retries of non-idempotent calls but we shouldn't make it the default.