yihua commented on code in PR #7476:
URL: https://github.com/apache/hudi/pull/7476#discussion_r1089544404


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -271,15 +272,15 @@ public class HoodieWriteConfig extends HoodieConfig {
       .defaultValue(String.valueOf(4 * 1024 * 1024))
       .withDocumentation("Size of in-memory buffer used for parallelizing 
network reads and lake storage writes.");
 
-  public static final ConfigProperty<String> WRITE_DISRUPTOR_BUFFER_SIZE = 
ConfigProperty
+  public static final ConfigProperty<String> 
WRITE_EXECUTOR_DISRUPTOR_BUFFER_SIZE = ConfigProperty

Review Comment:
   (Let's take this in a separate PR) What is the unit of this config (B, KB, 
or MB)? We should mention the unit in the config naming.



##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/execution/TestSimpleExecutionInSpark.java:
##########
@@ -156,10 +148,10 @@ public Integer finish() {
           }
         };
 
-    SimpleHoodieExecutor<HoodieRecord, Tuple2<HoodieRecord, 
Option<IndexedRecord>>, Integer> exec = null;
+    SimpleExecutor<HoodieRecord, HoodieRecord, Integer> exec = null;
 
     try {
-      exec = new SimpleHoodieExecutor(hoodieRecords.iterator(), consumer, 
getCloningTransformer(HoodieTestDataGenerator.AVRO_SCHEMA));
+      exec = new SimpleExecutor<>(hoodieRecords.iterator(), consumer, 
Function.identity());

Review Comment:
   similar here



##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/execution/TestDisruptorExecutionInSpark.java:
##########
@@ -143,9 +139,9 @@ public Integer finish() {
           }
         };
 
-    DisruptorExecutor<HoodieRecord, Tuple2<HoodieRecord, 
Option<IndexedRecord>>, Integer>
-        executor = new 
DisruptorExecutor(hoodieWriteConfig.getDisruptorWriteBufferSize(), 
hoodieRecords.iterator(), consumer,
-        getCloningTransformer(HoodieTestDataGenerator.AVRO_SCHEMA), 
Option.of(WaitStrategyFactory.DEFAULT_STRATEGY), getPreExecuteRunnable());
+    DisruptorExecutor<HoodieRecord, HoodieRecord, Integer>
+        executor = new DisruptorExecutor<>(1024, hoodieRecords.iterator(), 
consumer,
+        Function.identity(), WaitStrategyFactory.DEFAULT_STRATEGY, 
getPreExecuteRunnable());

Review Comment:
   similar here



##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -2857,8 +2863,15 @@ private void validate() {
     }
 
     public HoodieWriteConfig build() {
+      return build(true);
+    }
+
+    @VisibleForTesting
+    public HoodieWriteConfig build(boolean shouldValidate) {

Review Comment:
   OK.  We should think about how to hide this, so it is not used by users 
accidentally.



##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/execution/TestDisruptorExecutionInSpark.java:
##########
@@ -93,11 +91,11 @@ public Integer finish() {
             return count;
           }
         };
-    DisruptorExecutor<HoodieRecord, Tuple2<HoodieRecord, 
Option<IndexedRecord>>, Integer> exec = null;
+    DisruptorExecutor<HoodieRecord, HoodieRecord, Integer> exec = null;
 
     try {
-      exec = new 
DisruptorExecutor(hoodieWriteConfig.getDisruptorWriteBufferSize(), 
hoodieRecords.iterator(), consumer,
-          getCloningTransformer(HoodieTestDataGenerator.AVRO_SCHEMA), 
Option.of(WaitStrategyFactory.DEFAULT_STRATEGY), getPreExecuteRunnable());
+      exec = new 
DisruptorExecutor<>(writeConfig.getWriteExecutorDisruptorWriteBufferSize(), 
hoodieRecords.iterator(), consumer,
+          Function.identity(), WaitStrategyFactory.DEFAULT_STRATEGY, 
getPreExecuteRunnable());

Review Comment:
   Should this also use `getTransformer(HoodieTestDataGenerator.AVRO_SCHEMA, 
writeConfig)`?



##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/execution/TestSimpleExecutionInSpark.java:
##########
@@ -203,8 +194,8 @@ public Integer finish() {
           }
         };
 
-    SimpleHoodieExecutor<HoodieRecord, Tuple2<HoodieRecord, 
Option<IndexedRecord>>, Integer> exec =
-        new SimpleHoodieExecutor(iterator, consumer, 
getCloningTransformer(HoodieTestDataGenerator.AVRO_SCHEMA));
+    SimpleExecutor<HoodieRecord, HoodieRecord, Integer> exec =
+        new SimpleExecutor<>(iterator, consumer, Function.identity());

Review Comment:
   similar here



##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/execution/TestSimpleExecutionInSpark.java:
##########
@@ -88,10 +80,10 @@ public Integer finish() {
             return count;
           }
         };
-    SimpleHoodieExecutor<HoodieRecord, Tuple2<HoodieRecord, 
Option<IndexedRecord>>, Integer> exec = null;
+    SimpleExecutor<HoodieRecord, HoodieRecord, Integer> exec = null;
 
     try {
-      exec = new SimpleHoodieExecutor(hoodieRecords.iterator(), consumer, 
getCloningTransformer(HoodieTestDataGenerator.AVRO_SCHEMA));
+      exec = new SimpleExecutor<>(hoodieRecords.iterator(), consumer, 
Function.identity());

Review Comment:
   Similar here for `getTransformer`.  The production code passes the transfer 
in by calling `getTransformer` so let's follow the same in the test code.



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

Reply via email to