You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2021/10/03 07:01:16 UTC

[GitHub] [iotdb] ruanwenjun opened a new pull request #4073: [ISSUE-4072] Parallel insert records in Session

ruanwenjun opened a new pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073


   close #4072 


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] LebronAl commented on a change in pull request #4073: [ISSUE-4072] Parallel insert records in Session

Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073#discussion_r727630449



##########
File path: session/src/main/java/org/apache/iotdb/session/Session.java
##########
@@ -814,6 +830,7 @@ private void handleRedirection(String deviceId, EndPoint endpoint)
                 }
               });
       if (connection == null) {
+        deviceIdToEndpoint.remove(deviceId);
         throw new IoTDBConnectionException(tmp.get());

Review comment:
       As `tmp` may be accessd by multiple thread, we'd better make it a local variable of the function

##########
File path: session/src/main/java/org/apache/iotdb/session/Session.java
##########
@@ -814,6 +830,7 @@ private void handleRedirection(String deviceId, EndPoint endpoint)
                 }
               });
       if (connection == null) {
+        deviceIdToEndpoint.remove(deviceId);
         throw new IoTDBConnectionException(tmp.get());

Review comment:
       As `tmp` may be accessd by multiple thread now, we'd better make it a local variable in this function




-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] ruanwenjun commented on a change in pull request #4073: [ISSUE-4072] Parallel insert records in Session

Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on a change in pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073#discussion_r727101607



##########
File path: session/src/main/java/org/apache/iotdb/session/Session.java
##########
@@ -2096,6 +2023,72 @@ private TSCreateSchemaTemplateReq getTSCreateSchemaTemplateReq(
     return request;
   }
 
