Skip to content

Commit 921a758

Browse files
committed
fix(controller): track executor state with LRU eviction instead of id-based skip
The id-based skip in updateExecutorState dropped any executor whose ID exceeded MaxTrackedExecutorPerApp and never pruned the state map. On long-running drivers this made executors past the cap invisible and let stale terminated entries crowd out the cap budget, so on executor churn Status.ExecutorState could report zero running executors despite live pods. Replace it with a size-bounded policy enforced at the end of updateExecutorState: evict terminated/UNKNOWN entries oldest-ID-first, and only when the cap is fully occupied by live entries drop the newest-ID-first (logging a warning). Status.ExecutorState semantics shift from cumulative to at-most-cap instantaneous. Add util.ParseExecutorIDFromPodName to recover executor IDs from pod names for terminated entries whose pods are already gone. Signed-off-by: shashankchaudhary <shashankch292@gmail.com>
1 parent 968912a commit 921a758

4 files changed

Lines changed: 663 additions & 5 deletions

File tree

internal/controller/sparkapplication/controller.go

Lines changed: 74 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020
"context"
2121
"fmt"
2222
"os"
23-
"strconv"
23+
"sort"
2424
"strings"
2525
"time"
2626

@@ -1044,10 +1044,6 @@ func (r *Reconciler) updateExecutorState(ctx context.Context, app *v1beta2.Spark
10441044
var executorApplicationID string
10451045
for _, pod := range pods {
10461046
if util.IsExecutorPod(&pod) {
1047-
// If the executor number is higher than the `MaxTrackedExecutorPerApp` we want to stop persisting executors
1048-
if executorID, _ := strconv.Atoi(util.GetSparkExecutorID(&pod)); executorID > r.options.MaxTrackedExecutorPerApp {
1049-
continue
1050-
}
10511047
newState := util.GetExecutorState(&pod)
10521048
oldState, exists := app.Status.ExecutorState[pod.Name]
10531049
// Only record an executor event if the executor state is new or it has changed.
@@ -1106,9 +1102,82 @@ func (r *Reconciler) updateExecutorState(ctx context.Context, app *v1beta2.Spark
11061102
}
11071103
}
11081104

1105+
r.enforceExecutorStateCap(ctx, app)
1106+
11091107
return nil
11101108
}
11111109

1110+
// enforceExecutorStateCap bounds the size of app.Status.ExecutorState to
1111+
// MaxTrackedExecutorPerApp. Terminated entries are evicted oldest-first by
1112+
// executor ID; if the cap is still exceeded (all remaining entries are live),
1113+
// the newest live entries are dropped and a warning is logged.
1114+
//
1115+
// Ordering relies on Spark assigning executor IDs as a monotonically
1116+
// increasing per-application counter, so a larger ID means a more recently
1117+
// created executor. Names without a parseable trailing ID resolve to 0 and
1118+
// are thus treated as oldest; ID ties are broken by name for determinism.
1119+
func (r *Reconciler) enforceExecutorStateCap(ctx context.Context, app *v1beta2.SparkApplication) {
1120+
logger := log.FromContext(ctx)
1121+
capacity := r.options.MaxTrackedExecutorPerApp
1122+
if capacity <= 0 || len(app.Status.ExecutorState) <= capacity {
1123+
return
1124+
}
1125+
1126+
type entry struct {
1127+
name string
1128+
id int
1129+
}
1130+
var terminated, live []entry
1131+
for name, state := range app.Status.ExecutorState {
1132+
e := entry{name: name, id: util.ParseExecutorIDFromPodName(name)}
1133+
// UNKNOWN is set above when the pod is gone but the driver is still
1134+
// running, so treat it as evictable alongside terminal states.
1135+
if util.IsExecutorTerminated(state) || state == v1beta2.ExecutorStateUnknown {
1136+
terminated = append(terminated, e)
1137+
} else {
1138+
live = append(live, e)
1139+
}
1140+
}
1141+
1142+
// Terminated eviction: drop oldest-ID first until at cap or none left.
1143+
sort.Slice(terminated, func(i, j int) bool {
1144+
if terminated[i].id != terminated[j].id {
1145+
return terminated[i].id < terminated[j].id
1146+
}
1147+
return terminated[i].name < terminated[j].name
1148+
})
1149+
for _, e := range terminated {
1150+
if len(app.Status.ExecutorState) <= capacity {
1151+
return
1152+
}
1153+
delete(app.Status.ExecutorState, e.name)
1154+
}
1155+
1156+
if len(app.Status.ExecutorState) <= capacity {
1157+
return
1158+
}
1159+
1160+
// Live eviction: all remaining entries are live; drop newest-ID first.
1161+
sort.Slice(live, func(i, j int) bool {
1162+
if live[i].id != live[j].id {
1163+
return live[i].id > live[j].id
1164+
}
1165+
return live[i].name < live[j].name
1166+
})
1167+
dropped := 0
1168+
for _, e := range live {
1169+
if len(app.Status.ExecutorState) <= capacity {
1170+
break
1171+
}
1172+
delete(app.Status.ExecutorState, e.name)
1173+
dropped++
1174+
}
1175+
if dropped > 0 {
1176+
logger.Info("Dropped live executors from status; tracked-executor cap fully occupied by live entries",
1177+
"name", app.Name, "namespace", app.Namespace, "dropped", dropped, "cap", capacity)
1178+
}
1179+
}
1180+
11121181
func (r *Reconciler) getExecutorPods(ctx context.Context, app *v1beta2.SparkApplication) (*corev1.PodList, error) {
11131182
matchLabels := util.GetResourceLabels(app)
11141183
matchLabels[common.LabelSparkRole] = common.SparkRoleExecutor

0 commit comments

Comments
 (0)