Skip to content

Add optimistic_sync metric#8059

Merged
mergify[bot] merged 3 commits into
sigp:unstablefrom
chong-he:metrics-optimistic
Nov 12, 2025
Merged

Add optimistic_sync metric#8059
mergify[bot] merged 3 commits into
sigp:unstablefrom
chong-he:metrics-optimistic

Conversation

@chong-he

Copy link
Copy Markdown
Member

Issue Addressed

Proposed Changes

Display 0 if optimistic sync and 1 for normal sync

Additional Info

Tested and it is working, the optimistic sync is triggered by running lcli mock-el without the flag --all-payloads-valid true, so the beacon node sync is optimistic under this situation.

Screenshot from 2025-09-16 18-37-51

@chong-he chong-he added ready-for-review The code is ready for review low-hanging-fruit Easy to resolve, get it before someone else does! UX-and-logs labels Sep 16, 2025
Comment thread beacon_node/client/src/metrics.rs Outdated
pub static IS_OPTIMISTIC_SYNC: LazyLock<Result<IntGauge>> = LazyLock::new(|| {
try_create_int_gauge(
"optimistic_sync",
"Metric to check if the beacon chain is in optimistic sync mode. 0 if optimistic sync and 1 if synced",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is counterintuitive. In my original comment I was thinking we would name the metric something like execution_verified 0|1, with 0 representing the optimistic case and 1 representing full sync. Or if we keep the optimistic_sync name I think we should swap it so that 1 = optimistic, 0 = not optimistic. That might actually be the clearest. We would just need to display the 0 as "good" in our grafana chart

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Revised in revise

If you think the metric should be placed in another place (e.g., execution layer metrics) and rename to execution_verified, then I can revise to that

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah looks good! Thanks!

@michaelsproul michaelsproul added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Sep 22, 2025
@chong-he chong-he added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Oct 6, 2025
@michaelsproul michaelsproul added the v8.1.0 Post-Fulu release label Oct 29, 2025
@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Nov 12, 2025
@mergify mergify Bot added the queued label Nov 12, 2025
mergify Bot added a commit that referenced this pull request Nov 12, 2025
@mergify mergify Bot merged commit d54dc68 into sigp:unstable Nov 12, 2025
36 checks passed
@mergify mergify Bot removed the queued label Nov 12, 2025
@chong-he chong-he deleted the metrics-optimistic branch December 17, 2025 02:22
@jimmygchen jimmygchen mentioned this pull request Feb 5, 2026
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

low-hanging-fruit Easy to resolve, get it before someone else does! ready-for-merge This PR is ready to merge. UX-and-logs v8.1.0 Post-Fulu release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants