Small Bug Fixes and performance flags by SirTyson · Pull Request #437 · stellar/supercluster

SirTyson · 2026-09-23T21:07:08Z

Stack: #437 → #438 → #439

First of three stacked PRs with a set of changes I've been using to test our milestone targets via MinBlockTimeTest. Commits are as follows:

c2ba02a Serialize concurrent StatefulSet updates in WithLive. This is a bug fix. The mission restarts all nodes at once, and each restart updates shared state (the network config and the StatefulSet list) without
synchronization. Two restarts could race, so one node's update was lost: its StatefulSet came back with 0 replicas and the network waited forever for it. A lock around the update fixes this. Waiting for readiness
stays outside the lock, so nodes still start in parallel.
f60dff8 Add --core-env. A repeatable NAME=VALUE flag that adds environment variables to the stellar-core container, which the overlay child process inherits. This let's us play with ENV configs required by some rust libraries.
3cdf282 Adds --one-stellar-core-per-host. During testing, I found that scheduling multiple validators on a single node had significant perf implications. This flag makes it such that we schedule all validators on separate worker nodes. Note that for larger topologies, or when the cluster is being used, this might not be possible, in which case we fail relatively fast during scheduling.

Copilot

Copilot review overview

🟡 Changes recommended

Unresolved environment-variable validation and placement-handling issues remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 2

Open (3)

Allowing core-env collisions overrides fixed peer environment variables · New
Placement violation aborts mission instead of failing candidate · New
Empty core-env entries bypass malformed-input validation · New

What changed in this PR

Adds WithLive race protection and opt-in controls for core environment variables, startup cleanup, and validator placement.

Changes:

Serializes concurrent StatefulSet updates.
Adds --core-env and --skip-startup-sweep.
Adds validator anti-affinity and placement validation.

File	Description
`src/FSLibrary/StellarStatefulSets.fs`	Serializes live-state updates.
`src/FSLibrary/StellarOrphanSweep.fs`	Adds startup sweep decision logic.
`src/FSLibrary/StellarMissionContext.fs`	Adds mission fields and environment parsing.
`src/FSLibrary/StellarKubeSpecs.fs`	Adds environment injection and validator affinity.
`src/FSLibrary/StellarFormation.fs`	Provides formation state locking.
`src/FSLibrary/StellarCoreCfg.fs`	Defines validator labels.
`src/FSLibrary/MinBlockTimeTest.fs`	Validates validator placement.
`src/FSLibrary.Tests/Tests.fs`	Tests new controls and placement behavior.
`src/App/Program.fs`	Exposes and wires new options.
`doc/one-validator-per-host.md`	Documents placement behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 3

Open (4)

Allowing core-env collisions overrides fixed peer environment variables
Reject blank entries and invalid environment variable names · New
Empty core-env entries bypass malformed-input validation
Placement violation aborts mission instead of failing candidate

Copilot

Copilot review overview

🟡 Changes recommended

Four moderate issues remain around rate limiting, reserved environment variables, and malformed --core-env validation.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 4

Open (5)

Allowing core-env collisions overrides fixed peer environment variables
Rate-limit the new namespaced pod list request · New
Reject blank entries and invalid environment variable names
Empty core-env entries bypass malformed-input validation
Placement violation aborts mission instead of failing candidate

Copilot

Copilot review overview

🟡 Changes recommended

Unresolved moderate findings affect environment validation, autoscaler detection, and job-only mission behavior.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 5

Open (6)

Allowing core-env collisions overrides fixed peer environment variables
Stale pod events can misclassify replacement validator runs · New
Rate-limit the new namespaced pod list request
Reject blank entries and invalid environment variable names
Empty core-env entries bypass malformed-input validation
Placement violation aborts mission instead of failing candidate

Copilot

Copilot review overview

🔵 Needs a closer look

Two unresolved moderate placement and scheduling issues remain in StellarStatefulSets.fs.

Review effort: Lite
Findings: 2

Open (2)

Rate-limit the new namespaced pod list request
Placement violation aborts mission instead of failing candidate

Resolved since last review (4)

Allowing core-env collisions overrides fixed peer environment variables
Stale pod events can misclassify replacement validator runs
Reject blank entries and invalid environment variable names
Empty core-env entries bypass malformed-input validation

