You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org> on 2018/09/17 21:33:46 UTC

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Bikramjeet Vig has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11440


Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................

IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

This patch adds memory estimates for kudu scan nodes based on
empirically derived estimates for the scan's memory consumption
that were added in IMPALA-7096.

Testing:
Modified resource requirements planner test.

Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
---
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
M testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
6 files changed, 100 insertions(+), 61 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/11440/1
-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/11440

to look at the new patch set (#4).

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................

IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

This patch adds memory estimates for kudu scan nodes based on
empirically derived estimates for the scan's memory consumption
that were added in IMPALA-7096.

Testing:
Modified resource requirements planner test.

Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
---
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/ScanNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
M testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
9 files changed, 204 insertions(+), 79 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/11440/4
-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11440 )

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................


Patch Set 7: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 7
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Oct 2018 18:17:29 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11440 )

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................


Patch Set 4: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Sep 2018 17:50:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11440 )

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................


Patch Set 7: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 7
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Oct 2018 22:04:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/11440

to look at the new patch set (#5).

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................

IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

This patch adds memory estimates for kudu scan nodes based on
empirically derived estimates for the scan's memory consumption
that were added in IMPALA-7096.

Testing:
Modified resource requirements planner test.

Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
---
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/ScanNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
M testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
9 files changed, 205 insertions(+), 79 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/11440/5
-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 5
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11440 )

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/11440/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11440/1//COMMIT_MSG@14
PS1, Line 14: Modified resource requirements planner test.
I don't see any changes to this test (it seems like I didn't include a Kudu scan tests case, which is an oversight). I think we should add some tests there.


http://gerrit.cloudera.org:8080/#/c/11440/1/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java:

http://gerrit.cloudera.org:8080/#/c/11440/1/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@91
PS1, Line 91:   // Factor capturing the worst-case deviation from a uniform distribution of scan ranges
Put this in the ScanNode base class?


http://gerrit.cloudera.org:8080/#/c/11440/1/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@284
PS1, Line 284:     // The non-MT scan node requires at least one scanner thread.
             :     int maxScannerThreads;
             :     if (queryOptions.getMt_dop() >= 1) {
             :       maxScannerThreads = 1;
             :     } else {
             :       // TODO: what about heterogeneous hardware, i.e., different num of cores?
             :       maxScannerThreads = Math.min(perHostScanRanges, RuntimeEnv.INSTANCE.getNumCores());
             :       // Account for the max scanner threads query option.
             :       if (queryOptions.isSetNum_scanner_threads() &&
             :           queryOptions.getNum_scanner_threads() > 0) {
             :         maxScannerThreads =
             :             Math.min(maxScannerThreads, queryOptions.getNum_scanner_threads());
             :       }
             :     }
I think this is shared with HdfsScanNode. Can we factor out to a helper function in ScanNode?


http://gerrit.cloudera.org:8080/#/c/11440/1/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@298
PS1, Line 298:     int num_cols = desc_.getSlots().size();
Can you leave a comment briefly describing the heuristic and mentioning any known limitations? E.g. doesn't model the row batch queue. Otherwise it will be mysterious in a year's time when we've forgotten the context.


http://gerrit.cloudera.org:8080/#/c/11440/1/testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
File testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test:

PS1: 
You might know already, but we don't validate resource requirements for this test because PlannerTestOption.VALIDATE_RESOURCES isn't set for it. I don't think we need to validate resources here, but didn't want us to think we had coverage that we don't.



-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 17 Sep 2018 22:01:55 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11440 )

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/771/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Sep 2018 23:40:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11440 )

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11440/2/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java:

