[GitHub] [hudi] xushiyan commented on a diff in pull request #7037: [HUDI-5078] Fixing determination of table service for metadata calls

[GitBox <gi...@apache.org>]

xushiyan commented on code in PR #7037:
URL: https://github.com/apache/hudi/pull/7037#discussion_r1014078055


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/HoodieTable.java:
##########
@@ -858,10 +860,22 @@ public final Option<HoodieTableMetadataWriter> getMetadataWriter(String triggeri
 
   /**
    * Check if action type is a table service.
-   * @param actionType action type of interest.
+   * @param instant instant of interest
    * @return true if action represents a table service. false otherwise.
    */
-  public abstract boolean isTableServiceAction(String actionType);
+  public boolean isTableServiceAction(HoodieInstant instant) {
+    if (instant.getAction().equals(HoodieTimeline.REPLACE_COMMIT_ACTION)) {
+      Option<Pair<HoodieInstant, HoodieClusteringPlan>> instantPlan = ClusteringUtils.getClusteringPlan(getMetaClient(), instant);

Review Comment:
   ```suggestion
         Option<Pair<HoodieInstant, HoodieClusteringPlan>> instantPlan = ClusteringUtils.getClusteringPlan(metaClient, instant);
   ```



##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/BaseActionExecutor.java:
##########
@@ -58,7 +59,7 @@ public BaseActionExecutor(HoodieEngineContext context, HoodieWriteConfig config,
    */
   protected final void writeTableMetadata(HoodieCommitMetadata metadata, String actionType) {
     table.getMetadataWriter(instantTime).ifPresent(w -> w.update(
-        metadata, instantTime, table.isTableServiceAction(actionType)));
+        metadata, instantTime, table.isTableServiceAction(new HoodieInstant(HoodieInstant.State.INFLIGHT, actionType, instantTime))));

Review Comment:
   not a big fan of creating dummy instant to cater the API. it's the API that needs to fit better with the caller info.



##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java:
##########
@@ -656,7 +659,9 @@ private List<DirectoryInfo> listAllPartitions(HoodieTableMetaClient datasetMetaC
           partitionsToBootstrap.add(dirInfo);
         } else {
           // Add sub-dirs to the queue
-          pathsToList.addAll(dirInfo.getSubDirectories());
+          pathsToList.addAll(dirInfo.getSubDirectories().stream()
+              .map(path -> new SerializablePath(new CachingPath(path.toUri())))
+              .collect(Collectors.toList()));
         }

Review Comment:
   unrelated changes?



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

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