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.

Sign up for GitHub

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

Jump to bottom

issue 329: RafReplDev Metrics and Restart Leader Test #337

Merged
merged 9 commits into from
Feb 29, 2024
Merged

issue 329: RafReplDev Metrics and Restart Leader Test #337

merged 9 commits into from
Feb 29, 2024

Conversation

yamingk
Copy link
Contributor

@yamingk yamingk commented Feb 28, 2024 •
edited
Loading

  1. add some metrics around read/write/fetch for error cases.
  2. add restart leader test case.

Testing:

  1. Wrote a script to run restart leader for 100 times and passed. Verified leader changed to replica-1 after replica-0 (original leader) restarted with I/O running.

@yamingk yamingk requested a review from hkadayam February 28, 2024 21:01
@yamingk yamingk changed the title Yk repl metrix issue 329: RafReplDev Metrics and Restart Leader Test Feb 28, 2024
raakella1
raakella1 previously approved these changes Feb 29, 2024
View reviewed changes
Copy link
Contributor

@raakella1 raakella1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG!

@yamingk yamingk linked an issue Feb 29, 2024 that may be closed by this pull request
Homestore Raft Repl Dev metrics | Leader Restart Test Case Implement #329
Closed
xiaoxichen
xiaoxichen previously approved these changes Feb 29, 2024
View reviewed changes
yamingk
yamingk commented Feb 29, 2024
View reviewed changes
src/tests/test_raft_repl_dev.cpp

this->wait_for_all_writes(exp_entries);

if (g_helper->replica_num() != 0) { std::this_thread::sleep_for(std::chrono::seconds{10}); }
Copy link
Contributor Author

@yamingk yamingk Feb 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line is harmless and actually should not be needed since we have sync_for_verify_start() which will wait for all replicas to reach into the same state. It was here originally when I was debugging why some replica couldn't finish the I/O and after I root caused it I forget to remove this line.

I will remove it in my other PR if no other comments need to be addressed.

@yamingk yamingk dismissed stale reviews from xiaoxichen and raakella1 via 86e8180 February 29, 2024 18:39
@codecov-commenter
Copy link

codecov-commenter commented Feb 29, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 21 lines in your changes are missing coverage. Please review.

Project coverage is 47.67%. Comparing base (e0cd9a8) to head (1e97bb3).

Files Patch % Lines
src/lib/replication/repl_dev/raft_repl_dev.cpp 0.00% 12 Missing ⚠️
src/lib/replication/repl_dev/raft_repl_dev.h 0.00% 9 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #337      +/-   ##
==========================================
- Coverage   49.99%   47.67%   -2.32%     
==========================================
  Files         134      107      -27     
  Lines       14286     9357    -4929     
  Branches     1725     1212     -513     
==========================================
- Hits         7142     4461    -2681     
+ Misses       6503     4419    -2084     
+ Partials      641      477     -164

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

raakella1
raakella1 approved these changes Feb 29, 2024
View reviewed changes
Copy link
Contributor

@raakella1 raakella1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG!

@yamingk yamingk merged commit dcaad8f into eBay:master Feb 29, 2024
20 checks passed
@yamingk yamingk deleted the yk_repl_metrix branch February 29, 2024 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hkadayam hkadayam hkadayam left review comments

@raakella1 raakella1 raakella1 approved these changes

@xiaoxichen xiaoxichen xiaoxichen left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Homestore Raft Repl Dev metrics | Leader Restart Test Case Implement
5 participants
@yamingk @codecov-commenter @xiaoxichen @hkadayam @raakella1