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

GoCB Bucket Incr amt=0 logic not handling errors correctly

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: 2.0.0
    • Fix Version/s: 2.6.0
    • Component/s: SyncGateway
    • Security Level: Public
    • Sprint:
      CBG Sprint 21
    • Story Points:
      3

      Description

      Background

      In CouchbaseBucketGoCB's Incr method, when the amount specified to increment by is zero, we have special handling in place to instead do a normal Get op, in order to work around some GoCB-specific counter handling.

      https://github.com/couchbase/sync_gateway/blob/b0c7485d9933775bf951b368cdc31ca2189bf4a8/base/bucket_gocb.go#L1793-L1803

      This logic is sound in the case when err is ErrKeyNotFound, but under other circumstances, in the case where Get's retry loop aborts with an error, this logic is incorrect, and allows the Incr op to report an incorrect counter value of zero.

      Example error scenario

      In Sync Gateway 1.5.2, this manifests when the Couchbase Server node containing the _sync:seq doc is down and not failedover (the Get op persistently times out for each retry). Swallowing an ErrTimeout means SG happily continues to start up thinking it has a starting sequence of zero.

      Proposed fix

      We instead should explicitly check for ErrKeyNotFound in the special-handling code above when returning "0, nil". for any other errors we "0, err".

      Testing

      This should be easy enough to test in an integration test with LeakyBucket by forcing a timeout error on a Get() op when calling Incr() with amt=0.

        Attachments

          Issue Links

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

            Activity

            Hide
            build-team Couchbase Build Team added a comment -

            Build sync_gateway-2.6.0-37 contains sync_gateway commit cde8f82 with commit message:
            CBG-342 - Check for explicit KeyNotFound in gocb Incr's amt=0 handling (#4095)

            Show
            build-team Couchbase Build Team added a comment - Build sync_gateway-2.6.0-37 contains sync_gateway commit cde8f82 with commit message: CBG-342 - Check for explicit KeyNotFound in gocb Incr's amt=0 handling (#4095)
            Hide
            build-team Couchbase Build Team added a comment -

            Build sync_gateway-2.6.0-37 contains sync_gateway commit cde8f82 with commit message:
            CBG-342 - Check for explicit KeyNotFound in gocb Incr's amt=0 handling (#4095)

            Show
            build-team Couchbase Build Team added a comment - Build sync_gateway-2.6.0-37 contains sync_gateway commit cde8f82 with commit message: CBG-342 - Check for explicit KeyNotFound in gocb Incr's amt=0 handling (#4095)
            Hide
            build-team Couchbase Build Team added a comment -

            Build sync_gateway-2.5.1-9 contains sync_gateway commit 5d3b96d with commit message:
            CBG-432 - Backport CBG-342: Check for explicit KeyNotFound in gocb Incr's amt=0 handling (#4095) (#4182)

            Show
            build-team Couchbase Build Team added a comment - Build sync_gateway-2.5.1-9 contains sync_gateway commit 5d3b96d with commit message: CBG-432 - Backport CBG-342 : Check for explicit KeyNotFound in gocb Incr's amt=0 handling (#4095) (#4182)
            Hide
            build-team Couchbase Build Team added a comment -

            Build sync_gateway-2.5.1-9 contains sync_gateway commit 5d3b96d with commit message:
            CBG-432 - Backport CBG-342: Check for explicit KeyNotFound in gocb Incr's amt=0 handling (#4095) (#4182)

            Show
            build-team Couchbase Build Team added a comment - Build sync_gateway-2.5.1-9 contains sync_gateway commit 5d3b96d with commit message: CBG-432 - Backport CBG-342 : Check for explicit KeyNotFound in gocb Incr's amt=0 handling (#4095) (#4182)

              People

              • Assignee:
                ben.brooks Ben Brooks
                Reporter:
                ben.brooks Ben Brooks
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:

                  Gerrit Reviews

                  There are no open Gerrit changes

                    PagerDuty

                    Error rendering 'com.pagerduty.jira-server-plugin:PagerDuty'. Please contact your Jira administrators.