Skip to content

PohRecorder - sharable tick heights#7355

Merged
apfitzge merged 5 commits intoanza-xyz:masterfrom
apfitzge:poh_recorder_shared_tick_heights
Aug 11, 2025
Merged

PohRecorder - sharable tick heights#7355
apfitzge merged 5 commits intoanza-xyz:masterfrom
apfitzge:poh_recorder_shared_tick_heights

Conversation

@apfitzge
Copy link
Copy Markdown

@apfitzge apfitzge commented Aug 6, 2025

Problem

  • Observing tick height requires locking poh
  • For leader decision, we currently want to know how far from leader we are.
    • currently this means we lock poh recorder very often

Summary of Changes

  • Make tick_height and leader_first_tick_height into Arc<AtomicU64> (flag for None)
  • Wrap both in safe interfaces so that things external to PohRecorder cannot modify them

Fixes #

Comment thread poh/src/poh_recorder.rs
// Uses the flag of u64::MAX to indicate None; this does not
// need to be observable outside of PohRecorder.
#[derive(Clone)]
pub struct SharedLeaderFirstTickHeight(Arc<AtomicU64>);
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thought about having the same struct as above, but the loading is different since this can be None (via u64::MAX flag).

Thought cleaner to just separate them, even though they are very similar. Will also make it a bit harder to confuse the two once the shared structs are cloned outside of PohRecorder.

@apfitzge
Copy link
Copy Markdown
Author

apfitzge commented Aug 6, 2025

This, with other recent changes, can allow us to make DecisionMaker lock-free.
It will also be used in scheduler-bindings to provide leader-status messages:

  • how long until we become leader
  • progress in leader slot

until alpenglow**.

@apfitzge apfitzge mentioned this pull request Aug 6, 2025
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Aug 6, 2025

Codecov Report

❌ Patch coverage is 93.42105% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.2%. Comparing base (6e14749) to head (59ba4c8).
⚠️ Report is 31 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7355   +/-   ##
=======================================
  Coverage    83.2%    83.2%           
=======================================
  Files         797      797           
  Lines      361115   361154   +39     
=======================================
+ Hits       300699   300760   +61     
+ Misses      60416    60394   -22     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@apfitzge apfitzge marked this pull request as ready for review August 6, 2025 20:50
@apfitzge apfitzge requested a review from jstarry August 6, 2025 20:50
apfitzge added a commit to apfitzge/agave that referenced this pull request Aug 6, 2025
@apfitzge
Copy link
Copy Markdown
Author

apfitzge commented Aug 7, 2025

#7372 is a follow-up to this change.

Comment thread poh/src/poh_recorder.rs
Comment thread poh/src/poh_recorder.rs Outdated
Comment thread poh/src/poh_recorder.rs
@apfitzge apfitzge merged commit 7d25178 into anza-xyz:master Aug 11, 2025
40 checks passed
@apfitzge apfitzge deleted the poh_recorder_shared_tick_heights branch August 11, 2025 14: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.

3 participants