vinothchandar commented on a change in pull request #2359:
URL: https://github.com/apache/hudi/pull/2359#discussion_r579589397



##########
File path: 
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/heartbeat/HeartbeatUtils.java
##########
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.heartbeat;
+
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.common.util.ValidationUtils;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.exception.HoodieException;
+import org.apache.hudi.table.HoodieTable;
+import org.apache.log4j.LogManager;
+import org.apache.log4j.Logger;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Helper class to delete heartbeat for completed or failed instants with 
expired heartbeats.
+ */
+public class HeartbeatUtils {
+
+  private static final Logger LOG = LogManager.getLogger(HeartbeatUtils.class);
+
+  /**
+   * Deletes the heartbeat file for the specified instant.
+   * @param fs
+   * @param basePath
+   * @param instantTime
+   * @return
+   */
+  public static boolean deleteHeartbeatFile(FileSystem fs, String basePath, 
String instantTime) {
+    boolean deleted = false;
+    try {
+      String heartbeatFolderPath = 
HoodieTableMetaClient.getHeartbeatFolderPath(basePath);
+      deleted = fs.delete(new Path(heartbeatFolderPath + File.separator + 
instantTime), false);
+      if (!deleted) {
+        LOG.error("Failed to delete heartbeat for instant " + instantTime);
+      }
+    } catch (IOException io) {
+      LOG.error("Unable to delete heartbeat for instant " + instantTime, io);
+    }
+    return deleted;
+  }
+
+  /**
+   * Deletes the heartbeat files for instants with expired heartbeats without 
any active instant.
+   * @param allExistingHeartbeatInstants
+   * @param metaClient
+   * @param basePath
+   */
+  public static void cleanExpiredHeartbeats(List<String> 
allExistingHeartbeatInstants,
+                                            HoodieTableMetaClient metaClient, 
String basePath) {
+    Set<String> nonExpiredHeartbeatInstants = metaClient.getActiveTimeline()
+        
.filterCompletedInstants().getInstants().map(HoodieInstant::getTimestamp).collect(Collectors.toSet());
+    allExistingHeartbeatInstants.stream().forEach(instant -> {
+      if (!nonExpiredHeartbeatInstants.contains(instant)) {
+        deleteHeartbeatFile(metaClient.getFs(), basePath, instant);
+      }
+    });
+  }
+
+  /**
+   * Check if the heartbeat corresponding to instantTime has expired. If yes, 
abort by throwing an exception.
+   * @param instantTime
+   * @param table
+   * @param heartbeatClient
+   * @param config
+   */
+  public static void abortIfHeartbeatExpired(String instantTime, HoodieTable 
table,
+                                             HoodieHeartbeatClient 
heartbeatClient, HoodieWriteConfig config) {
+    ValidationUtils.checkArgument(heartbeatClient != null);
+    try {
+      if (config.getFailedWritesCleanPolicy().isLazy() && 
heartbeatClient.isHeartbeatExpired(instantTime)) {
+        throw new HoodieException("Heartbeat for instant " + instantTime + " 
has expired, last heartbeat "

Review comment:
       this deserves its own Exception class. 

##########
File path: 
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/heartbeat/HeartbeatUtils.java
##########
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.heartbeat;
+
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.common.util.ValidationUtils;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.exception.HoodieException;
+import org.apache.hudi.table.HoodieTable;
+import org.apache.log4j.LogManager;
+import org.apache.log4j.Logger;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Helper class to delete heartbeat for completed or failed instants with 
expired heartbeats.
+ */
+public class HeartbeatUtils {

Review comment:
       to our discussion today, can you add a `TestHeartbeatUtils` in a 
subsequent PR? 

##########
File path: 
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/heartbeat/HoodieHeartbeatClient.java
##########
@@ -0,0 +1,289 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.heartbeat;
+
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.util.ValidationUtils;
+import org.apache.hudi.exception.HoodieException;
+import org.apache.hudi.exception.HoodieHeartbeatException;
+import org.apache.log4j.LogManager;
+import org.apache.log4j.Logger;
+
+import javax.annotation.concurrent.NotThreadSafe;
+import java.io.File;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.Serializable;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.Timer;
+import java.util.TimerTask;
+
+/**
+ * This class creates heartbeat for hudi client. This heartbeat is used to 
ascertain whether the running job is or not.
+ * NOTE: Due to CPU contention on the driver/client node, the heartbeats could 
be delayed, hence it's important to set
+ *       the value high enough to avoid that possibility.
+ */
+@NotThreadSafe
+public class HoodieHeartbeatClient implements AutoCloseable, Serializable {
+
+  private static final Logger LOG = 
LogManager.getLogger(HoodieHeartbeatClient.class);
+
+  private final transient FileSystem fs;
+  private final String basePath;
+  // path to the heartbeat folder where all writers are updating their 
heartbeats
+  private String heartbeatFolderPath;
+  // heartbeat interval in millis
+  private final Long heartbeatIntervalInMs;
+  private Integer numTolerableHeartbeatMisses;
+  private final Long maxAllowableHeartbeatIntervalInMs;
+  private Map<String, Heartbeat> instantToHeartbeatMap;
+
+  public HoodieHeartbeatClient(FileSystem fs, String basePath, Long 
heartbeatIntervalInMs,
+                               Integer numTolerableHeartbeatMisses) {
+    ValidationUtils.checkArgument(heartbeatIntervalInMs >= 1000, "Cannot set 
heartbeat lower than 1 second");
+    this.fs = fs;
+    this.basePath = basePath;
+    this.heartbeatFolderPath = 
HoodieTableMetaClient.getHeartbeatFolderPath(basePath);
+    this.heartbeatIntervalInMs = heartbeatIntervalInMs;
+    this.numTolerableHeartbeatMisses = numTolerableHeartbeatMisses;
+    this.maxAllowableHeartbeatIntervalInMs = this.heartbeatIntervalInMs * 
this.numTolerableHeartbeatMisses;
+    this.instantToHeartbeatMap = new HashMap<>();
+  }
+
+  class Heartbeat {
+
+    private String instantTime;
+    private Boolean isHeartbeatStarted = false;
+    private Boolean isHeartbeatStopped = false;
+    private Long lastHeartbeatTime;
+    private Integer numHeartbeats = 0;
+    private Timer timer = new Timer();
+
+    public String getInstantTime() {
+      return instantTime;
+    }
+
+    public void setInstantTime(String instantTime) {
+      this.instantTime = instantTime;
+    }
+
+    public Boolean isHeartbeatStarted() {
+      return isHeartbeatStarted;
+    }
+
+    public void setHeartbeatStarted(Boolean heartbeatStarted) {
+      isHeartbeatStarted = heartbeatStarted;
+    }
+
+    public Boolean isHeartbeatStopped() {
+      return isHeartbeatStopped;
+    }
+
+    public void setHeartbeatStopped(Boolean heartbeatStopped) {
+      isHeartbeatStopped = heartbeatStopped;
+    }
+
+    public Long getLastHeartbeatTime() {
+      return lastHeartbeatTime;
+    }
+
+    public void setLastHeartbeatTime(Long lastHeartbeatTime) {
+      this.lastHeartbeatTime = lastHeartbeatTime;
+    }
+
+    public Integer getNumHeartbeats() {
+      return numHeartbeats;
+    }
+
+    public void setNumHeartbeats(Integer numHeartbeats) {
+      this.numHeartbeats = numHeartbeats;
+    }
+
+    public Timer getTimer() {
+      return timer;
+    }
+
+    public void setTimer(Timer timer) {
+      this.timer = timer;
+    }
+
+    @Override
+    public String toString() {
+      return "Heartbeat{"
+              + "instantTime='" + instantTime + '\''
+              + ", isHeartbeatStarted=" + isHeartbeatStarted
+              + ", isHeartbeatStopped=" + isHeartbeatStopped
+              + ", lastHeartbeatTime=" + lastHeartbeatTime
+              + ", numHeartbeats=" + numHeartbeats
+              + ", timer=" + timer
+              + '}';
+    }
+  }
+
+  class HeartbeatTask extends TimerTask {
+
+    private final String instantTime;
+
+    HeartbeatTask(String instantTime) {
+      this.instantTime = instantTime;
+    }
+
+    @Override
+    public void run() {
+      updateHeartbeat(instantTime);
+    }
+  }
+
+  /**
+   * Start a new heartbeat for the specified instant. If there is already one 
running, this will be a NO_OP
+   * @param instantTime
+   */
+  public void start(String instantTime) {
+    LOG.info("Received request to start heartbeat for instant time " + 
instantTime);
+    Heartbeat heartbeat = instantToHeartbeatMap.get(instantTime);
+    ValidationUtils.checkArgument(heartbeat == null || 
!heartbeat.isHeartbeatStopped(), "Cannot restart a stopped heartbeat for " + 
instantTime);
+    if (heartbeat != null && heartbeat.isHeartbeatStarted()) {

Review comment:
       invert the condition?  and avoid the `return ;` 

##########
File path: 
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/AbstractHoodieWriteClient.java
##########
@@ -181,7 +171,7 @@ public boolean commitStats(String instantTime, 
List<HoodieWriteStat> stats, Opti
     HoodieCommitMetadata metadata = CommitUtils.buildMetadata(stats, 
partitionToReplaceFileIds, extraMetadata, operationType, config.getSchema(), 
commitActionType);
     // Finalize write
     finalizeWrite(table, instantTime, stats);
-
+    HeartbeatUtils.abortIfHeartbeatExpired(instantTime, table, 
heartbeatClient, config);

Review comment:
       as long as the exception is clear, throwing runtime exceptions is 
actually better. (source: clean code book) 

##########
File path: 
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/heartbeat/HoodieHeartbeatClient.java
##########
@@ -0,0 +1,289 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.heartbeat;
+
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.util.ValidationUtils;
+import org.apache.hudi.exception.HoodieException;
+import org.apache.hudi.exception.HoodieHeartbeatException;
+import org.apache.log4j.LogManager;
+import org.apache.log4j.Logger;
+
+import javax.annotation.concurrent.NotThreadSafe;
+import java.io.File;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.Serializable;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.Timer;
+import java.util.TimerTask;
+
+/**
+ * This class creates heartbeat for hudi client. This heartbeat is used to 
ascertain whether the running job is or not.
+ * NOTE: Due to CPU contention on the driver/client node, the heartbeats could 
be delayed, hence it's important to set
+ *       the value high enough to avoid that possibility.
+ */
+@NotThreadSafe
+public class HoodieHeartbeatClient implements AutoCloseable, Serializable {
+
+  private static final Logger LOG = 
LogManager.getLogger(HoodieHeartbeatClient.class);
+
+  private final transient FileSystem fs;
+  private final String basePath;
+  // path to the heartbeat folder where all writers are updating their 
heartbeats
+  private String heartbeatFolderPath;
+  // heartbeat interval in millis
+  private final Long heartbeatIntervalInMs;
+  private Integer numTolerableHeartbeatMisses;
+  private final Long maxAllowableHeartbeatIntervalInMs;
+  private Map<String, Heartbeat> instantToHeartbeatMap;
+
+  public HoodieHeartbeatClient(FileSystem fs, String basePath, Long 
heartbeatIntervalInMs,
+                               Integer numTolerableHeartbeatMisses) {
+    ValidationUtils.checkArgument(heartbeatIntervalInMs >= 1000, "Cannot set 
heartbeat lower than 1 second");
+    this.fs = fs;
+    this.basePath = basePath;
+    this.heartbeatFolderPath = 
HoodieTableMetaClient.getHeartbeatFolderPath(basePath);
+    this.heartbeatIntervalInMs = heartbeatIntervalInMs;
+    this.numTolerableHeartbeatMisses = numTolerableHeartbeatMisses;
+    this.maxAllowableHeartbeatIntervalInMs = this.heartbeatIntervalInMs * 
this.numTolerableHeartbeatMisses;
+    this.instantToHeartbeatMap = new HashMap<>();
+  }
+
+  class Heartbeat {
+
+    private String instantTime;
+    private Boolean isHeartbeatStarted = false;
+    private Boolean isHeartbeatStopped = false;
+    private Long lastHeartbeatTime;
+    private Integer numHeartbeats = 0;
+    private Timer timer = new Timer();
+
+    public String getInstantTime() {
+      return instantTime;
+    }
+
+    public void setInstantTime(String instantTime) {
+      this.instantTime = instantTime;
+    }
+
+    public Boolean isHeartbeatStarted() {
+      return isHeartbeatStarted;
+    }
+
+    public void setHeartbeatStarted(Boolean heartbeatStarted) {
+      isHeartbeatStarted = heartbeatStarted;
+    }
+
+    public Boolean isHeartbeatStopped() {
+      return isHeartbeatStopped;
+    }
+
+    public void setHeartbeatStopped(Boolean heartbeatStopped) {
+      isHeartbeatStopped = heartbeatStopped;
+    }
+
+    public Long getLastHeartbeatTime() {
+      return lastHeartbeatTime;
+    }
+
+    public void setLastHeartbeatTime(Long lastHeartbeatTime) {
+      this.lastHeartbeatTime = lastHeartbeatTime;
+    }
+
+    public Integer getNumHeartbeats() {
+      return numHeartbeats;
+    }
+
+    public void setNumHeartbeats(Integer numHeartbeats) {
+      this.numHeartbeats = numHeartbeats;
+    }
+
+    public Timer getTimer() {
+      return timer;
+    }
+
+    public void setTimer(Timer timer) {
+      this.timer = timer;
+    }
+
+    @Override
+    public String toString() {
+      return "Heartbeat{"
+              + "instantTime='" + instantTime + '\''
+              + ", isHeartbeatStarted=" + isHeartbeatStarted
+              + ", isHeartbeatStopped=" + isHeartbeatStopped
+              + ", lastHeartbeatTime=" + lastHeartbeatTime
+              + ", numHeartbeats=" + numHeartbeats
+              + ", timer=" + timer
+              + '}';
+    }
+  }
+
+  class HeartbeatTask extends TimerTask {
+
+    private final String instantTime;
+
+    HeartbeatTask(String instantTime) {
+      this.instantTime = instantTime;
+    }
+
+    @Override
+    public void run() {
+      updateHeartbeat(instantTime);
+    }
+  }
+
+  /**
+   * Start a new heartbeat for the specified instant. If there is already one 
running, this will be a NO_OP
+   * @param instantTime
+   */
+  public void start(String instantTime) {
+    LOG.info("Received request to start heartbeat for instant time " + 
instantTime);
+    Heartbeat heartbeat = instantToHeartbeatMap.get(instantTime);
+    ValidationUtils.checkArgument(heartbeat == null || 
!heartbeat.isHeartbeatStopped(), "Cannot restart a stopped heartbeat for " + 
instantTime);
+    if (heartbeat != null && heartbeat.isHeartbeatStarted()) {
+      // heartbeat already started, NO_OP
+      return;
+    } else {
+      Heartbeat newHeartbeat = new Heartbeat();
+      newHeartbeat.setHeartbeatStarted(true);

Review comment:
       shuld the `Heartbeat()` constructor do this?

##########
File path: 
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/heartbeat/HeartbeatUtils.java
##########
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client.heartbeat;
+
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.common.util.ValidationUtils;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.exception.HoodieException;
+import org.apache.hudi.table.HoodieTable;
+import org.apache.log4j.LogManager;
+import org.apache.log4j.Logger;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Helper class to delete heartbeat for completed or failed instants with 
expired heartbeats.
+ */
+public class HeartbeatUtils {
+
+  private static final Logger LOG = LogManager.getLogger(HeartbeatUtils.class);
+
+  /**
+   * Deletes the heartbeat file for the specified instant.
+   * @param fs
+   * @param basePath
+   * @param instantTime
+   * @return
+   */
+  public static boolean deleteHeartbeatFile(FileSystem fs, String basePath, 
String instantTime) {
+    boolean deleted = false;
+    try {
+      String heartbeatFolderPath = 
HoodieTableMetaClient.getHeartbeatFolderPath(basePath);
+      deleted = fs.delete(new Path(heartbeatFolderPath + File.separator + 
instantTime), false);
+      if (!deleted) {
+        LOG.error("Failed to delete heartbeat for instant " + instantTime);
+      }
+    } catch (IOException io) {
+      LOG.error("Unable to delete heartbeat for instant " + instantTime, io);
+    }
+    return deleted;
+  }
+
+  /**
+   * Deletes the heartbeat files for instants with expired heartbeats without 
any active instant.
+   * @param allExistingHeartbeatInstants
+   * @param metaClient
+   * @param basePath
+   */
+  public static void cleanExpiredHeartbeats(List<String> 
allExistingHeartbeatInstants,
+                                            HoodieTableMetaClient metaClient, 
String basePath) {
+    Set<String> nonExpiredHeartbeatInstants = metaClient.getActiveTimeline()
+        
.filterCompletedInstants().getInstants().map(HoodieInstant::getTimestamp).collect(Collectors.toSet());
+    allExistingHeartbeatInstants.stream().forEach(instant -> {
+      if (!nonExpiredHeartbeatInstants.contains(instant)) {
+        deleteHeartbeatFile(metaClient.getFs(), basePath, instant);
+      }
+    });
+  }
+
+  /**
+   * Check if the heartbeat corresponding to instantTime has expired. If yes, 
abort by throwing an exception.
+   * @param instantTime
+   * @param table
+   * @param heartbeatClient
+   * @param config
+   */
+  public static void abortIfHeartbeatExpired(String instantTime, HoodieTable 
table,
+                                             HoodieHeartbeatClient 
heartbeatClient, HoodieWriteConfig config) {
+    ValidationUtils.checkArgument(heartbeatClient != null);
+    try {
+      if (config.getFailedWritesCleanPolicy().isLazy() && 
heartbeatClient.isHeartbeatExpired(instantTime)) {
+        throw new HoodieException("Heartbeat for instant " + instantTime + " 
has expired, last heartbeat "
+            + 
heartbeatClient.getLastHeartbeatTime(table.getMetaClient().getFs(), 
config.getBasePath(), instantTime));
+      }
+    } catch (IOException io) {
+      throw new HoodieException("Unable to read heartbeat", io);

Review comment:
       Pleas throw `HoodieIOException` whenever there is an IOException being 
rethrown

##########
File path: 
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/AbstractHoodieWriteClient.java
##########
@@ -749,22 +739,49 @@ private HoodieTimeline 
getInflightTimelineExcludeCompactionAndClustering(HoodieT
   }
 
   /**
-   * Cleanup all pending commits.
+   * Rollback all failed writes.
    */
-  private void rollbackPendingCommits() {
+  public Boolean rollbackFailedWrites() {

Review comment:
       to @nsivabalan 's point. can't the return type be void and use `Void` 
for the function. 

##########
File path: 
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java
##########
@@ -71,10 +72,10 @@
   private static final long serialVersionUID = 1L;
   private static final Logger LOG = 
LogManager.getLogger(HoodieTableMetaClient.class);
   public static final String METAFOLDER_NAME = ".hoodie";
-  public static final String TEMPFOLDER_NAME = METAFOLDER_NAME + 
Path.SEPARATOR + ".temp";
-  public static final String AUXILIARYFOLDER_NAME = METAFOLDER_NAME + 
Path.SEPARATOR + ".aux";
-  public static final String BOOTSTRAP_INDEX_ROOT_FOLDER_PATH = 
AUXILIARYFOLDER_NAME + Path.SEPARATOR + ".bootstrap";
-
+  public static final String TEMPFOLDER_NAME = METAFOLDER_NAME + 
File.separator + ".temp";

Review comment:
       why this change?

##########
File path: 
hudi-common/src/main/java/org/apache/hudi/common/table/log/AbstractHoodieLogRecordScanner.java
##########
@@ -145,6 +148,14 @@ public void scan() {
           // hit a block with instant time greater than should be processed, 
stop processing further
           break;
         }
+        if (r.getBlockType() != CORRUPT_BLOCK && r.getBlockType() != 
COMMAND_BLOCK) {

Review comment:
       test case for this?
   

##########
File path: 
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/io/TestHoodieTimelineArchiveLog.java
##########
@@ -408,11 +408,11 @@ public void testArchiveCommitCompactionNoHole() throws 
IOException {
     HoodieTable table = HoodieSparkTable.create(cfg, context, metaClient);
     HoodieTimelineArchiveLog archiveLog = new HoodieTimelineArchiveLog(cfg, 
table);
 
-    HoodieTimeline timeline = 
metaClient.getActiveTimeline().getCommitsAndCompactionTimeline();
+    HoodieTimeline timeline = 
metaClient.getActiveTimeline().getWriteTimeline();

Review comment:
       can we add a test case for archiving with inflight instants and lazy 
cleaning?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to