Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CDC] CHECK fails with pending RPCs on shutdown #2549

Closed
rahuldesirazu opened this issue Oct 9, 2019 · 0 comments
Closed

[CDC] CHECK fails with pending RPCs on shutdown #2549

rahuldesirazu opened this issue Oct 9, 2019 · 0 comments
Assignees
Labels
area/cdc Change Data Capture kind/bug This issue is a bug

Comments

@rahuldesirazu
Copy link
Contributor

As a CDC poller, if we go from leader to follower for a tablet, we trigger a shutdown for its apply changes client. This shutdown waits for all rpcs to finish before returning. This isn't happening in the allotted 30s, which causes the server to crash.

@rahuldesirazu rahuldesirazu added kind/bug This issue is a bug area/cdc Change Data Capture labels Oct 9, 2019
@rahuldesirazu rahuldesirazu self-assigned this Oct 9, 2019
ndeodhar added a commit that referenced this issue Oct 23, 2019
Summary:
Diff includes:
1. Move Rpcs to CDC consumer instead of CDC poller since it's unnecessary overhead to have this per tablet (CDC poller is per tablet).
2. Fixes potential race condition while checking for whether all CDC records are processed.
    The race condition was because:
       a) `processed_record_count_` was an atomic variable and it was increment atomically.
       b) There was then a check to see if `processed_record_count_` and `record_count_` matches, and then call some method if they match.
       c) Steps (a) and (b) were separate. Due to this, it's possible that two threads could update roughly around the same time and then instead of only the last thread running the method in (b), both threads would run the method in (b).
3. Clean up shutdown path. Since `~Rpcs()` shuts down `Rpcs`, we should not explicitly call `Shutdown`.

Test Plan: Jenkins

Reviewers: hector, nicolas, rahuldesirazu

Reviewed By: nicolas, rahuldesirazu

Subscribers: ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D7374
@ndeodhar ndeodhar self-assigned this Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cdc Change Data Capture kind/bug This issue is a bug
Projects
None yet
Development

No branches or pull requests

2 participants