Skip to content

Conversation

@carllin
Copy link
Contributor

@carllin carllin commented Feb 28, 2025

Problem

flag use_alpenglow_tick_producer is not enabled at startup

Summary of Changes

Fixes #

if !poh_exit.load(Ordering::Relaxed)
// Should be set by replay_stage after it sees a notarized
// block in the new alpenglow epoch
&& poh_recorder.read().unwrap().is_alpenglow_enabled
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you still need this test? Maybe just say the only way it exits the previous loop is either:

  1. poh_exit is true
  2. alpenglow is enabled
    So only exit the loop if 1 is true.
    Then you can move the info loop to line 169~175?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The info log is only necessary if we started in not alpenglow mode, and then migrated to alpenglow. If we started in alpenglow mode, there's no need to print that we're migrating

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm ok, we can also do:
if poh_exit.load(Ordering::Relaxed) {
return;
} else { // must exit because of Alpenglow
info(...)
}
Your choice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm actually I might like the original better, I want to explicitly check that alpenglow was enabled in case somebody else adds another path/error on which poh would exit

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, that's fair.

if !poh_exit.load(Ordering::Relaxed)
// Should be set by replay_stage after it sees a notarized
// block in the new alpenglow epoch
&& poh_recorder.read().unwrap().is_alpenglow_enabled
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, that's fair.

@carllin carllin merged commit af4ac1d into anza-xyz:master Feb 28, 2025
7 checks passed
carllin added a commit that referenced this pull request Mar 1, 2025
PohService needs to set `use_alpenglow_tick_producer` flag on startup  (#59)
carllin added a commit that referenced this pull request Mar 1, 2025
PohService needs to set `use_alpenglow_tick_producer` flag on startup  (#59)
bw-solana pushed a commit to bw-solana/alpenglow that referenced this pull request Aug 1, 2025
PohService needs to set `use_alpenglow_tick_producer` flag on startup  (anza-xyz#59)
bw-solana pushed a commit to bw-solana/alpenglow that referenced this pull request Aug 1, 2025
PohService needs to set `use_alpenglow_tick_producer` flag on startup  (anza-xyz#59)
bw-solana pushed a commit to bw-solana/alpenglow that referenced this pull request Aug 1, 2025
PohService needs to set `use_alpenglow_tick_producer` flag on startup  (anza-xyz#59)
bw-solana pushed a commit to bw-solana/alpenglow that referenced this pull request Aug 1, 2025
PohService needs to set `use_alpenglow_tick_producer` flag on startup  (anza-xyz#59)
bw-solana pushed a commit to bw-solana/alpenglow that referenced this pull request Aug 1, 2025
PohService needs to set `use_alpenglow_tick_producer` flag on startup  (anza-xyz#59)
bw-solana pushed a commit to bw-solana/alpenglow that referenced this pull request Aug 1, 2025
PohService needs to set `use_alpenglow_tick_producer` flag on startup  (anza-xyz#59)
bw-solana pushed a commit to bw-solana/alpenglow that referenced this pull request Aug 2, 2025
PohService needs to set `use_alpenglow_tick_producer` flag on startup  (anza-xyz#59)
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