Support timeoutMS inheritance for transaction operations

XMLWordPrintableJSON

    • Type: Bug
    • Resolution: Unresolved
    • Priority: Major - P3
    • None
    • Affects Version/s: None
    • Component/s: CSOT
    • None
    • Go Drivers
    • Hide

      1. What would you like to communicate to the user about this feature?
      2. Would you like the user to see examples of the syntax and/or executable code and its output?
      3. Which versions of the driver/connector does this apply to?

      Show
      1. What would you like to communicate to the user about this feature? 2. Would you like the user to see examples of the syntax and/or executable code and its output? 3. Which versions of the driver/connector does this apply to?
    • None
    • None
    • None
    • None
    • None
    • None

      Detailed steps to reproduce the problem?

      The CSOT specification requires that timeoutMS from the parent MongoClient be inherited by sessions and applied to transaction operations including withTransaction, commitTransaction, and abortTransaction. Currently, the Go Driver only supports the operation-level analogue.

      The following CSOT unified spec test can be used to repro:

      go test -run TestUnifiedSpec/client-side-operations-timeout/tests/sessions-inherit-timeoutMS.json ./internal/integration/unified -v
      

      Output: https://gist.github.com/prestonvasquez/416d33b8d0c3ae8f5fafcc69ef61edf5

      Definition of done: what must be done to consider the task complete?

      (1)Session.AbortTransaction and Session.CommitTransaction need to pass the client-level timeoutMS value through the operation pipeline.

      (2) session.WithTransaction needs to apply client timeout to context, if set:

      ctx, cancel := csot.WithTimeout(ctx, s.client.timeout)
      defer cancel()
      

      (3) The CSOT specifications say

      If the callback returns an error and the transaction must be aborted, drivers MUST refresh the `timeoutMS` value for the `abortTransaction` operation.

      The Go Driver is non-compliant with this, relying on legacy [and buggy] behavior that ignores timeouts and context cancelations while aborting transactions within the convenience API.

      If client-side abort fails, the server has its own transaction timeout (transactionLifetimeLimitSeconds, default 60 seconds). The transaction will eventually be cleaned up server-side.

      The spec rationale explicitly accepts this trade-off:

      This can cause the entire withTransaction call to take up to 2*timeoutMS, but it was decided that this risk is worthwhile given the importance of transaction cleanup.

      Suggested fix:

      abortCtx, abortCancel := csot.WithTimeout(newBackgroundContext(ctx), timeoutDur)
      _ = s.AbortTransaction(abortCtx)
      abortCancel()
      

      WLOG commitTransaction also needs a refresh timeout

      (4) The following tests should be removed from the skip-list:

      "TestUnifiedSpec/client-side-operations-timeout/tests/sessions-inherit-timeoutMS.json/timeoutMS_applied_to_commitTransaction"
      "TestUnifiedSpec/client-side-operations-timeout/tests/sessions-inherit-timeoutMS.json/timeoutMS_applied_to_abortTransaction"
      "TestUnifiedSpec/client-side-operations-timeout/tests/sessions-inherit-timeoutMS.json/timeoutMS_applied_to_withTransaction"
      "TestUnifiedSpec/client-side-operations-timeout/tests/sessions-override-operation-timeoutMS.json/timeoutMS_applied_to_withTransaction"
      

            Assignee:
            Unassigned
            Reporter:
            Preston Vasquez
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated: