Skip to content

Commit

Permalink
address review
Browse files Browse the repository at this point in the history
  • Loading branch information
blink1073 committed Sep 11, 2024
1 parent 2b89e09 commit da69ee0
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 29 deletions.
24 changes: 12 additions & 12 deletions source/transactions-convenient-api/tests/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ ______________________________________________________________________

The YAML and JSON files in this directory are platform-independent tests meant to exercise a driver's implementation of
the Convenient API for Transactions spec. These tests utilize the
[Unified Test Format](../../unified-test-format/unified-test-format.rst).
[Unified Test Format](../../unified-test-format/unified-test-format.md).

Several prose tests, which are not easily expressed in YAML, are also presented in this file. Those tests will need to
be manually implemented by each driver.
Expand All @@ -29,17 +29,17 @@ Write a callback that returns a custom value (e.g. boolean, string, object). Exe
Drivers should test that `withTransaction` enforces a non-configurable timeout before retrying both commits and entire
transactions. Specifically, three cases should be checked:

> - If the callback raises an error with the TransientTransactionError label and the retry timeout has been exceeded,
> `withTransaction` should propagate the error to its caller.
> - If committing raises an error with the UnknownTransactionCommitResult label, and the retry timeout has been
> exceeded, `withTransaction` should propagate the error to its caller.
> - If committing raises an error with the TransientTransactionError label and the retry timeout has been exceeded,
> `withTransaction` should propagate the error to its caller. This case may occur if the commit was internally retried
> against a new primary after a failover and the second primary returned a NoSuchTransaction error response.
>
> If possible, drivers should implement these tests without requiring the test runner to block for the full duration of
> the retry timeout. This might be done by internally modifying the timeout value used by `withTransaction` with some
> private API or using a mock timer.
- If the callback raises an error with the TransientTransactionError label and the retry timeout has been exceeded,
`withTransaction` should propagate the error to its caller.
- If committing raises an error with the UnknownTransactionCommitResult label, and the retry timeout has been exceeded,
`withTransaction` should propagate the error to its caller.
- If committing raises an error with the TransientTransactionError label and the retry timeout has been exceeded,
`withTransaction` should propagate the error to its caller. This case may occur if the commit was internally retried
against a new primary after a failover and the second primary returned a NoSuchTransaction error response.

If possible, drivers should implement these tests without requiring the test runner to block for the full duration of
the retry timeout. This might be done by internally modifying the timeout value used by `withTransaction` with some
private API or using a mock timer.

## Changelog

Expand Down
22 changes: 5 additions & 17 deletions source/transactions-convenient-api/transactions-convenient-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ The root object of a driver's API. The name of this object MAY vary across drive

<div id="TransactionOptions">

TransactionOptions\
Options for `ClientSession.startTransaction`, as defined in the
[Transactions](../transactions/transactions.md) specification. The structure of these options MAY vary across drivers
(e.g. dictionary, typed class).
**TransactionOptions**

Options for `ClientSession.startTransaction`, as defined in the [Transactions](../transactions/transactions.md)
specification. The structure of these options MAY vary across drivers (e.g. dictionary, typed class).

### Naming Deviations

Expand Down Expand Up @@ -314,10 +314,6 @@ to exceed the user's original intention for `wtimeout`. The current design of th
write concern timeouts, and simply retries `commitTransaction` within its timeout period for all errors bearing the
"UnknownTransactionCommitResult" label.

This change was made in light of the forthcoming Client-side Operations Timeout specification (see:
[Future Work](#future-work)), which we expect will allow the current 120-second timeout for `withTransaction` to be
customized and also obviate the need for users to specify `wtimeout`.

### The commit is not retried after a MaxTimeMSExpired error

This specification intentionally chooses not to retry commit operations after a MaxTimeMSExpired error as doing so would
Expand Down Expand Up @@ -361,13 +357,6 @@ a helper method to execute a user-defined function within a transaction has few
provides an implementation of a technique already described in the MongoDB 4.0 documentation
([DRIVERS-488](https://jira.mongodb.org/browse/DRIVERS-488)).

## Future Work

The forthcoming Client-side Operations Timeout specification
([DRIVERS-555](https://jira.mongodb.org/browse/DRIVERS-555)) may allow users to alter the default retry timeout, as a
client-side timeout could be applied to `withTransaction` and its retry logic. In the absence of a client-side operation
timeout, withTransaction can continue to use the 120-second default and thus preserve backwards compatibility.

## Changelog

- 2024-09-06: Migrated from reStructuredText to Markdown.
Expand All @@ -376,8 +365,7 @@ timeout, withTransaction can continue to use the 120-second default and thus pre

- 2022-10-05: Remove spec front matter and reformat changelog.

- 2022-01-19: withTransaction applies timeouts per the client-side operations\
timeout specification.
- 2022-01-19: withTransaction applies timeouts per the client-side operations timeout specification.

- 2019-04-24: withTransaction does not retry when commit fails with MaxTimeMSExpired.

Expand Down

0 comments on commit da69ee0

Please sign in to comment.