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

2023-07-11 Thread via GitHub


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


##
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:
   @leesf, `heartbeat != null && heartbeat.isHeartbeatStarted()` or `heartbeat 
!= null && heartbeat.isHeartbeatStopped()` is not enough. Because 
`isHeartbeatStarted` is true and `isHeartbeatStopped` is true when heartbeat is 
stopped.



-- 
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-unsubscr...@hudi.apache.org

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



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

2023-07-10 Thread via GitHub


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


##
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/testutils/HoodieClientTestBase.java:
##
@@ -770,6 +770,7 @@ protected void updateBatchWithoutCommit(String 
newCommitTime, List
 HoodieWriteConfig hoodieWriteConfig = 
getConfigBuilder(HoodieFailedWritesCleaningPolicy.LAZY)
 .withAutoCommit(false) // disable auto commit
 .withRollbackUsingMarkers(true)
+.withHeartbeatTolerableMisses(0)
 .build();

Review Comment:
   @danny0405, accelerate the expiration of the heartbeat for the deltacommit 
and not wait for the heartbeat expiration in [line 
308](https://github.com/apache/hudi/blob/master/hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/client/functional/TestSavepointRestoreMergeOnRead.java#L308)
 for `TestSavepointRestoreMergeOnRead#testCleaningCompletedRollback`.



-- 
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-unsubscr...@hudi.apache.org

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