slachiewicz commented on PR #285:
URL:
https://github.com/apache/flink-connector-kafka/pull/285#issuecomment-5471439470
@Efrat19 re `getSentSourceEvent()` vs the Whitebox read — I looked into it,
and unfortunately the public getter doesn't close this race in Flink 2.2/2.3:
```java
public Map<Integer, List<SourceEvent>> getSentSourceEvent() throws Exception
{
return workerExecutor.submit(() -> new HashMap<>(sentSourceEvent)).get();
}
```
Two problems for the racy call sites:
1. **The copy runs on the workerExecutor, but mutations run on the
mainExecutor** (`sendEventToSourceReader` mutates the HashMap directly when on
the main thread). In the tests that hit the flake (e.g. the
`awaitCloseStarted()` one at line ~1196), close keeps sending events on the
main executor while the test polls — so the copy can still throw
`ConcurrentModificationException`, same as the Whitebox read.
2. If it does, `.get()` wraps it in an `ExecutionException`, which the retry
`catch (AssertionError | ConcurrentModificationException)` doesn't catch — the
retry fix would be defeated. During close polling there's also a
`RejectedExecutionException` risk once the executor is shut down.
The non-racy helper next to it (`getLatestMetadataUpdateEvent`, line ~2388)
already uses the public getter — so reflection is confined to the one polling
path that races with the close. I'd keep the Whitebox read + multi-catch as-is,
but happy to switch and widen the catch to
`ExecutionException`/`RejectedExecutionException` if you prefer dropping the
reflection.
--
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]
