Add support for measuring tx e2e latency - #417
Conversation
There was a problem hiding this comment.
Pull request overview
Adds configurable load-generator nodes and gated transaction end-to-end latency metrics for pubnet performance missions.
Changes:
- Selects load generators from a public-key file.
- Enables and logs e2e latency metrics only for selected generators.
- Adds an e2e measurement preset and expands the SLP evaluation wrapper.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/FSLibrary/StellarNetworkData.fs |
Maps loadgen keys to dedicated core sets. |
src/FSLibrary/StellarMissionContext.fs |
Adds measurement and loadgen settings. |
src/FSLibrary/StellarCoreSet.fs |
Marks load-generating core sets. |
src/FSLibrary/StellarCoreCfg.fs |
Gates the stellar-core latency metric. |
src/FSLibrary/MinBlockTimeTest.fs |
Selects generators and logs latency metrics. |
src/FSLibrary/MaxTPSTest.fs |
Uses configured load generators. |
src/FSLibrary/json-type-samples/sample-metrics.json |
Defines latency metric shapes. |
src/FSLibrary.Tests/Tests.fs |
Updates the test mission context. |
src/App/Program.fs |
Adds and validates CLI options. |
scripts/slp_eval.sh |
Supports topology overrides and forwarded arguments. |
scripts/measure_e2e.sh |
Adds the e2e measurement preset. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for removedNode in Set.difference loadgenKeySet keptNodes do | ||
| LogWarn "Removed loadgen node %s from simulation" removedNode |
There was a problem hiding this comment.
seems it would be better to crash to explicitly flag a misconfiguration to the operator
| if self.measureE2eLatency && self.network.missionContext.measureE2eLatency then | ||
| t.Add("LOADGEN_MEASURE_TX_E2E_LATENCY_FOR_TESTING", true) |> ignore |
marta-lokhova
left a comment
marta-lokhova
left a comment
There was a problem hiding this comment.
Overall LGTM, though I think it'd be good to merge once p28 and CI stabilize, and avoid introducing new (potentially unstable) changes. Let's merge after the release is done, so likely next week!
| // from the HomeDomains of the public network but with periods replaced with dashes, | ||
| // and lowercased, so for example keybase.io turns into keybase-io. | ||
| let groupedOrgNodes : (HomeDomainName * PubnetNode.Root array) array = | ||
| // Additionally, each load generator needs to exist in a size-1 org (because |
There was a problem hiding this comment.
hmm, is this necessary? in other missions, loadgen nodes are just assigned to the first node in CoreSet ordering
Updated version of #394, trimmed down to just the changes needed for our e2e measurements.