http://gerrit.cloudera.org:8080/#/c/11440/2/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@284
PS2, Line 284:     int perHostScanRanges = (int) Math.ceil(((double) scanRangeSpecs_
> That was my initial thought too but this calculation is quite different bet
Isn't this formula that takes the number of ranges total and produces the number of per-host ranges the same though? Some of the other code flow is different but I think this line of code is basically the same.



-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Sep 2018 21:26:54 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/11440

to look at the new patch set (#3).

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................

IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

This patch adds memory estimates for kudu scan nodes based on
empirically derived estimates for the scan's memory consumption
that were added in IMPALA-7096.

Testing:
Modified resource requirements planner test.

Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
---
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/ScanNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
M testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
9 files changed, 194 insertions(+), 77 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/11440/3
-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11440 )

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................

IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

This patch adds memory estimates for kudu scan nodes based on
empirically derived estimates for the scan's memory consumption
that were added in IMPALA-7096.

Testing:
Modified resource requirements planner test.

Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Reviewed-on: http://gerrit.cloudera.org:8080/11440
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/ScanNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
M testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
9 files changed, 205 insertions(+), 79 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 8
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11440 )

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................


Patch Set 2:

(6 comments)

Just a few more cleanup and test suggestions

http://gerrit.cloudera.org:8080/#/c/11440/2/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java:

http://gerrit.cloudera.org:8080/#/c/11440/2/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@284
PS2, Line 284:     int perHostScanRanges = (int) Math.ceil(((double) scanRangeSpecs_
Can we factor out the per-host scan range calculation into a helper? I know it's short but the logic isn't trivial and I think we expect it to be identical between the scan node implementations (unless something changes in future).


http://gerrit.cloudera.org:8080/#/c/11440/2/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@296
PS2, Line 296:     .setMemEstimateBytes(mem_estimate_per_thread * maxScannerThreads)
This new indentation seems weird to me.


http://gerrit.cloudera.org:8080/#/c/11440/2/fe/src/main/java/org/apache/impala/planner/ScanNode.java
File fe/src/main/java/org/apache/impala/planner/ScanNode.java:

http://gerrit.cloudera.org:8080/#/c/11440/2/fe/src/main/java/org/apache/impala/planner/ScanNode.java@232
PS2, Line 232: ComputeMaxNumberOfScannerThreads
Initial letter of method name should be lower-case in java.


http://gerrit.cloudera.org:8080/#/c/11440/2/fe/src/main/java/org/apache/impala/planner/ScanNode.java@237
PS2, Line 237:       maxScannerThreads = 1;
We can just return early here (and eliminate the nesting of the else branch.


http://gerrit.cloudera.org:8080/#/c/11440/1/testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
File testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test:

PS1: 
> Yup, my main intention was to just keep track of how the estimates change o
Yeah, that seems fine to update then, no reason to have things diverge :)


http://gerrit.cloudera.org:8080/#/c/11440/2/testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
File testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test:

http://gerrit.cloudera.org:8080/#/c/11440/2/testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test@5256
PS2, Line 5256: select * from functional_kudu.alltypes
Can you also add a scan of a single column and a scan of count(*) and a scan of an unpartitioned table like tpch_kudu.nation, which I think should only be scanned on one node with one thread.



-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Sep 2018 16:14:03 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11440 )

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/750/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 21 Sep 2018 22:17:24 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11440 )

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/830/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 5
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Sep 2018 00:56:50 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11440 )

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................


Patch Set 6: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 6
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Oct 2018 18:04:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11440 )

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/780/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Sep 2018 17:53:51 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11440 )

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/11440/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11440/1//COMMIT_MSG@14
PS1, Line 14: Modified resource requirements planner test.
> I don't see any changes to this test (it seems like I didn't include a Kudu
Done


http://gerrit.cloudera.org:8080/#/c/11440/1/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java:

http://gerrit.cloudera.org:8080/#/c/11440/1/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@91
PS1, Line 91:   // Factor capturing the worst-case deviation from a uniform distribution of scan ranges
> Put this in the ScanNode base class?
Done


http://gerrit.cloudera.org:8080/#/c/11440/1/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@284
PS1, Line 284:     // The non-MT scan node requires at least one scanner thread.
             :     int maxScannerThreads;
             :     if (queryOptions.getMt_dop() >= 1) {
             :       maxScannerThreads = 1;
             :     } else {
             :       // TODO: what about heterogeneous hardware, i.e., different num of cores?
             :       maxScannerThreads = Math.min(perHostScanRanges, RuntimeEnv.INSTANCE.getNumCores());
             :       // Account for the max scanner threads query option.
             :       if (queryOptions.isSetNum_scanner_threads() &&
             :           queryOptions.getNum_scanner_threads() > 0) {
             :         maxScannerThreads =
             :             Math.min(maxScannerThreads, queryOptions.getNum_scanner_threads());
             :       }
             :     }
> I think this is shared with HdfsScanNode. Can we factor out to a helper fun
Done


http://gerrit.cloudera.org:8080/#/c/11440/1/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@298
PS1, Line 298:     int num_cols = desc_.getSlots().size();
> Can you leave a comment briefly describing the heuristic and mentioning any
Done


