rangareddy commented on PR #19486:
URL: https://github.com/apache/hudi/pull/19486#issuecomment-5252115778
Squashed to a single commit and rebased onto current master (31 commits of
drift; neither file had been touched upstream, so no conflicts). Re-ran
everything after the rebase rather than assuming it still held:
`org.apache.hudi.client.transaction.**` 256 tests green, checkstyle 0,
apache-rat 0.
Two changes while reviewing, both in the test:
1. The regression previously failed with a bare
`java.util.concurrent.TimeoutException` from `Future.get`, which tells a reader
nothing. It now converts that into an `AssertionError` naming the cause:
```
java.lang.AssertionError: tryLock never returned - it is blocked on the
monitor held by the unrelated
thread, which means the provider is synchronizing on the interned "lock"
literal again rather than on a
private monitor
```
Re-verified both arms afterwards: production file reverted to master ->
10 run / 1 failed with that message; with the fix -> 10 run / 0 failed.
2. The evidence for the hazard is sharper than the description claimed, so I
strengthened it: `FileSystemBasedLockProviderTestClass` does not merely
synchronize on its `LOCK = "lock"` constant, it calls
`LOCK.wait(retryWaitTimeMs)` inside that block (line 79). An unrelated provider
implementation is therefore waiting and notifying on the very monitor this
provider takes.
The description is updated for both, including the failure transcript, which
quoted the old `TimeoutException`. I also unchecked "CI passes on my PR" — the
checks are queued, and the module evidence above is local.
@hudi-agent's nit from the first round (do not name a test class in
production Javadoc) remains applied: the production comment states the
mechanism only, and the concrete instance lives here and in the commit message
where going stale costs nothing.
--
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]
