vinothchandar commented on a change in pull request #2359:
URL: https://github.com/apache/hudi/pull/2359#discussion_r549549304
##########
File path:
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/HoodieTimelineArchiveLog.java
##########
@@ -180,7 +186,14 @@ public boolean archiveIfRequired(HoodieEngineContext
context) throws IOException
return oldestPendingCompactionInstant
.map(instant ->
HoodieTimeline.compareTimestamps(instant.getTimestamp(), GREATER_THAN,
s.getTimestamp()))
.orElse(true);
- }).limit(commitTimeline.countInstants() - minInstantsToKeep);
+ });
+ if (config.getFailedWritesCleanPolicy() ==
HoodieCleaningPolicy.HoodieFailedWritesCleaningPolicy.LAZY
+ && config.getMultiWriterConcurrencyModel() !=
MultiWriterConcurrencyModel.LEGACY_SINGLE_WRITER) {
Review comment:
do we need to guard this by `config.getMultiWriterConcurrencyModel() !=
MultiWriterConcurrencyModel.LEGACY_SINGLE_WRITER` ? it should technically work
right? if so, best to keep just the one flag to fence
##########
File path:
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/BaseRollbackActionExecutor.java
##########
@@ -130,23 +131,29 @@ private void validateSavepointRollbacks() {
}
private void validateRollbackCommitSequence() {
- final String instantTimeToRollback = instantToRollback.getTimestamp();
- HoodieTimeline commitTimeline = table.getCompletedCommitsTimeline();
- HoodieTimeline inflightAndRequestedCommitTimeline =
table.getPendingCommitTimeline();
- // Make sure only the last n commits are being rolled back
- // If there is a commit in-between or after that is not rolled back, then
abort
- if ((instantTimeToRollback != null) && !commitTimeline.empty()
- && !commitTimeline.findInstantsAfter(instantTimeToRollback,
Integer.MAX_VALUE).empty()) {
- throw new HoodieRollbackException(
- "Found commits after time :" + instantTimeToRollback + ", please
rollback greater commits first");
- }
+ // Continue to provide the same behavior if policy is EAGER (similar to
pendingRollback logic). This is required
+ // since with LAZY rollback we support parallel writing which can allow a
new inflight while rollback is ongoing
+ // Remove this once we support LAZY rollback of failed writes by default
as parallel writing becomes the default
+ // writer mode.
+ if (config.getFailedWritesCleanPolicy() ==
HoodieCleaningPolicy.HoodieFailedWritesCleaningPolicy.EAGER) {
Review comment:
just return if the policy is not EAGER? easier to read that, than
nested if blocks .
##########
File path:
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/table/action/clean/SparkCleanActionExecutor.java
##########
@@ -23,9 +23,9 @@
import org.apache.hudi.avro.model.HoodieActionInstant;
import org.apache.hudi.avro.model.HoodieCleanerPlan;
import org.apache.hudi.client.WriteStatus;
-import org.apache.hudi.common.HoodieCleanStat;
import org.apache.hudi.client.common.HoodieEngineContext;
import org.apache.hudi.client.common.HoodieSparkEngineContext;
+import org.apache.hudi.common.HoodieCleanStat;
import org.apache.hudi.common.model.CleanFileInfo;
import org.apache.hudi.common.model.HoodieKey;
import org.apache.hudi.common.model.HoodieRecord;
Review comment:
I was expecting some changes in this file, to clean the failed writers
for LAZY? Can you point me to the files that do this.
##########
File path:
hudi-common/src/test/java/org/apache/hudi/common/testutils/FileCreateUtils.java
##########
@@ -83,6 +83,17 @@ private static void createMetaFile(String basePath, String
instantTime, String s
}
}
+ private static void createMetaFile(FileSystem fs, String basePath, String
instantTime, String suffix) throws IOException {
Review comment:
+1
----------------------------------------------------------------
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]
