You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Qifan Chen (Code Review)" <ge...@cloudera.org> on 2020/10/29 16:35:15 UTC
[Impala-ASF-CR] IMPALA-9355: TestExchangeMemUsage.test exchange mem usage scaling doesn't hit the memory limit
Qifan Chen has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16676
Change subject: IMPALA-9355: TestExchangeMemUsage.test_exchange_mem_usage_scaling doesn't hit the memory limit
......................................................................
IMPALA-9355: TestExchangeMemUsage.test_exchange_mem_usage_scaling doesn't hit the memory limit
This fix improved the robustness of the test by lowering the memory
limit from 170MB to 168MB. In 10 local tests, the memory allocation
shortage at EXCHANGE_NODE was as low as 8KB without exceeding the
limit of 170MB.
Testing:
1. Unit testing with 200 test iterations.
Change-Id: I3c33e3b254ce75054bf6cee0eff8b7206fa0c122
---
M testdata/workloads/functional-query/queries/QueryTest/exchange-mem-scaling.test
1 file changed, 5 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/16676/2
--
To view, visit http://gerrit.cloudera.org:8080/16676
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3c33e3b254ce75054bf6cee0eff8b7206fa0c122
Gerrit-Change-Number: 16676
Gerrit-PatchSet: 2
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
[Impala-ASF-CR] IMPALA-9355: TestExchangeMemUsage.test exchange mem usage scaling doesn't hit the memory limit
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16676 )
Change subject: IMPALA-9355: TestExchangeMemUsage.test_exchange_mem_usage_scaling doesn't hit the memory limit
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/7578/ : 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/16676
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c33e3b254ce75054bf6cee0eff8b7206fa0c122
Gerrit-Change-Number: 16676
Gerrit-PatchSet: 2
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 29 Oct 2020 16:57:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9355: TestExchangeMemUsage.test exchange mem usage scaling doesn't hit the memory limit
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16676 )
Change subject: IMPALA-9355: TestExchangeMemUsage.test_exchange_mem_usage_scaling doesn't hit the memory limit
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/7609/ : 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/16676
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c33e3b254ce75054bf6cee0eff8b7206fa0c122
Gerrit-Change-Number: 16676
Gerrit-PatchSet: 3
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Nov 2020 17:53:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9355: TestExchangeMemUsage.test exchange mem usage scaling doesn't hit the memory limit
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/16676 )
Change subject: IMPALA-9355: TestExchangeMemUsage.test_exchange_mem_usage_scaling doesn't hit the memory limit
......................................................................
Patch Set 3: Code-Review+2
Thanks for fixing this!
--
To view, visit http://gerrit.cloudera.org:8080/16676
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c33e3b254ce75054bf6cee0eff8b7206fa0c122
Gerrit-Change-Number: 16676
Gerrit-PatchSet: 3
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Nov 2020 17:43:10 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9355: TestExchangeMemUsage.test exchange mem usage scaling doesn't hit the memory limit
Posted by "Qifan Chen (Code Review)" <ge...@cloudera.org>.
Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/16676 )
Change subject: IMPALA-9355: TestExchangeMemUsage.test_exchange_mem_usage_scaling doesn't hit the memory limit
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16676/2/testdata/workloads/functional-query/queries/QueryTest/exchange-mem-scaling.test
File testdata/workloads/functional-query/queries/QueryTest/exchange-mem-scaling.test:
http://gerrit.cloudera.org:8080/#/c/16676/2/testdata/workloads/functional-query/queries/QueryTest/exchange-mem-scaling.test@7
PS2, Line 7: set mem_limit=168m;
: set num_scanner_threads=1;
: select *
: from tpch_parquet.lin
> From a purely style note, I think this comment is not necessary. The commen
Done
--
To view, visit http://gerrit.cloudera.org:8080/16676
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c33e3b254ce75054bf6cee0eff8b7206fa0c122
Gerrit-Change-Number: 16676
Gerrit-PatchSet: 3
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Nov 2020 17:31:34 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9355: TestExchangeMemUsage.test exchange mem usage scaling doesn't hit the memory limit
Posted by "Qifan Chen (Code Review)" <ge...@cloudera.org>.
Qifan Chen has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/16676 )
Change subject: IMPALA-9355: TestExchangeMemUsage.test_exchange_mem_usage_scaling doesn't hit the memory limit
......................................................................
IMPALA-9355: TestExchangeMemUsage.test_exchange_mem_usage_scaling doesn't hit the memory limit
This fix improved the robustness of the test by lowering the memory
limit from 170MB to 168MB. In 10 local tests, the memory allocation
shortage at EXCHANGE_NODE was as low as 8KB without exceeding the
limit of 170MB.
Testing:
1. Unit testing with 200 test iterations.
Change-Id: I3c33e3b254ce75054bf6cee0eff8b7206fa0c122
---
M testdata/workloads/functional-query/queries/QueryTest/exchange-mem-scaling.test
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/16676/3
--
To view, visit http://gerrit.cloudera.org:8080/16676
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3c33e3b254ce75054bf6cee0eff8b7206fa0c122
Gerrit-Change-Number: 16676
Gerrit-PatchSet: 3
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
[Impala-ASF-CR] IMPALA-9355: TestExchangeMemUsage.test exchange mem usage scaling doesn't hit the memory limit
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/16676 )
Change subject: IMPALA-9355: TestExchangeMemUsage.test_exchange_mem_usage_scaling doesn't hit the memory limit
......................................................................
Patch Set 2:
(1 comment)
The change looks good from a content point of view. I have one small style point.
http://gerrit.cloudera.org:8080/#/c/16676/2/testdata/workloads/functional-query/queries/QueryTest/exchange-mem-scaling.test
File testdata/workloads/functional-query/queries/QueryTest/exchange-mem-scaling.test:
http://gerrit.cloudera.org:8080/#/c/16676/2/testdata/workloads/functional-query/queries/QueryTest/exchange-mem-scaling.test@7
PS2, Line 7: # IMPALA-9355: TestExchangeMemUsage.test_exchange_mem_usage_scaling doesn't hit
: # the memory limit. The mem_limit is reduced from 170MB to 168MB since the memory
: # shortage observed at exchange nodes can be as small as 8KB without exceeding
: # the limit of 170MB.
From a purely style note, I think this comment is not necessary. The comment about IMPALA-5485 is describing why this test exists and is useful to a reader. The details about adjustments to the memory limit to avoid flakiness are important for the history, but it is usually not something a reader of this file needs to know about. All the history is accessible by git blame, so they can find the commit that set the mem_limit=168m if they need. So, I would lean toward not including a comment for this.
--
To view, visit http://gerrit.cloudera.org:8080/16676
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c33e3b254ce75054bf6cee0eff8b7206fa0c122
Gerrit-Change-Number: 16676
Gerrit-PatchSet: 2
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Nov 2020 01:30:07 +0000
Gerrit-HasComments: Yes