Details
-
Bug
-
Resolution: Fixed
-
Major
-
2.0.0
-
Security Level: Public
-
None
-
CBG Sprint 22
-
3
Description
Background
Whilst working on CBG-342 - I noticed the Get handling in Incr for when amt=0 is not taking into account the passed in "def" parameter. This seems wrong, and looks like it might cause issues when Incr is called with a zero amt non-zero def values.
A quick unit test comparing Walrus and gocb buckets show a mismatch of return values for the given Incr call.
Another thing to note is that the intial return value of Incr via Get is never persisted, which means non-zero def values are going to mess up the semantics of Incr on the bucket.
Proposed fix
When we do the initial Get with amt=0 and a non-zero def, we should write this to the bucket, using CAS to make sure we've not had the doc created underneath us in the meantime.
Unit/integration test to demonstrate issue
// TestIncrAmtZero covers the special handling in Incr for when amt=0 on an unknown key
|
func TestIncrAmtZero(t *testing.T) {
|
|
|
key := "TestIncrAmtZero"
|
|
|
bucket := GetTestBucketOrPanic()
|
defer bucket.Close()
|
|
|
// key hasn't been created, so we'll fall into the special 'Get' handling in Incr
|
val, err := bucket.Incr(key, 0, 5, 0)
|
assert.NoError(t, err)
|
assert.Equal(t, uint64(5), val)
|
|
|
// Actually increment key to create it
|
val, err = bucket.Incr(key, 1, 5, 0)
|
assert.NoError(t, err)
|
assert.Equal(t, uint64(6), val)
|
|
|
// Do another amt=0 to make sure we get the new incremented value
|
val, err = bucket.Incr(key, 0, 5, 0)
|
assert.NoError(t, err)
|
assert.Equal(t, uint64(6), val)
|
}
|
14:59 $ go test -run='^TestIncrAmtZero$' -v ./base
|
=== RUN TestIncrAmtZero
|
2019-04-30T14:59:33.996+01:00 [INF] Opening Walrus database test_data_bucket-83ff19130b65933d4dbefad8279e5ebd on <walrus:>
|
--- PASS: TestIncrAmtZero (0.00s)
|
PASS
|
ok github.com/couchbase/sync_gateway/base 0.019s
|
|
|
14:59 $ SG_TEST_BACKING_STORE=couchbase SG_TEST_COUCHBASE_SERVER_URL=couchbase://10.112.191.101 go test -run='^TestIncrAmtZero$' -v ./base
|
=== RUN TestIncrAmtZero
|
2019-04-30T14:59:21.963+01:00 [INF] Successfully opened bucket test_data_bucket
|
2019-04-30T14:59:21.972+01:00 [INF] Set query timeouts for bucket test_data_bucket to cluster:1m15s, bucket:1m15s
|
2019-04-30T14:59:21.972+01:00 [INF] Flushing bucket test_data_bucket
|
2019-04-30T14:59:24.103+01:00 [INF] GoCBCustomSGTranscoder Opening Couchbase database test_data_bucket on <couchbase://10.112.191.101> as user "test_data_bucket"
|
2019-04-30T14:59:24.567+01:00 [INF] Successfully opened bucket test_data_bucket
|
2019-04-30T14:59:24.574+01:00 [INF] Set query timeouts for bucket test_data_bucket to cluster:1m15s, bucket:1m15s
|
--- FAIL: TestIncrAmtZero (3.08s)
|
bucket_gocb_test.go:487:
|
Error Trace: bucket_gocb_test.go:487
|
Error: Not equal:
|
expected: 0x5
|
actual : 0x0
|
Test: TestIncrAmtZero
|
bucket_gocb_test.go:492:
|
Error Trace: bucket_gocb_test.go:492
|
Error: Not equal:
|
expected: 0x6
|
actual : 0x5
|
Test: TestIncrAmtZero
|
bucket_gocb_test.go:497:
|
Error Trace: bucket_gocb_test.go:497
|
Error: Not equal:
|
expected: 0x6
|
actual : 0x5
|
Test: TestIncrAmtZero
|
FAIL
|
FAIL github.com/couchbase/sync_gateway/base 3.099s
|