You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2022/03/11 11:59:33 UTC

[GitHub] [hudi] vinothchandar commented on a change in pull request #4264: [HUDI-2875] Make HoodieParquetWriter Thread safe and memory executor …

vinothchandar commented on a change in pull request #4264:
URL: https://github.com/apache/hudi/pull/4264#discussion_r824648413



##########
File path: hudi-common/src/main/java/org/apache/hudi/common/util/queue/BoundedInMemoryExecutor.java
##########
@@ -47,7 +48,7 @@
 public class BoundedInMemoryExecutor<I, O, E> {
 
   private static final Logger LOG = LogManager.getLogger(BoundedInMemoryExecutor.class);
-
+  private static final long TERMINATE_WAITING_TIME = 60L;

Review comment:
       rename: TERMINATE_TIMEOUT_SECS ?

##########
File path: hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/table/action/bootstrap/ParquetBootstrapMetadataHandler.java
##########
@@ -80,14 +80,15 @@ void executeBootstrap(HoodieBootstrapHandle<?, ?, ?, ?> bootstrapHandle,
         HoodieRecord rec = new HoodieAvroRecord(new HoodieKey(recKey, partitionPath), payload);
         return rec;
       }, table.getPreExecuteRunnable());
-      wrapper.execute();
+      executor.execute();
     } catch (Exception e) {
       throw new HoodieException(e);
     } finally {
-      bootstrapHandle.close();
-      if (null != wrapper) {
-        wrapper.shutdownNow();
+      reader.close();
+      if (null != executor) {
+        executor.shutdownNow();

Review comment:
       any reason we don't awaitTermination here?

##########
File path: hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/table/action/commit/SparkMergeHelper.java
##########
@@ -77,7 +77,7 @@ public void runMerge(HoodieTable<T, JavaRDD<HoodieRecord<T>>, JavaRDD<HoodieKey>
       readSchema = mergeHandle.getWriterSchemaWithMetaFields();
     }
 
-    BoundedInMemoryExecutor<GenericRecord, GenericRecord, Void> wrapper = null;
+    BoundedInMemoryExecutor<GenericRecord, GenericRecord, Void> executor = null;

Review comment:
       In future, could we do these renames in a separate PR?




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