voonhous commented on code in PR #19370:
URL: https://github.com/apache/hudi/pull/19370#discussion_r3643829498
##########
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/transaction/lock/HiveMetastoreBasedLockProvider.java:
##########
@@ -225,12 +234,33 @@ private void acquireLockInternal(long time, TimeUnit
unit, LockComponent lockCom
* takes a long time. Must be called only after {@link #lock} has been set.
*/
private void scheduleHeartbeat() {
- Heartbeat heartbeat = new Heartbeat(hiveClient, lock.getLockid());
+ Heartbeat heartbeat = new Heartbeat(hiveClient, lock.getLockid(),
this::onLockLost);
long heartbeatIntervalMs = lockConfiguration.getConfig()
.getLong(LOCK_HEARTBEAT_INTERVAL_MS_KEY,
DEFAULT_LOCK_HEARTBEAT_INTERVAL_MS);
future = executor.scheduleAtFixedRate(heartbeat, heartbeatIntervalMs / 2,
heartbeatIntervalMs, TimeUnit.MILLISECONDS);
}
+ /**
+ * Invoked from the heartbeat thread once the metastore reports the lock as
expired or aborted.
+ * The lock cannot be renewed anymore, so stop heartbeating it and stop
claiming it is held.
+ */
+ private void onLockLost(Exception cause) {
Review Comment:
`future.cancel(false)` doesn't stop a tick that's already running, so this
can fire for a lock we released normally: `unlock()` drops the lock at HMS
while a heartbeat RPC is in flight, that heartbeat comes back with
`NoSuchLockException`, and we end up here even though nothing was lost.
Depending on timing:
- if we've already re-acquired, this drops the new lock and cancels its
heartbeat, so the healthy lock quietly expires at HMS mid-commit
- with no re-acquire, `lockLostRemotely` stays latched and the next no-op
`unlock()` throws spuriously
- the heartbeat thread can also null `lock` between the assignment in
`acquireLockInternal` and `lock.getLockid()` in `scheduleHeartbeat()`, which
NPEs
A lock-id guard closes all three:
```java
long lockId = lock.getLockid();
Heartbeat heartbeat = new Heartbeat(hiveClient, lockId, e ->
onLockLost(lockId, e));
```
and here, return early unless the current `lock` is non-null with that same
id.
--
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]
