SEPURI-SAI-KRISHNA opened a new pull request, #59043:
URL: https://github.com/apache/spark/pull/59043
### What changes were proposed in this pull request?
This PR fixes the flaky test `driver side SQL metrics` in
`SQLAppStatusListenerSuite` (runs in both
`SQLAppStatusListenerWithInMemoryStoreSuite` and
`SQLAppStatusListenerWithRocksDBBackendSuite`).
The test now captures its own execution id inside
`SQLExecution.withNewExecutionId` and uses `eventually` to wait until the
status store has computed the metrics for that execution. The unbounded `while`
/ `Thread.sleep` wait loops and the lookup of the latest execution in the store
are removed.
### Why are the changes needed?
The test fails intermittently in CI, e.g. in the scheduled master and
branch-4.x builds:
```
- driver side SQL metrics *** FAILED *** (21 milliseconds)
Map(3105033 -> "2", 3105032 -> "1", 3105031 -> "total (min, med, max
(stageId: taskId))
0 ms (0 ms, 0 ms, 0 ms (stage 5.0: task 10))") did not contain key 3105125
(SQLAppStatusListenerSuite.scala:607)
```
The wait logic was racy:
1. `while (statusStore.executionsCount() < oldCount)` can never be true,
since the count only grows, so it never waited.
2. `SQLAppStatusListener` processes events asynchronously. If it has not yet
processed the new execution, `statusStore.executionsList().last` is an
execution left over from a previous test. That execution already has metric
values, so the second wait loop exited immediately and the test read the wrong
execution's metrics. The map in the failure above is such a previous
execution's metrics.
In addition, the old loops had no timeout, so a real hang would never fail
the test.
### Does this PR introduce _any_ user-facing change?
No. Test-only change.
### How was this patch tested?
- Reproduced the race deterministically with a temporary (not included)
suite that stalls the status listener queue for 2 seconds while the test runs:
the old wait logic fails every time with the same symptom as CI (it reads the
previous execution's metrics), and the new wait logic passes.
- `build/sbt 'sql/testOnly *SQLAppStatusListenerWithInMemoryStoreSuite
*SQLAppStatusListenerWithRocksDBBackendSuite'`: 38 tests passed.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5.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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
