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

raftstore: add drop message metrics #2316

Merged
merged 7 commits into from
Sep 26, 2017
Merged

raftstore: add drop message metrics #2316

merged 7 commits into from
Sep 26, 2017

Conversation

overvenus
Copy link
Member

Records dropped raft messages. Please let me know if I am missing something.

@@ -770,6 +770,7 @@ impl<T: Transport, C: PdClient> Store<T, C> {
fn on_raft_message(&mut self, mut msg: RaftMessage) -> Result<()> {
let region_id = msg.get_region_id();
if !self.is_raft_msg_valid(&msg) {
self.raft_metrics.message.drop_invalid_msg += 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

any reason to drop the message? stale, store not match or others?

I think we should more detailed metric tags.

@@ -88,6 +88,13 @@ pub struct RaftMessageMetrics {
pub heartbeat_resp: u64,
pub transfer_leader: u64,
pub timeout_now: u64,
pub drop_mismatch_store_id: u64,
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer using another struct for these drop messages.

Copy link
Contributor

@siddontang siddontang left a comment

Choose a reason for hiding this comment

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

Rest LGTM

PTAL @BusyJay

Add a new metrics for recording dropped messages
@overvenus
Copy link
Member Author

PTAL

@siddontang
Copy link
Contributor

PTAL @javaforfun

@siddontang
Copy link
Contributor

No need, here we just want to know the invalid message like stale, mismatched store ID, dropping.

Copy link
Member

@BusyJay BusyJay left a comment

Choose a reason for hiding this comment

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

check_msg and check_snapshot is not covered.

@@ -721,6 +723,7 @@ impl<T: Transport, C: PdClient> Store<T, C> {
region_id,
p
);
self.raft_metrics.message_dropped.destroy_stale_peer += 1;
Copy link
Member

Choose a reason for hiding this comment

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

This is the same as L695.

@overvenus
Copy link
Member Author

PTAL

@BusyJay
Copy link
Member

BusyJay commented Sep 26, 2017

LGTM

@overvenus
Copy link
Member Author

/run-all-test

@overvenus overvenus merged commit 88730a3 into master Sep 26, 2017
@overvenus overvenus deleted the ov/drop-raft-msg branch September 26, 2017 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants