TriggerTimerMixConcensus test#384
Open
SirTyson wants to merge 1 commit intostellar:mainfrom
Open
Conversation
6 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for exercising the experimental EXPERIMENTAL_TRIGGER_TIMER behavior in Supercluster missions, including a new mission that mixes enabled/disabled nodes and injects configurable clock drift.
Changes:
- Introduces
TriggerTimerMixConsensusmission with per-node trigger-timer enablement and clock drift distributions. - Threads new core configuration options (
experimentalTriggerTimer, per-node clock offsets) into core-set options and TOML generation. - Extends CLI and mission context to carry trigger-timer/drift parameters; optionally enables trigger timer in MaxTPSClassic.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/FSLibrary/StellarMissionContext.fs | Adds trigger-timer/drift fields to MissionContext. |
| src/FSLibrary/StellarMission.fs | Registers the new TriggerTimerMixConsensus mission. |
| src/FSLibrary/StellarCoreSet.fs | Adds core-set options for trigger timer and clock offsets with defaults. |
| src/FSLibrary/StellarCoreCfg.fs | Emits TOML settings for trigger timer and artificial clock offset; maps per-node offsets. |
| src/FSLibrary/MissionTriggerTimerMixConsensus.fs | New mission implementing mixed trigger-timer enablement and drift sampling. |
| src/FSLibrary/MissionMaxTPSMixed.fs | Updates maxTPSTest call-site for new signature. |
| src/FSLibrary/MissionMaxTPSClassic.fs | Wires MaxTPSClassic to optionally enable trigger timer. |
| src/FSLibrary/MaxTPSTest.fs | Adds enableTriggerTimer parameter and applies it to all CoreSets. |
| src/FSLibrary/FSLibrary.fsproj | Includes new mission source file in the build. |
| src/FSLibrary.Tests/Tests.fs | Updates test MissionContext literal with new required fields. |
| src/App/Program.fs | Adds CLI options for trigger-timer/drift and passes them into MissionContext. |
| run-trigger-timer.sh | Adds a helper script to run the new mission with drift settings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+35
to
+36
| if ms >= 0 then (ms + 999) / 1000 | ||
| else -((abs ms + 999) / 1000) |
Comment on lines
+109
to
+118
| let sampleFlag () = rng.Next(100) < flagPct | ||
|
|
||
| let sampleOffset () = | ||
| match drift with | ||
| | NoDrift -> 0 | ||
| | _ when rng.Next(100) >= driftPct -> 0 | ||
| | UniformDrift (lower, upper) -> rng.Next(lower, upper + 1) | ||
| | BimodalDrift (min1, max1, min2, max2) -> | ||
| if rng.Next(2) = 0 then rng.Next(min1, max1 + 1) | ||
| else rng.Next(min2, max2 + 1) |
Comment on lines
+120
to
+154
| // Walk through CoreSets, splitting each into single-node CoreSets so that | ||
| // each node gets its own name with flag/drift annotation. | ||
| let modifiedCoreSets = | ||
| baseCoreSets | ||
| |> List.collect (fun cs -> | ||
| let nc = cs.options.nodeCount | ||
|
|
||
| [ for j in 0 .. nc - 1 do | ||
| let flagEnabled = sampleFlag () | ||
| let offset = sampleOffset () | ||
|
|
||
| let baseName = | ||
| if nc > 1 then sprintf "%s-%d" cs.name.StringName j | ||
| else cs.name.StringName | ||
|
|
||
| let annotatedName = annotateName baseName flagEnabled offset | ||
|
|
||
| LogInfo | ||
| " Node %s: trigger_timer=%b, offset=%d" | ||
| annotatedName.StringName | ||
| flagEnabled | ||
| offset | ||
|
|
||
| { cs with | ||
| name = annotatedName | ||
| keys = [| cs.keys.[j] |] | ||
| options = | ||
| { cs.options with | ||
| nodeCount = 1 | ||
| nodeLocs = | ||
| cs.options.nodeLocs | ||
| |> Option.map (fun locs -> [ locs.[j] ]) | ||
| experimentalTriggerTimer = if flagEnabled then Some true else None | ||
| clockOffsets = if offset <> 0 then Some [ offset ] else None } } ]) | ||
|
|
Comment on lines
+629
to
+633
| [<Option("disable-trigger-timer", | ||
| HelpText = "Disable EXPERIMENTAL_TRIGGER_TIMER on all nodes in MaxTPSClassic (default: enabled)", | ||
| Required = false, | ||
| Default = false)>] | ||
| member self.DisableTriggerTimer = disableTriggerTimer |
Comment on lines
+270
to
+276
| match self.experimentalTriggerTimer with | ||
| | Some v -> t.Add("EXPERIMENTAL_TRIGGER_TIMER", v) |> ignore | ||
| | None -> () | ||
|
|
||
| match self.clockOffsetMs with | ||
| | Some offset -> t.Add("ARTIFICIALLY_SET_SYSTEM_CLOCK_OFFSET_FOR_TESTING", int64 offset) |> ignore | ||
| | None -> () |
Comment on lines
+3
to
+23
| IMAGE=docker-registry.services.stellar-ops.com/dev/stellar-core:25.1.2-3047.7a0d9bcd2.jammy-do-not-use-in-prd-perftests | ||
|
|
||
| PROJECT="/mnt/xvdf/supercluster/src/App/App.fsproj" | ||
|
|
||
| # -- Drift distribution (uncomment one) -- | ||
| # No drift: | ||
| #DRIFT_ARGS="" | ||
| # Uniform drift in [-2000, +2000]ms: | ||
| #DRIFT_ARGS="--uniform-drift=-2000,+2000 --drift-pct 70" | ||
| # Bimodal drift: first half [-5000,-2000]ms, second half [+2000,+5000]ms: | ||
| DRIFT_ARGS="--bimodal-drift=-5000,-2000,+2000,+5000 --drift-pct 70" | ||
|
|
||
| dotnet run --project $PROJECT clean --namespace=garand && dotnet run --project $PROJECT --configuration Release \ | ||
| -- mission TriggerTimerMixConsensus \ | ||
| --image=$IMAGE \ | ||
| --netdelay-image=docker-registry.services.stellar-ops.com/dev/sdf-netdelay:latest \ | ||
| --postgres-image=docker-registry.services.stellar-ops.com/dev/postgres:9.5.22 \ | ||
| --nginx-image=docker-registry.services.stellar-ops.com/dev/nginx:latest \ | ||
| --prometheus-exporter-image=docker-registry.services.stellar-ops.com/dev/stellar-core-prometheus-exporter:latest \ | ||
| --ingress-internal-domain=stellar-supercluster.kube001-ssc-eks.services.stellar-ops.com \ | ||
| --avoid-node-labels=purpose:ssc \ |
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.
Adds a testing mode the for new experimental trigger timer. Generates load on a topology, where you can specify clock drift and the percentage of nodes with the new flag enabled.