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

Redundancy metrics updates #5829

Merged
merged 3 commits into from Jan 16, 2024
Merged

Conversation

iulianpascalau
Copy link
Contributor

Reasoning behind the pull request

  • fixed the redundancy metric to include the multikey status
  • added a new metric that contains the reason of step-in, in case the node is not main node and the main node failed

Proposed changes

Testing procedure

  • standard system test
  • redundancy tests + metrics checks

Pre-requisites

Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:

  • was the PR targeted to the correct branch?
  • if this is a larger feature that probably needs more than one PR, is there a feat branch created?
  • if this is a feat branch merging, do all satellite projects have a proper tag inside go.mod?

- added a new metric that contains the reason of step-in, in case the node is not main node and the main node failed
@sstanculeanu sstanculeanu self-requested a review January 12, 2024 07:53
Copy link

codecov bot commented Jan 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6622473) 80.15% compared to head (161452b) 80.18%.
Report is 1 commits behind head on rc/sirius-patch-2.

❗ Current head 161452b differs from pull request most recent head f037a6f. Consider uploading reports for the commit f037a6f to get more accurate results

Additional details and impacted files
@@                  Coverage Diff                  @@
##           rc/sirius-patch-2    #5829      +/-   ##
=====================================================
+ Coverage              80.15%   80.18%   +0.03%     
=====================================================
  Files                    708      708              
  Lines                  94052    94072      +20     
=====================================================
+ Hits                   75384    75435      +51     
+ Misses                 13325    13293      -32     
- Partials                5343     5344       +1     

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

}

func (wrk *Worker) computeRedundancyMetrics() (bool, string) {
if !wrk.nodeRedundancyHandler.IsMainMachineActive() {
Copy link
Contributor

@raduchis raduchis Jan 15, 2024

Choose a reason for hiding this comment

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

the nodeRedundancyHandler returns always that the main machine is active for multi-key?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, it does since the key provided/auto-generated in multikey mode is always an observer (otherwise the node can not start). In this setup, the single key redundancy handler does not count "failures" for that key since the key was never part of the consensus.

Copy link
Collaborator

@gabi-vuls gabi-vuls left a comment

Choose a reason for hiding this comment

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

--- Specific errors ---

block hash does not match 8848
wrong nonce in block 3530
miniblocks does not match 0
num miniblocks does not match 0
miniblock hash does not match 0
block bodies does not match 0
receipts hash missmatch 0

/------/

--- Statistics ---

Nr. of all ERRORS: 0
Nr. of all WARNS: 666
Nr. of new ERRORS: 0
Nr. of new WARNS: 2
Nr. of PANICS: 0

/------/

--- ERRORS ---

/------/

--- WARNINGS ---

/------/

@iulianpascalau iulianpascalau merged commit 7177707 into rc/sirius-patch-2 Jan 16, 2024
6 checks passed
@iulianpascalau iulianpascalau deleted the fix-redundancy-metric branch January 16, 2024 09:05
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.

None yet

4 participants