I was wondering what the current status of this PR is and if we might see changes to commit_unknown_result in the next 6-12 months?
commit_unknown_result is a retry able error in most language bindings including Rust. Something that I am considering doing in my layer, is to make this a non-retryable error. Has this approach been tried before? If so, can you please share your experience.
@andrew.noyes had a different idea for this problem, but I can’t seem to find his write up at the moment.
My PR would mostly need to be correctness and performance tested. But currently this doesn’t have a high priority.
What’s the reasoning for this? I understand commit_unknown_result is harder to handle for applications, but there’s also value in having bindings being consistent. Of course, if you talk about a more high-level layer (I am not sure – are you talking about the rust bindings or some more complex layer?), this might make much more sense.
For a layer I think it would be cool if they would solve the problem and make every transaction idempotent. For idempotent transactions commit_unknown_result is a non-issue. Depending on the data model your layer uses this might be much easier than solving the problem in FDB.
Here’s the write-up: foundationdb/idempotency_keys.md at anoyes/idempotency-keys-design · sfc-gh-anoyes/foundationdb · GitHub.
It’s actually a bit out of date (one thing I plan to do is rewrite it to use the terminology of “idempotency id” instead of “idempotency key” because so far it has been confusing lol), but then main ideas are there. I would not plan on me landing this in the next 6-12 months, although it is a possibility.
Sorry, my last comment was not clear. I was referring to higher-level layer that I would build on top of the rust bindings. I had no plans of changing the behavior of rust bindings.
Since I started this thread, I’ve been thinking more about this issue and have sketched out an initial design for fdb-rl crate. It uses ideas from your design, but tries to implement it in a layer. 
I’ll share the design document early next week.
@markus.pilman @andrew.noyes here is the fdb-rl Rust Create Idempotency Design document.
I would really appreciate if you could please review it and let me know your feedback.
Also if you can suggest other FDB community members whose feedback would be valuable, please let me know and I will also request them for their inputs.
Thanks in advance!
I took a look - makes sense to me. One thing I’ll mention is that the way providing automatic idempotency interacts with setting a transaction timeout is very subtle and slightly different from commit_unknown_result. Before replying with commit_unknown_result the fdb client ensures that the commit is no longer in-flight, which is important. A commit that fails with transaction_timed_out is still in-flight, so it could commit after you read the sentinel key.
Also maybe I missed it but I don’t think the doc mentions what happens if the sentinel key could have been garbage-collected when you read it.
Thanks a lot for mentioning the issue with transaction_timed_out. I completely forgot about it!
You had also mentioned the following in a previous thread.
Are there any other error codes similar to cluster_version_changed that has the behavior of being retryable while the transaction is still in-flight?
In order to correctly deal with in-flight transactions for non-idempotent transactions, would the following mechanism work?
In the event we receive a transaction_timed_out or cluster_version_changed, wait for a specific duration of time, for example 10 seconds.
The in-flight transaction would have, by this time, either been committed, or the transaction would have been rejected due to the 5 second limit.
After 10 seconds, attempt to read the sentinel key again. This time if we are able to read the sentinel key, that would mean a successful commit. Missing sentinel key would mean the previous transaction failed to commit and would also not commit in the future.
That would be an error in the garbage collection algorithm. For the design to work correctly, the garbage collection process must leave a threshold (for example: 72 hours) of sentinel keys untouched.
I’ve updated the document reflect this (Tracking is now enabled, so you can easily find the edit).
I think that’s the only one.
The mechanism foundationdb uses internally to ensure that a commit is no longer in-flight is to commit a transaction with a write conflict range that intersects the in-flight transaction’s read conflict range, or to get a read version that’s more than MAX_WRITE_TRANSACTION_LIFE_VERSIONS (5e6) beyond that in-flight transaction’s read version. Clock-based approaches have the downside of relying on clocks being trustworthy.
No matter what threshold you choose it’s still theoretically possible to attempt to read a sentinel key that would have been garbage collected.
Thanks for the reply. Maybe I can use a very similar approach. Please let me know your thoughts on the following.
Currently the sentinel key-value has the following form.
TenantRoot/(TENANT_METADATA,
IDEMPOTENCY_DATA,
incarnation: int,
read_version: i64,
idempotency_uuid: uuid,) = (commit_vs: versionstamp,)
I could also introduce a “conflict” key range of the following form. There would be a one-to-one correspondence between a given sentinel-key and its associated “conflict” range.
(No data would be written in the “conflict” range).
TenantRoot/(TENANT_METADATA,
IDEMPOTENCY_CONFLICT_RANGE,
incarnation: int,
read_version: i64,
idempotency_uuid: uuid,)
When a transaction first attempts to write the sentinel key, I would include a read conflict range on the sentinel key’s “conflict” range. Lets name this transaction T1 and the conflict range T1_Conflict_Range.
In the event T1 fails with transaction_timed_out or cluster_version_changed, before attempting a retry, I would create a new transaction T2.
Transaction T2 would be very simple transaction that would attempt to do a write conflict range on T1_Conflict_Range, and commit.
After T2 successfully commits, I can attempt the retry logic. A successful T2 commit would indicate
In either of the cases after T2 commits successfully, I should be able to rely on the presence or absence of the sentinel-key for subsequent decisions.
Do you see any potential issues here?
@andrew.noyes As I was updating the design document, I noticed a discrepancy between documentation and a forum post for transaction_timed_out and transaction_cancelled.
In the FDB documentation section for Non-Retryable Errors, it is mentioned that we will get a transaction_timed_out error in-case we set a timeout in our transaction options.
However, this forum post by @alloc seems to indicate that we will get a transaction_cancelled error.
I was wondering if there was some subtlety that I might be missing here?
In order understand the error conditions better, I’ve created the following table. Could you please let me know in case there is a mistake.
| Error Name | on_error retry? | Possible in-flight transaction |
|---|---|---|
commit_unknown_result |
Yes | No |
cluster_version_changed |
Yes | Yes |
operation_cancelled |
No | Yes |
transaction_timed_out |
No | Yes |
transaction_cancelled |
??? | ??? |
Regarding transaction_cancelled, is it something on_error automatically retries? Also from a FDB client perspective, does the possibility of an in-flight transaction exist when we get a transaction_cancelled error?
I confirmed that a transaction which sets a timeout fails with error code transaction_timed_out (error code 1031), so it’s a different error than if you cancel the transaction.
Thanks @andrew.noyes. I noticed another potential inconsistency. In the section on Transaction cancellation, in the python code documentation it says,
On the 6th time, on_error() will throw retry_limit_exceeded rather than resetting and retrying.
I was unable to find retry_limit_exceeded in the list of documented error codes.
What error code will we get when the retry limit is exceeded?
That seems to be an error in the documentation. If you hit the retry limit, it should just return the error that you last encountered (i.e. it could be transaction_too_old, not_committed, etc.).
@ajbeamon @andrew.noyes @markus.pilman Thanks a lot for the replies to this thread.
I think design doc almost done. Below is the link to the latest version. When you get a chance, please give it a read, and let me know your feedback.
The design doc turned out to longer than what I had initially anticipated. I’ve tried to be exhaustive and enumerate all possible issues that can occur when working with non-idempotent transactions and provided links to relevant sections in the documentation and previous forum posts.
I am hoping this information will be useful to future language binding authors as well when they try to implement similar API for their language.