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 metric measuring number of successfully inserted push messages #20275

Merged
merged 3 commits into from
Sep 29, 2021

Conversation

carllin
Copy link
Contributor

@carllin carllin commented Sep 27, 2021

Problem

No easy way to tell ratio of new messages coming in from push vs pull to measure how effective push in the network currently is

Summary of Changes

Add a process_push_success metric that is a parallel to the process_pull_response_success metric

Fixes #

@@ -109,6 +109,7 @@ pub(crate) struct GossipStats {
pub(crate) new_push_requests2: Counter,
pub(crate) new_push_requests: Counter,
pub(crate) new_push_requests_num: Counter,
pub(crate) process_push_success: Counter,
Copy link
Contributor

Choose a reason for hiding this comment

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

can we plz keep this list sorted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

let results = self
.push
.process_push_message(&self.crds, from, values, now);
let success_count = results.len();
Copy link
Contributor

Choose a reason for hiding this comment

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

the inserted ones are the ones which are not error.
results.len() is the same as values.len().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ick, need to read more carefully. Fixed up, thanks!

@codecov
Copy link

codecov bot commented Sep 28, 2021

Codecov Report

Merging #20275 (9829999) into master (5810568) will increase coverage by 0.0%.
The diff coverage is 6.6%.

@@           Coverage Diff           @@
##           master   #20275   +/-   ##
=======================================
  Coverage    82.8%    82.8%           
=======================================
  Files         487      487           
  Lines      135612   135624   +12     
=======================================
+ Hits       112295   112309   +14     
+ Misses      23317    23315    -2     

@carllin carllin added the v1.7 label Sep 29, 2021
@carllin carllin merged commit ee8621a into solana-labs:master Sep 29, 2021
mergify bot pushed a commit that referenced this pull request Sep 29, 2021
…20275)

* Add number of successfully inserted push messages

(cherry picked from commit ee8621a)

# Conflicts:
#	gossip/src/cluster_info.rs
#	gossip/src/crds_gossip.rs
carllin added a commit that referenced this pull request Sep 29, 2021
…20275)

* Add number of successfully inserted push messages

(cherry picked from commit ee8621a)
carllin added a commit that referenced this pull request Sep 29, 2021
…20275)

* Add number of successfully inserted push messages

(cherry picked from commit ee8621a)
carllin added a commit that referenced this pull request Sep 29, 2021
…20275)

* Add number of successfully inserted push messages

(cherry picked from commit ee8621a)
carllin added a commit that referenced this pull request Sep 29, 2021
…20275)

* Add number of successfully inserted push messages

(cherry picked from commit ee8621a)
dankelleher pushed a commit to identity-com/solana that referenced this pull request Nov 24, 2021
frits-metalogix added a commit to identity-com/solana that referenced this pull request Nov 24, 2021
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.

2 participants