tigerquoll opened a new pull request, #1110:
URL: https://github.com/apache/yunikorn-core/pull/1110
### Description
Test-only. **No product code is touched.**
The scheduler's existing tests assert individual outcomes — "this ask was
allocated", "this queue is
over its limit". None assert its output *as a whole*: who gets what, on
which node, in what order,
and what is handed back for release. Most interesting scheduler changes are
ordering changes, and a
change to ask ordering, node scoring, headroom accounting, victim selection
or placeholder
replacement can leave every existing assertion green while changing where
work lands.
This adds two tests that make that class of change visible. Both drive a
fixed workload through the
real `RMProxy`/partition API and pin the exact, ordered sequence of
decisions the RM observes via
`UpdateAllocation` — `(applicationID, allocationKey, nodeID)` per
allocation, `(applicationID,
allocationKey, terminationType)` per release — compared verbatim against a
golden JSON file.
#### What's added
| File | |
|---|---|
| `golden_trace_test.go` | `TestGoldenDecisionTrace` — one mixed workload +
shared helpers |
| `golden_trace_scenarios_test.go` | `TestGoldenDecisionTraceScenarios` — 6
isolated sub-tests |
| `testdata/golden_trace*.json` | 7 goldens (14 entries for the mixed
workload, 3–4 each for scenarios) |
| `mock_rm_callback_test.go` | additive: `traceEntry`, an opt-in `trace`
field, `enableTrace()`, `getTrace()`, `getNodeAllocations()` |
Capture is opt-in — the mock is shared with the whole package, including
`BenchmarkScheduling`, so
capturing unconditionally would put an append inside the callback's write
lock on a measured path.
Two words are used throughout, for two different things that both happen to
number six:
* **Phases** — the six *sequential* stages inside `TestGoldenDecisionTrace`.
One cluster, each phase
building on the state the last left, one 14-entry golden for all of them.
* **Scenarios** — the six *independent* sub-tests of
`TestGoldenDecisionTraceScenarios`. Each gets a
fresh cluster, its own config and its own small golden.
They are not the same six. `gang` and `preemption` name both a phase and a
scenario, deliberately —
see the duplication note below.
**The mixed workload's phases** are: priority ordering within an app,
pending-ask removal, gang
placeholder replacement, restart + recovery, app failure with pending asks,
and priority preemption.
The whole pinned trace is short enough to read:
```
p-high, p-mid, p-low -> golden-node-main:1
r-1, r-3, r-4 -> golden-node-main:1 (r-2 removed while
pending)
g-placeholder -> main:1 ; g-placeholder :: PLACEHOLDER_REPLACED ; g-real ->
main:1
p-recover-high -> golden-node-main:1 (p-recover-low
does not fit)
low-1, low-2 -> preempt:1 ; low-2 :: PREEMPTED_BY_SCHEDULER ; high-1 ->
preempt:1
```
Three of those six phases only pin anything because the workload is sized to
make them decide
something:
* **Pending-ask removal** — four asks at four distinct priorities, no
scheduling cycle before the
middle one (`r-2`) is removed. The golden pins that the survivors are
still served in priority
order and that the removal never reaches the RM.
* **Restart and recovery** — recovered usage 55/7 against a 105/10 node
leaves 50/3 free; the two
post-recovery asks are 30/1 each, so exactly one fits. Lost usage
allocates both, double-counted
usage neither.
* **Failed-application ask cleanup** — the node still has 20/2 free and the
failed app's asks are
10/1 each, so they *would* be allocated had cleanup not happened.
Those figures aren't comments: `node-main`'s capacity is computed from the
per-phase ask sizes and
each phase's headroom is asserted, so resizing an earlier ask fails with a
message naming what to
recompute rather than quietly hollowing out a later phase.
**The scenarios** each isolate a single decision:
| Scenario | What it pins |
|---|---|
| `app-ordering-fifo` / `app-ordering-fair` | Two apps tie on priority for
one remaining slot; the policy's second key decides (submission time vs usage
ratio). Pre-loaded so the policies pick different winners. |
| `node-binpacking` / `node-fair` | Four equal asks, two identical nodes.
Binpacking packs, fair alternates. Traces differ **only** in the node column. |
| `gang` | Placeholder → real handshake on an otherwise empty cluster. |
| `preemption` | Victim selection, release, and reschedule, on an otherwise
empty cluster. |
Policy pairs share an identical workload, so the diff between a pair *is*
the policy's effect.
The `gang` and `preemption` scenarios repeat choreography the mixed workload
also runs, rather than
sharing a helper with it. That duplication is the point: a shared helper
would move both traces
together on a mistake, and "the shared trace broke but the scenario didn't,
so it's prior-state
interaction" would stop being a conclusion anyone could draw.
#### Determinism
* Fixed app/node IDs, pinned strictly-increasing creation times on every ask.
* Scheduling advanced by explicit `MultiStepSchedule`, never the background
loop.
* Trace lengths asserted **exactly**, not "at least": a `>= n` barrier is
satisfied early by a
surplus decision, which then drifts into a later wait and presents as a
timeout instead of a diff.
* Every assertion is preceded by a quiescence round trip
(`traceProbe.sync`), because polling a
length cannot prove *absence*: where no new decision is expected, the
first evaluation already
succeeds. Costs nothing measurable — 7.9s → 7.7s.
* WARN-level logging, re-applied after each `ms.Init` (which resets it) and
restored from a snapshot
on cleanup, since it is process-global.
* Candidate lists come from Go map iteration, so each scenario avoids ties
in the comparator that
decides it (distinct submission times *and* usage for app-sort; node IDs
that order unambiguously
under `nodeRef.Less`'s `NodeID` fallback).
One exception: creation times are **not** pinned across the restart —
`NewSIFromAllocation` copies
neither `AllocationTags` nor `Priority`, so replayed allocations get
`time.Now()` and priority 0.
Nothing pinned depends on their order; they are already allocated, and
`tryAllocate` skips those.
#### Scope and limitations
These pin decision *sequences* for the behaviours above on small clusters.
They are **not** a general
conformance suite — a green run does not mean "scheduling is unchanged". Not
covered:
* **Reservations lifecycle** — reservation happens internally during
preemption, but reserve/unreserve
is never pinned and reservation timeout is never reached.
* **Ordering of recovered allocations among themselves** — the recovery
phase pins that recovered
*usage* is accounted for, not that recovered requests sort like live ones;
the recovery path loses
the createTime and priority that would make such an assertion meaningful.
* **The partition column** — single-partition workloads make it a constant
carrying the normalised
name, which would write the RM ID into every golden line.
* **User and group quota limits** — no config sets `limits:`; all apps run
as one user, no groups.
* **Required-node / daemonset asks** — no ask carries a required-node tag.
* **Unschedulable-ask backoff** — `backoffDeadline` is never armed; no queue
sets a backoff threshold.
* **Placeholder timeout and gang failure paths** — only the successful
placeholder→real replacement.
* **Guaranteed-resource-driven preemption and preemption fencing** — both
preemption scenarios are
priority-driven; no queue uses `FencePreemptionPolicy`.
* **Nested queue hierarchies** — every config is `root` plus one level of
leaves.
* **Multiple partitions** — everything runs on the single `default`
partition.
* **Custom resource types** — only `memory` and `vcore`.
* **Foreign allocations** — none registered.
* **Queue placement rules** — apps name their target queue directly.
* **The placeholder-replacement revert** in
`Application.tryPlaceholderAllocate` — reaching it needs
two goroutines racing on one placeholder within a cycle, which a
step-driven harness can't script
without the timing nondeterminism a golden test must not have.
#### Running and regenerating
```
go test ./pkg/scheduler/tests/ -run '^TestGoldenDecisionTrace' -count=1 #
both tests
```
Raise `-count` to check determinism. A mismatch prints a side-by-side
listing naming the golden file,
marking differing rows and, for the mixed workload, attributing each row to
the phase that produced
it. A phase that produces no decisions owns no row, so it is named at its
boundary instead.
Regenerate only when a change *intentionally* alters scheduling decisions:
```
# all seven goldens
UPDATE_GOLDEN=1 go test ./pkg/scheduler/tests/ -run
'^TestGoldenDecisionTrace' -count=1
# a single scenario
UPDATE_GOLDEN=1 go test ./pkg/scheduler/tests/ -run
'^TestGoldenDecisionTraceScenarios$/^node-fair$' -count=1
```
Under `UPDATE_GOLDEN` the length barriers report their divergence and carry
on rather than failing
ahead of the rewrite, so the run's log doubles as a summary of what changed;
the workload's own
preconditions still fail hard. The value is parsed as a boolean; an
unparseable one fails rather than
silently verifying.
**The run always ends RED** — a green one would be indistinguishable from a
run that verified
something. So:
1. `git diff pkg/scheduler/tests/testdata/` — that diff **is** the behaviour
change.
2. Account for every changed line; one you can't explain is a side-effect,
not noise.
3. Update the trace-length arguments the run reported, so the barriers pin
the new behaviour.
4. Re-run **without** `UPDATE_GOLDEN` and confirm green — the regenerating
run says nothing about
correctness.
5. Include the golden diff in the PR, so the new sequence is reviewed as a
behaviour change.
Only goldens matched by `-run` are rewritten, and on an unmodified tree they
come back byte-identical
— so a dirty `git status` afterwards always means a real delta.
### Type of change
- [x] Improvement
### Jira issue
Jira ID : https://issues.apache.org/jira/browse/YUNIKORN-3338
- [x] I have created a Jira issue for this pull request.
- [x] The Jira ID is part of the title of this pull request.
### AI Tooling
Generated by Author with assistance from Claude Code.
If an AI tool was used:
- [x] The PR includes the phrase "Generated by \<tool>", where \<tool> is
the name of the AI tool used.
- [x] My use of AI contributions follows the ASF legal policy.
Check https://www.apache.org/legal/generative-tooling.html for details.
### How has this been tested?
- [x] New unit tests were added to cover new or changed code paths.
- [x] `make test_all` was run, and no failures reported.
- [ ] A pull request will be opened for new e2e tests
(apache/yunikorn-k8shim repository). — not
applicable: no product code changes, so there is no new behaviour for e2e
to cover.
Beyond `make test_all`:
| Check | Result |
|---|---|
| `git diff origin/master --name-only` | 10 files, test and testdata only |
| `-run '^TestGoldenDecisionTrace' -count=10` | ok, 74.2s |
| `go test ./pkg/scheduler/tests/ -count=1` | ok, 48.3s |
| `go test ./pkg/scheduler/tests/ -race -count=1` | ok, 49.7s |
| `-bench BenchmarkScheduling -benchtime=1x` | ok, unaffected by trace
capture |
Adds ~7.3s to a package run: 41.0s without these tests, 48.3s with them.
A golden test that never fails is decoration, so each perturbation below was
applied, observed, and
reverted in full:
1. **Recovery replay skipped** → restart phase fails: node reports 105/10
free against a 50/3 ledger,
and both post-recovery asks allocate instead of one.
2. **`LessThan` priority flipped** → reverses phase 1 (`p-high`↔`p-low`) and
phase 2 (`r-1`↔`r-4`);
order-only, invisible to any length assertion.
3. **`LessThan` createTime flipped** → 5 of 6 scenarios fail with `a1..a4`
reversed; `gang` correctly
stays green, never holding two comparable pending asks.
4. **`nodeRef.Less` by descending score** → the node pair fails *by swapping
into each other's
golden*: binpacking spreads, fair packs.
5. **`sortApplicationsByPriorityAndSubmissionTime` reversed** → only
`app-ordering-fifo` fails
(`e-3`→`l-2` on the contested slot); `app-ordering-fair` correctly stays
green.
6. **Failure cleanup bypassed** → 3 decisions where 1 was pinned: both
abandoned asks allocate,
confirming the node genuinely had room.
7. **Regeneration, both directions** → with (1) applied, `UPDATE_GOLDEN`
rewrites the golden (md5
`2a62d152…`→`98240929…`, 14→16 entries) and still ends red; reverting (1)
restores it byte-for-byte.
### Questions:
- [ ] The change needs documentation, a pull request for
apache/yunikorn-site repository will be created.
- [ ] There is breaking changes for older versions: jira is tagged with
`release-notes` label.
- [ ] The licenses files needs to be updated.
None apply: the change is test-only, adds no user-facing behaviour, and
introduces no dependencies.
### Screenshots or other details
A mismatch names the golden file, marks differing rows with `!!`, and
attributes each row to the
phase that produced it (a phase producing no decisions is named at its
boundary instead):
```
scheduling decisions do not match the golden file.
golden file: testdata/golden_trace.json (14 entries)
this run: 14 entries
6 phase 3 gang placeholder replacement golden: alloc g-placeholder
... got: alloc g-placeholder ...
!! 7 phase 3 gang placeholder replacement golden: release g-placeholder
of ... got: release g-placeholder of golden-app-gang ...
9 phase 4 restart and recovery golden: alloc p-recover-high
... got: alloc p-recover-high ...
--- phase 5 failed application ask cleanup: no decisions ---
10 phase 6 preemption golden: alloc low-1 ...
got: alloc low-1 ...
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
