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

   ### Change Logs
   
   Fixes three defects in the Hive-metastore-based lock provider 
(hudi-hive-sync, package transaction/lock):
   
   - `Heartbeat.run()` rethrew on any failure. Because the heartbeat runs under 
`ScheduledExecutorService.scheduleAtFixedRate`, a single thrown exception 
permanently cancels all subsequent executions (it is only observable through 
the never-read `ScheduledFuture`), so one transient HMS/network hiccup silently 
stops lock renewal while the writer still believes it holds the lock. It now 
logs a warning with the cause and continues, so the next tick retries. The 
previous throw also discarded the original cause.
   
   - `HiveMetastoreBasedLockProvider.close()` ran `executor.shutdown()` as the 
last statement inside the `try`, so if `unlock()`/`Hive.closeCurrent()` threw, 
the 2-thread scheduled pool was never shut down and leaked for the JVM 
lifetime. `executor.shutdown()` is moved into a `finally`, and the caught 
exception is now passed to `log.error` instead of being dropped.
   
   - When `acquireLock` timed out on `Future.get` but the lock was actually 
granted server-side, the provider recovered it via `checkLock` and returned 
success, but never scheduled a heartbeat, so a long-running commit let HMS 
expire the recovered lock mid-write. Heartbeat scheduling is extracted into 
`scheduleHeartbeat()` and is now also invoked on that recovery path.
   
   ### Impact
   
   The Hive-metastore lock provider (`HiveMetastoreBasedLockProvider`) becomes 
more robust: transient heartbeat failures no longer silently drop the lock 
(which could let a second writer acquire it and corrupt data), and repeated 
lock/close cycles in a long-lived JVM no longer leak scheduler threads. No 
public API or configuration change.
   
   ### Risk level (write none, low medium or high below)
   
   low
   
   Localized to the `transaction/lock` package. New unit tests cover the 
heartbeat no-rethrow behavior and executor shutdown on both the normal and 
unlock-throws paths; existing functional tests 
(`TestHiveMetastoreBasedLockProvider`) still cover 
acquire/unlock/reentrant/waiting flows.
   
   ### Documentation Update
   
   none
   
   ### Contributor's checklist
   
   - [ ] Read through [contributor's 
guide](https://hudi.apache.org/contribute/how-to-contribute)
   - [ ] Change Logs and Impact were stated clearly
   - [ ] Adequate tests were added if applicable
   - [ ] CI passed
   


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