Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kvclient: require explicit acknowledgement from client on restart #85980

Closed
ajwerner opened this issue Aug 11, 2022 · 3 comments
Closed

kvclient: require explicit acknowledgement from client on restart #85980

ajwerner opened this issue Aug 11, 2022 · 3 comments
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv KV Team

Comments

@ajwerner
Copy link
Contributor

ajwerner commented Aug 11, 2022

Is your feature request related to a problem? Please describe.

Today, when a transaction hits some restart errors, it increments its epoch and remains open. When that happens a TransactionRetryWithProtoRefreshError makes its way to the client. In various situations we've seen sql code failing to properly propagate the error and update its state machine accordingly (#85677, #84448). In those cases, the transaction in question will silently lose the writes preceding the restart. We've seen other cases where the transaction is aborted internally and needs to be restarted, but methods continue to work.

Describe the solution you'd like

A transaction which has been restarted internally should require an explicit acknowledgement from the client before it becomes usable again. An aborted transaction should return more errors from various methods to prevent misuse.

Jira issue: CRDB-18513

@ajwerner ajwerner added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Aug 11, 2022
@blathers-crl blathers-crl bot added the T-kv KV Team label Aug 11, 2022
@rafiss
Copy link
Collaborator

rafiss commented Aug 11, 2022

@nvanbenschoten points out that this is similar (or identical to) #22615, which was resolved in #74563

@ajwerner
Copy link
Contributor Author

I've been reviewing the information from the one case where I saw the transparent loss of writes and it appears to have been a situation involving LeafTxn objects and distsql. I'll file a separate issue when I figure it out.

@rafiss
Copy link
Collaborator

rafiss commented Aug 11, 2022

I see a098962 (#74563) relates to LeafTxns - just throwing that out there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv KV Team
Projects
None yet
Development

No branches or pull requests

2 participants