Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/16864 )
Change subject: IMPALA-10287: Include parallelism in cost comparison of broadcast vs partition ...................................................................... Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/16864/1/common/thrift/ImpalaInternalService.thrift File common/thrift/ImpalaInternalService.thrift: http://gerrit.cloudera.org:8080/#/c/16864/1/common/thrift/ImpalaInternalService.thrift@473 PS1, Line 473: 118: optional bool use_dop_for_costing = true; > I'd consider enabling it by default, it seems like a fairly clear win parti Enabled this. It didn't actually change the plans in the test suite since mt_dop is small for dev testing. http://gerrit.cloudera.org:8080/#/c/16864/1/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java File fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java: http://gerrit.cloudera.org:8080/#/c/16864/1/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java@515 PS1, Line 515: sqrt( > I wonder if natural log reduces the influence of DOP too much, e.g. log(16) I was actually debating whether to use sqrt at one point but fell back to a more conservative function. But given your feedback and also offline feedback from Kurt about this not being aggressive enough and also the fact that the whole costing change can be easily disabled, I went ahead and changed this to sqrt . I figured it would be easier to do the performance evaluation once this patch is merged and so I have created a follow-on JIRA (https://issues.apache.org/jira/browse/IMPALA-10395) to gather more performance data and use a curve-fitting to determine what the right equation should be. -- To view, visit http://gerrit.cloudera.org:8080/16864 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idff569299e5c78720ca17c616a531adac78208e1 Gerrit-Change-Number: 16864 Gerrit-PatchSet: 2 Gerrit-Owner: Aman Sinha <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Tue, 15 Dec 2020 00:31:14 +0000 Gerrit-HasComments: Yes
