wombatu-kun opened a new pull request, #19391:
URL: https://github.com/apache/hudi/pull/19391

   ### Describe the issue this Pull Request addresses
   
   This is breaking CI. `TestHiveDriverPool` failed three tests on #19389 
([Azure build 
15760](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15760))
 with `Expected org.apache.hudi.hive.HoodieHiveSyncException to be thrown, but 
nothing was thrown`; `awaitAllThrowsFirstError` exhausted all four surefire 
reruns and failed the build.
   
   Root cause is a race in `awaitAll`. A worker calls `dispatch.abort()` from 
its `catch` block, which releases the awaiting thread while that task is still 
unwinding - its exception only reaches the `FutureTask` after `call()` returns. 
`awaitAll` then runs `cancelPending()`, and `FutureTask.cancel(false)` succeeds 
on any task still in state `NEW`, one mid-unwind included. The cancel wins the 
state CAS, the later `setException` is a no-op, `get()` reports 
`CancellationException`, and `awaitAll` returns normally with `firstError` 
still null.
   
   `HiveQueryDDLExecutor.runSQLs` sends partition `ADD`/`TOUCH`/`ALTER` DDL 
through this path when `hoodie.datasource.hive_sync.batching.enabled` is on, so 
losing the race reports a failed partition batch as a successful sync and 
leaves the metastore short of partitions.
   
   ### Summary and Changelog
   
   - `Dispatch` records the first failing `Throwable`: `abort()` becomes 
`recordFailure(Throwable)`, which stores it before counting the latch down. 
`dispatchAll` and `runOnEachWorker` call that instead. The skip path still 
throws `CancellationException` outside the `try`, so unstarted statements keep 
counting as cancelled.
   - `awaitAll` seeds `firstError` from that record rather than from 
`Future.get()`, and widens it to `Throwable` so an `Error` out of `Driver.run` 
propagates too.
   - New test `awaitAllReportsFailureWhenFailingFutureIsCancelledMidFlight` 
pins the losing interleaving deterministically: it parks the Driver 
mid-statement, cancels that future from the test thread (what `cancelPending()` 
does when it wins), then lets the Driver throw. It fails 4/4 reruns without the 
fix; the three existing error-path tests stop being timing-dependent as a side 
effect.
   
   ### Impact
   
   A hive-sync that was silently leaving partitions unregistered will now fail 
with the `HoodieHiveSyncException` this code always intended to throw. Only the 
HiveQL sync path with batching enabled builds a pool at all. No API or config 
change.
   
   ### Risk Level
   
   low
   
   Error reporting only; the success path and the in-task abort check that 
bounds how much extra DDL a failed batch applies are both untouched. 
`hudi-hive-sync` unit suite 345/345, checkstyle clean.
   
   ### Documentation Update
   
   none
   
   ### Contributor's checklist
   
   - [x] Read through [contributor's 
guide](https://hudi.apache.org/contribute/how-to-contribute)
   - [x] Enough context is provided in the sections above
   - [x] Adequate tests were added if applicable
   


-- 
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]

Reply via email to