Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19033 )
Change subject: IMPALA-11604 Planner changes for CPU usage ...................................................................... Patch Set 42: (10 comments) http://gerrit.cloudera.org:8080/#/c/19033/42/be/src/scheduling/scheduler.h File be/src/scheduling/scheduler.h: http://gerrit.cloudera.org:8080/#/c/19033/42/be/src/scheduling/scheduler.h@441 PS42, Line 441: effective_instance_count nit: TPlanFragment.effective_instance_count. Otherwise, it's hard to find context here. http://gerrit.cloudera.org:8080/#/c/19033/42/be/src/util/backend-gflag-util.cc File be/src/util/backend-gflag-util.cc: http://gerrit.cloudera.org:8080/#/c/19033/42/be/src/util/backend-gflag-util.cc@206 PS42, Line 206: 1 These are relative costs, right? "1" is the minimum cost? http://gerrit.cloudera.org:8080/#/c/19033/42/be/src/util/backend-gflag-util.cc@207 PS42, Line 207: " nit: don't break line here http://gerrit.cloudera.org:8080/#/c/19033/42/be/src/util/backend-gflag-util.cc@211 PS42, Line 211: min_processing_cost_per_thread what's the normal range? why set default value as 1000000? http://gerrit.cloudera.org:8080/#/c/19033/42/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/19033/42/common/thrift/ImpalaService.thrift@767 PS42, Line 767: Control whether to display processing cost detail in query plan or not. The comment is different from the meaning of query option http://gerrit.cloudera.org:8080/#/c/19033/42/common/thrift/ImpalaService.thrift@771 PS42, Line 771: 64 Why the max value is 64? http://gerrit.cloudera.org:8080/#/c/19033/42/fe/src/main/java/org/apache/impala/planner/BroadcastProcessingCost.java File fe/src/main/java/org/apache/impala/planner/BroadcastProcessingCost.java: http://gerrit.cloudera.org:8080/#/c/19033/42/fe/src/main/java/org/apache/impala/planner/BroadcastProcessingCost.java@47 PS42, Line 47: Preconditions.checkState Could we move this Precondition.checkState to constructor? http://gerrit.cloudera.org:8080/#/c/19033/42/fe/src/main/java/org/apache/impala/planner/DataSink.java File fe/src/main/java/org/apache/impala/planner/DataSink.java: http://gerrit.cloudera.org:8080/#/c/19033/42/fe/src/main/java/org/apache/impala/planner/DataSink.java@68 PS42, Line 68: processingCost_ Should we add Preconditions to check if processingCost_ is valid? http://gerrit.cloudera.org:8080/#/c/19033/42/fe/src/main/java/org/apache/impala/planner/ExchangeNode.java File fe/src/main/java/org/apache/impala/planner/ExchangeNode.java: http://gerrit.cloudera.org:8080/#/c/19033/42/fe/src/main/java/org/apache/impala/planner/ExchangeNode.java@265 PS42, Line 265: per row batch per row? http://gerrit.cloudera.org:8080/#/c/19033/42/fe/src/main/java/org/apache/impala/util/ExprUtil.java File fe/src/main/java/org/apache/impala/util/ExprUtil.java: http://gerrit.cloudera.org:8080/#/c/19033/42/fe/src/main/java/org/apache/impala/util/ExprUtil.java@119 PS42, Line 119: e.getCost() : 1 What's cost range? Is the value 1 the minimum value? -- To view, visit http://gerrit.cloudera.org:8080/19033 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If32dc770dfffcdd0be2b5555a789a7720952c68a Gerrit-Change-Number: 19033 Gerrit-PatchSet: 42 Gerrit-Owner: Qifan Chen <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Qifan Chen <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Comment-Date: Mon, 06 Feb 2023 19:12:54 +0000 Gerrit-HasComments: Yes
