Conversation
Performance Measurements ⏳
|
There was a problem hiding this comment.
Pull request overview
This PR adjusts the snapshot restore pipeline to reduce stem burst pressure (especially on snapin output links), consolidates the “expected hash” and “expected capitalization” signaling for the vinyl path into a single message, and updates topology reliability for the snapin_txn placeholder link.
Changes:
- Combine expected lthash + capitalization into
FD_SNAPSHOT_HASH_MSG_EXP_AND_CAPITALforsnapin -> snapwm -> snaplv, and remove the standalone capitalization message. - Reduce/retune
STEM_BURSTvalues forsnapin,snapwm, andsnaplv. - Mark
snapin_txnas an unreliable topology link in production and dev topologies.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/discof/restore/utils/fd_ssctrl.h | Introduces combined expected-hash+capitalization message and removes old capitalization message type/struct. |
| src/discof/restore/fd_snapwm_tile.c | Forwards the new combined message and reduces STEM_BURST. |
| src/discof/restore/fd_snaplv_tile_private.h | Adjusts FD_SNAPLV_STEM_BURST definition and commentary. |
| src/discof/restore/fd_snaplv_tile.c | Consumes the combined message, updates capitalization types/formatting, and removes old expected-hash handling for snapwm->snaplv. |
| src/discof/restore/fd_snapin_tile.c | Emits the combined message in vinyl mode, adds capitalization range check, and reduces STEM_BURST. |
| src/app/firedancer/topology.c | Marks snapin_txn as FD_TOPOB_UNRELIABLE. |
| src/app/firedancer-dev/commands/snapshot_load.c | Marks snapin_txn as FD_TOPOB_UNRELIABLE in the dev snapshot-load topology. |
| src/app/firedancer-dev/commands/backtest.c | Marks snapin_txn as FD_TOPOB_UNRELIABLE in the backtest topology. |
You can also share your feedback on Copilot code review. Take the survey.
eac0e61 to
5bf6c5f
Compare
Performance Measurements ⏳
|
5bf6c5f to
8f20994
Compare
Performance Measurements ⏳
|
There was a problem hiding this comment.
Pull request overview
This PR adjusts the snapshot restore pipeline messaging and topology to reduce output-link credit pressure (notably by lowering STEM_BURST in several tiles), while also simplifying the expected-hash/expected-capitalization signaling in the vinyl path.
Changes:
- Reduce
STEM_BURSTforsnapin,snapwm, andsnaplvto better match per-cycle publish expectations and lowercr_availpressure. - Combine “expected hash” and “expected capitalization” into a single control message for the
snapin -> snapwm -> snaplvpath (reusingfd_ssctrl_hash_result_t). - Mark
snapin_txnas an unreliable topology link (since it is used as a dcache placeholder/scratch channel).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/discof/restore/utils/fd_ssctrl.h | Introduces combined expected-hash+capitalization sig; removes standalone capitalization type; updates sig-to-string mapping. |
| src/discof/restore/fd_snapwm_tile_vinyl.c | Adds a FIXME note regarding deprecated duplicate-accounts lthash path usage. |
| src/discof/restore/fd_snapwm_tile.c | Updates snapwm to forward the combined expected-hash+capitalization message; reduces STEM_BURST. |
| src/discof/restore/fd_snaplv_tile_private.h | Re-defines FD_SNAPLV_STEM_BURST based on duplicate-batch worst-case behavior. |
| src/discof/restore/fd_snaplv_tile.c | Consumes the combined expected-hash+capitalization message; switches manifest capitalization handling to long. |
| src/discof/restore/fd_snapin_tile.c | Sends combined expected-hash+capitalization message in vinyl mode; adds LONG_MAX guard for manifest capitalization; reduces STEM_BURST. |
| src/app/firedancer/topology.c | Marks snapin_txn input to snapwm as FD_TOPOB_UNRELIABLE. |
| src/app/firedancer-dev/commands/snapshot_load.c | Mirrors snapin_txn unreliability in the snapshot-load dev topology. |
| src/app/firedancer-dev/commands/backtest.c | Mirrors snapin_txn unreliability in the backtest dev topology. |
You can also share your feedback on Copilot code review. Take the survey.
8f20994 to
c66e969
Compare
Performance Measurements ⏳
|
Minimizing
snapinSTEM_BURST to 2.This reduces pressure on
cr_availonsnapinoutput links.Combining
expected hashandexpected capitalizationinto a single message (reusing existingfd_ssctrl_hash_result_t).Updating
snapin_txnlink as unreliable in topology.Minor updates to
snapwmandsnaplvSTEM_BURST as well.