nsivabalan commented on code in PR #11553:
URL: https://github.com/apache/hudi/pull/11553#discussion_r1667079266
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieTableServiceClient.java:
##########
@@ -880,8 +872,7 @@ protected Map<String, Option<HoodiePendingRollbackInfo>>
getPendingRollbackInfos
String instantToRollback =
rollbackPlan.getInstantToRollback().getCommitTime();
if (ignoreCompactionAndClusteringInstants) {
if (!HoodieTimeline.COMPACTION_ACTION.equals(action)) {
- boolean isClustering =
HoodieTimeline.REPLACE_COMMIT_ACTION.equals(action)
- && ClusteringUtils.getClusteringPlan(metaClient, new
HoodieInstant(true, action, instantToRollback)).isPresent();
+ boolean isClustering =
ClusteringUtils.isClusteringInstant(metaClient.getActiveTimeline(), new
HoodieInstant(true, action, instantToRollback));
Review Comment:
again, if its inflight instant that we are checking, action type would
suffice.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieTableServiceClient.java:
##########
@@ -821,16 +821,8 @@ protected void archive(HoodieTable table) {
* @return
*/
private HoodieTimeline
getInflightTimelineExcludeCompactionAndClustering(HoodieTableMetaClient
metaClient) {
- HoodieTimeline inflightTimelineWithReplaceCommit =
metaClient.getCommitsTimeline().filterPendingExcludingCompaction();
- HoodieTimeline inflightTimelineExcludeClusteringCommit =
inflightTimelineWithReplaceCommit.filter(instant -> {
- if (instant.getAction().equals(HoodieTimeline.REPLACE_COMMIT_ACTION)) {
- Option<Pair<HoodieInstant, HoodieClusteringPlan>> instantPlan =
ClusteringUtils.getClusteringPlan(metaClient, instant);
- return !instantPlan.isPresent();
- } else {
- return true;
- }
- });
- return inflightTimelineExcludeClusteringCommit;
+ HoodieTimeline inflightTimelineExcludingCompaction =
metaClient.getCommitsTimeline().filterPendingExcludingCompaction();
+ return inflightTimelineExcludingCompaction.filter(instant ->
!ClusteringUtils.isClusteringInstant(inflightTimelineExcludingCompaction,
instant));
Review Comment:
won't this be simplified w/ this patch.
inflightTimelineExcludingCompaction will only include pending instants and
not compaction.
so, we might as well just filter out CLUSTER_ACTION right? we do not need to
deser the requested to find if its clustering.
infact thats one of the major wins w/ this change.
##########
hudi-cli/src/main/java/org/apache/hudi/cli/commands/ArchivedCommitsCommand.java:
##########
@@ -241,6 +241,7 @@ private Comparable[] readCommit(GenericRecord record,
boolean skipMetadata) {
case HoodieTimeline.COMPACTION_ACTION:
return commitDetail(record, "hoodieCompactionMetadata", skipMetadata);
case HoodieTimeline.REPLACE_COMMIT_ACTION:
+ case HoodieTimeline.CLUSTER_ACTION:
Review Comment:
I would go with "CLUSTERING_ACTION", since we named "COMPACTION_ACTION" and
not "COMPACT_ACTION".
also, verb makes sense for timeline apis
getPendingCompactionTimeline vs getPendingCompactTimeline
getPendingClusteringTimeline vs getPendingClusterTimeline
--
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]
