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

add rpc to query raft status #3336

Merged
merged 7 commits into from
Nov 26, 2021
Merged

Conversation

kikimo
Copy link
Contributor

@kikimo kikimo commented Nov 22, 2021

What type of PR is this?

  • bug
  • feature
  • enhancement

What does this PR do?

This PR add an readonly raft rpc getState() to query raft info like: leadership, running status, current term and etc. We expect this rpc to help us find the correct leader as soon as possible when leader change happends during our test, it's a replacement for meta api which has high latency and sometime return incorrect leader information.

@codecov-commenter
Copy link

codecov-commenter commented Nov 22, 2021

Codecov Report

Merging #3336 (b4d5754) into master (6541438) will decrease coverage by 0.06%.
The diff coverage is 78.40%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3336      +/-   ##
==========================================
- Coverage   85.31%   85.25%   -0.07%     
==========================================
  Files        1289     1279      -10     
  Lines      120073   119010    -1063     
==========================================
- Hits       102442   101459     -983     
+ Misses      17631    17551      -80     
Impacted Files Coverage Δ
src/clients/meta/MetaClient.cpp 75.87% <ø> (-0.66%) ⬇️
src/clients/meta/MetaClient.h 95.65% <ø> (ø)
src/graph/optimizer/rule/TopNRule.cpp 95.83% <ø> (ø)
src/graph/planner/plan/Admin.cpp 56.14% <0.00%> (-2.05%) ⬇️
src/graph/planner/plan/PlanNode.h 81.25% <ø> (ø)
src/graph/util/ExpressionUtils.h 100.00% <ø> (ø)
src/graph/validator/MaintainValidator.cpp 84.21% <ø> (+1.55%) ⬆️
src/graph/validator/MaintainValidator.h 80.30% <ø> (+4.72%) ⬆️
src/kvstore/raftex/RaftPart.h 98.30% <ø> (ø)
src/kvstore/raftex/RaftexService.cpp 86.40% <0.00%> (-5.13%) ⬇️
... and 80 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a14d7b4...b4d5754. Read the comment docs.

liuyu85cn
liuyu85cn previously approved these changes Nov 22, 2021
Copy link
Contributor

@liuyu85cn liuyu85cn left a comment

Choose a reason for hiding this comment

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

LGTM

STOPPED = 2; // The part has been stopped
WAITING_SNAPSHOT = 3; // Waiting for the snapshot.
}

Copy link
Contributor

Choose a reason for hiding this comment

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

These enum types have been defined in following source file, could you unify the usage of these enum types?

enum class Status {
STARTING = 0, // The part is starting, not ready for service
RUNNING, // The part is running
STOPPED, // The part has been stopped
WAITING_SNAPSHOT // Waiting for the snapshot.
};
enum class Role {
LEADER = 1, // the leader
FOLLOWER, // following a leader
CANDIDATE, // Has sent AskForVote request
LEARNER // It is the same with FOLLOWER,
// except it does not participate in leader election
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yixinglu updated, PTAL

CANDIDATE = 3; // Has sent AskForVote request
LEARNER = 4; // same with FOLLOWER, except that it does
// not vote in leader election
}
Copy link
Contributor

Choose a reason for hiding this comment

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

could add a (cpp.enum_strict) here and below

Copy link
Contributor

@critical27 critical27 left a comment

Choose a reason for hiding this comment

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

Others LGTM

Copy link
Contributor

@yixinglu yixinglu left a comment

Choose a reason for hiding this comment

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

LGTM

@critical27 critical27 merged commit 5b754e5 into vesoft-inc:master Nov 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-testing PR: ready for the CI test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants