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

            ben.brooks Ben Brooks created issue -
            ben.brooks Ben Brooks made changes -
            Field Original Value New Value
            Link This issue relates to CBSE-10654 [ CBSE-10654 ]
            adamf Adam Fraser made changes -
            Fix Version/s Lithium [ 16180 ]
            adamf Adam Fraser made changes -
            Assignee The One [ the one ] Isaac Lambat [ JIRAUSER25602 ]
            adamf Adam Fraser made changes -
            Story Points 5
            adamf Adam Fraser made changes -
            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 SG 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.
            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.
            adamf Adam Fraser made changes -
            Sprint CBG Sprint 82 [ 1777 ]
            adamf Adam Fraser made changes -
            Rank Ranked lower
            adamf Adam Fraser made changes -
            Sprint CBG Sprint 82 [ 1777 ]
            adamf Adam Fraser made changes -
            Rank Ranked higher
            isaac.lambat Isaac Lambat made changes -
            Sprint CBG Sprint 82 [ 1777 ]
            isaac.lambat Isaac Lambat made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            isaac.lambat Isaac Lambat made changes -
            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.
            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.
            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 )
            Automated transition triggered when Isaac Lambat created pull request #5249 in GitHub -
            Status In Progress [ 3 ] In Review [ 10107 ]
            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 made changes -
            Status In Review [ 10107 ] In Progress [ 3 ]
            adamf Adam Fraser made changes -
            Sprint CBG Sprint 82 [ 1777 ] CBG Sprint 82, CBG Sprint 83 [ 1777, 1801 ]
            isaac.lambat Isaac Lambat made changes -
            Status In Progress [ 3 ] Open [ 1 ]
            isaac.lambat Isaac Lambat made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            isaac.lambat Isaac Lambat made changes -
            Status In Progress [ 3 ] In Review [ 10107 ]

            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)
            Automated transition triggered when Ben Brooks merged pull request #5249 in GitHub -
            Resolution Fixed [ 1 ]
            Status In Review [ 10107 ] Resolved [ 5 ]
            isaac.lambat Isaac Lambat made changes -
            Status Resolved [ 5 ] Closed [ 6 ]
            isaac.lambat Isaac Lambat made changes -
            Link This issue causes CBG-1719 [ CBG-1719 ]

            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