From 473acdb2f3d74b90811810500b9f4e8a1f5d0cd0 Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Wed, 26 Aug 2026 11:00:20 -0400 Subject: [PATCH 01/22] feat(gitignore): add docs/plans and logs-smoke to ignore list --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 8a57e922..20b55938 100644 --- a/.gitignore +++ b/.gitignore @@ -18,3 +18,6 @@ .fake .ionide .idea + +docs/plans +logs-smoke \ No newline at end of file From 9285bf392db0646c6615b94708ebe8506deabe19 Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Wed, 26 Aug 2026 13:45:12 -0400 Subject: [PATCH 02/22] feat(catchup): retire workers the queue cannot keep busy A run holds its full fleet for its whole duration, including the tail where almost nothing is left: 37.7% of prod worker-hours are idle, and at the extreme 1020 workers wait 4.5 hours on 4 remaining jobs. Each worker becomes its own single-replica StatefulSet, so any idle worker can be removed without regard to position; scaling one shared StatefulSet only ever deletes a suffix of its ordinals, which a single long range on a high ordinal can pin for hours. Each poll marks up to `ready - outstanding` idle workers into a Redis set that worker.sh checks before claiming, and deletes the ones marked on an earlier pass that are still idle. The one-pass gap is the safety property: a marked worker stops claiming within its 10s loop, so by the next poll it provably takes no new work. Busy workers come from reading job_owners directly rather than from the status snapshot, which the monitor only republishes after serially pinging every owner and so can be older than the mark. Capacity is Running pods, not the replica count: a Pending pod owns no job and so reads as idle capacity it cannot supply. Logs are collected before each deletion, since /data is emptyDir, and a failed collection skips the removal rather than losing them. --- .../MissionHistoryPubnetParallelCatchupV2.fs | 95 +++++++++++++++++-- .../parallel_catchup_helm/files/worker.sh | 7 ++ .../templates/catchup_workers.yaml | 56 ++++++----- 3 files changed, 126 insertions(+), 32 deletions(-) diff --git a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs index 001981ea..fd72cb32 100644 --- a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs +++ b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs @@ -47,6 +47,8 @@ let failedJobLogStreamLineCount = 1000 let mutable nonce : String = "" let mutable helmReleaseName : String = "" +// Pods not yet retired; module scope because cleanup runs from a signal handler. +let mutable livePods : Set = Set.empty let jobMonitorHostName (context: MissionContext) = match context.jobMonitorExternalHost with @@ -241,12 +243,9 @@ let installProject (context: MissionContext) = // 1. Automatically determines worker pod names from context.pubnetParallelCatchupNumWorkers // 2. For each pod, finds all files matching "stellar-core-*.log" in /data // 3. Creates a tar.gz archive and copies it to context.destination directory -let collectLogsFromPods (context: MissionContext) = - // Generate pod names based on number of workers - // Pod names follow the pattern: -stellar-core-0, -stellar-core-1, etc. - let podNames = - [ 0 .. context.pubnetParallelCatchupNumWorkers - 1 ] - |> List.map (fun i -> sprintf "%s-stellar-core-%d" helmReleaseName i) +// Returns the pods whose collection raised; an empty archive is success. +let collectLogsFromPods (context: MissionContext) (podNames: string list) : string list = + let mutable failed = [] LogInfo "Collecting logs from %d worker pods to directory: %s" (List.length podNames) context.destination.Path @@ -281,6 +280,40 @@ let collectLogsFromPods (context: MissionContext) = with ex -> LogWarn "Could not collect logs from pod %s (this is expected if pod doesn't exist): %s" podName ex.Message + failed <- podName :: failed + + failed + +// Running pods only, since a Pending pod reads as idle capacity it cannot supply. +let readyPods (context: MissionContext) : Set = + let selector = "app=" + helmReleaseName + "-stellar-core" + + let pods = + context.kube.ListNamespacedPod(context.namespaceProperty, labelSelector = selector) + + pods.Items + |> Seq.filter (fun pod -> pod.Status.Phase = "Running") + |> Seq.map (fun pod -> pod.Metadata.Name) + |> Set.ofSeq + +// Runs redis-cli in a ready worker and returns its output lines. +let redisIn (context: MissionContext) (host: string) (args: string) : string list = + let outFile = Path.Combine(Path.GetTempPath(), helmReleaseName + "-redis.txt") + let sh = sprintf "redis-cli -h \"$REDIS_HOST\" -p \"$REDIS_PORT\" %s" args + let cmd = [| "sh"; "-c"; sh |] + + RemoteCommandRunner.RunRemoteCommandAndCaptureOutput( + context.kube, + context.namespaceProperty, + host, + "stellar-core", + cmd, + outFile + ) + + File.ReadAllLines outFile + |> Array.toList + |> List.filter (fun l -> l.Trim() <> "") // Cleanup on exit. `signalTriggered` indicates we're running under a hard // deadline (Jenkins' SoftKillWaitSeconds, ~5s by default, before SIGKILL). @@ -313,7 +346,7 @@ let cleanup (signalTriggered: bool) (context: MissionContext) = try LogInfo "Attempting to collect worker logs before cleanup..." let stopwatch = Stopwatch.StartNew() - collectLogsFromPods context + collectLogsFromPods context (List.ofSeq livePods) |> ignore stopwatch.Stop() LogInfo "Log collection completed in %.2f seconds" stopwatch.Elapsed.TotalSeconds with ex -> LogWarn "Failed to collect some or all worker logs: %s" ex.Message @@ -395,6 +428,12 @@ let historyPubnetParallelCatchupV2 (context: MissionContext) = installProject context let mutable allJobsFinished = false + + livePods <- + Set.ofList [ for i in 0 .. context.pubnetParallelCatchupNumWorkers - 1 -> + sprintf "%s-stellar-core-%d-0" helmReleaseName i ] + // Marks from earlier passes only, so nothing is removed in the pass that marked it. + let mutable marked : Set = Set.empty let mutable timeoutLeft = jobMonitorStatusCheckTimeOutSecs let mutable timeBeforeNextMetricsCheck = jobMonitorMetricsCheckIntervalSecs let jobMonitorPath = "/" + context.namespaceProperty + "/" + helmReleaseName @@ -424,6 +463,48 @@ let historyPubnetParallelCatchupV2 (context: MissionContext) = failwith "Catch up failed, check logs for more info" + // `queue_remain_count`, not `num_remain`, which is 1 as a pre-first-poll sentinel. + let outstanding = status.Value("queue_remain_count") + JobsInProgress.Count + + try + let ready = readyPods context + // Read job_owners directly so it is current, not as old as the status snapshot. + let busy = redisIn context (Seq.head ready) "HVALS job_owners" |> Set.ofList + let removable = marked |> Set.filter (fun p -> ready.Contains p && not (busy.Contains p)) + + if not removable.IsEmpty then + // /data is emptyDir, so a pod removed before its logs are read loses them. + match collectLogsFromPods context (List.ofSeq removable) with + | [] -> + for pod in removable do + let sts = pod.Substring(0, pod.Length - 2) + + context.kube.DeleteNamespacedStatefulSet(sts, context.namespaceProperty) + |> ignore + + marked <- Set.difference marked removable + livePods <- Set.difference livePods removable + LogInfo "Retired %d workers (%d outstanding)" removable.Count outstanding + | failed -> LogWarn "Not retiring: log collection failed for %d workers" failed.Length + + let toMark = + ready + |> Seq.filter (fun p -> not (busy.Contains p) && not (marked.Contains p)) + |> Seq.truncate (max 0 (ready.Count - outstanding)) + |> List.ofSeq + + // Chunked because RunRemoteCommand rejects a command of 4096 bytes or more. + for chunk in List.chunkBySize 30 toMark do + let names = chunk |> List.map (sprintf "'%s'") |> String.concat " " + + redisIn context (Seq.head ready) (sprintf "SADD \"%s-retiring\" %s" helmReleaseName names) + |> ignore + + if not toMark.IsEmpty then + marked <- Set.union marked (Set.ofList toMark) + LogInfo "Marked %d retiring (%d ready, %d outstanding)" toMark.Length ready.Count outstanding + with ex -> LogWarn "Worker scale-down skipped this pass: %s" ex.Message + if remainSize = 0 && JobsInProgress.Count = 0 then // All jobs completed — perform a final query on the metrics queryJobMonitor (context, jobMonitorPath, jobMonitorMetricsEndPoint) |> ignore diff --git a/src/MissionParallelCatchup/parallel_catchup_helm/files/worker.sh b/src/MissionParallelCatchup/parallel_catchup_helm/files/worker.sh index b8efc18b..af757277 100644 --- a/src/MissionParallelCatchup/parallel_catchup_helm/files/worker.sh +++ b/src/MissionParallelCatchup/parallel_catchup_helm/files/worker.sh @@ -22,6 +22,13 @@ SLEEP_INTERVAL=10 LOG_DIR="/data" while true; do +# Stop claiming once the driver marks us, so it can remove us without interrupting a range. +if [ "$(redis-cli -h "$REDIS_HOST" -p "$REDIS_PORT" SISMEMBER "$RELEASE_NAME-retiring" "$POD_NAME")" = "1" ]; then + echo "$(date) $POD_NAME is retiring; not claiming." + sleep $SLEEP_INTERVAL + continue +fi + # Fetch the next job key from the Redis queue. # Our ranges are generated in the order we want to run them from left to right, so we always pull from the left JOB_KEY=$(redis-cli -h "$REDIS_HOST" -p "$REDIS_PORT" LMOVE "$JOB_QUEUE" "$PROGRESS_QUEUE" LEFT LEFT) diff --git a/src/MissionParallelCatchup/parallel_catchup_helm/templates/catchup_workers.yaml b/src/MissionParallelCatchup/parallel_catchup_helm/templates/catchup_workers.yaml index e861a3e3..cc97f333 100644 --- a/src/MissionParallelCatchup/parallel_catchup_helm/templates/catchup_workers.yaml +++ b/src/MissionParallelCatchup/parallel_catchup_helm/templates/catchup_workers.yaml @@ -21,39 +21,43 @@ metadata: {{- end }} {{- end }} --- +{{- range $i := until (int $.Values.worker.replicas) }} apiVersion: apps/v1 kind: StatefulSet metadata: - name: {{ .Release.Name }}-stellar-core + name: {{ $.Release.Name }}-stellar-core-{{ $i }} labels: - app: {{ .Release.Name }}-stellar-core + app: {{ $.Release.Name }}-stellar-core + worker-index: {{ $i | quote }} spec: - serviceName: "{{ .Release.Name }}-stellar-core" + serviceName: "{{ $.Release.Name }}-stellar-core" podManagementPolicy: Parallel - replicas: {{ .Values.worker.replicas }} + replicas: 1 selector: matchLabels: - app: {{ .Release.Name }}-stellar-core + app: {{ $.Release.Name }}-stellar-core + worker-index: {{ $i | quote }} template: metadata: labels: - app: {{ .Release.Name }}-stellar-core + app: {{ $.Release.Name }}-stellar-core + worker-index: {{ $i | quote }} spec: - serviceAccountName: stellar-supercluster-{{ .Release.Name }} - {{- if or .Values.worker.requireNodeLabels .Values.worker.avoidNodeLabels }} + serviceAccountName: stellar-supercluster-{{ $.Release.Name }} + {{- if or $.Values.worker.requireNodeLabels $.Values.worker.avoidNodeLabels }} affinity: nodeAffinity: requiredDuringSchedulingIgnoredDuringExecution: nodeSelectorTerms: - matchExpressions: - {{- range .Values.worker.requireNodeLabels }} + {{- range $.Values.worker.requireNodeLabels }} - key: {{ .key }} operator: {{ .operator }} {{- with .values }} values: {{ toJson . }} {{- end }} {{- end }} - {{- range .Values.worker.avoidNodeLabels }} + {{- range $.Values.worker.avoidNodeLabels }} - key: {{ .key }} operator: {{ .operator }} {{- with .values }} @@ -61,26 +65,26 @@ spec: {{- end }} {{- end }} {{- end }} - {{- if .Values.worker.tolerateNodeTaints }} + {{- if $.Values.worker.tolerateNodeTaints }} tolerations: - {{- range .Values.worker.tolerateNodeTaints }} + {{- range $.Values.worker.tolerateNodeTaints }} - key: {{ .key }} effect: {{ .effect }} {{- end }} {{- end }} containers: - name: stellar-core - image: {{ .Values.worker.stellar_core_image }} + image: {{ $.Values.worker.stellar_core_image }} imagePullPolicy: Always resources: requests: - cpu: "{{ .Values.worker.resources.requests.cpu}}" - memory: "{{ .Values.worker.resources.requests.memory}}" - ephemeral-storage: "{{ .Values.worker.resources.requests.ephemeral_storage}}" + cpu: "{{ $.Values.worker.resources.requests.cpu}}" + memory: "{{ $.Values.worker.resources.requests.memory}}" + ephemeral-storage: "{{ $.Values.worker.resources.requests.ephemeral_storage}}" limits: - cpu: "{{ .Values.worker.resources.limits.cpu}}" - memory: "{{ .Values.worker.resources.limits.memory}}" - ephemeral-storage: "{{ .Values.worker.resources.limits.ephemeral_storage}}" + cpu: "{{ $.Values.worker.resources.limits.cpu}}" + memory: "{{ $.Values.worker.resources.limits.memory}}" + ephemeral-storage: "{{ $.Values.worker.resources.limits.ephemeral_storage}}" command: ["/bin/sh", "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/scripts/worker.sh"] ports: - containerPort: 11626 @@ -90,10 +94,10 @@ spec: fieldRef: fieldPath: metadata.name - name: ASAN_OPTIONS - value: {{ .Values.worker.asanOptions | quote }} + value: {{ $.Values.worker.asanOptions | quote }} envFrom: - configMapRef: - name: {{ .Release.Name }}-worker-config + name: {{ $.Release.Name }}-worker-config volumeMounts: - name: config mountPath: /config @@ -104,17 +108,17 @@ spec: volumes: - name: config configMap: - name: {{ .Release.Name }}-stellar-core-config + name: {{ $.Release.Name }}-stellar-core-config - name: script configMap: - name: {{ .Release.Name }}-worker-script + name: {{ $.Release.Name }}-worker-script - emptyDir: {} name: data-volume - {{- if not .Values.worker.unevenSched }} + {{- if not $.Values.worker.unevenSched }} topologySpreadConstraints: - labelSelector: matchLabels: - app: {{ .Release.Name }}-stellar-core + app: {{ $.Release.Name }}-stellar-core # Note: maxSkew affects dynamic node scheduling with karpenter # See https://github.com/stellar/supercluster/issues/330 maxSkew: 2 @@ -122,6 +126,8 @@ spec: whenUnsatisfiable: DoNotSchedule {{- end }} --- +{{- end }} +--- apiVersion: v1 kind: ConfigMap metadata: From d9d9eb6f241d74b0e29f81af29785d426b5511fd Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Wed, 26 Aug 2026 13:58:21 -0400 Subject: [PATCH 03/22] fix(catchup): a Terminating pod is not ready A pod being deleted still reports phase Running, so readyPods counted pods on their way out. That inflated capacity, marked more workers than the queue could spare, and re-selected the pods just deleted -- whose StatefulSets were already gone, so the delete threw NotFound and aborted the whole pass. A run lost a pass after every successful wave. The ready set also has to drop the workers removed earlier in the same pass, or the marking step reads a fleet size that is one wave stale and re-marks what it just deleted. --- src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs index fd72cb32..63a15dbc 100644 --- a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs +++ b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs @@ -292,7 +292,7 @@ let readyPods (context: MissionContext) : Set = context.kube.ListNamespacedPod(context.namespaceProperty, labelSelector = selector) pods.Items - |> Seq.filter (fun pod -> pod.Status.Phase = "Running") + |> Seq.filter (fun pod -> pod.Status.Phase = "Running" && isNull (box pod.Metadata.DeletionTimestamp)) |> Seq.map (fun pod -> pod.Metadata.Name) |> Set.ofSeq @@ -467,7 +467,7 @@ let historyPubnetParallelCatchupV2 (context: MissionContext) = let outstanding = status.Value("queue_remain_count") + JobsInProgress.Count try - let ready = readyPods context + let mutable ready = readyPods context // Read job_owners directly so it is current, not as old as the status snapshot. let busy = redisIn context (Seq.head ready) "HVALS job_owners" |> Set.ofList let removable = marked |> Set.filter (fun p -> ready.Contains p && not (busy.Contains p)) @@ -484,6 +484,7 @@ let historyPubnetParallelCatchupV2 (context: MissionContext) = marked <- Set.difference marked removable livePods <- Set.difference livePods removable + ready <- Set.difference ready removable LogInfo "Retired %d workers (%d outstanding)" removable.Count outstanding | failed -> LogWarn "Not retiring: log collection failed for %d workers" failed.Length From 233285a5afa787dccd75d706ffa05a004df14684 Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Wed, 26 Aug 2026 14:08:46 -0400 Subject: [PATCH 04/22] perf(catchup): skip the pod list and exec when nothing can come of them Both reads ran on every poll of a multi-hour run even while the queue outran the fleet and nothing was marked, and the pod list at 1024 workers is not cheap. They are now skipped unless something is marked or the queue has dropped below the fleet, which also stops the ramp logging a skipped pass every poll: with no pod Running yet, taking the head of an empty ready set threw and the handler reported it as a real failure. The guard reads livePods rather than the ready set, since consulting the ready set is the cost being avoided. livePods is always the larger of the two, so the guard errs toward doing the work rather than skipping it. redisIn now takes the ready set and picks a host itself, returning nothing when there is none, so callers stop reaching for Seq.head. --- .../MissionHistoryPubnetParallelCatchupV2.fs | 46 +++++++++++-------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs index 63a15dbc..dc334efd 100644 --- a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs +++ b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs @@ -296,24 +296,27 @@ let readyPods (context: MissionContext) : Set = |> Seq.map (fun pod -> pod.Metadata.Name) |> Set.ofSeq -// Runs redis-cli in a ready worker and returns its output lines. -let redisIn (context: MissionContext) (host: string) (args: string) : string list = - let outFile = Path.Combine(Path.GetTempPath(), helmReleaseName + "-redis.txt") - let sh = sprintf "redis-cli -h \"$REDIS_HOST\" -p \"$REDIS_PORT\" %s" args - let cmd = [| "sh"; "-c"; sh |] - - RemoteCommandRunner.RunRemoteCommandAndCaptureOutput( - context.kube, - context.namespaceProperty, - host, - "stellar-core", - cmd, - outFile - ) +// Runs redis-cli in a ready worker and returns its output lines, or none if there is no host. +let redisIn (context: MissionContext) (ready: Set) (args: string) : string list = + match Seq.tryHead ready with + | None -> [] + | Some host -> + let outFile = Path.Combine(Path.GetTempPath(), helmReleaseName + "-redis.txt") + let sh = sprintf "redis-cli -h \"$REDIS_HOST\" -p \"$REDIS_PORT\" %s" args + let cmd = [| "sh"; "-c"; sh |] + + RemoteCommandRunner.RunRemoteCommandAndCaptureOutput( + context.kube, + context.namespaceProperty, + host, + "stellar-core", + cmd, + outFile + ) - File.ReadAllLines outFile - |> Array.toList - |> List.filter (fun l -> l.Trim() <> "") + File.ReadAllLines outFile + |> Array.toList + |> List.filter (fun l -> l.Trim() <> "") // Cleanup on exit. `signalTriggered` indicates we're running under a hard // deadline (Jenkins' SoftKillWaitSeconds, ~5s by default, before SIGKILL). @@ -467,9 +470,12 @@ let historyPubnetParallelCatchupV2 (context: MissionContext) = let outstanding = status.Value("queue_remain_count") + JobsInProgress.Count try - let mutable ready = readyPods context + // Each read costs an apiserver call or a pod exec, so skip both when nothing is marked and the queue still outruns the fleet. + let doReads = not marked.IsEmpty || outstanding < livePods.Count + let mutable ready = if doReads then readyPods context else Set.empty // Read job_owners directly so it is current, not as old as the status snapshot. - let busy = redisIn context (Seq.head ready) "HVALS job_owners" |> Set.ofList + let busy = redisIn context ready "HVALS job_owners" |> Set.ofList + let removable = marked |> Set.filter (fun p -> ready.Contains p && not (busy.Contains p)) if not removable.IsEmpty then @@ -498,7 +504,7 @@ let historyPubnetParallelCatchupV2 (context: MissionContext) = for chunk in List.chunkBySize 30 toMark do let names = chunk |> List.map (sprintf "'%s'") |> String.concat " " - redisIn context (Seq.head ready) (sprintf "SADD \"%s-retiring\" %s" helmReleaseName names) + redisIn context ready (sprintf "SADD \"%s-retiring\" %s" helmReleaseName names) |> ignore if not toMark.IsEmpty then From 6b4f927e99e90aa1c35c7ab88d3f4ea4d3b68178 Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Wed, 26 Aug 2026 14:33:27 -0400 Subject: [PATCH 05/22] fix(catchup): bound how many workers one pass drains Log collection is serial and runs inside the poll loop, so an unbounded batch blocks it for as long as the batch takes. A 400-worker run retired 213 in one pass and spent 103 seconds collecting, and that was with tiny archives: each worker had run only 2 ranges. A prod worker churns dozens over hours, so per-pod cost is seconds rather than half a second, and an uncapped pass late in a 1024-worker run would block for tens of minutes with no timeout on any single exec. Capped at 32 per pass, so the worst case is bounded by the cap rather than by the fleet size. Later passes drain the rest. --- src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs index dc334efd..c57fa635 100644 --- a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs +++ b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs @@ -49,6 +49,8 @@ let mutable nonce : String = "" let mutable helmReleaseName : String = "" // Pods not yet retired; module scope because cleanup runs from a signal handler. let mutable livePods : Set = Set.empty +// Log collection is serial and runs inside the poll loop, so bound what one pass can block on. +let maxRetiredPerPass = 32 let jobMonitorHostName (context: MissionContext) = match context.jobMonitorExternalHost with @@ -476,7 +478,8 @@ let historyPubnetParallelCatchupV2 (context: MissionContext) = // Read job_owners directly so it is current, not as old as the status snapshot. let busy = redisIn context ready "HVALS job_owners" |> Set.ofList - let removable = marked |> Set.filter (fun p -> ready.Contains p && not (busy.Contains p)) + let idle p = ready.Contains p && not (busy.Contains p) + let removable = marked |> Seq.filter idle |> Seq.truncate maxRetiredPerPass |> Set.ofSeq if not removable.IsEmpty then // /data is emptyDir, so a pod removed before its logs are read loses them. From ed59cf0c94f03990f34d97d0e16934e4770cdb54 Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Wed, 26 Aug 2026 15:32:49 -0400 Subject: [PATCH 06/22] fix(catchup): pass --namespace to every helm call helm was left to use the kubeconfig's current namespace while every other call in the mission honours context.namespaceProperty. A run launched with --namespace ssc-config-test therefore installed its chart into stellar-supercluster: 1024 workers and a monitor landed in the production namespace, while the driver polled and scaled an empty one, so scale-down silently did nothing for the whole run. Both uninstall paths get it too. They were previously consistent with the install only by accident -- both defaulted to the same current namespace -- which would have broken the moment the install was fixed alone. --- .../MissionHistoryPubnetParallelCatchupV2.fs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs index c57fa635..10cd5861 100644 --- a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs +++ b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs @@ -223,10 +223,15 @@ let installProject (context: MissionContext) = let expandedKubeCfg = ExpandHomeDirTilde context.kubeCfg Environment.SetEnvironmentVariable("KUBECONFIG", expandedKubeCfg) + // --namespace is not optional: without it helm uses the kubeconfig's current + // namespace while every other call honours context.namespaceProperty, so a run + // targeted at one namespace installs its chart into another. RunShellCommand [| "helm" "install" helmReleaseName helmChartPath + "--namespace" + context.namespaceProperty "--values" valuesFilePath "--set" @@ -236,7 +241,9 @@ let installProject (context: MissionContext) = match RunShellCommand [| "helm" "get" "values" - helmReleaseName |] with + helmReleaseName + "--namespace" + context.namespaceProperty |] with | Some valuesOutput -> LogInfo "%s" valuesOutput | _ -> () @@ -341,7 +348,9 @@ let cleanup (signalTriggered: bool) (context: MissionContext) = RunShellCommand [| "helm" "uninstall" - helmReleaseName |] + helmReleaseName + "--namespace" + context.namespaceProperty |] |> ignore else // Normal / legitimate-failure path: pods are still alive through @@ -358,7 +367,9 @@ let cleanup (signalTriggered: bool) (context: MissionContext) = RunShellCommand [| "helm" "uninstall" - helmReleaseName |] + helmReleaseName + "--namespace" + context.namespaceProperty |] |> ignore let mutable cleanupContext : MissionContext option = None From cfe2a5e44f08fed9b0c59e2e663995c2cbf8bd79 Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Wed, 26 Aug 2026 20:12:14 -0400 Subject: [PATCH 07/22] fix(catchup): keep a reserve of workers unmarked Once every worker is marked retiring, a job the monitor puts back on the queue can never be claimed: worker.sh refuses to claim while marked, and there is no way to unmark. A single recoverable orphan then hangs the run. Observed on a 1024-worker run: all 4009 ranges were accounted for, one job lost its owner upstream, the monitor correctly requeued it, and nothing was left willing to take it. The run had to be aborted. Two changes, both in the mark step. The reserve is now counted against unmarked workers rather than `ready`, which includes marked pods that have not been deleted yet and so let the reserve erode away over successive passes. And the reserve never drops below minUnmarkedWorkers, so a run whose queue empties still has somewhere to put returned work. Three rather than one so the reserve survives a worker restarting or its node being disrupted, and so several returned jobs drain in parallel. The cost is three idle workers, about two nodes, against a tail that otherwise idles the whole fleet. --- .../MissionHistoryPubnetParallelCatchupV2.fs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs index 10cd5861..06f9aa75 100644 --- a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs +++ b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs @@ -52,6 +52,9 @@ let mutable livePods : Set = Set.empty // Log collection is serial and runs inside the poll loop, so bound what one pass can block on. let maxRetiredPerPass = 32 +// A job the monitor requeues needs a worker still willing to claim it. +let minUnmarkedWorkers = 3 + let jobMonitorHostName (context: MissionContext) = match context.jobMonitorExternalHost with | Some host -> host @@ -508,12 +511,17 @@ let historyPubnetParallelCatchupV2 (context: MissionContext) = LogInfo "Retired %d workers (%d outstanding)" removable.Count outstanding | failed -> LogWarn "Not retiring: log collection failed for %d workers" failed.Length - let toMark = + // Counted against unmarked workers, not `ready`: marked pods linger until + // they are deleted, and counting them erodes the reserve to nothing. + let markable = ready |> Seq.filter (fun p -> not (busy.Contains p) && not (marked.Contains p)) - |> Seq.truncate (max 0 (ready.Count - outstanding)) |> List.ofSeq + let toMark = + markable + |> List.truncate (max 0 (markable.Length - max outstanding minUnmarkedWorkers)) + // Chunked because RunRemoteCommand rejects a command of 4096 bytes or more. for chunk in List.chunkBySize 30 toMark do let names = chunk |> List.map (sprintf "'%s'") |> String.concat " " From e29f2f2cb42da3da553e70b64375995da7be87a4 Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Thu, 27 Aug 2026 09:24:19 -0400 Subject: [PATCH 08/22] fix(gitignore): remove unnecessary entries for docs/plans and logs-smoke --- .gitignore | 3 --- 1 file changed, 3 deletions(-) diff --git a/.gitignore b/.gitignore index 20b55938..8a57e922 100644 --- a/.gitignore +++ b/.gitignore @@ -18,6 +18,3 @@ .fake .ionide .idea - -docs/plans -logs-smoke \ No newline at end of file From 56c99f5e861095cbe125471e2710bdd27f69b9f0 Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Thu, 27 Aug 2026 11:47:15 -0400 Subject: [PATCH 09/22] fix(catchup): act on the exec exit code instead of printing it RunRemoteCommandAndCaptureOutput returned void and wrote the k8s status channel to the console, so neither caller could tell a failed exec from a command that produced no output. Two fail-open paths followed. `redisIn` reads job_owners to decide which workers are idle. A failed exec left it with zero lines, which reads as "no worker is busy" and makes every worker look retirable -- the exact check that is supposed to stop a busy worker being retired. Two such passes in a row would mark a worker and then delete it mid-range. `collectLogsFromPods` treats an exception as the only failure, but tar exiting non-zero raises nothing, so a failed archive counted as collected and the worker was deleted with its logs. The exit code was already available: channel 3 carries a V1Status whose causes hold it, and Kubernetes.GetExitCodeOrThrow already parses exactly that for RunRemoteCommand in the same file. Renamed the locals and fixed the comments too -- a variable called `stderr` holding the status channel is what made this invisible. Both callers now fail closed: redisIn raises into the poll loop's handler, which skips the pass with no state mutated, and a failed tar puts the pod on the failed list so retirement is skipped for it. --- src/CSLibrary/RemoteCommandRunner.cs | 32 +++++++------- .../MissionHistoryPubnetParallelCatchupV2.fs | 42 ++++++++++++------- 2 files changed, 42 insertions(+), 32 deletions(-) diff --git a/src/CSLibrary/RemoteCommandRunner.cs b/src/CSLibrary/RemoteCommandRunner.cs index ca9b2f58..8052374d 100644 --- a/src/CSLibrary/RemoteCommandRunner.cs +++ b/src/CSLibrary/RemoteCommandRunner.cs @@ -66,14 +66,16 @@ await kube.MuxedStreamNamespacedPodExecAsync(name: podName, @namespace: ns, } // Execute a command and capture stdout to a file (for copying files from pod) - public static void RunRemoteCommandAndCaptureOutput(Kubernetes kube, string ns, string podName, + // Returns the command's exit code; callers that ignore it cannot tell a + // failed exec from a command that legitimately produced no output. + public static int RunRemoteCommandAndCaptureOutput(Kubernetes kube, string ns, string podName, string containerName, string[] command, string outputFilePath) { - Task task = RunRemoteCommandAndCaptureOutputAsync(kube, ns, podName, containerName, command, outputFilePath); - task.Wait(); + Task task = RunRemoteCommandAndCaptureOutputAsync(kube, ns, podName, containerName, command, outputFilePath); + return task.Result; } - public static async Task RunRemoteCommandAndCaptureOutputAsync(Kubernetes kube, string ns, string podName, + public static async Task RunRemoteCommandAndCaptureOutputAsync(Kubernetes kube, string ns, string podName, string containerName, string[] command, string outputFilePath) { // The `using` lifetime guard ensure these objects lifetimes last the entire task, @@ -89,30 +91,28 @@ await kube.MuxedStreamNamespacedPodExecAsync( stderr: true, tty: false).ConfigureAwait(false)) using (System.IO.Stream stdout = mstr.GetStream(ChannelIndex.StdOut, null)) - using (System.IO.Stream stderr = mstr.GetStream(ChannelIndex.Error, null)) - using (System.IO.StreamReader errorReader = new System.IO.StreamReader(stderr)) + using (System.IO.Stream statusChannel = mstr.GetStream(ChannelIndex.Error, null)) + using (System.IO.StreamReader statusReader = new System.IO.StreamReader(statusChannel)) using (System.IO.FileStream fileStream = new System.IO.FileStream(outputFilePath, FileMode.Create, FileAccess.Write, FileShare.None, bufferSize: 128 * 1024, useAsync: true)) { // Start the MuxStream, this establishes the connection and routes bytes back into separate channels. - // We only care about stdout(1) and stderr(2). + // We only care about stdout(1) and the status channel(3). mstr.Start(); - // Copy stdout → file asynchronously, and drain stderr concurrently. + // Copy stdout → file asynchronously, and drain the status channel concurrently. var copyTask = stdout.CopyToAsync(fileStream); - var errorTask = errorReader.ReadToEndAsync(); + var statusTask = statusReader.ReadToEndAsync(); - await Task.WhenAll(copyTask, errorTask).ConfigureAwait(false); + await Task.WhenAll(copyTask, statusTask).ConfigureAwait(false); // Flush the file stream to ensure all data is written await fileStream.FlushAsync().ConfigureAwait(false); - // Log any errors to console - string errors = errorTask.Result; - if (!string.IsNullOrEmpty(errors)) - { - Console.WriteLine($"Command stderr from pod {podName}: {errors}"); - } + // Channel 3 is the k8s status channel, not stderr: it carries a + // V1Status whose causes hold the command's real exit code. + string status = statusTask.Result; + return Kubernetes.GetExitCodeOrThrow(SafeJsonConvert.DeserializeObject(status)); } } } diff --git a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs index 06f9aa75..19d22671 100644 --- a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs +++ b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs @@ -274,14 +274,19 @@ let collectLogsFromPods (context: MissionContext) (podNames: string list) : stri let outputFile = Path.Combine(context.destination.Path, sprintf "%s-logs.tar.gz" podName) // Execute the command and capture the tar output to a local file - RemoteCommandRunner.RunRemoteCommandAndCaptureOutput( - kube = context.kube, - ns = context.namespaceProperty, - podName = podName, - containerName = "stellar-core", - command = command, - outputFilePath = outputFile - ) + let rc = + RemoteCommandRunner.RunRemoteCommandAndCaptureOutput( + kube = context.kube, + ns = context.namespaceProperty, + podName = podName, + containerName = "stellar-core", + command = command, + outputFilePath = outputFile + ) + + // tar exiting non-zero raises nothing, so without this a failed + // archive counts as collected and the pod is deleted with its logs. + if rc <> 0 then failwithf "tar exited %d" rc let fileInfo = FileInfo(outputFile) @@ -317,14 +322,19 @@ let redisIn (context: MissionContext) (ready: Set) (args: string) : stri let sh = sprintf "redis-cli -h \"$REDIS_HOST\" -p \"$REDIS_PORT\" %s" args let cmd = [| "sh"; "-c"; sh |] - RemoteCommandRunner.RunRemoteCommandAndCaptureOutput( - context.kube, - context.namespaceProperty, - host, - "stellar-core", - cmd, - outFile - ) + // A failed exec would otherwise return no lines, which reads as "no worker + // is busy" and makes every worker look retirable. + let rc = + RemoteCommandRunner.RunRemoteCommandAndCaptureOutput( + context.kube, + context.namespaceProperty, + host, + "stellar-core", + cmd, + outFile + ) + + if rc <> 0 then failwithf "redis-cli in %s exited %d" host rc File.ReadAllLines outFile |> Array.toList From 520696c681ce1914cae8d74895f152bf47faf8db Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Thu, 27 Aug 2026 13:07:11 -0400 Subject: [PATCH 10/22] fix(catchup): update comments for clarity and increase max retired pods per pass --- src/CSLibrary/RemoteCommandRunner.cs | 5 +---- src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs | 5 +---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/CSLibrary/RemoteCommandRunner.cs b/src/CSLibrary/RemoteCommandRunner.cs index 8052374d..bc18a101 100644 --- a/src/CSLibrary/RemoteCommandRunner.cs +++ b/src/CSLibrary/RemoteCommandRunner.cs @@ -66,8 +66,7 @@ await kube.MuxedStreamNamespacedPodExecAsync(name: podName, @namespace: ns, } // Execute a command and capture stdout to a file (for copying files from pod) - // Returns the command's exit code; callers that ignore it cannot tell a - // failed exec from a command that legitimately produced no output. + // Returns the command's exit code public static int RunRemoteCommandAndCaptureOutput(Kubernetes kube, string ns, string podName, string containerName, string[] command, string outputFilePath) { @@ -109,8 +108,6 @@ await kube.MuxedStreamNamespacedPodExecAsync( // Flush the file stream to ensure all data is written await fileStream.FlushAsync().ConfigureAwait(false); - // Channel 3 is the k8s status channel, not stderr: it carries a - // V1Status whose causes hold the command's real exit code. string status = statusTask.Result; return Kubernetes.GetExitCodeOrThrow(SafeJsonConvert.DeserializeObject(status)); } diff --git a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs index 19d22671..c0367258 100644 --- a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs +++ b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs @@ -50,7 +50,7 @@ let mutable helmReleaseName : String = "" // Pods not yet retired; module scope because cleanup runs from a signal handler. let mutable livePods : Set = Set.empty // Log collection is serial and runs inside the poll loop, so bound what one pass can block on. -let maxRetiredPerPass = 32 +let maxRetiredPerPass = 64 // A job the monitor requeues needs a worker still willing to claim it. let minUnmarkedWorkers = 3 @@ -226,9 +226,6 @@ let installProject (context: MissionContext) = let expandedKubeCfg = ExpandHomeDirTilde context.kubeCfg Environment.SetEnvironmentVariable("KUBECONFIG", expandedKubeCfg) - // --namespace is not optional: without it helm uses the kubeconfig's current - // namespace while every other call honours context.namespaceProperty, so a run - // targeted at one namespace installs its chart into another. RunShellCommand [| "helm" "install" helmReleaseName From 960cb34b7deaad1e93b2898690a7702610394afa Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Thu, 27 Aug 2026 13:40:36 -0400 Subject: [PATCH 11/22] fix(catchup): measure surplus against the whole unmarked fleet The reserve was subtracted from the idle pool alone, but `outstanding` counts in-progress jobs too, and those already have a worker each. With 1000 workers, 700 busy and 700 jobs outstanding, the surplus came out as 300 - 700 and nothing was marked at all: 300 idle workers were held to cover work that was already being served. Counting the surplus against every unmarked worker gives 1000 - 700 = 300, and those 300 idle workers are marked. Only idle workers are ever marked, and the reserve still keeps `max outstanding minUnmarkedWorkers` workers unmarked, so a requeued job still has somewhere to go -- an unmarked busy worker becomes claimable as soon as it finishes. Run #192 never sat in that state: in-progress fell from 1022 to 80 in three minutes, so the largest instantaneous gap was about 80 workers and retirement was capped at 32 a pass throughout. A run whose ranges vary enough for in-progress to decay gradually would sit there and mark nothing. --- src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs index c0367258..cd70a877 100644 --- a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs +++ b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs @@ -520,14 +520,12 @@ let historyPubnetParallelCatchupV2 (context: MissionContext) = // Counted against unmarked workers, not `ready`: marked pods linger until // they are deleted, and counting them erodes the reserve to nothing. - let markable = - ready - |> Seq.filter (fun p -> not (busy.Contains p) && not (marked.Contains p)) - |> List.ofSeq + let unmarked = ready |> Seq.filter (fun p -> not (marked.Contains p)) |> List.ofSeq let toMark = - markable - |> List.truncate (max 0 (markable.Length - max outstanding minUnmarkedWorkers)) + unmarked + |> List.filter (fun p -> not (busy.Contains p)) + |> List.truncate (max 0 (unmarked.Length - max outstanding minUnmarkedWorkers)) // Chunked because RunRemoteCommand rejects a command of 4096 bytes or more. for chunk in List.chunkBySize 30 toMark do From 753f2552f8355ff8d69128861177aec6e6d03fc0 Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Thu, 27 Aug 2026 13:49:51 -0400 Subject: [PATCH 12/22] fix(catchup): stop treating an empty log archive as a failed collection A worker that never claimed a job has an empty /data, so the tar glob matches nothing and tar exits 2. Checking that exit code put those workers on the failed list, which blocks retirement for the whole batch and never clears, because the batch is rebuilt the same way every pass. It lands on precisely the workers scale-down exists to remove: a surplus worker that never got work. Seen on ssc-test with 12 workers and 8 ranges, where the 4 that never claimed blocked retirement indefinitely. redisIn keeps its exit-code check. There a failed exec yields an empty busy set, which reads as "no worker is busy" and makes every worker look retirable, so failing closed there is worth having. --- .../MissionHistoryPubnetParallelCatchupV2.fs | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs index cd70a877..980f71f1 100644 --- a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs +++ b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs @@ -271,19 +271,15 @@ let collectLogsFromPods (context: MissionContext) (podNames: string list) : stri let outputFile = Path.Combine(context.destination.Path, sprintf "%s-logs.tar.gz" podName) // Execute the command and capture the tar output to a local file - let rc = - RemoteCommandRunner.RunRemoteCommandAndCaptureOutput( - kube = context.kube, - ns = context.namespaceProperty, - podName = podName, - containerName = "stellar-core", - command = command, - outputFilePath = outputFile - ) - - // tar exiting non-zero raises nothing, so without this a failed - // archive counts as collected and the pod is deleted with its logs. - if rc <> 0 then failwithf "tar exited %d" rc + RemoteCommandRunner.RunRemoteCommandAndCaptureOutput( + kube = context.kube, + ns = context.namespaceProperty, + podName = podName, + containerName = "stellar-core", + command = command, + outputFilePath = outputFile + ) + |> ignore let fileInfo = FileInfo(outputFile) From 8e9bcf2e5ce4714ee883ff5f113b095f11a39690 Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Thu, 27 Aug 2026 14:13:56 -0400 Subject: [PATCH 13/22] fix(catchup): take the worker index from the chart, not the pod name Per-worker StatefulSets renamed pods to -stellar-core--0, so the last hyphen-separated segment is the StatefulSet's own -0 suffix rather than the worker index. Every metrics record in run #192 reported core_id 0. Nothing consumes the field -- the monitor discards it and keeps only the two durations -- but a value that is always 0 reads as real data, so pass the index the chart already has. --- .../parallel_catchup_helm/files/worker.sh | 3 +-- .../parallel_catchup_helm/templates/catchup_workers.yaml | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/MissionParallelCatchup/parallel_catchup_helm/files/worker.sh b/src/MissionParallelCatchup/parallel_catchup_helm/files/worker.sh index af757277..ef9234f6 100644 --- a/src/MissionParallelCatchup/parallel_catchup_helm/files/worker.sh +++ b/src/MissionParallelCatchup/parallel_catchup_helm/files/worker.sh @@ -84,8 +84,7 @@ if [ $LMOVE_EXIT_CODE -eq 0 ] && [ -n "$JOB_KEY" ]; then fi # Push metrics to redis in a transaction to ensure data consistency. Retry for 5min on failures - # Extract the pod ordinal (last hyphen-separated segment) from pod name like "release-name-stellar-core-0" - core_id=$(echo "$POD_NAME" | awk -F'-' '{print $NF}') + core_id="$WORKER_INDEX" # Validate core_id was extracted successfully if [ -z "$core_id" ]; then echo "Error: Failed to extract core_id from POD_NAME: $POD_NAME" diff --git a/src/MissionParallelCatchup/parallel_catchup_helm/templates/catchup_workers.yaml b/src/MissionParallelCatchup/parallel_catchup_helm/templates/catchup_workers.yaml index cc97f333..5975f275 100644 --- a/src/MissionParallelCatchup/parallel_catchup_helm/templates/catchup_workers.yaml +++ b/src/MissionParallelCatchup/parallel_catchup_helm/templates/catchup_workers.yaml @@ -95,6 +95,8 @@ spec: fieldPath: metadata.name - name: ASAN_OPTIONS value: {{ $.Values.worker.asanOptions | quote }} + - name: WORKER_INDEX + value: {{ $i | quote }} envFrom: - configMapRef: name: {{ $.Release.Name }}-worker-config From e178fc233164ca90c310c375b9c26348e50ae0d4 Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Thu, 27 Aug 2026 14:28:32 -0400 Subject: [PATCH 14/22] fix(catchup): recognise per-worker StatefulSets in the orphan sweep The sweep helm-uninstalls a PCv2 release before deleting resources individually, so the release secret does not end up dangling at deleted workloads. It found the release by matching a StatefulSet name ending in -stellar-core, which no longer happens: per-worker StatefulSets end in -stellar-core-. Abandoned runs therefore had their workloads deleted one by one with the release left behind. Matching on the -stellar-core segment instead, and collapsing the names to a set, so a 1024-worker release is uninstalled once rather than 1024 times. --- src/FSLibrary/StellarOrphanSweep.fs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/FSLibrary/StellarOrphanSweep.fs b/src/FSLibrary/StellarOrphanSweep.fs index dd56aaf5..ed94fc20 100644 --- a/src/FSLibrary/StellarOrphanSweep.fs +++ b/src/FSLibrary/StellarOrphanSweep.fs @@ -72,20 +72,20 @@ let private sweepWithCutoff (cutoff: DateTime) (kube: Kubernetes) (ns: string) ( let stsItems = kube.ListNamespacedStatefulSet(namespaceParameter = ns).Items - for sts in stsItems do - if isOlderThan cutoff sts.Metadata then - let name = sts.Metadata.Name - - if name.StartsWith("parallel-catchup-") && name.EndsWith("-stellar-core") then - let release = name.Substring(0, name.Length - "-stellar-core".Length) - LogInfo "Orphan sweep: helm uninstall %s" release - - RunShellCommand [| "helm" - "uninstall" - release - "-n" - ns |] - |> ignore + for release in stsItems + |> Seq.filter (fun sts -> isOlderThan cutoff sts.Metadata) + |> Seq.map (fun sts -> sts.Metadata.Name) + |> Seq.filter (fun name -> name.StartsWith("parallel-catchup-") && name.Contains("-stellar-core")) + |> Seq.map (fun name -> name.Substring(0, name.IndexOf("-stellar-core"))) + |> Set.ofSeq do + LogInfo "Orphan sweep: helm uninstall %s" release + + RunShellCommand [| "helm" + "uninstall" + release + "-n" + ns |] + |> ignore // 2. Delete the same resource type set the retired `clean` verb targeted. // The order matches the old NamespaceContent.Cleanup so dependent From 95851b0aae5cfeafa8b45a0934c97f88f368cf9d Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Thu, 27 Aug 2026 14:50:00 -0400 Subject: [PATCH 15/22] fix(catchup): read the job-owners hash name from the pod environment redis.job_owners is a chart value plumbed to workers and the monitor as JOB_OWNERS, and worker.sh uses it for both HSET and HDEL. The driver hardcoded the default name instead, so setting that value would leave the busy set permanently empty: every worker reads as idle, gets marked, and is deleted a pass later while running a range. The same command already expands $REDIS_HOST and $REDIS_PORT inside the pod, so the key name resolves the same way. --- src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs index 980f71f1..ef93e432 100644 --- a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs +++ b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs @@ -493,7 +493,7 @@ let historyPubnetParallelCatchupV2 (context: MissionContext) = let doReads = not marked.IsEmpty || outstanding < livePods.Count let mutable ready = if doReads then readyPods context else Set.empty // Read job_owners directly so it is current, not as old as the status snapshot. - let busy = redisIn context ready "HVALS job_owners" |> Set.ofList + let busy = redisIn context ready "HVALS \"$JOB_OWNERS\"" |> Set.ofList let idle p = ready.Contains p && not (busy.Contains p) let removable = marked |> Seq.filter idle |> Seq.truncate maxRetiredPerPass |> Set.ofSeq From c489d09d551114679e897e22cfd3b0976e36ee63 Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Thu, 27 Aug 2026 14:58:31 -0400 Subject: [PATCH 16/22] fix(catchup): split release names at the last -stellar-core, record marks per chunk --tag is user-supplied and validated only as [a-z0-9-]+, so a tag may contain -stellar-core and the release name inherits it. Splitting at the first occurrence then yields a prefix that is not a release at all: the sweep uninstalls something nonexistent and leaves the real orphan installed. Marks are recorded per chunk instead of once at the end. Above 30 retirements the SADD is chunked, and a failure after an early chunk left those workers in the retiring set but absent from the driver's own set. They then never claim and never retire, and once the properly marked workers are gone they are all that remains: the reserve is made up entirely of workers that cannot claim, so a requeued job has nowhere to go, which is the stall this branch already fixed once. --- src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs | 3 ++- src/FSLibrary/StellarOrphanSweep.fs | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs index ef93e432..a4703f84 100644 --- a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs +++ b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs @@ -530,8 +530,9 @@ let historyPubnetParallelCatchupV2 (context: MissionContext) = redisIn context ready (sprintf "SADD \"%s-retiring\" %s" helmReleaseName names) |> ignore + marked <- Set.union marked (Set.ofList chunk) + if not toMark.IsEmpty then - marked <- Set.union marked (Set.ofList toMark) LogInfo "Marked %d retiring (%d ready, %d outstanding)" toMark.Length ready.Count outstanding with ex -> LogWarn "Worker scale-down skipped this pass: %s" ex.Message diff --git a/src/FSLibrary/StellarOrphanSweep.fs b/src/FSLibrary/StellarOrphanSweep.fs index ed94fc20..12c5074f 100644 --- a/src/FSLibrary/StellarOrphanSweep.fs +++ b/src/FSLibrary/StellarOrphanSweep.fs @@ -76,7 +76,7 @@ let private sweepWithCutoff (cutoff: DateTime) (kube: Kubernetes) (ns: string) ( |> Seq.filter (fun sts -> isOlderThan cutoff sts.Metadata) |> Seq.map (fun sts -> sts.Metadata.Name) |> Seq.filter (fun name -> name.StartsWith("parallel-catchup-") && name.Contains("-stellar-core")) - |> Seq.map (fun name -> name.Substring(0, name.IndexOf("-stellar-core"))) + |> Seq.map (fun name -> name.Substring(0, name.LastIndexOf("-stellar-core"))) |> Set.ofSeq do LogInfo "Orphan sweep: helm uninstall %s" release From 8a186abe20f3bcba9454a595f7448b15a3cd6d15 Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Tue, 8 Sep 2026 11:50:23 -0400 Subject: [PATCH 17/22] Drop stale collectLogsFromPods header lines The function no longer derives pod names from pubnetParallelCatchupNumWorkers; callers supply them. The remaining bullets restated the tar command and output path, both already commented inline. Co-Authored-By: Claude Opus 5 --- src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs index 3a5cc378..e66f87fa 100644 --- a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs +++ b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs @@ -251,11 +251,7 @@ let installProject (context: MissionContext) = | Some valuesOutput -> LogInfo "%s" valuesOutput | _ -> () -// Collect log files from all parallel catchup worker pods -// This function: -// 1. Automatically determines worker pod names from context.pubnetParallelCatchupNumWorkers -// 2. For each pod, finds all files matching "stellar-core-*.log" in /data -// 3. Creates a tar.gz archive and copies it to context.destination directory +// Collect log files from the given parallel catchup worker pods. // Returns the pods whose collection raised; an empty archive is success. let collectLogsFromPods (context: MissionContext) (podNames: string list) : string list = let mutable failed = [] From 72a5baa62ba220e060ef980a5e5bf453290b3fe4 Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Tue, 22 Sep 2026 10:47:14 -0400 Subject: [PATCH 18/22] Move scale-down marking into job monitor; workers as bare pods - job_monitor.py marks surplus idle workers (reserve counted against all unmarked, MIN_UNMARKED_WORKERS=8), skips Pending/dead pods via DNS, publishes `retirable` - driver only collects logs and deletes pods listed in `retirable`; cap 64/pass; redis exec, readyPods and marking math removed - workers are bare Pods with hostname/subdomain instead of per-worker StatefulSets - worker.sh: retiring key unprefixed; RELEASE_NAME injection dropped - orphan sweep discovers pc-v2 releases via pods Co-Authored-By: Claude Fable 5.1 --- .../MissionHistoryPubnetParallelCatchupV2.fs | 131 +++--------- src/FSLibrary/StellarOrphanSweep.fs | 23 ++- src/MissionParallelCatchup/job_monitor.py | 34 +++- .../parallel_catchup_helm/files/worker.sh | 5 +- .../templates/catchup_workers.yaml | 188 +++++++++--------- 5 files changed, 173 insertions(+), 208 deletions(-) diff --git a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs index e66f87fa..c32b3a75 100644 --- a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs +++ b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs @@ -56,9 +56,6 @@ let mutable livePods : Set = Set.empty // Log collection is serial and runs inside the poll loop, so bound what one pass can block on. let maxRetiredPerPass = 64 -// A job the monitor requeues needs a worker still willing to claim it. -let minUnmarkedWorkers = 3 - let jobMonitorHostName (context: MissionContext) = match context.jobMonitorExternalHost with | Some host -> host @@ -251,11 +248,8 @@ let installProject (context: MissionContext) = | Some valuesOutput -> LogInfo "%s" valuesOutput | _ -> () -// Collect log files from the given parallel catchup worker pods. -// Returns the pods whose collection raised; an empty archive is success. -let collectLogsFromPods (context: MissionContext) (podNames: string list) : string list = - let mutable failed = [] - +// Collect log files from the given worker pods; a pod whose collection fails is skipped. +let collectLogsFromPods (context: MissionContext) (podNames: string list) : unit = LogInfo "Collecting logs from %d worker pods to directory: %s" (List.length podNames) context.destination.Path for podName in podNames do @@ -288,50 +282,7 @@ let collectLogsFromPods (context: MissionContext) (podNames: string list) : stri else LogWarn "No logs found or empty archive for pod %s" podName - with ex -> - LogWarn "Could not collect logs from pod %s (this is expected if pod doesn't exist): %s" podName ex.Message - failed <- podName :: failed - - failed - -// Running pods only, since a Pending pod reads as idle capacity it cannot supply. -let readyPods (context: MissionContext) : Set = - let selector = "app=" + helmReleaseName + "-stellar-core" - - let pods = - context.kube.ListNamespacedPod(context.namespaceProperty, labelSelector = selector) - - pods.Items - |> Seq.filter (fun pod -> pod.Status.Phase = "Running" && isNull (box pod.Metadata.DeletionTimestamp)) - |> Seq.map (fun pod -> pod.Metadata.Name) - |> Set.ofSeq - -// Runs redis-cli in a ready worker and returns its output lines, or none if there is no host. -let redisIn (context: MissionContext) (ready: Set) (args: string) : string list = - match Seq.tryHead ready with - | None -> [] - | Some host -> - let outFile = Path.Combine(Path.GetTempPath(), helmReleaseName + "-redis.txt") - let sh = sprintf "redis-cli -h \"$REDIS_HOST\" -p \"$REDIS_PORT\" %s" args - let cmd = [| "sh"; "-c"; sh |] - - // A failed exec would otherwise return no lines, which reads as "no worker - // is busy" and makes every worker look retirable. - let rc = - RemoteCommandRunner.RunRemoteCommandAndCaptureOutput( - context.kube, - context.namespaceProperty, - host, - "stellar-core", - cmd, - outFile - ) - - if rc <> 0 then failwithf "redis-cli in %s exited %d" host rc - - File.ReadAllLines outFile - |> Array.toList - |> List.filter (fun l -> l.Trim() <> "") + with ex -> LogWarn "Could not collect logs from pod %s: %s" podName ex.Message // Cleanup on exit. `signalTriggered` indicates we're running under a hard // deadline (Jenkins' SoftKillWaitSeconds, ~5s by default, before SIGKILL). @@ -366,7 +317,7 @@ let cleanup (signalTriggered: bool) (context: MissionContext) = try LogInfo "Attempting to collect worker logs before cleanup..." let stopwatch = Stopwatch.StartNew() - collectLogsFromPods context (List.ofSeq livePods) |> ignore + collectLogsFromPods context (List.ofSeq livePods) stopwatch.Stop() LogInfo "Log collection completed in %.2f seconds" stopwatch.Elapsed.TotalSeconds with ex -> LogWarn "Failed to collect some or all worker logs: %s" ex.Message @@ -453,9 +404,8 @@ let historyPubnetParallelCatchupV2 (context: MissionContext) = livePods <- Set.ofList [ for i in 0 .. context.pubnetParallelCatchupNumWorkers - 1 -> - sprintf "%s-stellar-core-%d-0" helmReleaseName i ] - // Marks from earlier passes only, so nothing is removed in the pass that marked it. - let mutable marked : Set = Set.empty + sprintf "%s-stellar-core-%d" helmReleaseName i ] + let mutable timeoutLeft = jobMonitorStatusCheckTimeOutSecs let mutable timeBeforeNextMetricsCheck = jobMonitorMetricsCheckIntervalSecs let mutable stalledForSecs = 0 @@ -490,52 +440,31 @@ let historyPubnetParallelCatchupV2 (context: MissionContext) = // `queue_remain_count`, not `num_remain`, which is 1 as a pre-first-poll sentinel. let outstanding = status.Value("queue_remain_count") + JobsInProgress.Count + // The job monitor owns the marking decision; we only collect logs and delete. try - // Each read costs an apiserver call or a pod exec, so skip both when nothing is marked and the queue still outruns the fleet. - let doReads = not marked.IsEmpty || outstanding < livePods.Count - let mutable ready = if doReads then readyPods context else Set.empty - // Read job_owners directly so it is current, not as old as the status snapshot. - let busy = redisIn context ready "HVALS \"$JOB_OWNERS\"" |> Set.ofList - - let idle p = ready.Contains p && not (busy.Contains p) - let removable = marked |> Seq.filter idle |> Seq.truncate maxRetiredPerPass |> Set.ofSeq - - if not removable.IsEmpty then - // /data is emptyDir, so a pod removed before its logs are read loses them. - match collectLogsFromPods context (List.ofSeq removable) with - | [] -> - for pod in removable do - let sts = pod.Substring(0, pod.Length - 2) - - context.kube.DeleteNamespacedStatefulSet(sts, context.namespaceProperty) - |> ignore - - marked <- Set.difference marked removable - livePods <- Set.difference livePods removable - ready <- Set.difference ready removable - LogInfo "Retired %d workers (%d outstanding)" removable.Count outstanding - | failed -> LogWarn "Not retiring: log collection failed for %d workers" failed.Length - - // Counted against unmarked workers, not `ready`: marked pods linger until - // they are deleted, and counting them erodes the reserve to nothing. - let unmarked = ready |> Seq.filter (fun p -> not (marked.Contains p)) |> List.ofSeq - - let toMark = - unmarked - |> List.filter (fun p -> not (busy.Contains p)) - |> List.truncate (max 0 (unmarked.Length - max outstanding minUnmarkedWorkers)) - - // Chunked because RunRemoteCommand rejects a command of 4096 bytes or more. - for chunk in List.chunkBySize 30 toMark do - let names = chunk |> List.map (sprintf "'%s'") |> String.concat " " - - redisIn context ready (sprintf "SADD \"%s-retiring\" %s" helmReleaseName names) - |> ignore - - marked <- Set.union marked (Set.ofList chunk) - - if not toMark.IsEmpty then - LogInfo "Marked %d retiring (%d ready, %d outstanding)" toMark.Length ready.Count outstanding + let retirable = status.["retirable"] :?> JArray + + // Already-deleted names stay in the monitor's set, so filter by what is still live. + let toRetire = + retirable + |> Seq.map (fun name -> name.ToString()) + |> Seq.filter livePods.Contains + |> Seq.truncate maxRetiredPerPass + |> List.ofSeq + + if not toRetire.IsEmpty then + // /data is emptyDir, so collect before deleting or the logs are lost. + collectLogsFromPods context toRetire + + for pod in toRetire do + try + context.kube.DeleteNamespacedPod(pod, context.namespaceProperty) |> ignore + with :? k8s.Autorest.HttpOperationException as ex when + ex.Response.StatusCode = Net.HttpStatusCode.NotFound -> + LogInfo "Pod %s already gone" pod + + livePods <- Set.difference livePods (Set.ofList toRetire) + LogInfo "Retired %d workers (%d outstanding)" toRetire.Length outstanding with ex -> LogWarn "Worker scale-down skipped this pass: %s" ex.Message // Detect if the mission is stuck from two signals: 1. job queue // has in progress items but no live workers 2. the job monitor diff --git a/src/FSLibrary/StellarOrphanSweep.fs b/src/FSLibrary/StellarOrphanSweep.fs index 12c5074f..bf78cbe5 100644 --- a/src/FSLibrary/StellarOrphanSweep.fs +++ b/src/FSLibrary/StellarOrphanSweep.fs @@ -64,17 +64,18 @@ let private sweepKind let private sweepWithCutoff (cutoff: DateTime) (kube: Kubernetes) (ns: string) (apiRateLimit: int) : unit = LogInfo "Orphan sweep starting: namespace=%s cutoff=%s" ns (cutoff.ToString("o")) - // 1. helm-uninstall PCv2 releases whose StatefulSet is older than the cutoff. + // 1. helm-uninstall PCv2 releases whose worker pod is older than the cutoff. // Done before the kubectl-style deletes so helm's release secrets get // cleaned up properly, rather than left dangling pointing at deleted // resources. ApiRateLimit.sleepUntilNextRateLimitedApiCallTime apiRateLimit let stsItems = kube.ListNamespacedStatefulSet(namespaceParameter = ns).Items + let podItems = kube.ListNamespacedPod(namespaceParameter = ns).Items - for release in stsItems - |> Seq.filter (fun sts -> isOlderThan cutoff sts.Metadata) - |> Seq.map (fun sts -> sts.Metadata.Name) + for release in podItems + |> Seq.filter (fun pod -> isOlderThan cutoff pod.Metadata) + |> Seq.map (fun pod -> pod.Metadata.Name) |> Seq.filter (fun name -> name.StartsWith("parallel-catchup-") && name.Contains("-stellar-core")) |> Seq.map (fun name -> name.Substring(0, name.LastIndexOf("-stellar-core"))) |> Set.ofSeq do @@ -111,6 +112,20 @@ let private sweepWithCutoff (cutoff: DateTime) (kube: Kubernetes) (ns: string) ( |> ignore) cutoff + // PCv2 workers are bare pods, so nothing else in this sweep would reap them. + // Restricted to ownerless pods: everything else goes away with its controller. + sweepKind + apiRateLimit + "Pod" + (fun () -> + podItems + |> Seq.filter (fun p -> isNull p.Metadata.OwnerReferences || p.Metadata.OwnerReferences.Count = 0) + |> Seq.map (fun p -> p.Metadata)) + (fun n -> + kube.DeleteNamespacedPod(namespaceParameter = ns, name = n, propagationPolicy = "Foreground") + |> ignore) + cutoff + sweepKind apiRateLimit "Deployment" diff --git a/src/MissionParallelCatchup/job_monitor.py b/src/MissionParallelCatchup/job_monitor.py index bbebdd6e..289aaa89 100644 --- a/src/MissionParallelCatchup/job_monitor.py +++ b/src/MissionParallelCatchup/job_monitor.py @@ -1,5 +1,6 @@ import os import redis +import socket import requests import json import sys @@ -26,6 +27,8 @@ PROGRESS_QUEUE = os.getenv('PROGRESS_QUEUE', 'in_progress') #LIST METRICS = os.getenv('METRICS', 'metrics') # SET JOB_OWNERS = os.getenv('JOB_OWNERS', 'job_owners') # HASH +RETIRING = os.getenv('RETIRING', 'retiring') # SET +MIN_UNMARKED_WORKERS = int(os.getenv('MIN_UNMARKED_WORKERS', 8)) WORKER_PREFIX = os.getenv('WORKER_PREFIX', 'stellar-core') NAMESPACE = os.getenv('NAMESPACE', 'default') WORKER_COUNT = int(os.getenv('WORKER_COUNT', 3)) @@ -77,6 +80,7 @@ def get_logging_level(): 'jobs_failed': [], 'jobs_in_progress': [], 'workers': [], + 'retirable': [], 'workers_up': 0, 'workers_down': 0, 'workers_refresh_duration': 0, @@ -148,6 +152,14 @@ def ping_worker(pod_name, retries=1): time.sleep(STUCK_JOB_PING_DELAY_SECS) return False +def pod_exists(pod_name): + # Trailing dot skips the resolv.conf search list, so a miss costs one query. + try: + socket.gethostbyname(f"{pod_name}.{WORKER_PREFIX}.{NAMESPACE}.svc.cluster.local.") + return True + except socket.gaierror: + return False + def update_status_and_metrics(): global status mission_start_time = time.time() @@ -174,8 +186,27 @@ def update_status_and_metrics(): queue_in_progress_count = len(jobs_in_progress) queue_remain_count = redis_client.llen(JOB_QUEUE) + # --- Phase 1c: Mark surplus idle workers as retiring; worker.sh stops claiming once marked --- + # Names here are pod names ("{WORKER_PREFIX}-{i}") as worker.sh stores them in JOB_OWNERS. + busy = set(job_owners.values()) + retiring = redis_client.smembers(RETIRING) + candidates = sorted(w for w in (f"{WORKER_PREFIX}-{i}" for i in range(WORKER_COUNT)) + if w not in busy and w not in retiring) + outstanding = queue_remain_count + queue_in_progress_count + keep = max(outstanding, MIN_UNMARKED_WORKERS) + to_mark = [] + # Resolve only when the name count says something could be marked: at t=0 outstanding >= fleet, so zero lookups. + if len(busy - retiring) + len(candidates) > keep: + unmarked_idle = [w for w in candidates if pod_exists(w)] + to_mark = unmarked_idle[:max(0, len(busy - retiring) + len(unmarked_idle) - keep)] + if to_mark: + redis_client.sadd(RETIRING, *to_mark) + logger.info("Marked %d workers retiring (%d outstanding)", len(to_mark), outstanding) + # Marked on an earlier pass and still idle; names stay here after the driver deletes them. + retirable = sorted(retiring - busy) + # --- Phase 2: Quick single-ping check of workers that own in-progress jobs --- - active_workers = set(job_owners.values()) + active_workers = busy worker_statuses = [] workers_up = 0 workers_down = 0 @@ -231,6 +262,7 @@ def update_status_and_metrics(): 'jobs_failed': jobs_failed, 'jobs_in_progress': jobs_in_progress, 'workers': worker_statuses, + 'retirable': retirable, 'workers_up': workers_up, 'workers_down': workers_down, 'workers_refresh_duration': workers_refresh_duration, diff --git a/src/MissionParallelCatchup/parallel_catchup_helm/files/worker.sh b/src/MissionParallelCatchup/parallel_catchup_helm/files/worker.sh index b391b76f..0d4872be 100644 --- a/src/MissionParallelCatchup/parallel_catchup_helm/files/worker.sh +++ b/src/MissionParallelCatchup/parallel_catchup_helm/files/worker.sh @@ -9,7 +9,6 @@ if [ -z "$FAILED_QUEUE" ]; then echo "FAILED_QUEUE not set"; exit 1; fi if [ -z "$SUCCESS_QUEUE" ]; then echo "SUCCESS_QUEUE not set"; exit 1; fi if [ -z "$METRICS" ]; then echo "METRICS not set"; exit 1; fi if [ -z "$JOB_OWNERS" ]; then echo "JOB_OWNERS not set"; exit 1; fi -if [ -z "$RELEASE_NAME" ]; then echo "RELEASE_NAME not set"; exit 1; fi if [ -z "$POD_NAME" ]; then echo "POD_NAME not set"; exit 1; fi # ensure redis-cli is available @@ -27,8 +26,8 @@ if job then redis.call("HSET", KEYS[3], job, ARGV[1]) end return job' while true; do -# Stop claiming once the driver marks us, so it can remove us without interrupting a range. -if [ "$(redis-cli -h "$REDIS_HOST" -p "$REDIS_PORT" SISMEMBER "$RELEASE_NAME-retiring" "$POD_NAME")" = "1" ]; then +# Stop claiming once the job monitor marks us, so the driver can remove us without interrupting a range. +if [ "$(redis-cli -h "$REDIS_HOST" -p "$REDIS_PORT" SISMEMBER "retiring" "$POD_NAME")" = "1" ]; then echo "$(date) $POD_NAME is retiring; not claiming." sleep $SLEEP_INTERVAL continue diff --git a/src/MissionParallelCatchup/parallel_catchup_helm/templates/catchup_workers.yaml b/src/MissionParallelCatchup/parallel_catchup_helm/templates/catchup_workers.yaml index 5975f275..7c1d844b 100644 --- a/src/MissionParallelCatchup/parallel_catchup_helm/templates/catchup_workers.yaml +++ b/src/MissionParallelCatchup/parallel_catchup_helm/templates/catchup_workers.yaml @@ -22,111 +22,102 @@ metadata: {{- end }} --- {{- range $i := until (int $.Values.worker.replicas) }} -apiVersion: apps/v1 -kind: StatefulSet +apiVersion: v1 +kind: Pod metadata: name: {{ $.Release.Name }}-stellar-core-{{ $i }} labels: app: {{ $.Release.Name }}-stellar-core worker-index: {{ $i | quote }} spec: - serviceName: "{{ $.Release.Name }}-stellar-core" - podManagementPolicy: Parallel - replicas: 1 - selector: - matchLabels: - app: {{ $.Release.Name }}-stellar-core - worker-index: {{ $i | quote }} - template: - metadata: - labels: + # Bare pods have no controller to give them stable DNS, so hostname/subdomain + # supply the same `...svc` name the old controller did. + hostname: {{ $.Release.Name }}-stellar-core-{{ $i }} + subdomain: {{ $.Release.Name }}-stellar-core + serviceAccountName: stellar-supercluster-{{ $.Release.Name }} + {{- if or $.Values.worker.requireNodeLabels $.Values.worker.avoidNodeLabels }} + affinity: + nodeAffinity: + requiredDuringSchedulingIgnoredDuringExecution: + nodeSelectorTerms: + - matchExpressions: + {{- range $.Values.worker.requireNodeLabels }} + - key: {{ .key }} + operator: {{ .operator }} + {{- with .values }} + values: {{ toJson . }} + {{- end }} + {{- end }} + {{- range $.Values.worker.avoidNodeLabels }} + - key: {{ .key }} + operator: {{ .operator }} + {{- with .values }} + values: {{ toJson . }} + {{- end }} + {{- end }} + {{- end }} + {{- if $.Values.worker.tolerateNodeTaints }} + tolerations: + {{- range $.Values.worker.tolerateNodeTaints }} + - key: {{ .key }} + effect: {{ .effect }} + {{- end }} + {{- end }} + containers: + - name: stellar-core + image: {{ $.Values.worker.stellar_core_image }} + imagePullPolicy: Always + resources: + requests: + cpu: "{{ $.Values.worker.resources.requests.cpu}}" + memory: "{{ $.Values.worker.resources.requests.memory}}" + ephemeral-storage: "{{ $.Values.worker.resources.requests.ephemeral_storage}}" + limits: + cpu: "{{ $.Values.worker.resources.limits.cpu}}" + memory: "{{ $.Values.worker.resources.limits.memory}}" + ephemeral-storage: "{{ $.Values.worker.resources.limits.ephemeral_storage}}" + command: ["/bin/sh", "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/scripts/worker.sh"] + ports: + - containerPort: 11626 + env: + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name + - name: ASAN_OPTIONS + value: {{ $.Values.worker.asanOptions | quote }} + - name: WORKER_INDEX + value: {{ $i | quote }} + envFrom: + - configMapRef: + name: {{ $.Release.Name }}-worker-config + volumeMounts: + - name: config + mountPath: /config + - name: script + mountPath: /scripts + - name: data-volume + mountPath: /data + volumes: + - name: config + configMap: + name: {{ $.Release.Name }}-stellar-core-config + - name: script + configMap: + name: {{ $.Release.Name }}-worker-script + - emptyDir: {} + name: data-volume + {{- if not $.Values.worker.unevenSched }} + topologySpreadConstraints: + - labelSelector: + matchLabels: app: {{ $.Release.Name }}-stellar-core - worker-index: {{ $i | quote }} - spec: - serviceAccountName: stellar-supercluster-{{ $.Release.Name }} - {{- if or $.Values.worker.requireNodeLabels $.Values.worker.avoidNodeLabels }} - affinity: - nodeAffinity: - requiredDuringSchedulingIgnoredDuringExecution: - nodeSelectorTerms: - - matchExpressions: - {{- range $.Values.worker.requireNodeLabels }} - - key: {{ .key }} - operator: {{ .operator }} - {{- with .values }} - values: {{ toJson . }} - {{- end }} - {{- end }} - {{- range $.Values.worker.avoidNodeLabels }} - - key: {{ .key }} - operator: {{ .operator }} - {{- with .values }} - values: {{ toJson . }} - {{- end }} - {{- end }} - {{- end }} - {{- if $.Values.worker.tolerateNodeTaints }} - tolerations: - {{- range $.Values.worker.tolerateNodeTaints }} - - key: {{ .key }} - effect: {{ .effect }} - {{- end }} - {{- end }} - containers: - - name: stellar-core - image: {{ $.Values.worker.stellar_core_image }} - imagePullPolicy: Always - resources: - requests: - cpu: "{{ $.Values.worker.resources.requests.cpu}}" - memory: "{{ $.Values.worker.resources.requests.memory}}" - ephemeral-storage: "{{ $.Values.worker.resources.requests.ephemeral_storage}}" - limits: - cpu: "{{ $.Values.worker.resources.limits.cpu}}" - memory: "{{ $.Values.worker.resources.limits.memory}}" - ephemeral-storage: "{{ $.Values.worker.resources.limits.ephemeral_storage}}" - command: ["/bin/sh", "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/scripts/worker.sh"] - ports: - - containerPort: 11626 - env: - - name: POD_NAME - valueFrom: - fieldRef: - fieldPath: metadata.name - - name: ASAN_OPTIONS - value: {{ $.Values.worker.asanOptions | quote }} - - name: WORKER_INDEX - value: {{ $i | quote }} - envFrom: - - configMapRef: - name: {{ $.Release.Name }}-worker-config - volumeMounts: - - name: config - mountPath: /config - - name: script - mountPath: /scripts - - name: data-volume - mountPath: /data - volumes: - - name: config - configMap: - name: {{ $.Release.Name }}-stellar-core-config - - name: script - configMap: - name: {{ $.Release.Name }}-worker-script - - emptyDir: {} - name: data-volume - {{- if not $.Values.worker.unevenSched }} - topologySpreadConstraints: - - labelSelector: - matchLabels: - app: {{ $.Release.Name }}-stellar-core - # Note: maxSkew affects dynamic node scheduling with karpenter - # See https://github.com/stellar/supercluster/issues/330 - maxSkew: 2 - topologyKey: kubernetes.io/hostname - whenUnsatisfiable: DoNotSchedule - {{- end }} + # Note: maxSkew affects dynamic node scheduling with karpenter + # See https://github.com/stellar/supercluster/issues/330 + maxSkew: 2 + topologyKey: kubernetes.io/hostname + whenUnsatisfiable: DoNotSchedule + {{- end }} --- {{- end }} --- @@ -172,4 +163,3 @@ data: PROGRESS_QUEUE: "{{ .Values.redis.progress_queue }}" METRICS: "{{ .Values.redis.metrics }}" JOB_OWNERS: "{{ .Values.redis.job_owners }}" - RELEASE_NAME: "{{ .Release.Name }}" From e2ef41fa02287fa732bb3b3c405613fe37d61e7e Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Tue, 22 Sep 2026 12:26:22 -0400 Subject: [PATCH 19/22] Restore unrelated lines to match main Co-Authored-By: Claude Fable 5.1 --- src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs | 3 ++- .../parallel_catchup_helm/files/worker.sh | 1 + .../parallel_catchup_helm/templates/catchup_workers.yaml | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs index c32b3a75..8fc922d5 100644 --- a/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs +++ b/src/FSLibrary/MissionHistoryPubnetParallelCatchupV2.fs @@ -282,7 +282,8 @@ let collectLogsFromPods (context: MissionContext) (podNames: string list) : unit else LogWarn "No logs found or empty archive for pod %s" podName - with ex -> LogWarn "Could not collect logs from pod %s: %s" podName ex.Message + with ex -> + LogWarn "Could not collect logs from pod %s (this is expected if pod doesn't exist): %s" podName ex.Message // Cleanup on exit. `signalTriggered` indicates we're running under a hard // deadline (Jenkins' SoftKillWaitSeconds, ~5s by default, before SIGKILL). diff --git a/src/MissionParallelCatchup/parallel_catchup_helm/files/worker.sh b/src/MissionParallelCatchup/parallel_catchup_helm/files/worker.sh index 0d4872be..dfce03b4 100644 --- a/src/MissionParallelCatchup/parallel_catchup_helm/files/worker.sh +++ b/src/MissionParallelCatchup/parallel_catchup_helm/files/worker.sh @@ -9,6 +9,7 @@ if [ -z "$FAILED_QUEUE" ]; then echo "FAILED_QUEUE not set"; exit 1; fi if [ -z "$SUCCESS_QUEUE" ]; then echo "SUCCESS_QUEUE not set"; exit 1; fi if [ -z "$METRICS" ]; then echo "METRICS not set"; exit 1; fi if [ -z "$JOB_OWNERS" ]; then echo "JOB_OWNERS not set"; exit 1; fi +if [ -z "$RELEASE_NAME" ]; then echo "RELEASE_NAME not set"; exit 1; fi if [ -z "$POD_NAME" ]; then echo "POD_NAME not set"; exit 1; fi # ensure redis-cli is available diff --git a/src/MissionParallelCatchup/parallel_catchup_helm/templates/catchup_workers.yaml b/src/MissionParallelCatchup/parallel_catchup_helm/templates/catchup_workers.yaml index 7c1d844b..690bea9b 100644 --- a/src/MissionParallelCatchup/parallel_catchup_helm/templates/catchup_workers.yaml +++ b/src/MissionParallelCatchup/parallel_catchup_helm/templates/catchup_workers.yaml @@ -163,3 +163,4 @@ data: PROGRESS_QUEUE: "{{ .Values.redis.progress_queue }}" METRICS: "{{ .Values.redis.metrics }}" JOB_OWNERS: "{{ .Values.redis.job_owners }}" + RELEASE_NAME: "{{ .Release.Name }}" From e79e2da713dd9c6a1290804dad22406b219a5e7d Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Tue, 22 Sep 2026 14:45:42 -0400 Subject: [PATCH 20/22] Document ownerless-Pod deletion in the sweep Co-Authored-By: Claude Fable 5.1 --- src/App/Program.fs | 2 +- src/FSLibrary/StellarOrphanSweep.fs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/App/Program.fs b/src/App/Program.fs index 5b584bf4..11f8a9c3 100644 --- a/src/App/Program.fs +++ b/src/App/Program.fs @@ -30,7 +30,7 @@ type PollOptions(kubeConfig: string, namespaceProperty: string option) = inherit KubeOption(kubeConfig, namespaceProperty) [] + HelpText = "Delete every Service/ConfigMap/StatefulSet/Ingress/Job/DaemonSet/Deployment and ownerless Pod in the namespace and helm-uninstall any parallel-catchup-* releases. Destructive: meant for explicit recovery of a shared namespace, not for routine use.")>] type ForceCleanNamespaceOptions(kubeConfig: string, namespaceProperty: string option) = inherit KubeOption(kubeConfig, namespaceProperty) diff --git a/src/FSLibrary/StellarOrphanSweep.fs b/src/FSLibrary/StellarOrphanSweep.fs index bf78cbe5..78bc2444 100644 --- a/src/FSLibrary/StellarOrphanSweep.fs +++ b/src/FSLibrary/StellarOrphanSweep.fs @@ -54,9 +54,9 @@ let private sweepKind // Delete resources older than `cutoff` in the given namespace. Targets the same // resource type set the retired `clean` verb did (Service, ConfigMap, -// StatefulSet, Ingress, Job, DaemonSet, Deployment) and also helm-uninstalls -// any `parallel-catchup-*` releases (PCv2) so helm's release secrets get -// tidied along with the workloads. +// StatefulSet, Ingress, Job, DaemonSet, Deployment) plus ownerless Pods, and +// also helm-uninstalls any `parallel-catchup-*` releases (PCv2) so helm's +// release secrets get tidied along with the workloads. // // Used by both the automatic on-startup orphan sweep (cutoff = now - 2 days) // and the explicit `force-clean-namespace` verb (cutoff = DateTime.MaxValue, From 8046411daedd223085ef375b2233fe049964d560 Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Mon, 5 Oct 2026 10:17:16 -0400 Subject: [PATCH 21/22] worker.sh: fail closed on the retiring check; plumb RETIRING through the chart Co-Authored-By: Claude Fable 5.1 --- .../parallel_catchup_helm/files/worker.sh | 6 ++++-- .../parallel_catchup_helm/templates/catchup_workers.yaml | 1 + .../parallel_catchup_helm/templates/job_monitor.yaml | 1 + .../parallel_catchup_helm/values.yaml | 1 + 4 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/MissionParallelCatchup/parallel_catchup_helm/files/worker.sh b/src/MissionParallelCatchup/parallel_catchup_helm/files/worker.sh index dfce03b4..9000427e 100644 --- a/src/MissionParallelCatchup/parallel_catchup_helm/files/worker.sh +++ b/src/MissionParallelCatchup/parallel_catchup_helm/files/worker.sh @@ -9,6 +9,7 @@ if [ -z "$FAILED_QUEUE" ]; then echo "FAILED_QUEUE not set"; exit 1; fi if [ -z "$SUCCESS_QUEUE" ]; then echo "SUCCESS_QUEUE not set"; exit 1; fi if [ -z "$METRICS" ]; then echo "METRICS not set"; exit 1; fi if [ -z "$JOB_OWNERS" ]; then echo "JOB_OWNERS not set"; exit 1; fi +if [ -z "$RETIRING" ]; then echo "RETIRING not set"; exit 1; fi if [ -z "$RELEASE_NAME" ]; then echo "RELEASE_NAME not set"; exit 1; fi if [ -z "$POD_NAME" ]; then echo "POD_NAME not set"; exit 1; fi @@ -28,8 +29,9 @@ return job' while true; do # Stop claiming once the job monitor marks us, so the driver can remove us without interrupting a range. -if [ "$(redis-cli -h "$REDIS_HOST" -p "$REDIS_PORT" SISMEMBER "retiring" "$POD_NAME")" = "1" ]; then - echo "$(date) $POD_NAME is retiring; not claiming." +# Fail closed: anything but an explicit 0 (marked, or redis-cli error) means don't claim. +if [ "$(redis-cli -h "$REDIS_HOST" -p "$REDIS_PORT" SISMEMBER "$RETIRING" "$POD_NAME")" != "0" ]; then + echo "$(date) $POD_NAME is retiring or redis unreachable; not claiming." sleep $SLEEP_INTERVAL continue fi diff --git a/src/MissionParallelCatchup/parallel_catchup_helm/templates/catchup_workers.yaml b/src/MissionParallelCatchup/parallel_catchup_helm/templates/catchup_workers.yaml index 690bea9b..f55f24fb 100644 --- a/src/MissionParallelCatchup/parallel_catchup_helm/templates/catchup_workers.yaml +++ b/src/MissionParallelCatchup/parallel_catchup_helm/templates/catchup_workers.yaml @@ -163,4 +163,5 @@ data: PROGRESS_QUEUE: "{{ .Values.redis.progress_queue }}" METRICS: "{{ .Values.redis.metrics }}" JOB_OWNERS: "{{ .Values.redis.job_owners }}" + RETIRING: "{{ .Values.redis.retiring }}" RELEASE_NAME: "{{ .Release.Name }}" diff --git a/src/MissionParallelCatchup/parallel_catchup_helm/templates/job_monitor.yaml b/src/MissionParallelCatchup/parallel_catchup_helm/templates/job_monitor.yaml index 4c428077..f949f0bf 100644 --- a/src/MissionParallelCatchup/parallel_catchup_helm/templates/job_monitor.yaml +++ b/src/MissionParallelCatchup/parallel_catchup_helm/templates/job_monitor.yaml @@ -103,6 +103,7 @@ data: PROGRESS_QUEUE: "{{ .Values.redis.progress_queue }}" METRICS: "{{ .Values.redis.metrics }}" JOB_OWNERS: "{{ .Values.redis.job_owners }}" + RETIRING: "{{ .Values.redis.retiring }}" WORKER_PREFIX: "{{ .Release.Name }}-stellar-core" WORKER_COUNT: "{{ .Values.worker.replicas }}" LOGGING_INTERVAL_SECONDS: "{{ .Values.monitor.logging_interval_seconds }}" diff --git a/src/MissionParallelCatchup/parallel_catchup_helm/values.yaml b/src/MissionParallelCatchup/parallel_catchup_helm/values.yaml index ab7fc4a1..4f1cf127 100644 --- a/src/MissionParallelCatchup/parallel_catchup_helm/values.yaml +++ b/src/MissionParallelCatchup/parallel_catchup_helm/values.yaml @@ -7,6 +7,7 @@ redis: progress_queue: "in_progress" metrics: "metrics" job_owners: "job_owners" + retiring: "retiring" resources: requests: cpu: "100m" From 775e5159308e211411b84cef67fdd6a013be4404 Mon Sep 17 00:00:00 2001 From: Jonathan Eid Date: Mon, 5 Oct 2026 10:28:49 -0400 Subject: [PATCH 22/22] Orphan sweep: list pods after the helm uninstalls Co-Authored-By: Claude Fable 5.1 --- src/FSLibrary/StellarOrphanSweep.fs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/FSLibrary/StellarOrphanSweep.fs b/src/FSLibrary/StellarOrphanSweep.fs index 78bc2444..8c6236be 100644 --- a/src/FSLibrary/StellarOrphanSweep.fs +++ b/src/FSLibrary/StellarOrphanSweep.fs @@ -114,16 +114,15 @@ let private sweepWithCutoff (cutoff: DateTime) (kube: Kubernetes) (ns: string) ( // PCv2 workers are bare pods, so nothing else in this sweep would reap them. // Restricted to ownerless pods: everything else goes away with its controller. + // Listed here, after the helm uninstalls above, so pods they already removed are not deleted again. sweepKind apiRateLimit "Pod" (fun () -> - podItems + kube.ListNamespacedPod(namespaceParameter = ns).Items |> Seq.filter (fun p -> isNull p.Metadata.OwnerReferences || p.Metadata.OwnerReferences.Count = 0) |> Seq.map (fun p -> p.Metadata)) - (fun n -> - kube.DeleteNamespacedPod(namespaceParameter = ns, name = n, propagationPolicy = "Foreground") - |> ignore) + (fun n -> kube.DeleteNamespacedPod(namespaceParameter = ns, name = n) |> ignore) cutoff sweepKind