SEPURI-SAI-KRISHNA opened a new pull request, #29577:
URL: https://github.com/apache/flink/pull/29577
## What is the purpose of the change
`ExecutionVertexTest.testResetForNewExecutionReleasesPartitions` is
unstable. The ticket reports it
on master and on release-2.3, and every occurrence that names a line fails
at line 95, the first
`assertThat(releasePartitionsFuture).isNotDone()`, before `markFinished()`,
with the producer's
partition already released. The review on #29183 confirmed the two runs
linked in the description
fail at the same line.
The test builds the scheduler with
`ComponentMainThreadExecutorServiceAdapter.forMainThread()`, a
direct executor pinned to the test thread, and
`TestingUtils.defaultExecutorExtension()`. The 3 arg
`DefaultSchedulerBuilder` constructor uses that second executor as the io,
future and delay
executor, and it reaches `Execution` through
`ExecutionVertex#createNewExecution`. `Execution#deploy`
hops to it to build the TDD and then composes back "to the main thread", but
a direct executor runs
that stage inline on the pool thread. The thread assert in `forMainThread`
then trips inside the
executor's `execute`, where `CompletableFuture` swallows it into the chain.
If both hops dispatch off the test thread the chain dies silently, the task
stays in `DEPLOYING` and
nothing is released, so the test passes. The failure needs the first hop to
fail off-thread before
`whenCompleteAsync` attaches. That handler is then dispatched from the test
thread, actually runs,
`finalizeDeploymentAndCleanUpInMainThread` sees the failure, and
`markFailed` reaches
`handlePartitionCleanup(true, true)`, which releases the producer's blocking
partitions while
`startScheduling()` has not returned. Hence the release before the first
assertion.
Same root cause and same fix as FLINK-40682, which covered
`ArchivedExecutionGraphTest` and
`CachedShuffleDescriptorsTest`.
## Brief change log
- Have the class executor extension of `ExecutionVertexTest` create a
`DirectScheduledExecutorService`, so the deployment callbacks run inline
on the test thread. Both
`DefaultSchedulerBuilder` call sites in the test read
`EXECUTOR_RESOURCE.getExecutor()`, so both
get the direct executor from this one change and neither call site needs
editing. Doing it on the
extension also keeps the shutdown, which an inline `new
DirectScheduledExecutorService()` per call
site would lose. Both `isNotDone()` assertions are kept. FLINK-40682 made
the same change the same
way.
## Verifying this change
- Forcing the failing interleaving, with a 5ms delay in the executor side
supplier, a 50ms delay in
`deploy()` before `whenCompleteAsync` attaches, and a 100ms pause before
the first assertion, the
old test fails every run (10 of 10) with the CI message, and the new test
passes every run
(10 of 10) with no main thread violations at all.
- Each delay alone is not enough: delaying only the supplier passes 5 of 5
and the continuation
never runs, and pausing only before the assertion passes 5 of 5. Both
orderings are needed.
- Unmodified, the test passes 30 of 30 locally, which is expected at the
rate measured on the
ticket, 5 failures in 12000.
- The whole `ExecutionVertexTest` class passes 5 of 5, and the 223 tests of
`org.apache.flink.runtime.executiongraph` pass with the change.
- Making `handlePartitionCleanup` release an empty set still fails the final
assertion, and making
`markFinished()` release the blocking partitions still fails the second
`isNotDone()`, so neither
assertion is vacuous.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): **no**
- The public API, i.e., is any changed class annotated with
`@Public(Evolving)`: **no**
- The serializers: **no**
- The runtime per-record code paths (performance sensitive): **no**
- Anything that affects deployment or recovery: JobManager (and its
components), Checkpointing, Kubernetes/Yarn, ZooKeeper: **no**
- The S3 file system connector: **no**
## Documentation
- Does this pull request introduce a new feature? **no**
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (please specify the tool below)
Generated-by: Claude Code (Opus 5)
--
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]
