Uploaded image for project: 'C Driver'
  1. C Driver
  2. CDRIVER-3543

Consider refactoring how retryable writes labels are appended

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major - P3
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.17.0-beta, 1.17.0
    • Component/s: libmongoc
    • Labels:
      None

      Description

      The driver adds error labels for retryable writes as part of _mongoc_write_error_get_type per CDRIVER-3462.

      In the review of CDRIVER-3462 I added some suggestions for refactoring this to be more consistent with existing code we have for adding error labels:

      I wonder if it would make more sense to put this logic deeper in the command codepath.

      For the network error check, we could follow suit with where a
      TransientTransactionError label is added for network errors (see https://github.com/mongodb/specifications/blob/master/source/transactions/transactions.rst#error-reporting-changes).

      That's in the function "network_error_reply" of mongoc-cluster.c. It could append a RetryableWriteError if cmd->is_retryable_write is true.

      That would also apply in more cases, which might be an existing bug in our retry logic. Because network_reply_error is called in cases like failure to decompress a server response (https://github.com/mongodb/mongo-c-driver/blob/1.16.0/src/libmongoc/src/mongoc/mongoc-cluster.c/#L3013) which sets an error domain MONGOC_ERROR_PROTOCOL (not MONGOC_ERROR_STREAM) but still calls network_error_reply.

      For the server error check, _mongoc_write_error_is_retryable could be moved into mongoc-cluster.c and that check could be done around here: https://github.com/mongodb/mongo-c-driver/blob/1.16.0/src/libmongoc/src/mongoc/mongoc-cluster.c/#L3049

      Though this is just a suggestion. A nice thing about adding the error label it here is that it keeps the retryable writes logic a bit more self contained. But I'm up to discuss this further.

        Attachments

          Activity

            People

            Assignee:
            andreas.braun Andreas Braun
            Reporter:
            kevin.albertson Kevin Albertson
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved: