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

Do not retry a read operation when in a transaction #982

Merged
merged 4 commits into from
Jul 25, 2022

Conversation

jyemin
Copy link
Contributor

@jyemin jyemin commented Jul 22, 2022

Fixes a regression introduced in 4.4.0 in scope of JAVA-4034, and
was undetected due to the lack of a specification test for the
required behavior.

JAVA-4684

Fixes a regression introduced in 4.4.0 in scope of JAVA-4034, and
was undetected due to the lack of a specification test for the
required behavior.

JAVA-4684
@jyemin jyemin self-assigned this Jul 22, 2022
@jyemin jyemin requested review from rozza and katcharov July 22, 2022 17:07
@@ -437,21 +437,12 @@ static boolean canRetryWrite(final ServerDescription serverDescription, final Co
return true;
}

static boolean isRetryableRead(final boolean retryReads, final ServerDescription serverDescription,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In JAVA-4034, use of this method was removed with the addition of the RetryState class, but that caused us to lost the check for sessionContext.hasActiveTransaction(). Since the RetryState class already has the check for whether retryReads is enabled, I chose to just remove this unused method and move that check into canRetryRead below.

@jyemin jyemin merged commit afb5d0a into mongodb:master Jul 25, 2022
@jyemin jyemin deleted the j4684 branch July 25, 2022 12:33
jyemin added a commit that referenced this pull request Jul 25, 2022
Fixes a regression introduced in 4.4.0 in scope of JAVA-4034, and
was undetected due to the lack of a specification test for the
required behavior.

JAVA-4684
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants