[GitHub] [hudi] leesf commented on a diff in pull request #9160: [HUDI-6501] HoodieHeartbeatClient should stop all heartbeats and not delete heartbeat files for close

["leesf (via GitHub)" <gi...@apache.org>]

leesf commented on code in PR #9160:
URL: https://github.com/apache/hudi/pull/9160#discussion_r1260443951


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/heartbeat/HoodieHeartbeatClient.java:
##########
@@ -185,36 +179,55 @@ public void start(String instantTime) {
   }
 
   /**
-   * Stops the heartbeat for the specified instant.
-   * @param instantTime
+   * Stops the heartbeat and deletes the heartbeat file for the specified instant.
+   *
+   * @param instantTime The instant time for the heartbeat.
    * @throws HoodieException
    */
   public void stop(String instantTime) throws HoodieException {
     Heartbeat heartbeat = instantToHeartbeatMap.get(instantTime);
-    if (heartbeat != null && heartbeat.isHeartbeatStarted() && !heartbeat.isHeartbeatStopped()) {
-      LOG.info("Stopping heartbeat for instant " + instantTime);
-      heartbeat.getTimer().cancel();
-      heartbeat.setHeartbeatStopped(true);
-      LOG.info("Stopped heartbeat for instant " + instantTime);
+    if (isHeartbeatStarted(heartbeat)) {
+      stopHeartbeatTimer(heartbeat);
       HeartbeatUtils.deleteHeartbeatFile(fs, basePath, instantTime);
       LOG.info("Deleted heartbeat file for instant " + instantTime);
     }
   }
 
   /**
-   * Stops all heartbeats started via this instance of the client.
+   * Stops all timers of heartbeats started via this instance of the client.
+   *
    * @throws HoodieException
    */
-  public void stop() throws HoodieException {
-    instantToHeartbeatMap.values().forEach(heartbeat -> stop(heartbeat.getInstantTime()));
+  public void stopHeartbeatTimers() throws HoodieException {
+    instantToHeartbeatMap.values().stream().filter(this::isHeartbeatStarted).forEach(this::stopHeartbeatTimer);
+  }
+
+  /**
+   * Whether the given heartbeat is started.
+   *
+   * @param heartbeat The heartbeat to check whether is started.
+   * @return Whether the heartbeat is started.
+   * @throws IOException
+   */
+  private boolean isHeartbeatStarted(Heartbeat heartbeat) {
+    return heartbeat != null && heartbeat.isHeartbeatStarted() && !heartbeat.isHeartbeatStopped();

Review Comment:
   use `heartbeat != null && heartbeat.isHeartbeatStarted()` or `heartbeat != null && heartbeat.isHeartbeatStopped()` is enough?



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org