[MB-44773] Race condition when logging DCP state (retries may be logged as -1) - Couchbase database

XML

Word

Printable

Details

Type: Bug
Status: Closed
Priority: Critical
Resolution: Fixed
Affects Version/s: Cheshire-Cat
Fix Version/s: 7.0.0
Component/s: tools
Labels:
None

Triage:
Untriaged
Story Points:
1
Is this a Regression?:
No
Sprint:
Tools: CC Final Sprint

Description

What's the issue?
It looks like we might have a race condition when logging DCP state for very small transfers. Once a stream is started, it operates completely asynchronously, an extremely small stream may complete before the retries counter has been incremented; this may result in the number of retries being logged as -1 (which makes no sense)

Attachments

Issue Links

relates to

MB-44772 Backup Service test one_off_merge fails with permission denied

Closed

MB-45747 cbdocloader race condition

Closed

Gerrit Reviews

- Issue Only
- Show All Reviews
- Show Open Reviews
- Show All Issues
- Show Open Issues

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

Activity

Ascending order - Click to sort in descending order

Couchbase Build Team added a comment - 08/Mar/21 1:30 AM

Build couchbase-server-7.0.0-4631 contains backup commit 02a8cad with commit message:
~~MB-44773~~ Ensure we don't log a negative retry count

Couchbase Build Team added a comment - 08/Mar/21 1:30 AM Build couchbase-server-7.0.0-4631 contains backup commit 02a8cad with commit message: MB-44773 Ensure we don't log a negative retry count

Asad Zaidi (Inactive) added a comment - 11/Mar/21 5:50 AM

Closing based on the fact that this has been unit tested.

Asad Zaidi (Inactive) added a comment - 11/Mar/21 5:50 AM Closing based on the fact that this has been unit tested.

James Lee added a comment - 22/Apr/21 2:57 AM

Reopening because the current solution to this issue is inadequate and doesn't actually eliminate the race condition (as we've seen from ~~MB-45747~~).

This resulted in a workaround in 'master' which uses an atomic operation to get the retry count, however, this just eliminates the data race (accessing the variable itself); the value may well still be stale.

James Lee added a comment - 22/Apr/21 2:57 AM Reopening because the current solution to this issue is inadequate and doesn't actually eliminate the race condition (as we've seen from MB-45747 ). This resulted in a workaround in ' master ' which uses an atomic operation to get the retry count, however, this just eliminates the data race (accessing the variable itself); the value may well still be stale.

Couchbase Build Team added a comment - 22/Apr/21 8:36 AM

Build couchbase-server-7.0.0-5009 contains backup commit b720543 with commit message:
~~MB-44773~~ Ensure we log an accurate retry count

Couchbase Build Team added a comment - 22/Apr/21 8:36 AM Build couchbase-server-7.0.0-5009 contains backup commit b720543 with commit message: MB-44773 Ensure we log an accurate retry count

Arunkumar Senthilnathan added a comment - 26/Apr/21 2:39 PM

Closing this as this is unit tested already

Arunkumar Senthilnathan added a comment - 26/Apr/21 2:39 PM Closing this as this is unit tested already

Couchbase Build Team added a comment - 30/Apr/21 10:42 AM

Build couchbase-server-7.0.0-5075 contains backup commit 62e9626 with commit message:
~~MB-44773~~ Account for pre-stream request incrementing of retry count

Couchbase Build Team added a comment - 30/Apr/21 10:42 AM Build couchbase-server-7.0.0-5075 contains backup commit 62e9626 with commit message: MB-44773 Account for pre-stream request incrementing of retry count