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

Fix counters in validator monitor totals mode #3332

Merged
merged 1 commit into from
Jan 31, 2022
Merged

Conversation

arnetheduck
Copy link
Member

The current counters set gauges etc to the value of the last validator
to be processed - as the name of the feature implies, we should be using
sums instead.

  • fix missing beacon state metrics on startup, pre-first-head-selection

@github-actions
Copy link

github-actions bot commented Jan 28, 2022

Unit Test Results

     12 files  ±0     806 suites  ±0   33m 20s ⏱️ +16s
1 652 tests ±0  1 604 ✔️ ±0    48 💤 ±0  0 ±0 
9 665 runs  ±0  9 561 ✔️ ±0  104 💤 ±0  0 ±0 

Results for commit ea26439. ± Comparison against base commit d583e8e.

♻️ This comment has been updated with latest results.

epoch = current_epoch,
validator = id

if state.next_sync_committee.pubkeys.data.contains(pubkey):
Copy link
Contributor

Choose a reason for hiding this comment

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

How often is this code executed? There are ways to avoid the repeated per-validator linear scans here.

Copy link
Member Author

Choose a reason for hiding this comment

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

once per epoch, which is why I didn't bother for now - might do a followup but I want the metrics to be correct first, and this PR is messy enough

@zah
Copy link
Contributor

zah commented Jan 30, 2022

Needs rebasing

The current counters set gauges etc to the value of the _last_ validator
to be processed - as the name of the feature implies, we should be using
sums instead.

* fix missing beacon state metrics on startup, pre-first-head-selection
* fix epoch metrics not being updated on cross-epoch reorg
@arnetheduck arnetheduck merged commit ad327a8 into unstable Jan 31, 2022
@arnetheduck arnetheduck deleted the val-mon-totals branch January 31, 2022 07:36
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