wombatu-kun commented on code in PR #19334:
URL: https://github.com/apache/hudi/pull/19334#discussion_r3626890772
##########
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/transaction/lock/HiveMetastoreBasedLockProvider.java:
##########
@@ -192,17 +195,15 @@ private void acquireLockInternal(long time, TimeUnit
unit, LockComponent lockCom
final LockRequest lockRequestFinal = lockRequest;
this.lock = executor.submit(() -> hiveClient.lock(lockRequestFinal))
.get(time, unit);
-
- // refresh lock in case that certain commit takes a long time.
- Heartbeat heartbeat = new Heartbeat(hiveClient, lock.getLockid());
- long heartbeatIntervalMs = lockConfiguration.getConfig()
- .getLong(LOCK_HEARTBEAT_INTERVAL_MS_KEY,
DEFAULT_LOCK_HEARTBEAT_INTERVAL_MS);
- future = executor.scheduleAtFixedRate(heartbeat, heartbeatIntervalMs /
2, heartbeatIntervalMs, TimeUnit.MILLISECONDS);
+ scheduleHeartbeat();
} catch (InterruptedException | TimeoutException e) {
if (this.lock == null || this.lock.getState() != LockState.ACQUIRED) {
LockResponse lockResponse =
this.hiveClient.checkLock(lockRequest.getTxnid());
if (lockResponse.getState() == LockState.ACQUIRED) {
this.lock = lockResponse;
+ // The lock was granted server-side even though the client timed out
waiting on the
+ // future; it still needs a heartbeat, otherwise a long-running
commit lets HMS expire it.
+ scheduleHeartbeat();
Review Comment:
`checkLock()` in this catch and `unlock()` in the finally already run on the
caller thread while the submitted `lock()` task may still be in flight, so the
heartbeat is not the first concurrent user of `hiveClient` here. Cancelling
that future would not close the window either: `cancel(false)` is a no-op once
the task has started and `cancel(true)` cannot interrupt a blocking thrift
socket read, so serializing `IMetaStoreClient` access belongs in a separate
change.
--
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]