http://gerrit.cloudera.org:8080/#/c/11440/1/testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
File testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test:

PS1: 
> You might know already, but we don't validate resource requirements for thi
Yup, my main intention was to just keep track of how the estimates change over time. If you believe that this might mislead someone to think that we have extra coverage then we can remove these changes or maybe add a comment to the commit message clarifying that estimate changes to this file are not actually checked, but were only made for the purpose of updating the file.



-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 21 Sep 2018 21:40:11 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11440 )

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/11440/2/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java:

http://gerrit.cloudera.org:8080/#/c/11440/2/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@284
PS2, Line 284:     int perHostScanRanges = (int) Math.ceil(((double) scanRangeSpecs_
> Can we factor out the per-host scan range calculation into a helper? I know
That was my initial thought too but this calculation is quite different between hdfs and kudu scan nodes


http://gerrit.cloudera.org:8080/#/c/11440/2/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@296
PS2, Line 296:     .setMemEstimateBytes(mem_estimate_per_thread * maxScannerThreads)
> This new indentation seems weird to me.
Done


http://gerrit.cloudera.org:8080/#/c/11440/2/fe/src/main/java/org/apache/impala/planner/ScanNode.java
File fe/src/main/java/org/apache/impala/planner/ScanNode.java:

http://gerrit.cloudera.org:8080/#/c/11440/2/fe/src/main/java/org/apache/impala/planner/ScanNode.java@232
PS2, Line 232: ComputeMaxNumberOfScannerThreads
> Initial letter of method name should be lower-case in java.
Done


http://gerrit.cloudera.org:8080/#/c/11440/2/fe/src/main/java/org/apache/impala/planner/ScanNode.java@237
PS2, Line 237:       maxScannerThreads = 1;
> We can just return early here (and eliminate the nesting of the else branch
Done


http://gerrit.cloudera.org:8080/#/c/11440/2/testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
File testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test:

http://gerrit.cloudera.org:8080/#/c/11440/2/testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test@5256
PS2, Line 5256: select * from functional_kudu.alltypes
> Can you also add a scan of a single column and a scan of count(*) and a sca
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Sep 2018 21:20:38 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11440 )

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11440/2/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java:

http://gerrit.cloudera.org:8080/#/c/11440/2/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@284
PS2, Line 284:     int perHostScanRanges = (int) Math.ceil(((double) scanRangeSpecs_
> Isn't this formula that takes the number of ranges total and produces the n
Done. I guess I wanted to preserve the importance of the context around the difference in how this is used in hdfs vs kudu, but I see how it would make sense have this factored out in the scan node separately.



-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Sep 2018 23:18:43 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11440 )

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................


Patch Set 5: Code-Review+2

Rebased and carrying forward Tim's +2


-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 5
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Sep 2018 00:17:35 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/11440

to look at the new patch set (#6).

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................

IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

This patch adds memory estimates for kudu scan nodes based on
empirically derived estimates for the scan's memory consumption
that were added in IMPALA-7096.

Testing:
Modified resource requirements planner test.

Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
---
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/ScanNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
M testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
9 files changed, 205 insertions(+), 79 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/11440/6
-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 6
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11440 )

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/938/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 6
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Oct 2018 18:34:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11440 )

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/696/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 17 Sep 2018 22:05:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11440 )

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................


Patch Set 7:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3269/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 7
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Oct 2018 18:17:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11440 )

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/767/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Sep 2018 21:55:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/11440

to look at the new patch set (#2).

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................

IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

This patch adds memory estimates for kudu scan nodes based on
empirically derived estimates for the scan's memory consumption
that were added in IMPALA-7096.

Testing:
Modified resource requirements planner test.

Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
---
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/ScanNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
M testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
9 files changed, 139 insertions(+), 78 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/11440/2
-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Kudu Scan Nodes

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11440 )

Change subject: IMPALA-7351: Improve memory estimates for Kudu Scan Nodes
......................................................................


Patch Set 6:

Rebased, carrying over +2


-- 
To view, visit http://gerrit.cloudera.org:8080/11440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9bb52530271b0bff91311a67d222a2e9fac1229
Gerrit-Change-Number: 11440
Gerrit-PatchSet: 6
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Oct 2018 18:04:37 +0000
Gerrit-HasComments: No