danny0405 commented on code in PR #18904:
URL: https://github.com/apache/hudi/pull/18904#discussion_r3354710908
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/heartbeat/HoodieHeartbeatClient.java:
##########
@@ -197,20 +223,31 @@ public boolean isHeartbeatExpired(String instantTime)
throws IOException {
private void updateHeartbeat(String instantTime) throws
HoodieHeartbeatException {
try {
Long newHeartbeatTime = System.currentTimeMillis();
- OutputStream outputStream =
- this.storage.create(
- new StoragePath(heartbeatFolderPath, instantTime), true);
- outputStream.close();
+ writeHeartbeatFile(instantTime);
Heartbeat heartbeat = instantToHeartbeatMap.get(instantTime);
if (heartbeat.getLastHeartbeatTime() != null &&
isHeartbeatExpired(instantTime)) {
- log.error("Aborting, missed generating heartbeat within allowable
interval {} ms", this.maxAllowableHeartbeatIntervalInMs);
- // Since TimerTask allows only java.lang.Runnable, cannot throw an
exception and bubble to the caller thread, hence
- // explicitly interrupting the timer thread.
- Thread.currentThread().interrupt();
+ // A previous refresh was delayed past the tolerable interval. Stop
refreshing this heartbeat
+ // (cancel the timer) and do NOT advance the last heartbeat time, so
the heartbeat stays expired
+ // and the writer aborts at commit time via
HeartbeatUtils.abortIfHeartbeatExpired(). We must not
+ // keep refreshing here: a concurrent process (e.g. an async cleaner
under LAZY failed-writes
+ // policy) may already have started rolling back this instant once it
observed the expiry, and
+ // resurrecting the heartbeat could let this writer commit on top of
rolled-back files.
+ // The timer is cancelled cleanly rather than via Thread.interrupt(),
which would permanently
+ // kill the timer thread (turning a transient delay into a permanent
blackout on the first miss).
+ log.error("Missed generating heartbeat for instant {} within allowable
interval {} ms; stopping heartbeat refresh",
+ instantTime, this.maxAllowableHeartbeatIntervalInMs);
+ heartbeat.getTimer().cancel();
Review Comment:
can we replace the Timer with a Java `ScheduledExecutorService`,
`java.util.Timer. Timer` is a legacy class (introduced in Java 1.3) with severe
design flaws. If a `TimerTask` throws an uncaught exception, the entire timer
thread dies, killing all other scheduled tasks permanently.
--
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]
