You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "JackieTien97 (via GitHub)" <gi...@apache.org> on 2023/07/13 01:10:49 UTC

[GitHub] [iotdb] JackieTien97 commented on a diff in pull request #10532: Use customer timeout in schema fetcher

JackieTien97 commented on code in PR #10532:
URL: https://github.com/apache/iotdb/pull/10532#discussion_r1261863941


##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/analyze/schema/AutoCreateSchemaExecutor.java:
##########
@@ -54,21 +59,33 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.function.Function;
 import java.util.function.IntFunction;
 import java.util.stream.Collectors;
 
 import static org.apache.iotdb.db.utils.EncodingInferenceUtils.getDefaultEncoding;
 
 class AutoCreateSchemaExecutor {
-
+  private final IoTDBConfig config = IoTDBDescriptor.getInstance().getConfig();
+  private final Coordinator coordinator;
   private final ITemplateManager templateManager;
-  private final Function<Statement, ExecutionResult> statementExecutor;
+  private final ISchemaFetcher schemaFetcher;
 
   AutoCreateSchemaExecutor(
-      ITemplateManager templateManager, Function<Statement, ExecutionResult> statementExecutor) {
+      Coordinator coordinator, ITemplateManager templateManager, ISchemaFetcher schemaFetcher) {
+    this.coordinator = coordinator;
     this.templateManager = templateManager;
-    this.statementExecutor = statementExecutor;
+    this.schemaFetcher = schemaFetcher;
+  }
+
+  private ExecutionResult executeStatement(Statement statement) {
+    return coordinator.execute(
+        statement,
+        SessionManager.getInstance().requestQueryId(),
+        null,

Review Comment:
   try to gey `SessionInfo` here.



##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/analyze/schema/ClusterSchemaFetchExecutor.java:
##########
@@ -46,42 +52,54 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
-import java.util.function.BiFunction;
 import java.util.function.Consumer;
-import java.util.function.Supplier;
 
 class ClusterSchemaFetchExecutor {
 
+  private final IoTDBConfig config = IoTDBDescriptor.getInstance().getConfig();
   private final Coordinator coordinator;
   private final ITemplateManager templateManager;
-  private final Supplier<Long> queryIdProvider;
-  private final BiFunction<Long, Statement, ExecutionResult> statementExecutor;
+  private final ISchemaFetcher schemaFetcher;
   private final Consumer<ClusterSchemaTree> schemaCacheUpdater;
 
   ClusterSchemaFetchExecutor(
       Coordinator coordinator,
       ITemplateManager templateManager,
-      Supplier<Long> queryIdProvider,
-      BiFunction<Long, Statement, ExecutionResult> statementExecutor,
+      ISchemaFetcher schemaFetcher,
       Consumer<ClusterSchemaTree> schemaCacheUpdater) {
     this.coordinator = coordinator;
     this.templateManager = templateManager;
-    this.queryIdProvider = queryIdProvider;
-    this.statementExecutor = statementExecutor;
+    this.schemaFetcher = schemaFetcher;
     this.schemaCacheUpdater = schemaCacheUpdater;
   }
 
+  private ExecutionResult executionStatement(
+      Long queryId, Statement statement, MPPQueryContext context) {
+    return coordinator.execute(
+        statement,
+        queryId,
+        context == null ? null : context.getSession(),
+        "",
+        ClusterPartitionFetcher.getInstance(),
+        schemaFetcher,
+        context == null || context.getQueryType().equals(QueryType.WRITE)

Review Comment:
   write should never  have a timeout.



##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/analyze/schema/ClusterSchemaFetchExecutor.java:
##########
@@ -46,42 +52,54 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
-import java.util.function.BiFunction;
 import java.util.function.Consumer;
-import java.util.function.Supplier;
 
 class ClusterSchemaFetchExecutor {
 
+  private final IoTDBConfig config = IoTDBDescriptor.getInstance().getConfig();
   private final Coordinator coordinator;
   private final ITemplateManager templateManager;
-  private final Supplier<Long> queryIdProvider;
-  private final BiFunction<Long, Statement, ExecutionResult> statementExecutor;
+  private final ISchemaFetcher schemaFetcher;
   private final Consumer<ClusterSchemaTree> schemaCacheUpdater;
 
   ClusterSchemaFetchExecutor(
       Coordinator coordinator,
       ITemplateManager templateManager,
-      Supplier<Long> queryIdProvider,
-      BiFunction<Long, Statement, ExecutionResult> statementExecutor,
+      ISchemaFetcher schemaFetcher,
       Consumer<ClusterSchemaTree> schemaCacheUpdater) {
     this.coordinator = coordinator;
     this.templateManager = templateManager;
-    this.queryIdProvider = queryIdProvider;
-    this.statementExecutor = statementExecutor;
+    this.schemaFetcher = schemaFetcher;
     this.schemaCacheUpdater = schemaCacheUpdater;
   }
 
+  private ExecutionResult executionStatement(
+      Long queryId, Statement statement, MPPQueryContext context) {
+    return coordinator.execute(
+        statement,
+        queryId,
+        context == null ? null : context.getSession(),
+        "",
+        ClusterPartitionFetcher.getInstance(),
+        schemaFetcher,
+        context == null || context.getQueryType().equals(QueryType.WRITE)

Review Comment:
   Actually, we have already set the timeout field in `Context` for write to `Long.MAX_VALUE` in Coordinator.



##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/analyze/schema/AutoCreateSchemaExecutor.java:
##########
@@ -54,21 +59,33 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.function.Function;
 import java.util.function.IntFunction;
 import java.util.stream.Collectors;
 
 import static org.apache.iotdb.db.utils.EncodingInferenceUtils.getDefaultEncoding;
 
 class AutoCreateSchemaExecutor {
-
+  private final IoTDBConfig config = IoTDBDescriptor.getInstance().getConfig();
+  private final Coordinator coordinator;
   private final ITemplateManager templateManager;
-  private final Function<Statement, ExecutionResult> statementExecutor;
+  private final ISchemaFetcher schemaFetcher;
 
   AutoCreateSchemaExecutor(
-      ITemplateManager templateManager, Function<Statement, ExecutionResult> statementExecutor) {
+      Coordinator coordinator, ITemplateManager templateManager, ISchemaFetcher schemaFetcher) {
+    this.coordinator = coordinator;
     this.templateManager = templateManager;
-    this.statementExecutor = statementExecutor;
+    this.schemaFetcher = schemaFetcher;
+  }
+
+  private ExecutionResult executeStatement(Statement statement) {
+    return coordinator.execute(
+        statement,
+        SessionManager.getInstance().requestQueryId(),
+        null,
+        "",
+        ClusterPartitionFetcher.getInstance(),
+        schemaFetcher,
+        config.getQueryTimeoutThreshold());

Review Comment:
   get timeout from context.



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