[GitHub] [hudi] suryaprasanna commented on a diff in pull request #8829: [HUDI-6277][UBER] Clustering enhancements

["suryaprasanna (via GitHub)" <gi...@apache.org>]

suryaprasanna commented on code in PR #8829:
URL: https://github.com/apache/hudi/pull/8829#discussion_r1210969971


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/execution/HoodieLazyInsertIterable.java:
##########
@@ -90,12 +90,16 @@ public R getResult() {
     }
   }
 
-  static <T> Function<HoodieRecord<T>, HoodieInsertValueGenResult<HoodieRecord>> getTransformer(Schema schema,
+  /**
+   * Transformer function to help transform a HoodieRecord. This transformer is used by BufferedIterator to offload some
+   * expensive operations of transformation to the reader thread.
+   */
+  public <T> Function<HoodieRecord<T>, HoodieInsertValueGenResult<HoodieRecord>> getTransformer(Schema schema,
                                                                                                 HoodieWriteConfig writeConfig) {

Review Comment:
   In the runtime, we are providing a class, that transforms the record. The logic resides in other repository, so, we need this method to be extensible.



##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/client/validator/SparkPreCommitValidator.java:
##########
@@ -73,7 +79,23 @@ public void validate(String instantTime, HoodieWriteMetadata<O> writeResult, Dat
     try {
       validateRecordsBeforeAndAfter(before, after, getPartitionsModified(writeResult));
     } finally {
-      LOG.info(getClass() + " validator took " + timer.endTimer() + " ms");
+      long duration = timer.endTimer();
+      LOG.info(getClass() + " validator took " + duration + " ms" + ", metrics on? " + getWriteConfig().isMetricsOn());
+      publishRunStats(instantTime, duration);
+    }
+  }
+
+  /**
+   * Publish pre-commit validator run stats for a given commit action.
+   */
+  protected void publishRunStats(String instantTime, long duration) {
+    // Record validator duration metrics.

Review Comment:
   Thanks for catching this, updated the scope to private.



##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/HoodieMetrics.java:
##########
@@ -274,6 +274,17 @@ String getMetricsName(String action, String metric) {
     return config == null ? null : String.format("%s.%s.%s", config.getMetricReporterMetricsNamePrefix(), action, metric);
   }
 
+  public void updateClusteringFileCreationMetrics(long durationInMs) {
+    metrics.registerGauge(getMetricsName("replacecommit", "fileCreationTime"), durationInMs);
+  }
+
+  /**
+   * Given a commit action, metrics name and value this method publishes custom metrics.
+   */
+  public void publishMetrics(String commitAction, String metricsName, long value) {

Review Comment:
   Thanks for catching this, changed the param and method name.



##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/HoodieMetrics.java:
##########
@@ -274,6 +274,17 @@ String getMetricsName(String action, String metric) {
     return config == null ? null : String.format("%s.%s.%s", config.getMetricReporterMetricsNamePrefix(), action, metric);
   }
 
+  public void updateClusteringFileCreationMetrics(long durationInMs) {
+    metrics.registerGauge(getMetricsName("replacecommit", "fileCreationTime"), durationInMs);

Review Comment:
   Done the change to redirect the call.



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