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

GODRIVER-2565 Abort transaction before CommitLoop if context errored. #1101

Merged
merged 4 commits into from Oct 31, 2022
Merged

GODRIVER-2565 Abort transaction before CommitLoop if context errored. #1101

merged 4 commits into from Oct 31, 2022

Conversation

benjirewis
Copy link
Contributor

@benjirewis benjirewis commented Oct 25, 2022

GODRIVER-2565

Summary

If the context has errored right before the CommitLoop in WithTransaction, run AbortTransaction and return the context error.

Background & Motivation

This behavior is not specified in the convenient transactions API, as it has to do with contexts (see DRIVERS-2471). If the context has errored after the callback but before CommitLoop, there is no chance the transaction commit will succeed, and resources will be left open server-side. We should run AbortTransaction in this case as a best effort to clean up server-side.

@@ -416,7 +450,7 @@ func TestConvenientTransactions(t *testing.T) {
assert.True(t, ok, "expected result type %T, got %T", false, res)
assert.False(t, resBool, "expected result false, got %v", resBool)
})
t.Run("expired context before commitTransaction does not retry", func(t *testing.T) {
t.Run("expired context before callback does not retry", func(t *testing.T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests were inaccurately named.

@benjirewis benjirewis marked this pull request as ready for review October 26, 2022 14:21
Copy link
Collaborator

@matthewdale matthewdale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

mongo/session.go Show resolved Hide resolved
Copy link
Collaborator

@qingyang-hu qingyang-hu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM

@benjirewis benjirewis merged commit edfc51c into mongodb:master Oct 31, 2022
@benjirewis benjirewis deleted the 2565 branch October 31, 2022 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants