Uploaded image for project: 'Couchbase Gateway'
  1. Couchbase Gateway
  2. CBG-1672

Return 422 status for unprocessible deltas instead of 404 to use non-delta retry handling

    XMLWordPrintable

Details

    • Improvement
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 2.8.0
    • 3.0
    • SyncGateway
    • Security Level: Public
    • None
    • CBG Sprint 82, CBG Sprint 83
    • 5

    Description

      Sync Gateway returns a 404 in the case where a deltaSrc revision was either not found or was a tombstone (meaning we couldn't apply a delta on top of it), and the rev is considered a failed write and is not retried.

      CBL in these cases return a 422 status code, to ask the sender to retry without deltas (CBG-881)

      We should make ISGR return 422 status codes in the above cases to align with CBL and get the automatic rev retry handling. In addition, we may also want to consider retrying without deltas when we get 404, in order to allow for mixed-version ISGR compatibility.

      Attachments

        Issue Links

          No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

            Build sync_gateway-3.0.0-431 contains sync_gateway commit 5ce7abf with commit message:
            CBG-1672 - Return 422 status for unprocessible deltas instead of 404 to use non-delta retry handling (#5249)

            build-team Couchbase Build Team added a comment - Build sync_gateway-3.0.0-431 contains sync_gateway commit 5ce7abf with commit message: CBG-1672 - Return 422 status for unprocessible deltas instead of 404 to use non-delta retry handling (#5249)
            adamf Adam Fraser added a comment -

            Ben Brooks To confirm my understanding - the change here is to have an active peer return a 422 if it hits an error attempting to apply a delta in handleRev processing.  This will end up with SG behaving the same way CBL does, and trigger the handling already implemented on the active peer in CBG-881.

            Changing this in handleRev would also return a 422 on a passive peer when it can't apply a revision being pushed.  This would apply to both CBL and ISGR.  Is this intended, and do we know whether CBL retries on a 422 today?  It may be an unlikely scenario for CBL replication, since it's always pushing revisions as a child of the active rev (no conflicts), but still possible in cases like badly formatted deltas.

            Isaac Lambat Assuming the above is correct, we should ensure that ISGR test coverage handles both the active and passive cases.

            adamf Adam Fraser added a comment - Ben Brooks To confirm my understanding - the change here is to have an active peer return a 422 if it hits an error attempting to apply a delta in handleRev processing.  This will end up with SG behaving the same way CBL does, and trigger the handling already implemented on the active peer in CBG-881. Changing this in handleRev would also return a 422 on a passive peer when it can't apply a revision being pushed.  This would apply to both CBL and ISGR.  Is this intended, and do we know whether CBL retries on a 422 today?  It may be an unlikely scenario for CBL replication, since it's always pushing revisions as a child of the active rev (no conflicts), but still possible in cases like badly formatted deltas. Isaac Lambat  Assuming the above is correct, we should ensure that ISGR test coverage handles both the active and passive cases.
            isaac.lambat Isaac Lambat added a comment -

            404 no longer returned for FromRev tombstoned revisions (CBG-1427)

            isaac.lambat Isaac Lambat added a comment - 404 no longer returned for FromRev tombstoned revisions ( CBG-1427 )

            People

              isaac.lambat Isaac Lambat
              ben.brooks Ben Brooks
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Gerrit Reviews

                  There are no open Gerrit changes

                  PagerDuty