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

Switch Heartbeater to replace ResettableTimer with Periodic timer #386

Closed
rven1 opened this issue Jul 17, 2018 · 1 comment
Closed

Switch Heartbeater to replace ResettableTimer with Periodic timer #386

rven1 opened this issue Jul 17, 2018 · 1 comment
Assignees
Labels
kind/enhancement This is an enhancement of an existing feature

Comments

@rven1
Copy link
Contributor

rven1 commented Jul 17, 2018

Currently the heartbeater uses Resettable timer for keeping track of heartbeats. This has the disadvantage that there is a thread for each tablet peer that a tablet is tracking. We need to move this to Periodic timer so that we can use the ev thread as needed and not use a dedicated thread.

@rven1 rven1 self-assigned this Jul 17, 2018
@kmuthukk kmuthukk added the kind/enhancement This is an enhancement of an existing feature label Jul 17, 2018
yugabyte-ci pushed a commit that referenced this issue Sep 13, 2018
Summary:
Currently the raft consensus uses Resettable timer for keeping track of
heartbeats from other peers.
This has the disadvantage that there is a thread for each tablet peer that a
tablet is tracking. We need to move this to Periodic timer so that we can
use the ev thread as needed and not use a dedicated thread. This does not
change the heartbeater thread communicating from the tserver to the leader
master.

Test Plan: consensus_peer-test, Jenkins

Reviewers: mikhail, sergei

Reviewed By: sergei

Subscribers: kannan, ybase, bharat

Differential Revision: https://phabricator.dev.yugabyte.com/D5449
yugabyte-ci pushed a commit that referenced this issue Sep 14, 2018
This reverts commit f4d6fdfaff603fa4dd60b5d78e8f5035ceb219f7 because a
lot of TSAN tests were broken in it.
yugabyte-ci pushed a commit that referenced this issue Sep 15, 2018
Summary:
Currently the raft consensus uses Resettable timer for keeping track of
heartbeats from other peers.
This has the disadvantage that there is a thread for each tablet peer that a
tablet is tracking. We need to move this to Periodic timer so that we can
use the ev thread as needed and not use a dedicated thread. This does not
change the heartbeater thread communicating from the tserver to the leader
master.

Test Plan: consensus_peer-test, Jenkins

Reviewers: sergei, mikhail

Reviewed By: sergei

Subscribers: kannan, ybase, bharat

Differential Revision: https://phabricator.dev.yugabyte.com/D5449
yugabyte-ci pushed a commit that referenced this issue Sep 16, 2018
This reverts commit b14f0bf8aa8b5801f90c08b938cf10eee62d0f94.

Unfortunately, I have to revert this revision again due to "Check failed:
state_ == kPeerRunning (0 vs. 2)" caught by
RaftConsensusITest.TestChurnyElections in both pre-commit and
post-commit testing.
yugabyte-ci pushed a commit that referenced this issue Sep 19, 2018
Summary:
Prior to this revision, Raft consensus used Resettable timer for keeping track of
heartbeats from other peers.
This has the disadvantage that there is a thread for each tablet peer that a
tablet is tracking. We need to move this to Periodic timer so that we can
use the ev thread as needed and not use a dedicated thread. This does not
change the heartbeater thread communicating from the tserver to the leader
master.

Test Plan:
consensus_peer-test, Jenkins
ybd tsan --cxx-test integration-tests_raft_consensus-itest --gtest_filter RaftConsensusITest.TestChurnyElections -n 100

Reviewers: sergei, mikhail

Reviewed By: sergei, mikhail

Subscribers: kannan, ybase, bharat

Differential Revision: https://phabricator.dev.yugabyte.com/D5449
@rven1
Copy link
Contributor Author

rven1 commented Oct 3, 2018

Fixed. We now use periodic timer instead of resettable heartbeat for consensus peers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement This is an enhancement of an existing feature
Projects
None yet
Development

No branches or pull requests

2 participants