Add txset construction mode to apply load - #5448
Open
SirTyson wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a leader-side txset-construction-and-apply benchmarking mode to measure transaction-set construction, validation, and application.
Changes:
- Adds queue-overfilling construction mode and timing metrics.
- Extends configuration, tests, and benchmark examples.
- Documents the new mode and metrics.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Review |
|---|---|
src/simulation/test/LoadGeneratorTests.cpp |
Adds SAC coverage; missing construction-mode coverage for CUSTOM_TOKEN and SOROSWAP (nit). |
src/simulation/ApplyLoad.h |
Declares construction timing helpers and state. |
src/simulation/ApplyLoad.cpp |
Implements construction timing, but duplicates and omits production nomination behavior and inflates global queue metrics (moderate). |
src/main/Config.h |
Adds the timing-mode enum value. |
src/main/Config.cpp |
Parses the new configuration value. |
src/herder/HerderImpl.h |
Declares the transaction-set build timer. |
src/herder/HerderImpl.cpp |
Instruments construction, but measures a mismatched span (moderate) and lacks performance evidence (nit). |
docs/software/commands.md |
Documents the mode as production nomination despite its synthetic implementation (moderate). |
docs/metrics.md |
Documents the build metric. |
docs/apply-load-ledger-limits.cfg |
Documents queue multiplier configuration. |
docs/apply-load-for-meta.cfg |
Documents queue multiplier configuration. |
docs/apply-load-benchmark-token.cfg |
Documents token benchmark configuration. |
docs/apply-load-benchmark-sac.cfg |
Documents SAC benchmark configuration. |
Suppressed comments (1)
src/herder/HerderImpl.cpp:1669
- The new production-path timer adds unconditional clock reads and metric updates to every leader nomination, but the PR provides no performance evidence (the checklist item is unchecked).
CONTRIBUTING.md:56-60requires evidence of improvement and no regression for performance-impacting changes; please include measurements that quantify this instrumentation and the new benchmark mode before merging.
auto const txSetBuildStart = std::chrono::steady_clock::now();
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+46
to
+50
| transactions are submitted to the real transaction queues (outside the | ||
| measured span, leaving the caches warm as overlay admission would on a | ||
| live node), then the ledger is triggered through production nomination: | ||
| the Herder pulls the queues, builds and validates the proposed tx set, | ||
| nominates it through local consensus (with the node as its own |
| // Since we are not currently applying, it is safe to use read-only LCL, as | ||
| // it's guaranteed to be up-to-date | ||
| auto lcl = mLedgerManager.getLastClosedLedgerHeader(); | ||
| auto const txSetBuildStart = std::chrono::steady_clock::now(); |
Comment on lines
+1106
to
+1108
| ClassicTransactionQueue classicQueue(mApp, 1, 1, txQueueMultiplier(false)); | ||
| SorobanTransactionQueue sorobanQueue(mApp, 1, 1, txQueueMultiplier(true), | ||
| {}); |
Comment on lines
+1104
to
+1108
| // Use production queue classes with a fresh backlog for each ledger. | ||
| // No aging or flooding is needed; admission warms the validation caches. | ||
| ClassicTransactionQueue classicQueue(mApp, 1, 1, txQueueMultiplier(false)); | ||
| SorobanTransactionQueue sorobanQueue(mApp, 1, 1, txQueueMultiplier(true), | ||
| {}); |
Comment on lines
1127
to
1130
| auto const timingPhases = | ||
| GENERATE(ApplyLoadTimingPhases::APPLY_ONLY, | ||
| ApplyLoadTimingPhases::TX_SET_CONSTRUCTION_AND_APPLY, | ||
| ApplyLoadTimingPhases::TX_SET_VALIDATION_AND_APPLY); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds a new mode to apply load, "txset-construction-and-apply". This mode times all leader work in a ledger, without any network work: tx set construction, validation, then application. It's the leader end-to-end variant of txset-validation-and-apply, which simulates a non-leader node with only tx set validation and application. In testing, it seems like the number of TXs in the queue has a significant impact on TX set building times, so unlike other apply load modes, this test will generate more TXs than will actually fit in the final block in order to overfill the queue.
Checklist
clang-formatv8.0.0 (viamake formator the Visual Studio extension)