+  /**
+   * @param recordsGroup connection to record map
+   * @param insertConsumer insert function
+   * @param <T>
+   *     <ul>
+   *       <li>{@link TSInsertRecordsReq}
+   *       <li>{@link TSInsertStringRecordsReq}
+   *       <li>{@link TSInsertTabletsReq}
+   *     </ul>
+   *
+   * @throws IoTDBConnectionException
+   * @throws StatementExecutionException
+   */
+  private <T> void insertByGroup(
+      Map<SessionConnection, T> recordsGroup, InsertConsumer<T> insertConsumer)
+      throws IoTDBConnectionException, StatementExecutionException {
+    List<CompletableFuture<Void>> completableFutures =
+        recordsGroup.entrySet().stream()
+            .map(
+                entry -> {
+                  SessionConnection connection = entry.getKey();
+                  T recordsReq = entry.getValue();
+                  return CompletableFuture.runAsync(
+                      () -> {
+                        try {
+                          insertConsumer.insert(connection, recordsReq);
+                        } catch (RedirectException e) {
+                          e.getDeviceEndPointMap()
+                              .forEach(
+                                  (deviceId, endpoint) -> {
+                                    try {
+                                      handleRedirection(deviceId, endpoint);
+                                    } catch (IoTDBConnectionException ioTDBConnectionException) {
+                                      throw new CompletionException(ioTDBConnectionException);

Review comment:
       This is the previous logic, but you are right, here we need to remove the broken endpoint.
   I have added this logic in  `handleRedirection` please review.




-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] neuyilan commented on a change in pull request #4073: [ISSUE-4072] Parallel insert records in Session

Posted by GitBox <gi...@apache.org>.
neuyilan commented on a change in pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073#discussion_r727049390



##########
File path: session/src/main/java/org/apache/iotdb/session/Session.java
##########
@@ -2096,6 +2023,72 @@ private TSCreateSchemaTemplateReq getTSCreateSchemaTemplateReq(
     return request;
   }
 
+  /**
+   * @param recordsGroup connection to record map
+   * @param insertConsumer insert function
+   * @param <T>
+   *     <ul>
+   *       <li>{@link TSInsertRecordsReq}
+   *       <li>{@link TSInsertStringRecordsReq}
+   *       <li>{@link TSInsertTabletsReq}
+   *     </ul>
+   *
+   * @throws IoTDBConnectionException
+   * @throws StatementExecutionException
+   */
+  private <T> void insertByGroup(
+      Map<SessionConnection, T> recordsGroup, InsertConsumer<T> insertConsumer)
+      throws IoTDBConnectionException, StatementExecutionException {
+    List<CompletableFuture<Void>> completableFutures =
+        recordsGroup.entrySet().stream()
+            .map(
+                entry -> {
+                  SessionConnection connection = entry.getKey();
+                  T recordsReq = entry.getValue();
+                  return CompletableFuture.runAsync(
+                      () -> {
+                        try {
+                          insertConsumer.insert(connection, recordsReq);
+                        } catch (RedirectException e) {
+                          e.getDeviceEndPointMap()
+                              .forEach(
+                                  (deviceId, endpoint) -> {
+                                    try {
+                                      handleRedirection(deviceId, endpoint);
+                                    } catch (IoTDBConnectionException ioTDBConnectionException) {
+                                      throw new CompletionException(ioTDBConnectionException);

Review comment:
       Here why not remove the broken session?




-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] neuyilan commented on a change in pull request #4073: [ISSUE-4072] Parallel insert records in Session

Posted by GitBox <gi...@apache.org>.
neuyilan commented on a change in pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073#discussion_r727711374



##########
File path: session/src/main/java/org/apache/iotdb/session/Session.java
##########
@@ -814,6 +830,7 @@ private void handleRedirection(String deviceId, EndPoint endpoint)
                 }
               });
       if (connection == null) {
+        deviceIdToEndpoint.remove(deviceId);
         throw new IoTDBConnectionException(tmp.get());

Review comment:
       The same as `handleMetaRedirection` method?




-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] coveralls edited a comment on pull request #4073: [ISSUE-4072] Parallel insert records in Session

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073#issuecomment-932882313






-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] coveralls edited a comment on pull request #4073: [ISSUE-4072] Parallel insert records in Session

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073#issuecomment-932882313


   
   [![Coverage Status](https://coveralls.io/builds/43462812/badge)](https://coveralls.io/builds/43462812)
   
   Coverage decreased (-0.04%) to 67.723% when pulling **3946c7ab6f427444fa395cedb0f8506e48b6a814 on ruanwenjun:dev_wenjun_patch4072** into **da269383089752a6bbaa9b8f09fc3be9e6a118b5 on apache:master**.
   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] neuyilan commented on a change in pull request #4073: [ISSUE-4072] Parallel insert records in Session

Posted by GitBox <gi...@apache.org>.
neuyilan commented on a change in pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073#discussion_r727711374



##########
File path: session/src/main/java/org/apache/iotdb/session/Session.java
##########
@@ -814,6 +830,7 @@ private void handleRedirection(String deviceId, EndPoint endpoint)
                 }
               });
       if (connection == null) {
+        deviceIdToEndpoint.remove(deviceId);
         throw new IoTDBConnectionException(tmp.get());

Review comment:
       The same as `handleMetaRedirection` method?




-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] coveralls edited a comment on pull request #4073: [ISSUE-4072] Parallel insert records in Session

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073#issuecomment-932882313


   
   [![Coverage Status](https://coveralls.io/builds/43467672/badge)](https://coveralls.io/builds/43467672)
   
   Coverage decreased (-0.01%) to 67.745% when pulling **4b77504277f0ace79274f6bf64af90e632eb4a05 on ruanwenjun:dev_wenjun_patch4072** into **da269383089752a6bbaa9b8f09fc3be9e6a118b5 on apache:master**.
   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] coveralls edited a comment on pull request #4073: [ISSUE-4072] Parallel insert records in Session

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073#issuecomment-932882313


   
   [![Coverage Status](https://coveralls.io/builds/43439392/badge)](https://coveralls.io/builds/43439392)
   
   Coverage increased (+0.03%) to 67.784% when pulling **6500a5eae13ef546c1ab37a9d41374559e265e96 on ruanwenjun:dev_wenjun_patch4072** into **da269383089752a6bbaa9b8f09fc3be9e6a118b5 on apache:master**.
   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] HTHou merged pull request #4073: [ISSUE-4072] Parallel insert records in Session

Posted by GitBox <gi...@apache.org>.
HTHou merged pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073


   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] LebronAl commented on a change in pull request #4073: [ISSUE-4072] Parallel insert records in Session

Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073#discussion_r727630449



##########
File path: session/src/main/java/org/apache/iotdb/session/Session.java
##########
@@ -814,6 +830,7 @@ private void handleRedirection(String deviceId, EndPoint endpoint)
                 }
               });
       if (connection == null) {
+        deviceIdToEndpoint.remove(deviceId);
         throw new IoTDBConnectionException(tmp.get());

Review comment:
       As `tmp` may be accessd by multiple thread now, we'd better make it a local variable in this function




-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] coveralls commented on pull request #4073: [ISSUE-4072] Parallel insert records in Session

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073#issuecomment-932882313


   
   [![Coverage Status](https://coveralls.io/builds/43232856/badge)](https://coveralls.io/builds/43232856)
   
   Coverage decreased (-0.009%) to 67.774% when pulling **8ecc10b55c5ec42a5e80c938d7ab83d0f235ef86 on ruanwenjun:dev_wenjun_patch4072** into **d4babb28236c10fdf525eb253314eedafd7109fe on apache:master**.
   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] coveralls edited a comment on pull request #4073: [ISSUE-4072] Parallel insert records in Session

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073#issuecomment-932882313


   
   [![Coverage Status](https://coveralls.io/builds/43441247/badge)](https://coveralls.io/builds/43441247)
   
   Coverage decreased (-0.03%) to 67.729% when pulling **6f47a050ba31e6cdd449727048ee2d45bdb5de3e on ruanwenjun:dev_wenjun_patch4072** into **da269383089752a6bbaa9b8f09fc3be9e6a118b5 on apache:master**.
   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] ruanwenjun commented on a change in pull request #4073: [ISSUE-4072] Parallel insert records in Session

Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on a change in pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073#discussion_r727817304



##########
File path: session/src/main/java/org/apache/iotdb/session/Session.java
##########
@@ -814,6 +830,7 @@ private void handleRedirection(String deviceId, EndPoint endpoint)
                 }
               });
       if (connection == null) {
+        deviceIdToEndpoint.remove(deviceId);
         throw new IoTDBConnectionException(tmp.get());

Review comment:
       Done, remove tmp in Session.




-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] ruanwenjun commented on a change in pull request #4073: [ISSUE-4072] Parallel insert records in Session

Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on a change in pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073#discussion_r727088794



##########
File path: session/src/main/java/org/apache/iotdb/session/Session.java
##########
@@ -2096,6 +2010,71 @@ private TSCreateSchemaTemplateReq getTSCreateSchemaTemplateReq(
     return request;
   }
 
+  /**
+   * @param recordsGroup connection to record map
+   * @param insertConsumer insert function
+   * @param <T>
+   *     <ul>
+   *       <li>{@link TSInsertRecordsReq}
+   *       <li>{@link TSInsertStringRecordsReq}
+   *       <li>{@link TSInsertTabletsReq}
+   *     </ul>
+   *
+   * @throws IoTDBConnectionException
+   * @throws StatementExecutionException
+   */
+  private <T> void insertRecordsGroup(

Review comment:
       Done

##########
File path: session/src/main/java/org/apache/iotdb/session/Session.java
##########
@@ -2096,6 +2010,71 @@ private TSCreateSchemaTemplateReq getTSCreateSchemaTemplateReq(
     return request;
   }
 
+  /**
+   * @param recordsGroup connection to record map
+   * @param insertConsumer insert function
+   * @param <T>
+   *     <ul>
+   *       <li>{@link TSInsertRecordsReq}
+   *       <li>{@link TSInsertStringRecordsReq}
+   *       <li>{@link TSInsertTabletsReq}
+   *     </ul>
+   *
+   * @throws IoTDBConnectionException
+   * @throws StatementExecutionException
+   */
+  private <T> void insertRecordsGroup(
+      Map<SessionConnection, T> recordsGroup, InsertConsumer<T> insertConsumer)
+      throws IoTDBConnectionException, StatementExecutionException {
+    List<CompletableFuture<Void>> completableFutures =
+        recordsGroup.entrySet().stream()
+            .map(
+                entry -> {

Review comment:
       Done




-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] ruanwenjun commented on a change in pull request #4073: [ISSUE-4072] Parallel insert records in Session

Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on a change in pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073#discussion_r727679982



##########
File path: session/src/main/java/org/apache/iotdb/session/Session.java
##########
@@ -814,6 +830,7 @@ private void handleRedirection(String deviceId, EndPoint endpoint)
                 }
               });
       if (connection == null) {
+        deviceIdToEndpoint.remove(deviceId);
         throw new IoTDBConnectionException(tmp.get());

Review comment:
       Yes, agree, done.

##########
File path: session/src/main/java/org/apache/iotdb/session/Session.java
##########
@@ -814,6 +830,7 @@ private void handleRedirection(String deviceId, EndPoint endpoint)
                 }
               });
       if (connection == null) {
+        deviceIdToEndpoint.remove(deviceId);
         throw new IoTDBConnectionException(tmp.get());

Review comment:
       Done, remove tmp in Session.




-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] ruanwenjun commented on a change in pull request #4073: [ISSUE-4072] Parallel insert records in Session

Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on a change in pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073#discussion_r727679982



##########
File path: session/src/main/java/org/apache/iotdb/session/Session.java
##########
@@ -814,6 +830,7 @@ private void handleRedirection(String deviceId, EndPoint endpoint)
                 }
               });
       if (connection == null) {
+        deviceIdToEndpoint.remove(deviceId);
         throw new IoTDBConnectionException(tmp.get());

Review comment:
       Yes, agree, done.




-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] coveralls edited a comment on pull request #4073: [ISSUE-4072] Parallel insert records in Session

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073#issuecomment-932882313


   
   [![Coverage Status](https://coveralls.io/builds/43232870/badge)](https://coveralls.io/builds/43232870)
   
   Coverage decreased (-0.004%) to 67.779% when pulling **8ecc10b55c5ec42a5e80c938d7ab83d0f235ef86 on ruanwenjun:dev_wenjun_patch4072** into **d4babb28236c10fdf525eb253314eedafd7109fe on apache:master**.
   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] LebronAl commented on a change in pull request #4073: [ISSUE-4072] Parallel insert records in Session

Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073#discussion_r727630449



##########
File path: session/src/main/java/org/apache/iotdb/session/Session.java
##########
@@ -814,6 +830,7 @@ private void handleRedirection(String deviceId, EndPoint endpoint)
                 }
               });
       if (connection == null) {
+        deviceIdToEndpoint.remove(deviceId);
         throw new IoTDBConnectionException(tmp.get());

Review comment:
       As `tmp` may be accessd by multiple thread, we'd better make it a local variable of the function




-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] coveralls edited a comment on pull request #4073: [ISSUE-4072] Parallel insert records in Session

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073#issuecomment-932882313


   
   [![Coverage Status](https://coveralls.io/builds/43439189/badge)](https://coveralls.io/builds/43439189)
   
   Coverage decreased (-0.02%) to 67.741% when pulling **6500a5eae13ef546c1ab37a9d41374559e265e96 on ruanwenjun:dev_wenjun_patch4072** into **da269383089752a6bbaa9b8f09fc3be9e6a118b5 on apache:master**.
   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] coveralls edited a comment on pull request #4073: [ISSUE-4072] Parallel insert records in Session

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073#issuecomment-932882313


   
   [![Coverage Status](https://coveralls.io/builds/43441548/badge)](https://coveralls.io/builds/43441548)
   
   Coverage increased (+0.004%) to 67.762% when pulling **6f47a050ba31e6cdd449727048ee2d45bdb5de3e on ruanwenjun:dev_wenjun_patch4072** into **da269383089752a6bbaa9b8f09fc3be9e6a118b5 on apache:master**.
   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] LebronAl commented on a change in pull request #4073: [ISSUE-4072] Parallel insert records in Session

Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #4073:
URL: https://github.com/apache/iotdb/pull/4073#discussion_r726706159



##########
File path: session/src/main/java/org/apache/iotdb/session/Session.java
##########
@@ -2096,6 +2010,71 @@ private TSCreateSchemaTemplateReq getTSCreateSchemaTemplateReq(
     return request;
   }
 
+  /**
+   * @param recordsGroup connection to record map
+   * @param insertConsumer insert function
+   * @param <T>
+   *     <ul>
+   *       <li>{@link TSInsertRecordsReq}
+   *       <li>{@link TSInsertStringRecordsReq}
+   *       <li>{@link TSInsertTabletsReq}
+   *     </ul>
+   *
+   * @throws IoTDBConnectionException
+   * @throws StatementExecutionException
+   */
+  private <T> void insertRecordsGroup(

Review comment:
       In our dialect, `record` is a row writing interface and `tablet` is a column writing interface. It is not recommended to use `record` or `tablet` as part of the function name. How about call it `insertByGroups` or `insertByGroupsParallel`

##########
File path: session/src/main/java/org/apache/iotdb/session/Session.java
##########
@@ -2096,6 +2010,71 @@ private TSCreateSchemaTemplateReq getTSCreateSchemaTemplateReq(
     return request;
   }
 
+  /**
+   * @param recordsGroup connection to record map
+   * @param insertConsumer insert function
+   * @param <T>
+   *     <ul>
+   *       <li>{@link TSInsertRecordsReq}
+   *       <li>{@link TSInsertStringRecordsReq}
+   *       <li>{@link TSInsertTabletsReq}
+   *     </ul>
+   *
+   * @throws IoTDBConnectionException
+   * @throws StatementExecutionException
+   */
+  private <T> void insertRecordsGroup(
+      Map<SessionConnection, T> recordsGroup, InsertConsumer<T> insertConsumer)
+      throws IoTDBConnectionException, StatementExecutionException {
+    List<CompletableFuture<Void>> completableFutures =
+        recordsGroup.entrySet().stream()
+            .map(
+                entry -> {

Review comment:
       Since this is an IO intensive loads, it is recommended not to use the default thread pool `ForkJoinPool.commonPool()`, which is suitable for CPU intensive loads. We can use our own thread pool (coreSize set to 2x cpus)?




-- 
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: reviews-unsubscribe@iotdb.apache.org

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