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.

Sign up for GitHub

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

DRIVERS-2695 Add a case of CSOT not enabled for the retryable writes. #1466

Merged
merged 2 commits into from
Oct 19, 2023
Merged

DRIVERS-2695 Add a case of CSOT not enabled for the retryable writes. #1466

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a70f3c8
DRIVERS-2695 Add a case of CSOT not enabled for the retryable writes.
qingyang-hu Oct 4, 2023
efc2bfc
DRIVERS-2695 Correct the error propagation logic.
qingyang-hu Oct 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions source/retryable-writes/retryable-writes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,7 @@ The above rules are implemented in the following pseudo-code:
retryableCommand = addTransactionIdToCommand(command, session);

Exception previousError = null;
retrying = false;
while true {
try {
return executeCommand(server, retryableCommand);
Expand All @@ -468,7 +469,7 @@ The above rules are implemented in the following pseudo-code:
* numbers" MUST be re-raised with an actionable error message.
*/
if (!currentError.hasErrorLabel("RetryableWriteError")) {
if ( currentError.code == 20 && originalError.errmsg.startsWith("Transaction numbers") ) {
if ( currentError.code == 20 && previousError.errmsg.startsWith("Transaction numbers") ) {
currentError.errmsg = "This MongoDB deployment does not support retryable...";
}
throw currentError;
Expand All @@ -490,7 +491,7 @@ The above rules are implemented in the following pseudo-code:
* from the connection pool), we should raise the previous error if there
* was one.
*/
if (currentError is not DriverException && ! originalError.hasErrorLabel("NoWritesPerformed")) {
if (currentError is not DriverException && ! previousError.hasErrorLabel("NoWritesPerformed")) {
previousError = currentError;
}
}
Expand All @@ -516,10 +517,18 @@ The above rules are implemented in the following pseudo-code:
throw previousError;
}

/* CSOT is enabled and the operation has timed out. */
if (timeoutMS != null && isExpired(timeoutMS) {
if (timeoutMS == null) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have not merged the conditions to keep the reference code straightforward with the comments.

/* If CSOT is not enabled, allow any retryable error from the second
* attempt to propagate to our caller, as it will be just as relevant
* (if not more relevant) than the original error. */
if (retrying) {
throw previousError;
}
} else if (isExpired(timeoutMS)) {
/* CSOT is enabled and the operation has timed out. */
throw previousError;
}
retrying = true;
}
}

Expand Down Expand Up @@ -829,6 +838,7 @@ inconsistent with the server and potentially confusing to developers.
Changelog
=========

:2023-10-02: When CSOT is not enabled, one retry attempt occurs.
:2023-08-26: Require that in a sharded cluster the server on which the
operation failed MUST be provided to the server selection
mechanism as a deprioritized server.
Expand Down
Loading