yihua commented on code in PR #12888:
URL: https://github.com/apache/hudi/pull/12888#discussion_r1981939975
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/ListingBasedRollbackStrategy.java:
##########
@@ -164,7 +174,9 @@ public List<HoodieRollbackRequest>
getRollbackRequests(HoodieInstant instantToRo
// We do not know fileIds for inserts (first inserts are either
log files or base files),
// delete all files for the corresponding failed commit, if
present (same as COW)
hoodieRollbackRequests.addAll(getHoodieRollbackRequests(partitionPath,
filesToDelete.get()));
-
+ if
(metaClient.getTableConfig().getTableVersion().lesserThan(HoodieTableVersion.EIGHT))
{
+
hoodieRollbackRequests.addAll(getRollbackRequestToAppendForVersionSix(partitionPath,
instantToRollback, commitMetadataOptional.get(), table));
+ }
Review Comment:
Can we have the diverging logic within `getHoodieRollbackRequests` instead
of polluting the core logic here? Does other place calling
`getHoodieRollbackRequests` need revisiting too?
##########
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieReaderConfig.java:
##########
@@ -89,4 +95,18 @@ public class HoodieReaderConfig extends HoodieConfig {
"hoodie.write.record.merge.custom.implementation.classes";
public static final String
RECORD_MERGE_IMPL_CLASSES_DEPRECATED_WRITE_CONFIG_KEY =
"hoodie.datasource.write.record.merger.impls";
+
+ public static boolean isFileGroupReaderEnabled(HoodieTableVersion
tableVersion, HoodieConfig config) {
+ return tableVersion.greaterThanOrEquals(HoodieTableVersion.EIGHT) &&
config.getBooleanOrDefault(HoodieReaderConfig.FILE_GROUP_READER_ENABLED);
Review Comment:
We should not differentiate the reader logic at this layer. If there are
gaps regarding the file group reader reading the base and log files written in
table version 6, let's fix that within the file group reader. Otherwise, we'll
never deprecate the legacy log scanner and reader path.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/compact/ScheduleCompactionActionExecutor.java:
##########
@@ -109,9 +109,6 @@ public Option<HoodieCompactionPlan> execute() {
.getWriteTimeline().filterCompletedAndCompactionInstants().getInstantsAsStream()
.filter(instant -> compareTimestamps(instant.requestedTime(),
GREATER_THAN_OR_EQUALS, instantTime))
.collect(Collectors.toList());
- ValidationUtils.checkArgument(conflictingInstants.isEmpty(),
Review Comment:
Do we need this for backwards compatible compaction for table version 6?
--
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]
