PoiBlackTea opened a new pull request, #1132:
URL: https://github.com/apache/yunikorn-core/pull/1132
### Description
Remove redundant initial allocation phase and conflicting assertions in
TestSchedulerRecoveryQuotaPreemption, aligning the test with its actual
recovery quota preemption verification goals.
#### Root Cause Analysis (RCA):
- **Race Condition in Pre-restart Phase**: In
`TestSchedulerRecoveryQuotaPreemption`,
`ms.proxy.UpdateAllocation("allocation-key-01", ...)` was called asynchronously
via the event channel. Right after the call, synchronous assertions
(`assert.Equal(part.GetTotalAllocationCount(), 0)` and
`part.GetNode(...).GetAllocatedResource() == 0`) were executed immediately
without waiting for event processing.
- When the background goroutine was scheduled slower than the main
test thread, the assertions passed by chance because the node resource was
still at its initial state (`0`).
- When the background goroutine finished processing faster (e.g.,
under CI load), `node-1:1234` was successfully allocated `1024` memory, causing
the assertion to fail (`1024 != 0`).
- Adding `time.Sleep` or `waitForAllocations` 100% reproduces this
assertion failure deterministically.
- **Inconsistent Pre-restart Assertion**: In
`TestSchedulerRecoveryQuotaPreemption`, the application was already registered
(`addApp = true`), meaning `allocation-key-01` would succeed
once processed, contradicting the assertions that expected
`TotalAllocationCount == 0` and node memory usage `== 0`.
- **Substantive Test Intent**: The core purpose of
`TestSchedulerRecoveryQuotaPreemption` is to verify the 6 table-driven recovery
quota preemption behaviors under different configurations. The actual state
injection is done via `queueA.IncAllocatedResource(tt.allocated, false)` after
restart, and validated by `ms.mockRM.waitForAllocations(t, tt.addedAllocations,
1000)`. The initial allocation in Phase 1 was completely discarded after
`ms.serviceContext.StopAll()` and never propagated to the restart phase.
### Type of change
Please delete options that are not relevant.
- [ ] Bug Fix
- [X] Improvement
- [ ] Feature
- [ ] Refactoring
- [ ] Documentation
### Jira issue
Jira ID : https://issues.apache.org/jira/browse/YUNIKORN-3378
- [ ] 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
If an AI tool was used:
- [X] The PR includes the phrase "Generated by Antigravity IDE", where
Antigravity IDE 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?
- [ ] New unit tests were added to cover new or changed code paths.
- [ ] `make test_all` was run, and no failures reported.
- [ ] A pull request will be opened for new e2e tests
(apache/yunikorn-k8shim repository).
### 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.
### Screenshots or other details
N/A
--
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]