Copilot

Copilot review overview

🔵 Needs a closer look

Address the two moderate issues involving parallel-catchup environment propagation and stale autoscaler events.

Review effort: Lite
Findings: 2

Open (2)

Rate-limit the new namespaced pod list request
Placement violation aborts mission instead of failing candidate

drebelsky

The first two commits look fine to me (just some small nits that could be ignored). The third one feels a little more odd to me. From the naming, I would've expected --one-validator-per-host to only apply to validators but it also applies to watchers. Then, there are some places where the checking might get missed (e.g., restarts in max tps). More holistically, though, it seems like this might be the wrong layer to make the change. If we're only scheduling one stellar-core instance per host, the kube spec sizes and Karpenter sizes are very mismatched (perhaps we should instead do something like how the MaxTPS Jenkins job got adjusted to have one properly sized host per pod).

SirTyson · 2026-09-25T22:37:07Z

Thanks for your feedback! I believe it's been addressed

Copilot

Copilot review overview

🔵 Needs a closer look

One or more issues must be addressed before approval.

Review effort: Lite
Findings: None

Resolved since last review (2)

Rate-limit the new namespaced pod list request
Placement violation aborts mission instead of failing candidate

drebelsky

The first two commits seem fine. I don't fully understand the third commit, but since it is gated behind the flag, intended to unstick perf-testing, and is otherwise mostly a no-op, it seems okay.

Missions stop and start core sets concurrently (e.g. MaxTPS and MinBlockTime restart every node with Async.Parallel). WithLive's read-modify-write of the formation's network config and StatefulSet list was unsynchronized, so two core sets could race and one's live update was lost: its StatefulSet was rebuilt with 0 replicas and the network waited forever for the missing peer. Guard the state update and the StatefulSet replace with a per-formation lock. The readiness wait stays outside the lock so nodes still come up in parallel. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

NAME=VALUE entries, given after one --core-env as `--core-env A=1 B=2`, are added to the stellar-core container environment after the fixed variables; the overlay child process inherits them. Blank or malformed entries, names that are not environment variable names and duplicate names are rejected, so a typo cannot silently drop a setting. So are STELLAR_CORE_PEER_SHORT_NAME, which selects the pod's config, and ASAN_OPTIONS (set by --asan-options): the harness sets both itself. Without the flag the container environment is unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

The soft topology spread lets the scheduler pack several stellar-core pods on one worker node when few nodes are available, and co-located nodes interfere through resources their requests do not reserve (cores, caches, memory bandwidth, the NIC and the kernel's network stack), which silently changes what a benchmark measures. With the flag, the run's stellar-core StatefulSet pods, validators and watchers alike, get a label and a required pod anti-affinity against each other on kubernetes.io/hostname, scoped to this run so job pods and the HTTP proxy are unaffected. Each core container also requests its limits, so an autoscaler provisions a node sized to what the pod may use rather than to its much smaller request. For identical nodes across runs, pin the instance type too, e.g. --require-node-labels node.kubernetes.io/instance-type:<type>. The placement is checked in one place, WaitForAllReplicasReady, every time a core set's pods come up: at formation creation and on every restart, in every mission. It logs where the pods landed and fails unless they are scheduled and no two of the run's stellar-core pods share a node. When the cluster cannot provide the nodes, the mission fails with the scheduler's reason instead of waiting forever for replicas that can never become ready. It fails as soon as the cluster autoscaler (Karpenter or cluster-autoscaler) reports it cannot provision a node for a pod, or once a pod has been unschedulable for 3 minutes (10 while an autoscaler is provisioning nodes; that normally takes under a minute). Only namespaced pod and event reads are used: the harness typically cannot list nodes, so an up-front node count is not possible. Off by default; without it, pod specs are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Copilot AI lite review requested due to automatic review settings September 23, 2026 21:07

This was referenced Sep 23, 2026

Fixes MinBlockTime and adds artificial tier 1 topology flag #438

Open

Add --overlay-v2-optimized for the Rust-overlay stellar-core image #439

Open

Copilot started reviewing on behalf of SirTyson September 23, 2026 21:07 View session