Skip to content

metrics: Synchronous EWMA#27918

Closed
zeim839 wants to merge 4 commits intoethereum:masterfrom
zeim839:ewma_sync
Closed

metrics: Synchronous EWMA#27918
zeim839 wants to merge 4 commits intoethereum:masterfrom
zeim839:ewma_sync

Conversation

@zeim839
Copy link
Copy Markdown
Contributor

@zeim839 zeim839 commented Aug 13, 2023

Implements #27846

Note:
Updating the EWMA every nanosecond would yield erratically inflated results, especially if Rate() was called immediately after EWMA initialization. I've thus opted for a 1-second interval (allowing uncounted events to accumulate) before updating the EWMA.

Previously, the StandardMeter struct held a snapshot that would be continuously updated by the Tick() function. Since metrics are no longer governed by a clock, I've reworked the Snapshot() function to create a new snapshot struct whenever it is called.

@zeim839 zeim839 force-pushed the ewma_sync branch 2 times, most recently from 3a3971f to c76c6cf Compare August 14, 2023 15:09
@zeim839 zeim839 marked this pull request as draft August 15, 2023 11:20
@zeim839 zeim839 marked this pull request as ready for review August 16, 2023 07:36
@holiman
Copy link
Copy Markdown
Contributor

holiman commented Oct 10, 2023

Hi, and thanks for your PR.
Unfortunately it came at a unfortunate time, I was working on some older metrics-related PRs and finally realized that the entire metrics-system needed a thorough refactoring, which was implemented in #28035

With that change, this PR became very bitrotted. If you would like to rebase it on top of the changes, we can make another attempt at getting this PR in.

Apologies for not getting to this sooner -- the metrics subsystem/package is a bit of an orphan in go-ethereum, so we don't often prioritize.

@zeim839
Copy link
Copy Markdown
Contributor Author

zeim839 commented Oct 10, 2023

No problem. I'm not sure if I'll have the time to look into this anytime soon, so I will be closing this PR for the time being.

@zeim839 zeim839 closed this Oct 10, 2023
@zeim839 zeim839 deleted the ewma_sync branch October 10, 2023 14:14
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