wombatu-kun opened a new pull request, #19370: URL: https://github.com/apache/hudi/pull/19370
### Describe the issue this Pull Request addresses Part of #19345 - this covers point 2 of that issue only. It is a follow-up of #19334, whose review surfaced both points of that issue. #19334 made `Heartbeat.run` swallow every failure so that a transient error does not cancel the whole `scheduleAtFixedRate` chain. That is right for transient errors, but `NoSuchLockException`, `NoSuchTxnException` and `TxnAbortedException` are terminal: the metastore has already expired or aborted the lock and no later tick can renew it. Today each of them logs a `WARN` once per heartbeat interval (60s by default) for the rest of the commit, while `HiveMetastoreBasedLockProvider` keeps its `lock` field set, so the writer goes on believing it still holds exclusivity. Point 1 of the issue, the `checkLock(lockRequest.getTxnid())` recovery branch, is out of scope here and stays open. ### Summary and Changelog When the metastore reports the lock as gone, the provider now stops renewing it, stops claiming to hold it, and says so. - `Heartbeat` takes an `onLockLost` callback and catches the three terminal exceptions declared by `IMetaStoreClient.heartbeat(long, long)` ahead of the existing broad `catch`, which keeps handling transient failures exactly as before. It latches the terminal state, so a tick already queued when the schedule was cancelled does not issue another doomed heartbeat or report the loss twice. - `HiveMetastoreBasedLockProvider.onLockLost` logs the loss once at `ERROR` with the database and table, cancels the scheduled future and clears `lock`. - `unlock()` throws `HoodieLockException` naming the metastore expiry when the lock was dropped that way, instead of returning silently. This keeps the writer as loudly informed as it is today, when unlocking the stale lock id fails with a bare `NoSuchLockException`, while skipping the RPC that cannot succeed. - Because the heartbeat thread can now clear `lock`, `close()` and the `finally` block of `acquireLockInternal` read it into a local before use, matching what `unlock()` already does, and `future` is now `volatile`. - `TestHeartbeat` covers all three terminal exceptions plus the transient path; the new `TestHiveMetastoreBasedLockProviderLockLoss` drives the real scheduler against a mocked `IMetaStoreClient` and asserts that the lock is dropped, that renewal stops, that `unlock()` fails and that no stale unlock is sent. ### Impact Affects users of `HiveMetastoreBasedLockProvider` whose lock is expired or aborted by the metastore mid-commit. Instead of a `WARN` per minute and a writer that believes it is still exclusive, there is one `ERROR` at the moment of loss and a failure at release time. A release that fails today with a bare thrift `NoSuchLockException` now fails with a message naming the cause, so the failure mode is unchanged in kind. No public API, config or storage format change. ### Risk Level low The change is confined to the hive-sync lock heartbeat path. Transient failures keep the existing retry behaviour, verified by test. The terminal path is verified end to end against the real `ScheduledExecutorService` with a mocked metastore client, including that no further heartbeat is attempted after the loss. Targeted tests and checkstyle pass for `hudi-sync/hudi-hive-sync`. ### 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]
