[Impala-ASF-CR] IMPALA-5269: Fix issue with final line of query followed by a comment

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9191 )

Change subject: IMPALA-5269: Fix issue with final line of query followed by a 
comment
..


Patch Set 8: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I54f9a8f65214023520eaa010fc462a663d02d258
Gerrit-Change-Number: 9191
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Wed, 14 Feb 2018 07:38:27 +
Gerrit-HasComments: No

[Impala-ASF-CR] Use unqualified table refs in TPCH planner tests.

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9270 )

Change subject: Use unqualified table refs in TPCH planner tests.
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I886c451ab61a1739af96eeb765821dfd8e951b07
Gerrit-Change-Number: 9270
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Comment-Date: Wed, 14 Feb 2018 05:54:24 +
Gerrit-HasComments: No

[Impala-ASF-CR] Use unqualified table refs in TPCH planner tests.

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9270 )

Change subject: Use unqualified table refs in TPCH planner tests.
..

Use unqualified table refs in TPCH planner tests.

There were a few places where we accidentally used fully-qualified
table references. As a result, the testTpchViews() test did not
exactly cover what was intended.

Change-Id: I886c451ab61a1739af96eeb765821dfd8e951b07
Reviewed-on: http://gerrit.cloudera.org:8080/9270
Reviewed-by: Taras Bobrovytsky 
Tested-by: Impala Public Jenkins
---
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-views.test
2 files changed, 31 insertions(+), 31 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I886c451ab61a1739af96eeb765821dfd8e951b07
Gerrit-Change-Number: 9270
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-6516: Log catalog update only if the catalog version changes

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9311 )

Change subject: IMPALA-6516: Log catalog update only if the catalog version 
changes
..

IMPALA-6516: Log catalog update only if the catalog version changes

Impalad writes a line of log whenever a statestore catalog update comes
in. This patch removes the logging when the catalog version doesn't
change.

Change-Id: I04b8dd05c588d4cd91e9ca2251f8f66325bb45e2
Reviewed-on: http://gerrit.cloudera.org:8080/9311
Reviewed-by: Michael Ho 
Reviewed-by: anujphadke 
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M be/src/service/impala-server.cc
1 file changed, 5 insertions(+), 3 deletions(-)

Approvals:
  Michael Ho: Looks good to me, approved
  anujphadke: Looks good to me, but someone else must approve
  Alex Behm: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I04b8dd05c588d4cd91e9ca2251f8f66325bb45e2
Gerrit-Change-Number: 9311
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: anujphadke 

[Impala-ASF-CR] IMPALA-6516: Log catalog update only if the catalog version changes

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9311 )

Change subject: IMPALA-6516: Log catalog update only if the catalog version 
changes
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04b8dd05c588d4cd91e9ca2251f8f66325bb45e2
Gerrit-Change-Number: 9311
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 14 Feb 2018 05:26:46 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3193: Show table's comment on show tables

[Kim Jin Chul (Code Review)]

Kim Jin Chul has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8851 )

Change subject: IMPALA-3193: Show table's comment on show tables
..


Patch Set 17: Code-Review-1

Let me look into the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I44f814af05db6f3c027718ade9f474f8b8153bcd
Gerrit-Change-Number: 8851
Gerrit-PatchSet: 17
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Comment-Date: Wed, 14 Feb 2018 04:59:18 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5752: Add support for DECIMAL on Kudu tables

[Grant Henke (Code Review)]

Grant Henke has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9306


Change subject: IMPALA-5752: Add support for DECIMAL on Kudu tables
..

IMPALA-5752: Add support for DECIMAL on Kudu tables

Adds support for the Kudu DECIMAL type introduced in Kudu 1.7.0.

Change-Id: I2f927fce25dc4fa9529a4e0b688825699f5c0ea6
---
M be/src/exec/kudu-scanner.cc
M be/src/exec/kudu-table-sink.cc
M be/src/exec/kudu-util.cc
M be/src/exec/kudu-util.h
M be/src/exprs/kudu-partition-expr.cc
M fe/src/main/java/org/apache/impala/catalog/KuduColumn.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_create.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_delete.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_describe.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_insert.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_update.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_upsert.test
M tests/query_test/test_kudu.py
18 files changed, 689 insertions(+), 598 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2f927fce25dc4fa9529a4e0b688825699f5c0ea6
Gerrit-Change-Number: 9306
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Thomas Tauber-Marshall 

[Impala-ASF-CR] IMPALA-5269: Fix issue with final line of query followed by a comment

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9191 )

Change subject: IMPALA-5269: Fix issue with final line of query followed by a 
comment
..


Patch Set 8:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1935/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I54f9a8f65214023520eaa010fc462a663d02d258
Gerrit-Change-Number: 9191
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Wed, 14 Feb 2018 03:52:22 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5269: Fix issue with final line of query followed by a comment

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9191 )

Change subject: IMPALA-5269: Fix issue with final line of query followed by a 
comment
..


Patch Set 8: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I54f9a8f65214023520eaa010fc462a663d02d258
Gerrit-Change-Number: 9191
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Wed, 14 Feb 2018 03:51:38 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5269: Fix issue with final line of query followed by a comment

[Fredy Wijaya (Code Review)]

Fredy Wijaya has uploaded a new patch set (#8). ( 
http://gerrit.cloudera.org:8080/9191 )

Change subject: IMPALA-5269: Fix issue with final line of query followed by a 
comment
..

IMPALA-5269: Fix issue with final line of query followed by a comment

The patch is to remove any comments in a statement when checking if a
statement ends with a semicolon delimiter.

For example:

Before (semicolon delimiter is needed at the end):
select 1 + 1; -- comment\n;

After (semicolon delimiter is no longer needed):
select 1 + 1; -- comment

Testing:
- Ran end-to-end tests in shell

Change-Id: I54f9a8f65214023520eaa010fc462a663d02d258
---
M shell/impala_shell.py
M tests/shell/test_shell_interactive.py
2 files changed, 43 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/9191/8
--
To view, visit http://gerrit.cloudera.org:8080/9191
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I54f9a8f65214023520eaa010fc462a663d02d258
Gerrit-Change-Number: 9191
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Thomas Tauber-Marshall 

[Impala-ASF-CR] IMPALA-6517: bootstrap toolchain.py fails to recognize lsb release output from RHEL OS

[Vincent Tran (Code Review)]

Hello Philip Zeyliger,

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

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

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

Change subject: IMPALA-6517: bootstrap_toolchain.py fails to recognize 
lsb_release output from RHEL OS
..

IMPALA-6517: bootstrap_toolchain.py fails to recognize lsb_release
output from RHEL OS

The OS map that we currently use to check platform / release
against in bootstrap_toolchain.py does not contain key-value pairs
for:
lsb_release -irs
RedHatEnterpriseServer 6.9

This change adds RHEL5, RHEL6 and RHEL7 to the OS map.

Testing was done by cloning a repo locally and calling
bootstrap_toolchain.py
Testing was done against RHEL6, RHEL7, Ubuntu16.x and Centos7.x

Change-Id: I83874220bd424a452df49520b5dad7bfa2124ca6
---
M bin/bootstrap_toolchain.py
1 file changed, 7 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I83874220bd424a452df49520b5dad7bfa2124ca6
Gerrit-Change-Number: 9310
Gerrit-PatchSet: 3
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vincent Tran 

[Impala-ASF-CR] IMPALA-6517: bootstrap toolchain.py fails to recognize lsb release output from RHEL OS

[Vincent Tran (Code Review)]

Vincent Tran has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9310 )

Change subject: IMPALA-6517: bootstrap_toolchain.py fails to recognize 
lsb_release output from RHEL OS
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9310/1/bin/bootstrap_toolchain.py
File bin/bootstrap_toolchain.py:

http://gerrit.cloudera.org:8080/#/c/9310/1/bin/bootstrap_toolchain.py@63
PS1, Line 63:   "ubuntu12" : "ec2-package-ubuntu-12-04",
> This is problematic. Ubuntu has a 12.10 and 14.10 and 16.10 release per htt
Ah. I didn't realize that.
The way we mapped Ubuntu 14.04, 15.04, and 15.10 all to 
ec2-package-ubuntu-14-04 had me thinking that minor version was unimportant.

Fixed



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I83874220bd424a452df49520b5dad7bfa2124ca6
Gerrit-Change-Number: 9310
Gerrit-PatchSet: 1
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vincent Tran 
Gerrit-Comment-Date: Wed, 14 Feb 2018 03:01:59 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] KUDU-2296: Fix deserialization of messages larger than 64MB

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9313 )

Change subject: KUDU-2296: Fix deserialization of messages larger than 64MB
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57d3f3ca6ec0aa8be0e67e6a13c4b560c9d2c63a
Gerrit-Change-Number: 9313
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 14 Feb 2018 03:01:57 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5269: Fix issue with final line of query followed by a comment

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9191 )

Change subject: IMPALA-5269: Fix issue with final line of query followed by a 
comment
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9191/7/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/9191/7/tests/shell/test_shell_interactive.py@442
PS7, Line 442:   def test_line_ends_with_comment(self):
Looks like this test failed in the GVO build. Have you tried running it locally 
on your machine? Does it pass?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I54f9a8f65214023520eaa010fc462a663d02d258
Gerrit-Change-Number: 9191
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Wed, 14 Feb 2018 02:58:10 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5269: Fix issue with final line of query followed by a comment

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9191 )

Change subject: IMPALA-5269: Fix issue with final line of query followed by a 
comment
..


Patch Set 7: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1931/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I54f9a8f65214023520eaa010fc462a663d02d258
Gerrit-Change-Number: 9191
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Wed, 14 Feb 2018 02:49:04 +
Gerrit-HasComments: No

[Impala-ASF-CR] Use unqualified table refs in TPCH planner tests.

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9270 )

Change subject: Use unqualified table refs in TPCH planner tests.
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I886c451ab61a1739af96eeb765821dfd8e951b07
Gerrit-Change-Number: 9270
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Comment-Date: Wed, 14 Feb 2018 02:10:11 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6516: Log catalog update only if the catalog version changes

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9311 )

Change subject: IMPALA-6516: Log catalog update only if the catalog version 
changes
..


Patch Set 1:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1933/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04b8dd05c588d4cd91e9ca2251f8f66325bb45e2
Gerrit-Change-Number: 9311
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 14 Feb 2018 01:48:01 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6516: Log catalog update only if the catalog version changes

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9311 )

Change subject: IMPALA-6516: Log catalog update only if the catalog version 
changes
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04b8dd05c588d4cd91e9ca2251f8f66325bb45e2
Gerrit-Change-Number: 9311
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 14 Feb 2018 01:46:59 +
Gerrit-HasComments: No

[Impala-ASF-CR] [docs] Removed the obsolete Llama options files

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9219 )

Change subject: [docs] Removed the obsolete Llama options files
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c2b8160af9c95ec1e1b744b558d9537dd2550d
Gerrit-Change-Number: 9219
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 14 Feb 2018 01:21:35 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6497: add "Last row fetched" and AC events

[Tim Armstrong (Code Review)]

Hello Lars Volker, Bikramjeet Vig,

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

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

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

Change subject: IMPALA-6497: add "Last row fetched" and AC events
..

IMPALA-6497: add "Last row fetched" and AC events

This makes it more observable that all rows were returned to the client
and also that resources were released for admission control.

Testing:
Manually inspected some query profiles.

Added a basic observability test that ensures that the expected events
appear in the profile. Ran it in a loop for a bit to make sure it wasn't
flaky.

Change-Id: I32a707e4660061e75c86ad967f1fac6f6737da7e
---
M be/src/runtime/coordinator.cc
M tests/query_test/test_observability.py
2 files changed, 50 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/9271/7
--
To view, visit http://gerrit.cloudera.org:8080/9271
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I32a707e4660061e75c86ad967f1fac6f6737da7e
Gerrit-Change-Number: 9271
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6519: API to allocate unreserved buffer

[Tim Armstrong (Code Review)]

Hello Michael Ho, Dan Hecht,

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

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

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

Change subject: IMPALA-6519: API to allocate unreserved buffer
..

IMPALA-6519: API to allocate unreserved buffer

The motivation is to allow allocation of buffers without reservation in
ExchangeNode. Currently this it not possible because
IncreaseReservationToFit() followed by AllocateBuffer() is non-atomic.
We need to handle concurrent allocations in ExchangeNode because there
may be multiple batches being received at a given time.

Testing:
Added basic unit test.

Change-Id: Ia4d17b3db25491f796484de22405fbdee7a0f983
---
M be/src/runtime/bufferpool/buffer-pool-internal.h
M be/src/runtime/bufferpool/buffer-pool-test.cc
M be/src/runtime/bufferpool/buffer-pool.cc
M be/src/runtime/bufferpool/buffer-pool.h
M be/src/runtime/bufferpool/reservation-tracker.cc
M be/src/runtime/bufferpool/reservation-tracker.h
6 files changed, 106 insertions(+), 23 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia4d17b3db25491f796484de22405fbdee7a0f983
Gerrit-Change-Number: 9250
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6519: API to allocate unreserved buffer

[Tim Armstrong (Code Review)]

Hello Michael Ho, Dan Hecht,

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

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

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

Change subject: IMPALA-6519: API to allocate unreserved buffer
..

IMPALA-6519: API to allocate unreserved buffer

Testing:
Added basic unit test.

Change-Id: Ia4d17b3db25491f796484de22405fbdee7a0f983
---
M be/src/runtime/bufferpool/buffer-pool-internal.h
M be/src/runtime/bufferpool/buffer-pool-test.cc
M be/src/runtime/bufferpool/buffer-pool.cc
M be/src/runtime/bufferpool/buffer-pool.h
M be/src/runtime/bufferpool/reservation-tracker.cc
M be/src/runtime/bufferpool/reservation-tracker.h
6 files changed, 106 insertions(+), 23 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia4d17b3db25491f796484de22405fbdee7a0f983
Gerrit-Change-Number: 9250
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6519: API to allocate unreserved buffer

[Tim Armstrong (Code Review)]

Hello Michael Ho, Dan Hecht,

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

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

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

Change subject: IMPALA-6519: API to allocate unreserved buffer
..

IMPALA-6519: API to allocate unreserved buffer

Testing:
Added basic unit test.

Change-Id: Ia4d17b3db25491f796484de22405fbdee7a0f983
---
M be/src/runtime/bufferpool/buffer-pool-internal.h
M be/src/runtime/bufferpool/buffer-pool-test.cc
M be/src/runtime/bufferpool/buffer-pool.cc
M be/src/runtime/bufferpool/buffer-pool.h
M be/src/runtime/bufferpool/reservation-tracker.cc
M be/src/runtime/bufferpool/reservation-tracker.h
6 files changed, 107 insertions(+), 23 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia4d17b3db25491f796484de22405fbdee7a0f983
Gerrit-Change-Number: 9250
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6516: Log catalog update only if the catalog version changes

[anujphadke (Code Review)]

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9311 )

Change subject: IMPALA-6516: Log catalog update only if the catalog version 
changes
..


Patch Set 1: Code-Review+1

Thanks for fixing this. My impalad.INFO was flooded with these messages.
update applied with version: 1095 new min catalog object version: 2
I0213 15:29:33.942580  9332 impala-server.cc:1354] Catalog topic update applied 
with version: 1095 new min catalog object version: 2
I0213 15:29:35.944761  9332 impala-server.cc:1354] Catalog topic update applied 
with version: 1095 new min catalog object version: 2
I0213 15:29:37.946624  9332 impala-server.cc:1354] Catalog topic update applied 
with version: 1095 new min catalog object version: 2
I0213 15:29:39.948545  9332 impala-server.cc:1354] Catalog topic update applied 
with version: 1095 new min catalog object version: 2
I0213 15:29:41.950410  9332 impala-server.cc:1354] Catalog topic update applied 
with version: 1095 new min catalog object version: 2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04b8dd05c588d4cd91e9ca2251f8f66325bb45e2
Gerrit-Change-Number: 9311
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 14 Feb 2018 00:28:38 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5690: Part 1: Rename ostream operators for thrift types

[Tianyi Wang (Code Review)]

Tianyi Wang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9168 )

Change subject: IMPALA-5690: Part 1: Rename ostream operators for thrift types
..


Patch Set 8: Code-Review-1

part1+2 passed core tests. part1 failed the scanner fuzz test. I'm looking into 
it.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9c303997411237e988ef960157f781776f6fcb60
Gerrit-Change-Number: 9168
Gerrit-PatchSet: 8
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Wed, 14 Feb 2018 00:26:35 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6516: Log catalog update only if the catalog version changes

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9311 )

Change subject: IMPALA-6516: Log catalog update only if the catalog version 
changes
..


Patch Set 1: Code-Review+2

(1 comment)

Seems simple enough that I can +2 it. Please feel free to have someone more 
familiar with catalog to take a look too before merging.

http://gerrit.cloudera.org:8080/#/c/9311/1/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/9311/1/be/src/service/impala-server.cc@1350
PS1, Line 1350: catalog_update_info_.catalog_version != resp.new_catalog_version
> The change of min_catalog_object_version may have nothing to do with the st
Talked to Dimitris about what min_catalog_object_version is about. Makes sense.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04b8dd05c588d4cd91e9ca2251f8f66325bb45e2
Gerrit-Change-Number: 9311
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Wed, 14 Feb 2018 00:17:57 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6497: add "Last row fetched" and AC events

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9271 )

Change subject: IMPALA-6497: add "Last row fetched" and AC events
..


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9271/5/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/9271/5/tests/query_test/test_observability.py@206
PS5, Line 206: event_regexes = [r'Fragment Instance Lifecycle Event 
Timeline',
> I figured it was a convention to use raw strings for regexes in python. I t
That makes sense, thanks for the explanation.


http://gerrit.cloudera.org:8080/#/c/9271/5/tests/query_test/test_observability.py@209
PS5, Line 209:  r'First Batch Sent',
> Nice catch.
IMPALA-6511 has been fixed so this list should now include "Open Finished" 
after "Prepare Finished".



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32a707e4660061e75c86ad967f1fac6f6737da7e
Gerrit-Change-Number: 9271
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 14 Feb 2018 00:05:24 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4953,IMPALA-6437: separate AC/scheduler from catalog topic updates

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9123 )

Change subject: IMPALA-4953,IMPALA-6437: separate AC/scheduler from catalog 
topic updates
..


Patch Set 15:

I was able to borrow a 140 node cluster and ran a workload with a few streams 
of concurrent queries. I looked at the statestored topics page, impalad and 
statestored metrics, "top" and "perf top" to make sure that resource 
consumption was as expected. Things looks good. The statestore does remain 
active consuming a moderate amount of CPU just to poll the 140 subscribers for 
updates every 100ms, but that is expected. One thing that is interesting is 
that the request-queue topic updates very frequently when running without 
mem_limits, because it tracks the actual memory consumption in that case, which 
tends to fluctuate a lot. Detailed notes follow.

On an idle 140 node cluster, statestored consumes ~40% cpu and ~500kb/s 
network. Perf shows that time is mainly spent checking topic versions.


 9.19%  impalad   [.] 
impala::Statestore::GetMinSubscriberTopicVersion(std::string const&, 
std::string*)
   7.50%  [kernel]  [k] find_busiest_group
   4.47%  impalad   [.] 
_ZN5boost9unordered6detail12mix64_policyImE10apply_hashINS_4hashISsEESsEEmRKT_RKT0_.isra.271
   1.82%  [kernel]  [k] find_next_bit
   1.69%  impalad   [.] 
impala::Statestore::Subscriber::LastTopicVersionProcessed(std::string const&) 
const
   1.68%  [kernel]  [k] _spin_lock
   1.27%  libc-2.12.so  [.] __memcmp_sse4_1
   1.25%  [kernel]  [k] cpumask_next_and
   0.98%  [kernel]  [k] thread_return
   0.80%  [kernel]  [k] smaps_pte_entry
   0.74%  libjvm.so [.] 
GenericTaskQueueSet, 64ul>, (unsigned short
   0.70%  [kernel]  [k] schedule
   0.60%  libc-2.12.so  [.] memcpy
   0.54%  [kernel]  [k] _spin_lock_irqsave
   0.54%  [kernel]  [k] ixgbe_poll

If I ran a light workload of queries with no mem_limits, the request-queue 
topic updated frequently with the current memory consumption and statestored 
CPU consumption increased to 50-60% with a light workload of queries with no 
mem_limits.  If I set a default pool mem_limit the topic version only 
increments rarely and there is no noticeable increase in load.

On an idle cluster, prioritized updates were delivered in a timely manner and 
took minimal time to process. Snapshot of metrics from an Impalad while idle 
after running some queries:

statestore-subscriber.connected trueWhether the Impala Daemon 
considers itself connected to the StateStore.
statestore-subscriber.last-recovery-timeN/A The local time that the 
last statestore recovery happened.
statestore-subscriber.topic-update-interval-timeLast (of 67983): 
0.100672. Min: 0, max: 2.43858, avg: 0.144938  The time (sec) between 
Statestore subscriber topic updates.
statestore-subscriber.topic-update-duration Last (of 34783): 
0.000256518. Min: 0, max: 1.06605, avg: 0.00530736 The time (sec) taken to 
process Statestore subscriber topic updates.
statestore-subscriber.heartbeat-interval-time   Last (of 3344): 
1.1. Min: 0, max: 1.82638, avg: 1.00039 The time (sec) between 
Statestore heartbeats.
statestore-subscriber.topic-impala-request-queue.update-intervalLast 
(of 33200): 0.100672. Min: 0, max: 2.43848, avg: 0.100697  Interval between 
topic updates for Topic impala-request-queue
statestore-subscriber.topic-impala-membership.update-interval   Last 
(of 33200): 0.100671. Min: 0, max: 2.43858, avg: 0.100696  Interval between 
topic updates for Topic impala-membership
statestore-subscriber.topic-impala-request-queue.processing-time-s  Last 
(of 33200): 0.000254981. Min: 0, max: 0.0113791, avg: 0.000155621  Statestore 
Subscriber Topic impala-request-queue Processing Time
statestore-subscriber.topic-impala-membership.processing-time-s Last 
(of 33200): 0.000177222. Min: 0, max: 0.0113666, avg: 0.000120381  Statestore 
Subscriber Topic impala-membership Processing Time
statestore-subscriber.topic-catalog-update.update-interval  Last (of 1583): 
2.0009. Min: 1, max: 2.20628, avg: 2.00069  Interval between topic updates 
for Topic catalog-update
statestore-subscriber.topic-catalog-update.processing-time-sLast 
(of 1583): 0.136297. Min: 0, max: 1.06605, avg: 0.113322   Statestore 
Subscriber Topic catalog-update Processing Time

[Impala-ASF-CR] IMPALA-6511: Fix state machine in FIS::UpdateState()

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9294 )

Change subject: IMPALA-6511: Fix state machine in FIS::UpdateState()
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7304205d245289cc3d7ca2217e212c844ee75e7b
Gerrit-Change-Number: 9294
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 13 Feb 2018 23:44:13 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6511: Fix state machine in FIS::UpdateState()

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9294 )

Change subject: IMPALA-6511: Fix state machine in FIS::UpdateState()
..

IMPALA-6511: Fix state machine in FIS::UpdateState()

LAST_BATCH_SENT must always happen in state PRODUCING_DATA. This change
also marks "Open Finished" when receiving WAITING_FOR_FIRST_BATCH.

Change-Id: I7304205d245289cc3d7ca2217e212c844ee75e7b
Reviewed-on: http://gerrit.cloudera.org:8080/9294
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins
---
M be/src/runtime/fragment-instance-state.cc
M tests/query_test/test_observability.py
2 files changed, 3 insertions(+), 5 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7304205d245289cc3d7ca2217e212c844ee75e7b
Gerrit-Change-Number: 9294
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] KUDU-2296: Fix deserialization of messages larger than 64MB

[Joe McDonnell (Code Review)]

Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9313 )

Change subject: KUDU-2296: Fix deserialization of messages larger than 64MB
..


Patch Set 2: Code-Review+2

Carry +2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57d3f3ca6ec0aa8be0e67e6a13c4b560c9d2c63a
Gerrit-Change-Number: 9313
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 13 Feb 2018 23:18:02 +
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2296: Fix deserialization of messages larger than 64MB

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9313 )

Change subject: KUDU-2296: Fix deserialization of messages larger than 64MB
..


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1932/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57d3f3ca6ec0aa8be0e67e6a13c4b560c9d2c63a
Gerrit-Change-Number: 9313
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 13 Feb 2018 23:18:35 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5690: Part 1: Rename ostream operators for thrift types

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9168 )

Change subject: IMPALA-5690: Part 1: Rename ostream operators for thrift types
..


Patch Set 8: Code-Review+1

(2 comments)

> Thrift is still changing this operator<< in 0.11, adding enum
 > types. If we patch it now we may need to rework it on every thrift
 > upgrade. Generally I think it's unnecessary to patch a library only
 > for a name conflict.

Works for me.

http://gerrit.cloudera.org:8080/#/c/9168/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9168/8//COMMIT_MSG@13
PS8, Line 13:
Since this touches a lot of stuff, have you done a full test run with it?


http://gerrit.cloudera.org:8080/#/c/9168/8/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:

http://gerrit.cloudera.org:8080/#/c/9168/8/be/src/runtime/coordinator-backend-state.cc@64
PS8, Line 64: DCHECK(host_ == instance_params->host);  // all hosts must be 
the same
> DCHECK_EQ uses operator << to print the value of lhs and rhs. Operator << i
Could you mention this in the commit message?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9c303997411237e988ef960157f781776f6fcb60
Gerrit-Change-Number: 9168
Gerrit-PatchSet: 8
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Tue, 13 Feb 2018 23:03:41 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] KUDU-2296: Fix deserialization of messages larger than 64MB

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9313 )

Change subject: KUDU-2296: Fix deserialization of messages larger than 64MB
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57d3f3ca6ec0aa8be0e67e6a13c4b560c9d2c63a
Gerrit-Change-Number: 9313
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 13 Feb 2018 22:51:19 +
Gerrit-HasComments: No

[native-toolchain-CR](cdh6.x) Quiet down file transfers, rename target platform variable

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9315 )

Change subject: Quiet down file transfers, rename target platform variable
..

Quiet down file transfers, rename target platform variable

1. File download with wget and maven upload all have verbose progress
indicators. This increases noise in the build logs making it hard
to navigate to relevant details.

This patch:
- changes wget to use a less chatty progress bar
- changes maven upload to batch mode and bumps log level to warnings
  or worse
- tells "aws s3 cp" to report errors only, adding an explicit echo to
  report file copy source and estination.

2. When the toolchain is built in an automated environment (e.g. using
a Jenkins job), the build target platform identifier is communicated
in an environment variable. This patch renames this variable to
BUILD_TARGET_LABEL to untangle it from Jenkins-defined variables and
to make its role more explicit.

Tested by rebuilding the toolchain with the new script, using
a conforming Jenkins job; all artifacts were prodiced with correct
package name suffixes. Results are stored at:
s3://impala-toolchain-test/build/33-a25ad7b25c/

Change-Id: Iaa4349a8ae0ab5bde2dabf5ff65db0f2db4b9ba9
---
M functions.sh
M init.sh
2 files changed, 17 insertions(+), 10 deletions(-)

Approvals:
  Thomas Tauber-Marshall: Looks good to me, approved; Verified

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

Gerrit-Project: native-toolchain
Gerrit-Branch: cdh6.x
Gerrit-MessageType: merged
Gerrit-Change-Id: Iaa4349a8ae0ab5bde2dabf5ff65db0f2db4b9ba9
Gerrit-Change-Number: 9315
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Thomas Tauber-Marshall 

[native-toolchain-CR](cdh6.x) Quiet down file transfers, rename target platform variable

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9315 )

Change subject: Quiet down file transfers, rename target platform variable
..


Patch Set 1: Verified+1


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

Gerrit-Project: native-toolchain
Gerrit-Branch: cdh6.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa4349a8ae0ab5bde2dabf5ff65db0f2db4b9ba9
Gerrit-Change-Number: 9315
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Tue, 13 Feb 2018 22:50:33 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6516: Log catalog update only if the catalog version changes

[Tianyi Wang (Code Review)]

Tianyi Wang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9311 )

Change subject: IMPALA-6516: Log catalog update only if the catalog version 
changes
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9311/1/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/9311/1/be/src/service/impala-server.cc@1350
PS1, Line 1350: catalog_update_info_.catalog_version != resp.new_catalog_version
> Do we also care about change in min_catalog_object_version ?
The change of min_catalog_object_version may have nothing to do with the 
statestore update so I think we don't care specifically.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04b8dd05c588d4cd91e9ca2251f8f66325bb45e2
Gerrit-Change-Number: 9311
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Tue, 13 Feb 2018 22:50:33 +
Gerrit-HasComments: Yes

[native-toolchain-CR](cdh6.x) Quiet down file transfers, rename target platform variable

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9315


Change subject: Quiet down file transfers, rename target platform variable
..

Quiet down file transfers, rename target platform variable

1. File download with wget and maven upload all have verbose progress
indicators. This increases noise in the build logs making it hard
to navigate to relevant details.

This patch:
- changes wget to use a less chatty progress bar
- changes maven upload to batch mode and bumps log level to warnings
  or worse
- tells "aws s3 cp" to report errors only, adding an explicit echo to
  report file copy source and estination.

2. When the toolchain is built in an automated environment (e.g. using
a Jenkins job), the build target platform identifier is communicated
in an environment variable. This patch renames this variable to
BUILD_TARGET_LABEL to untangle it from Jenkins-defined variables and
to make its role more explicit.

Tested by rebuilding the toolchain with the new script, using
a conforming Jenkins job; all artifacts were prodiced with correct
package name suffixes. Results are stored at:
s3://impala-toolchain-test/build/33-a25ad7b25c/

Change-Id: Iaa4349a8ae0ab5bde2dabf5ff65db0f2db4b9ba9
---
M functions.sh
M init.sh
2 files changed, 17 insertions(+), 10 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/native-toolchain 
refs/changes/15/9315/1
--
To view, visit http://gerrit.cloudera.org:8080/9315
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: native-toolchain
Gerrit-Branch: cdh6.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iaa4349a8ae0ab5bde2dabf5ff65db0f2db4b9ba9
Gerrit-Change-Number: 9315
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Laszlo Gaal 

[native-toolchain-CR](cdh5.x) Quiet down file transfers, rename target platform variable

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9314 )

Change subject: Quiet down file transfers, rename target platform variable
..

Quiet down file transfers, rename target platform variable

1. File download with wget and maven upload all have verbose progress
indicators. This increases noise in the build logs making it hard
to navigate to relevant details.

This patch:
- changes wget to use a less chatty progress bar
- changes maven upload to batch mode and bumps log level to warnings
  or worse
- tells "aws s3 cp" to report errors only, adding an explicit echo to
  report file copy source and estination.

2. When the toolchain is built in an automated environment (e.g. using
a Jenkins job), the build target platform identifier is communicated
in an environment variable. This patch renames this variable to
BUILD_TARGET_LABEL to untangle it from Jenkins-defined variables and
to make its role more explicit.

Tested by rebuilding the toolchain with the new script, using
a conforming Jenkins job; all artifacts were prodiced with correct
package name suffixes. Results are stored at:
s3://impala-toolchain-test/build/33-a25ad7b25c/

Change-Id: Iaa4349a8ae0ab5bde2dabf5ff65db0f2db4b9ba9
---
M functions.sh
M init.sh
2 files changed, 17 insertions(+), 10 deletions(-)

Approvals:
  Thomas Tauber-Marshall: Looks good to me, approved; Verified

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

Gerrit-Project: native-toolchain
Gerrit-Branch: cdh5.x
Gerrit-MessageType: merged
Gerrit-Change-Id: Iaa4349a8ae0ab5bde2dabf5ff65db0f2db4b9ba9
Gerrit-Change-Number: 9314
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Thomas Tauber-Marshall 

[native-toolchain-CR](cdh5.x) Quiet down file transfers, rename target platform variable

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9314 )

Change subject: Quiet down file transfers, rename target platform variable
..


Patch Set 1: Code-Review+2

Clean cherry-pick


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

Gerrit-Project: native-toolchain
Gerrit-Branch: cdh5.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa4349a8ae0ab5bde2dabf5ff65db0f2db4b9ba9
Gerrit-Change-Number: 9314
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Tue, 13 Feb 2018 22:49:26 +
Gerrit-HasComments: No

[native-toolchain-CR](cdh5.x) Quiet down file transfers, rename target platform variable

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9314 )

Change subject: Quiet down file transfers, rename target platform variable
..


Patch Set 1: Verified+1


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

Gerrit-Project: native-toolchain
Gerrit-Branch: cdh5.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa4349a8ae0ab5bde2dabf5ff65db0f2db4b9ba9
Gerrit-Change-Number: 9314
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Tue, 13 Feb 2018 22:49:28 +
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2296: Fix deserialization of messages larger than 64MB

[Joe McDonnell (Code Review)]

Joe McDonnell has removed Kudu Jenkins from this change.  ( 
http://gerrit.cloudera.org:8080/9313 )

Change subject: KUDU-2296: Fix deserialization of messages larger than 64MB
..


Removed reviewer Kudu Jenkins.
--
To view, visit http://gerrit.cloudera.org:8080/9313
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I57d3f3ca6ec0aa8be0e67e6a13c4b560c9d2c63a
Gerrit-Change-Number: 9313
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Todd Lipcon 

[Impala-ASF-CR] KUDU-2296: Fix deserialization of messages larger than 64MB

[Joe McDonnell (Code Review)]

Hello Kudu Jenkins, Todd Lipcon,

I'd like you to do a code review. Please visit

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

to review the following change.


Change subject: KUDU-2296: Fix deserialization of messages larger than 64MB
..

KUDU-2296: Fix deserialization of messages larger than 64MB

Protobuf's CodedInputStream has a 64MB total byte limit by
default. When trying to deserialize messages larger than
this, ParseMessage() hits this limit and mistakenly
think that the packet is too short. This issue is dormant
due to Kudu's default rpc_max_message_size of 50MB.
However, Impala will be using a larger value for
rpc_max_message_size and requires this fix.

The fix is to override the default 64MB limit by calling
CodedInputStream::SetTotalByteLimit() with the buffer's
size.

Change-Id: I57d3f3ca6ec0aa8be0e67e6a13c4b560c9d2c63a
Reviewed-on: http://gerrit.cloudera.org:8080/9312
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M be/src/kudu/rpc/serialization.cc
1 file changed, 4 insertions(+), 0 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I57d3f3ca6ec0aa8be0e67e6a13c4b560c9d2c63a
Gerrit-Change-Number: 9313
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[Impala-ASF-CR] KUDU-2296: Fix deserialization of messages larger than 64MB

[Joe McDonnell (Code Review)]

Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9313 )

Change subject: KUDU-2296: Fix deserialization of messages larger than 64MB
..


Patch Set 1:

Clean cherry-pick


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57d3f3ca6ec0aa8be0e67e6a13c4b560c9d2c63a
Gerrit-Change-Number: 9313
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 13 Feb 2018 22:46:17 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5139: Update mvn-quiet.sh to print execution content to log file

[Nithya Janarthanan (Code Review)]

Nithya Janarthanan has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/9273 )

Change subject: IMPALA-5139: Update mvn-quiet.sh to print execution content to 
log file
..

IMPALA-5139: Update mvn-quiet.sh to print execution content to log file

Address David's review comment

remove white space

Update comments

code and comments cleanup mvn-quiet.sh

Change-Id: I475b17a4dccfa624dda61402491b461c53473f8b
---
M bin/impala-config.sh
M bin/mvn-quiet.sh
2 files changed, 15 insertions(+), 8 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I475b17a4dccfa624dda61402491b461c53473f8b
Gerrit-Change-Number: 9273
Gerrit-PatchSet: 5
Gerrit-Owner: Nithya Janarthanan 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 

[Impala-ASF-CR] IMPALA-4835: Part 2: Allocate scan range buffers upfront

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8707 )

Change subject: IMPALA-4835: Part 2: Allocate scan range buffers upfront
..


Patch Set 24:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8707/24/be/src/runtime/io/disk-io-mgr.h
File be/src/runtime/io/disk-io-mgr.h:

http://gerrit.cloudera.org:8080/#/c/8707/24/be/src/runtime/io/disk-io-mgr.h@308
PS24, Line 308:   int64_t max_bytes);
> I'm not arguing for one way or the other at this point. I'm just trying to
The code before this patch limited the number of queued buffers rather than the 
total number of buffers. I.e. it didn't factor in buffers in the client so 
couldn't control total reservation consumption. So there are some changes 
needed to correctly account for that and unblock the ranges in ReturnBuffer() 
instead of GetNext().

Giving each scanner a reservation for 3 * buffer size doesn't work for columnar 
formats because there's a scan range per column rather than per scanner. The 
commit message for Part 3 summarises the challenge there. Given a reservation 
of 3 * 8MB * num columns would result in really huge reservations for wide 
parquet files. We want to reserve less than that to avoid bad regressions. That 
then results in some interesting cases where we have to divide some amount of 
memory between many irregular-sized small scan ranges for the different Parquet 
columns where we want to use the available reservation efficiently.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia243bf8b62feeea602b122e0503ea4ba7d3ee70f
Gerrit-Change-Number: 8707
Gerrit-PatchSet: 24
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 13 Feb 2018 22:34:57 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] Bump Kudu version to b315d0e

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has abandoned this change. ( 
http://gerrit.cloudera.org:8080/9217 )

Change subject: Bump Kudu version to b315d0e
..


Abandoned

Forgot about this, and we're going to bump the version again to pull in the 
DECIMAL changes
--
To view, visit http://gerrit.cloudera.org:8080/9217
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Ida77c208aa575a9fa644c3acd01adeea08b5ebcf
Gerrit-Change-Number: 9217
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 

[Impala-ASF-CR] IMPALA-6516: Log catalog update only if the catalog version changes

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9311 )

Change subject: IMPALA-6516: Log catalog update only if the catalog version 
changes
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9311/1/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/9311/1/be/src/service/impala-server.cc@1350
PS1, Line 1350: catalog_update_info_.catalog_version != resp.new_catalog_version
Do we also care about change in min_catalog_object_version ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04b8dd05c588d4cd91e9ca2251f8f66325bb45e2
Gerrit-Change-Number: 9311
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Tue, 13 Feb 2018 22:29:38 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5690: Part 1: Rename ostream operators for thrift types

[Tianyi Wang (Code Review)]

Tianyi Wang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9168 )

Change subject: IMPALA-5690: Part 1: Rename ostream operators for thrift types
..


Patch Set 8:

(1 comment)

> Patch Set 8:
>
> (1 comment)
>
> > > Patch Set 5:
>  > >
>  > > Since this is a very large, complicated patch, is it possible to
>  > break it up to make it easier to review?
>  > >
>  > > In particular, I think that all of the changes around '<<' could
>  > be done in an initial patch, and then have a followup patch that
>  > contains the actual version bump and other things.
>  >
>  > I've split it. The part 2 is at https://gerrit.cloudera.org/c/9300/.
>
> Thanks for breaking this up. Much easier to wrap my head around now.
>
> What was the reason for not going with Henry's suggestion on the JIRA to 
> patch thrift not to emit '<<'?

Thrift is still changing this operator<< in 0.11, adding enum types. If we 
patch it now we may need to rework it on every thrift upgrade. Generally I 
think it's unnecessary to patch a library only for a name conflict.

http://gerrit.cloudera.org:8080/#/c/9168/8/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:

http://gerrit.cloudera.org:8080/#/c/9168/8/be/src/runtime/coordinator-backend-state.cc@64
PS8, Line 64: DCHECK(host_ == instance_params->host);  // all hosts must be 
the same
> Why was thing change made?
DCHECK_EQ uses operator << to print the value of lhs and rhs. Operator << is 
removed and the error message won't include the values after this change. If it 
matters I can do it manually. Another option is to revert this line in part 2, 
using the operator << defined by thrift.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9c303997411237e988ef960157f781776f6fcb60
Gerrit-Change-Number: 9168
Gerrit-PatchSet: 8
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Tue, 13 Feb 2018 22:09:10 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3193: Show table's comment on show tables

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8851 )

Change subject: IMPALA-3193: Show table's comment on show tables
..


Patch Set 17: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1929/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I44f814af05db6f3c027718ade9f474f8b8153bcd
Gerrit-Change-Number: 8851
Gerrit-PatchSet: 17
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Comment-Date: Tue, 13 Feb 2018 22:00:34 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4835: Part 2: Allocate scan range buffers upfront

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8707 )

Change subject: IMPALA-4835: Part 2: Allocate scan range buffers upfront
..


Patch Set 24:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8707/24/be/src/runtime/io/disk-io-mgr.h
File be/src/runtime/io/disk-io-mgr.h:

http://gerrit.cloudera.org:8080/#/c/8707/24/be/src/runtime/io/disk-io-mgr.h@308
PS24, Line 308:   int64_t max_bytes);
> To guarantee progress, the scanner definitely has to have enough reservatio
I'm not arguing for one way or the other at this point. I'm just trying to 
understand the change and why various parts are the way they are.

Regarding about still have the same logic for block/unblock ranges:
Why is that? Doesn't the existing code already limit each range to 3 buffers? 
i.e. why isn't it sufficient to just say that each scanner's reservation is 3 * 
buffer_size?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia243bf8b62feeea602b122e0503ea4ba7d3ee70f
Gerrit-Change-Number: 8707
Gerrit-PatchSet: 24
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 13 Feb 2018 21:51:52 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5690: Part 1: Rename ostream operators for thrift types

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9168 )

Change subject: IMPALA-5690: Part 1: Rename ostream operators for thrift types
..


Patch Set 8:

(1 comment)

> > Patch Set 5:
 > >
 > > Since this is a very large, complicated patch, is it possible to
 > break it up to make it easier to review?
 > >
 > > In particular, I think that all of the changes around '<<' could
 > be done in an initial patch, and then have a followup patch that
 > contains the actual version bump and other things.
 >
 > I've split it. The part 2 is at https://gerrit.cloudera.org/c/9300/.

Thanks for breaking this up. Much easier to wrap my head around now.

What was the reason for not going with Henry's suggestion on the JIRA to patch 
thrift not to emit '<<'?

http://gerrit.cloudera.org:8080/#/c/9168/8/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:

http://gerrit.cloudera.org:8080/#/c/9168/8/be/src/runtime/coordinator-backend-state.cc@64
PS8, Line 64: DCHECK(host_ == instance_params->host);  // all hosts must be 
the same
Why was thing change made?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9c303997411237e988ef960157f781776f6fcb60
Gerrit-Change-Number: 9168
Gerrit-PatchSet: 8
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Tue, 13 Feb 2018 21:44:34 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6517: bootstrap toolchain.py fails to recognize lsb release output from RHEL OS

[Vincent Tran (Code Review)]

Vincent Tran has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9310


Change subject: IMPALA-6517: bootstrap_toolchain.py fails to recognize 
lsb_release output from RHEL OS
..

IMPALA-6517: bootstrap_toolchain.py fails to recognize lsb_release
output from RHEL OS

The OS map that we currently use to check platform / release
against in bootstrap_toolchain.py does not contain key-value pairs
for:
lsb_release -irs
RedHatEnterpriseServer 6.9

This change adds RHEL5, RHEL6 and RHEL7  to the list and
relaxes the matching rule to only check the platform's major
release.

Testing was done by cloning a repo locally and calling
bootstrap_toolchain.py
Testing was done against RHEL6, RHEL7, Ubuntu16.x and Centos7.x

Change-Id: I83874220bd424a452df49520b5dad7bfa2124ca6
---
M bin/bootstrap_toolchain.py
1 file changed, 11 insertions(+), 7 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I83874220bd424a452df49520b5dad7bfa2124ca6
Gerrit-Change-Number: 9310
Gerrit-PatchSet: 1
Gerrit-Owner: Vincent Tran 

[Impala-ASF-CR] IMPALA-5269: Fix issue with final line of query followed by a comment

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9191 )

Change subject: IMPALA-5269: Fix issue with final line of query followed by a 
comment
..


Patch Set 7: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I54f9a8f65214023520eaa010fc462a663d02d258
Gerrit-Change-Number: 9191
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Tue, 13 Feb 2018 21:16:51 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5269: Fix issue with final line of query followed by a comment

[Fredy Wijaya (Code Review)]

Fredy Wijaya has uploaded a new patch set (#7). ( 
http://gerrit.cloudera.org:8080/9191 )

Change subject: IMPALA-5269: Fix issue with final line of query followed by a 
comment
..

IMPALA-5269: Fix issue with final line of query followed by a comment

The patch is to remove any comments in a statement when checking if a
statement ends with a semicolon delimiter.

For example:

Before (semicolon delimiter is needed at the end):
select 1 + 1; -- comment\n;

After (semicolon delimiter is no longer needed):
select 1 + 1; -- comment

Testing:
- Ran end-to-end tests in shell

Change-Id: I54f9a8f65214023520eaa010fc462a663d02d258
---
M shell/impala_shell.py
M tests/shell/test_shell_interactive.py
2 files changed, 43 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/9191/7
--
To view, visit http://gerrit.cloudera.org:8080/9191
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I54f9a8f65214023520eaa010fc462a663d02d258
Gerrit-Change-Number: 9191
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Thomas Tauber-Marshall 

[Impala-ASF-CR] IMPALA-6479: Update DESCRIBE to respect column privileges

[Adam Holley (Code Review)]

Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9276 )

Change subject: IMPALA-6479: Update DESCRIBE to respect column privileges
..


Patch Set 1:

(1 comment)

Quick clarification.

http://gerrit.cloudera.org:8080/#/c/9276/1/common/thrift/Frontend.thrift
File common/thrift/Frontend.thrift:

http://gerrit.cloudera.org:8080/#/c/9276/1/common/thrift/Frontend.thrift@173
PS1, Line 173:   4: optional ImpalaInternalService.TSessionState session
> I don't think this is needed.
Since it's not the only parameter, if I need to pass the session through the 
JNI call, doesn't it need to be added? getTableName and getDbs both have it as 
parameters.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic96ae184fccdc88ba970b5adcd501da1966accb9
Gerrit-Change-Number: 9276
Gerrit-PatchSet: 1
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Comment-Date: Tue, 13 Feb 2018 21:10:58 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5139: Update mvn-quiet.sh to print execution content to log file

[Nithya Janarthanan (Code Review)]

Nithya Janarthanan has abandoned this change. ( 
http://gerrit.cloudera.org:8080/9308 )

Change subject: IMPALA-5139: Update mvn-quiet.sh to print execution content to 
log file
..


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I83034a879643f74cfab141a50fba22e5fb07ab17
Gerrit-Change-Number: 9308
Gerrit-PatchSet: 1
Gerrit-Owner: Nithya Janarthanan 

[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics

[Alex Behm (Code Review)]

Alex Behm has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9223 )

Change subject: IMPALA-6392: Consistent explain format for parquet predicate 
statistics
..

IMPALA-6392: Consistent explain format for parquet predicate statistics

In EXPLAIN_LEVEL=2+, change the explain format for parquet predicate
statistics to output each tuple descriptor per line. This change is to
make it consistent with the output of other predicates.

Before:
parquet statistics predicates: c_custkey < 10, o_orderkey < 5, l_linenumber < 3

After:
parquet statistics predicates: c_custkey < 10
parquet statistics predicates on o: o_orderkey < 5
parquet statistics predicates on o_lineitems: l_linenumber < 3

Testing:
- Ran existing planner tests and updated the ones that are affected by
  this change.
- Ran end-to-end tests in query_test

Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Reviewed-on: http://gerrit.cloudera.org:8080/9223
Tested-by: Impala Public Jenkins
Reviewed-by: Alex Behm 
---
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
4 files changed, 51 insertions(+), 18 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 14
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9223 )

Change subject: IMPALA-6392: Consistent explain format for parquet predicate 
statistics
..


Patch Set 13: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 13
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 13 Feb 2018 21:02:15 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9223 )

Change subject: IMPALA-6392: Consistent explain format for parquet predicate 
statistics
..


Patch Set 13: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 13
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 13 Feb 2018 21:00:13 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5139: Update mvn-quiet.sh to print execution content to log file

[Nithya Janarthanan (Code Review)]

Nithya Janarthanan has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9308


Change subject: IMPALA-5139: Update mvn-quiet.sh to print execution content to 
log file
..

IMPALA-5139: Update mvn-quiet.sh to print execution content to log file

Change-Id: I475b17a4dccfa624dda61402491b461c53473f8b

Address David's review comment

remove white space

Update comments

code and comments cleanup mvn-quiet.sh

Change-Id: I83034a879643f74cfab141a50fba22e5fb07ab17
---
M bin/impala-config.sh
M bin/mvn-quiet.sh
2 files changed, 15 insertions(+), 8 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I83034a879643f74cfab141a50fba22e5fb07ab17
Gerrit-Change-Number: 9308
Gerrit-PatchSet: 1
Gerrit-Owner: Nithya Janarthanan 

[Impala-ASF-CR] IMPALA-5269: Fix issue with final line of query followed by a comment

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9191 )

Change subject: IMPALA-5269: Fix issue with final line of query followed by a 
comment
..


Patch Set 6: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9191/6/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/9191/6/shell/impala_shell.py@391
PS6, Line 391:  make this statement
nit: "Strip any comments to make a statement such as the following be 
considered as ending with a delimiter: "



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I54f9a8f65214023520eaa010fc462a663d02d258
Gerrit-Change-Number: 9191
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Tue, 13 Feb 2018 20:58:14 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5139: Update mvn-quiet.sh to print execution content to log file

[Nithya Janarthanan (Code Review)]

Nithya Janarthanan has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/9273 )

Change subject: IMPALA-5139: Update mvn-quiet.sh to print execution content to 
log file
..

IMPALA-5139: Update mvn-quiet.sh to print execution content to log file

Change-Id: I475b17a4dccfa624dda61402491b461c53473f8b

Address David's review comment

Change-Id: I475b17a4dccfa624dda61402491b461c53473f8b
---
M bin/impala-config.sh
M bin/mvn-quiet.sh
2 files changed, 16 insertions(+), 8 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I475b17a4dccfa624dda61402491b461c53473f8b
Gerrit-Change-Number: 9273
Gerrit-PatchSet: 4
Gerrit-Owner: Nithya Janarthanan 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 

[Impala-ASF-CR] IMPALA-6116: Bound memory usage of DataStreamSevice's service queue

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9282 )

Change subject: IMPALA-6116: Bound memory usage of DataStreamSevice's service 
queue
..


Patch Set 1:

(6 comments)

I went through the change and found that I would prefer to not access 
dependencies through the ExecEnv singleton, but instead inject them through the 
ctors and Init(). That way it feels easier to me to follow the code. I don't 
feel strongly about it though.

However, if we want to go with ExecEnv, we should probably remove some of the 
injected dependencies as well to keep it consistent.

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

http://gerrit.cloudera.org:8080/#/c/9282/1//COMMIT_MSG@15
PS1, Line 15: previou
nit: typo


http://gerrit.cloudera.org:8080/#/c/9282/1/be/src/rpc/rpc-mgr-test-base.h
File be/src/rpc/rpc-mgr-test-base.h:

http://gerrit.cloudera.org:8080/#/c/9282/1/be/src/rpc/rpc-mgr-test-base.h@133
PS1, Line 133:   // Takes over ownership of the newly created 'service'.
Can you explain why this method is needed? Could you just pass in a pointer 
here?


http://gerrit.cloudera.org:8080/#/c/9282/1/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:

http://gerrit.cloudera.org:8080/#/c/9282/1/be/src/runtime/exec-env.cc@304
PS1, Line 304: RETURN_IF_ERROR(data_svc_->Init(rpc_mgr_.get()));
The RpcMgr has a getter in the ExecEnv, too, so you could remove it from the 
ctor.


http://gerrit.cloudera.org:8080/#/c/9282/1/be/src/runtime/exec-env.cc@305
PS1, Line 305: data_svc_
this one, too


http://gerrit.cloudera.org:8080/#/c/9282/1/be/src/runtime/krpc-data-stream-mgr.cc
File be/src/runtime/krpc-data-stream-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/9282/1/be/src/runtime/krpc-data-stream-mgr.cc@83
PS1, Line 83:   mem_tracker_.reset(new MemTracker(-1, "Data Stream Manager 
Deferred RPCs",
I found this easier to follow when the MemTrackers were created in ExecEnv with 
the rest of the trackers. I don't feel strongly about it though.


http://gerrit.cloudera.org:8080/#/c/9282/1/be/src/service/data-stream-service.cc
File be/src/service/data-stream-service.cc:

http://gerrit.cloudera.org:8080/#/c/9282/1/be/src/service/data-stream-service.cc@46
PS1, Line 46: DataStreamService::DataStreamService(RpcMgr* rpc_mgr)
It seems odd to pass the rpc_mgr into both the ctor and Init().



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idea4262dfb0e0aa8d58ff6ea6a8248e880b9
Gerrit-Change-Number: 9282
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 13 Feb 2018 20:33:43 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5139: Update mvn-quiet.sh to print execution content to log file

[Nithya Janarthanan (Code Review)]

Nithya Janarthanan has abandoned this change. ( 
http://gerrit.cloudera.org:8080/9303 )

Change subject: IMPALA-5139: Update mvn-quiet.sh to print execution content to 
log file
..


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I37f27788410e8aab8c41266329556103a92864e3
Gerrit-Change-Number: 9303
Gerrit-PatchSet: 1
Gerrit-Owner: Nithya Janarthanan 

[Impala-ASF-CR] IMPALA-5139: Update mvn-quiet.sh to print execution content to log file

[Nithya Janarthanan (Code Review)]

Nithya Janarthanan has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9303


Change subject: IMPALA-5139: Update mvn-quiet.sh to print execution content to 
log file
..

IMPALA-5139: Update mvn-quiet.sh to print execution content to log file

Change-Id: I475b17a4dccfa624dda61402491b461c53473f8b

Address David's review comment

Change-Id: I475b17a4dccfa624dda61402491b461c53473f8b

remove white space

Change-Id: I37f27788410e8aab8c41266329556103a92864e3
---
M bin/impala-config.sh
M bin/mvn-quiet.sh
2 files changed, 15 insertions(+), 8 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I37f27788410e8aab8c41266329556103a92864e3
Gerrit-Change-Number: 9303
Gerrit-PatchSet: 1
Gerrit-Owner: Nithya Janarthanan 

[Impala-ASF-CR] IMPALA-5269: Fix issue with final line of query followed by a comment

[Fredy Wijaya (Code Review)]

Fredy Wijaya has uploaded a new patch set (#6). ( 
http://gerrit.cloudera.org:8080/9191 )

Change subject: IMPALA-5269: Fix issue with final line of query followed by a 
comment
..

IMPALA-5269: Fix issue with final line of query followed by a comment

The patch is to remove any comments in a statement when checking if a
statement ends with a semicolon delimiter.

For example:

Before (semicolon delimiter is needed at the end):
select 1 + 1; -- comment\n;

After (semicolon delimiter is no longer needed):
select 1 + 1; -- comment

Testing:
- Ran end-to-end tests in shell

Change-Id: I54f9a8f65214023520eaa010fc462a663d02d258
---
M shell/impala_shell.py
M tests/shell/test_shell_interactive.py
2 files changed, 42 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I54f9a8f65214023520eaa010fc462a663d02d258
Gerrit-Change-Number: 9191
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Thomas Tauber-Marshall 

[Impala-ASF-CR] IMPALA-6511: Fix state machine in FIS::UpdateState()

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9294 )

Change subject: IMPALA-6511: Fix state machine in FIS::UpdateState()
..


Patch Set 1:

Thanks for the review! :)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7304205d245289cc3d7ca2217e212c844ee75e7b
Gerrit-Change-Number: 9294
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 13 Feb 2018 20:04:40 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4835: Part 2+3: switch I/O buffers to buffer pool

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9302 )

Change subject: IMPALA-4835: Part 2+3: switch I/O buffers to buffer pool
..


Patch Set 1:

Dan requested a combined version of the two patches. This is the two patches 
squash
ed together


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94704bbcb2c438f8fffd1168e42e6b2ac496dc0d
Gerrit-Change-Number: 9302
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 13 Feb 2018 20:03:41 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4835: Part 2: Allocate scan range buffers upfront

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8707 )

Change subject: IMPALA-4835: Part 2: Allocate scan range buffers upfront
..


Patch Set 24:

See https://gerrit.cloudera.org/#/c/9302/ for the combined patch


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia243bf8b62feeea602b122e0503ea4ba7d3ee70f
Gerrit-Change-Number: 8707
Gerrit-PatchSet: 24
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 13 Feb 2018 20:03:16 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4835: Part 2: Allocate scan range buffers upfront

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8707 )

Change subject: IMPALA-4835: Part 2: Allocate scan range buffers upfront
..


Patch Set 24:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8707/24/be/src/runtime/io/disk-io-mgr.h
File be/src/runtime/io/disk-io-mgr.h:

http://gerrit.cloudera.org:8080/#/c/8707/24/be/src/runtime/io/disk-io-mgr.h@308
PS24, Line 308:   int64_t max_bytes);
> But given that a scanner will be scanning multiple scan ranges, don't we ha
To guarantee progress, the scanner definitely has to have enough reservation to 
allocate at least one buffer. Beyond that, it could decide to give more or less 
additional memory to a given scan range. My current implementation in Part 3 
implements the simple policy of setting aside the exact same reservation per 
range and doesn't try to do anything with unused reservation in cases where we 
have small or cached ranges. I originally was going to do something a bit more 
adaptive where scanner threads could "give up" reservation they didn't need for 
a given input split but decided to simplify that for now.

I think the dilemma for me is narrowing the interface by exploiting our 
knowledge that it's used in one specific way in the HdfsScanNode/HdfsScanner 
layer versus implementing a slightly more general interface where I can explain 
the logic behind the interface without reference to the higher layers.

Anyway, if you feel strongly I can combine them, it wouldn't be that hard to 
restore the generality later.

We could definitely move the allocation of buffers to be done on-demand in the 
DiskIoMgr. We'd still have the same logic for deciding when to block/unblock 
the ranges, but it would just be an accounting mechanism, rather than a queue 
of physical buffers. I think the main benefit of the upfront allocation 
approach is that it's more deterministic - we always allocate the same amount 
and it's always done in the scanner thread. I.e. if we mess up the reservation 
accounting, it will fail more deterministically instead of 
non-deterministically depending on how fast scanner and I/O mgr threads make 
progress.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia243bf8b62feeea602b122e0503ea4ba7d3ee70f
Gerrit-Change-Number: 8707
Gerrit-PatchSet: 24
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 13 Feb 2018 20:03:02 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4835: Part 2+3: switch I/O buffers to buffer pool

[Tim Armstrong (Code Review)]

Tim Armstrong has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9302


Change subject: IMPALA-4835: Part 2+3: switch I/O buffers to buffer pool
..

IMPALA-4835: Part 2+3: switch I/O buffers to buffer pool

This is the final patch to switch the Disk I/O manager to allocate all
buffer from the buffer pool and to reserve the buffers required for
a query upfront.

* The planner reserves enough memory to run a single scanner per
  scan node.
* The multi-threaded scan node must increase reservation before
  spinning up more threads.
* The scanner implementations must be careful to stay within their
  assigned reservation.

The row-oriented scanners were most straightforward, since they only
have a single scan range active at a time. A single I/O buffer is
sufficient to scan the whole file but more I/O buffers can improve I/O
throughput.

Parquet is more complex because it issues a scan range per column and
the sizes of the columns on disk are not known during planning. To
deal with this, the reservation in the frontend is based on a
heuristic involving the file size and # columns. The Parquet scanner
can then divvy up reservation to columns based on the size of column
data on disk.

I adjusted how the 'mem_limit' is divided between buffer pool and non
buffer pool memory for low mem_limits to account for the increase in
buffer pool memory.

DiskIoMgr changes:
-
This change is a step towards reserving memory for buffers from the
buffer pool and constraining per-scanner memory requirements. This
change restructures the DiskIoMgr code so that each ScanRange operates
with a fixed set of buffers that are allocated upfront and recycled as
the I/O mgr works through the ScanRange.

One major change is that ScanRanges get blocked when a buffer is not
available and get unblocked when a client returns a buffer via
ReturnBuffer(). I was able to remove the logic to maintain the
blocked_ranges_ list by instead adding a separate set with all ranges
that are active.

Testing:
* Added more planner tests to cover reservation calcs for scan node.
* Test scanners for all file formats with the reservation denial debug
  action, to test behaviour when the scanners hit reservation limits.
* Updated memory and buffer pool limits for tests.

Perf:
I ran TPC-H and targeted perf locally comparing with master. Both
showed small improvements of a few percent and no regressions of
note. I still need to run cluster perf tests.

Change-Id: I94704bbcb2c438f8fffd1168e42e6b2ac496dc0d
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-parquet-scanner.h
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M be/src/exec/hdfs-scan-node-mt.cc
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/hdfs-scan-node.h
M be/src/exec/parquet-column-readers.cc
M be/src/exec/parquet-column-readers.h
M be/src/exec/scanner-context.cc
M be/src/exec/scanner-context.h
M be/src/runtime/bufferpool/buffer-pool.h
M be/src/runtime/bufferpool/reservation-tracker-test.cc
M be/src/runtime/bufferpool/reservation-util.cc
M be/src/runtime/io/disk-io-mgr-stress-test.cc
M be/src/runtime/io/disk-io-mgr-stress.cc
M be/src/runtime/io/disk-io-mgr-stress.h
M be/src/runtime/io/disk-io-mgr-test.cc
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/disk-io-mgr.h
M be/src/runtime/io/request-context.cc
M be/src/runtime/io/request-context.h
M be/src/runtime/io/request-ranges.h
M be/src/runtime/io/scan-range.cc
M be/src/runtime/tmp-file-mgr.cc
M be/src/runtime/tmp-file-mgr.h
M be/src/util/bit-util-test.cc
M be/src/util/bit-util.h
M be/src/util/runtime-profile-counters.h
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/util/BitUtil.java
M fe/src/test/java/org/apache/impala/util/BitUtilTest.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/disable-codegen.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/partition-pruning.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/sort-expr-materialization.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
M testdata/workloads/functional-planner/queries/PlannerTest/union.test
M 

[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

[David Knupp (Code Review)]

David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 7:

> Patch Set 7:
>
> David, it'd be great if you also had time for a look here. Thanks :)

Will do, although probably not until tomorrow morning.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 7
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 13 Feb 2018 20:00:58 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 7:

David, it'd be great if you also had time for a look here. Thanks :)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 7
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 13 Feb 2018 19:56:32 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p3

[Tianyi Wang (Code Review)]

Tianyi Wang has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/9300 )

Change subject: IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p3
..

IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p3

Dependency changes:
- BE and python use thrift 0.9.3-p3 from native-toolchain.
- FE uses thrift 0.9.3 from apache maven repo.
- Fb303 and http components dependencies are no longer needed in FE and
  are removed.
- The minimum openssl version requirement is increased to 1.0.1.

Configuration change:
- Thrift codegen option movable_type is enabled. New code no longer
  needs to use std::swap to avoid copying.

Change-Id: I639227721502eaa10398d9490ff6ac63aa71b3a6
---
M CMakeLists.txt
M be/src/codegen/llvm-codegen.cc
M be/src/common/init.cc
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/TAcceptQueueServer.h
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/rpc/thrift-thread.h
M be/src/rpc/thrift-util.cc
M be/src/runtime/coordinator.cc
M bin/impala-config.sh
M buildall.sh
M common/thrift/CMakeLists.txt
M fe/pom.xml
M infra/python/deps/compiled-requirements.txt
16 files changed, 85 insertions(+), 177 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I639227721502eaa10398d9490ff6ac63aa71b3a6
Gerrit-Change-Number: 9300
Gerrit-PatchSet: 3
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 

[Impala-ASF-CR] IMPALA-5690: Part 1: Rename ostream operators for thrift types

[Tianyi Wang (Code Review)]

Tianyi Wang has uploaded a new patch set (#8). ( 
http://gerrit.cloudera.org:8080/9168 )

Change subject: IMPALA-5690: Part 1: Rename ostream operators for thrift types
..

IMPALA-5690: Part 1: Rename ostream operators for thrift types

Thrift 0.9.3 implements "ostream& operator<<(ostream&, T)" for thrift
data types while impala did the same to enums and special types
including TNetworkAddress and TUniqueId. This patch renames these
impala defined functions to avoid the conflict.

Change-Id: I9c303997411237e988ef960157f781776f6fcb60
---
M be/src/benchmarks/scheduler-benchmark.cc
M be/src/codegen/llvm-codegen.cc
M be/src/exec/exchange-node.cc
M be/src/exec/exec-node.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/parquet-column-readers.cc
M be/src/exec/parquet-metadata-utils.cc
M be/src/rpc/rpc-trace.cc
M be/src/rpc/thrift-client.cc
M be/src/rpc/thrift-util.cc
M be/src/rpc/thrift-util.h
M be/src/runtime/client-cache.cc
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator.cc
M be/src/runtime/data-stream-mgr.cc
M be/src/runtime/data-stream-recvr.cc
M be/src/runtime/data-stream-sender.cc
M be/src/runtime/fragment-instance-state.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M be/src/runtime/krpc-data-stream-recvr.cc
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/runtime/mem-tracker.cc
M be/src/runtime/query-exec-mgr.cc
M be/src/runtime/query-state.cc
M be/src/runtime/runtime-filter-bank.cc
M be/src/runtime/runtime-state.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/scheduler.cc
M be/src/service/child-query.cc
M be/src/service/client-request-state.cc
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-internal-service.cc
M be/src/service/impala-server.cc
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-result-set.cc
M be/src/statestore/statestore.cc
M be/src/util/collection-metrics.h
M be/src/util/debug-util.cc
M be/src/util/debug-util.h
M be/src/util/histogram-metric.h
M be/src/util/metrics.h
M be/src/util/network-util.cc
M be/src/util/network-util.h
M be/src/util/webserver.cc
M testdata/workloads/functional-query/queries/QueryTest/set.test
M tests/shell/test_shell_commandline.py
50 files changed, 260 insertions(+), 275 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/9168/8
--
To view, visit http://gerrit.cloudera.org:8080/9168
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9c303997411237e988ef960157f781776f6fcb60
Gerrit-Change-Number: 9168
Gerrit-PatchSet: 8
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 

[Impala-ASF-CR] IMPALA-5690: Part 1: Rename ostream operators for thrift types

[Tianyi Wang (Code Review)]

Tianyi Wang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9168 )

Change subject: IMPALA-5690: Part 1: Rename ostream operators for thrift types
..


Patch Set 7:

> Patch Set 5:
>
> Since this is a very large, complicated patch, is it possible to break it up 
> to make it easier to review?
>
> In particular, I think that all of the changes around '<<' could be done in 
> an initial patch, and then have a followup patch that contains the actual 
> version bump and other things.

I've split it. The part 2 is at https://gerrit.cloudera.org/c/9300/.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9c303997411237e988ef960157f781776f6fcb60
Gerrit-Change-Number: 9168
Gerrit-PatchSet: 7
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Tue, 13 Feb 2018 19:40:10 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5690: Part 1: Rename ostream operators for thrift types

[Tianyi Wang (Code Review)]

Tianyi Wang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9168 )

Change subject: IMPALA-5690: Part 1: Rename ostream operators for thrift types
..


Patch Set 7:

Why is there "Merge Conflict" while the parent of this change is the latest 
master? 
https://git-wip-us.apache.org/repos/asf?p=impala.git;a=commit;h=5f7599687748b1e1ce0a5a34d38cc49a4c4cd9f5


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9c303997411237e988ef960157f781776f6fcb60
Gerrit-Change-Number: 9168
Gerrit-PatchSet: 7
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Tue, 13 Feb 2018 19:37:34 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p3

[Tianyi Wang (Code Review)]

Tianyi Wang has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/9300 )

Change subject: IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p3
..

IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p3

Dependency changes:
- BE and python use thrift 0.9.3-p3 from native-toolchain.
- FE uses thrift 0.9.3 from apache maven repo.
- Fb303 and http components dependencies are no longer needed in FE and
  are removed.
- The minimum openssl version requirement is increased to 1.0.1.

Configuration change:
- Thrift codegen option movable_type is enabled. New code no longer
  needs to use std::swap to avoid copying.

Change-Id: I639227721502eaa10398d9490ff6ac63aa71b3a6
---
M CMakeLists.txt
M be/src/codegen/llvm-codegen.cc
M be/src/common/init.cc
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/TAcceptQueueServer.h
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/rpc/thrift-thread.h
M be/src/rpc/thrift-util.cc
M be/src/runtime/coordinator.cc
M bin/impala-config.sh
M buildall.sh
M common/thrift/CMakeLists.txt
M fe/pom.xml
M infra/python/deps/compiled-requirements.txt
16 files changed, 85 insertions(+), 177 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I639227721502eaa10398d9490ff6ac63aa71b3a6
Gerrit-Change-Number: 9300
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 

[Impala-ASF-CR] IMPALA-5690: Part 1: Rename ostream operators for thrift types

[Tianyi Wang (Code Review)]

Tianyi Wang has uploaded a new patch set (#7). ( 
http://gerrit.cloudera.org:8080/9168 )

Change subject: IMPALA-5690: Part 1: Rename ostream operators for thrift types
..

IMPALA-5690: Part 1: Rename ostream operators for thrift types

Thrift 0.9.3 implements "ostream& operator<<(ostream&, T)" for thrift
data types while impala did the same to enums and special types
including TNetworkAddress and TUniqueId. This patch renames these
impala defined functions to avoid the conflict.

Change-Id: I9c303997411237e988ef960157f781776f6fcb60
---
A .idea/codeStyles/codeStyleConfig.xml
M be/src/benchmarks/scheduler-benchmark.cc
M be/src/codegen/llvm-codegen.cc
M be/src/exec/exchange-node.cc
M be/src/exec/exec-node.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/parquet-column-readers.cc
M be/src/exec/parquet-metadata-utils.cc
M be/src/rpc/rpc-trace.cc
M be/src/rpc/thrift-client.cc
M be/src/rpc/thrift-util.cc
M be/src/rpc/thrift-util.h
M be/src/runtime/client-cache.cc
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator.cc
M be/src/runtime/data-stream-mgr.cc
M be/src/runtime/data-stream-recvr.cc
M be/src/runtime/data-stream-sender.cc
M be/src/runtime/fragment-instance-state.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M be/src/runtime/krpc-data-stream-recvr.cc
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/runtime/mem-tracker.cc
M be/src/runtime/query-exec-mgr.cc
M be/src/runtime/query-state.cc
M be/src/runtime/runtime-filter-bank.cc
M be/src/runtime/runtime-state.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/scheduler.cc
M be/src/service/child-query.cc
M be/src/service/client-request-state.cc
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-internal-service.cc
M be/src/service/impala-server.cc
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-result-set.cc
M be/src/statestore/statestore.cc
M be/src/util/collection-metrics.h
M be/src/util/debug-util.cc
M be/src/util/debug-util.h
M be/src/util/histogram-metric.h
M be/src/util/metrics.h
M be/src/util/network-util.cc
M be/src/util/network-util.h
M be/src/util/webserver.cc
M testdata/workloads/functional-query/queries/QueryTest/set.test
M tests/shell/test_shell_commandline.py
51 files changed, 265 insertions(+), 275 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/9168/7
--
To view, visit http://gerrit.cloudera.org:8080/9168
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9c303997411237e988ef960157f781776f6fcb60
Gerrit-Change-Number: 9168
Gerrit-PatchSet: 7
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 

[Impala-ASF-CR] IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p3

[Tianyi Wang (Code Review)]

Tianyi Wang has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9300


Change subject: IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p3
..

IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p3

Dependency changes:
- BE and python use thrift 0.9.3-p3 from native-toolchain.
- FE uses thrift 0.9.3 from apache maven repo.
- Fb303 and http components dependencies are no longer needed in FE and
  are removed.
- The minimum openssl version requirement is increased to 1.0.1.

Configuration change:
- Thrift codegen option movable_type is enabled. New code no longer
  needs to use std::swap to avoid copying.

Change-Id: I639227721502eaa10398d9490ff6ac63aa71b3a6
---
M CMakeLists.txt
M be/src/codegen/llvm-codegen.cc
M be/src/common/init.cc
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/TAcceptQueueServer.h
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/rpc/thrift-thread.h
M be/src/rpc/thrift-util.cc
M be/src/runtime/coordinator.cc
M bin/impala-config.sh
M buildall.sh
M common/thrift/CMakeLists.txt
M fe/pom.xml
M infra/python/deps/compiled-requirements.txt
16 files changed, 85 insertions(+), 177 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I639227721502eaa10398d9490ff6ac63aa71b3a6
Gerrit-Change-Number: 9300
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 

[Impala-ASF-CR] IMPALA-5690: Part 1: Rename ostream operators for thrift types

[Tianyi Wang (Code Review)]

Tianyi Wang has uploaded a new patch set (#6). ( 
http://gerrit.cloudera.org:8080/9168 )

Change subject: IMPALA-5690: Part 1: Rename ostream operators for thrift types
..

IMPALA-5690: Part 1: Rename ostream operators for thrift types

Thrift 0.9.3 implements "ostream& operator<<(ostream&, T)" for thrift
data types while impala did the same to enums and special types
including TNetworkAddress and TUniqueId. This patch renames these
impala defined functions to avoid the conflict.

Change-Id: I9c303997411237e988ef960157f781776f6fcb60
---
A .idea/codeStyles/codeStyleConfig.xml
M be/src/benchmarks/scheduler-benchmark.cc
M be/src/codegen/llvm-codegen.cc
M be/src/exec/exchange-node.cc
M be/src/exec/exec-node.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/parquet-column-readers.cc
M be/src/exec/parquet-metadata-utils.cc
M be/src/rpc/rpc-trace.cc
M be/src/rpc/thrift-client.cc
M be/src/rpc/thrift-util.cc
M be/src/rpc/thrift-util.h
M be/src/runtime/client-cache.cc
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator.cc
M be/src/runtime/data-stream-mgr.cc
M be/src/runtime/data-stream-recvr.cc
M be/src/runtime/data-stream-sender.cc
M be/src/runtime/fragment-instance-state.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M be/src/runtime/krpc-data-stream-recvr.cc
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/runtime/mem-tracker.cc
M be/src/runtime/query-exec-mgr.cc
M be/src/runtime/query-state.cc
M be/src/runtime/runtime-filter-bank.cc
M be/src/runtime/runtime-state.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/scheduler.cc
M be/src/service/child-query.cc
M be/src/service/client-request-state.cc
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-internal-service.cc
M be/src/service/impala-server.cc
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-result-set.cc
M be/src/statestore/statestore.cc
M be/src/util/collection-metrics.h
M be/src/util/debug-util.cc
M be/src/util/debug-util.h
M be/src/util/histogram-metric.h
M be/src/util/metrics.h
M be/src/util/network-util.cc
M be/src/util/network-util.h
M be/src/util/webserver.cc
M testdata/workloads/functional-query/queries/QueryTest/set.test
M tests/shell/test_shell_commandline.py
51 files changed, 265 insertions(+), 275 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9c303997411237e988ef960157f781776f6fcb60
Gerrit-Change-Number: 9168
Gerrit-PatchSet: 6
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 

[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 7:

I ran private core and exhaustive tests and added skips for tests that didn't 
work with KRPC. I filed IMPALA-6512 and IMPALA-6513 for them. All other tests 
passed.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 7
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 13 Feb 2018 18:48:59 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

[Lars Volker (Code Review)]

Hello Michael Ho, Michael Brown, Sailesh Mukil,

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

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

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

Change subject: IMPALA-6508: add KRPC test flag
..

IMPALA-6508: add KRPC test flag

This change adds a flag "--use_krpc" to start-impala-cluster.py. The
flag is currently passed as an argument to the impalad daemon. In the
future it will also enable KRPC for the catalogd and statestored
daemons.

This change also adds a flag "--test_krpc" to pytest. When running tests
using "impala-py.test --test_krpc", the test cluster will be started
by passing "--use_krpc" to start-impala-cluster.py (see above).

This change also adds a SkipIf to skip tests based on whether the
cluster was started with KRPC support or not.

- SkipIf.not_krpc can be used to mark a test that depends on KRPC.
- SkipIf.not_thrift can be used to mark a test that depends on Thrift
  RPC.

This change adds a meta test to make sure that the new SkipIf decorators
work correctly. The test should be removed as soon as real tests have
been added with the new decorators.

Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
---
M bin/run-all-tests.sh
M bin/start-impala-cluster.py
M tests/common/custom_cluster_test_suite.py
M tests/common/skip.py
A tests/common/test_skip.py
M tests/conftest.py
M tests/custom_cluster/test_exchange_delays.py
M tests/custom_cluster/test_krpc_mem_usage.py
M tests/custom_cluster/test_rpc_exception.py
9 files changed, 85 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/9291/7
--
To view, visit http://gerrit.cloudera.org:8080/9291
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 7
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-3193: Show table's comment on show tables

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8851 )

Change subject: IMPALA-3193: Show table's comment on show tables
..


Patch Set 17:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1929/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I44f814af05db6f3c027718ade9f474f8b8153bcd
Gerrit-Change-Number: 8851
Gerrit-PatchSet: 17
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Comment-Date: Tue, 13 Feb 2018 18:17:45 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3193: Show table's comment on show tables

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8851 )

Change subject: IMPALA-3193: Show table's comment on show tables
..


Patch Set 17: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I44f814af05db6f3c027718ade9f474f8b8153bcd
Gerrit-Change-Number: 8851
Gerrit-PatchSet: 17
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Comment-Date: Tue, 13 Feb 2018 18:17:02 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3193: Show table's comment on show tables

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8851 )

Change subject: IMPALA-3193: Show table's comment on show tables
..


Patch Set 16: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I44f814af05db6f3c027718ade9f474f8b8153bcd
Gerrit-Change-Number: 8851
Gerrit-PatchSet: 16
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Comment-Date: Tue, 13 Feb 2018 18:16:54 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/custom_cluster_test_suite.py@137
PS5, Line 137: if pytest.config.option.test_krpc:
 :   cmd.append("--impalad_args=-use_krpc")
> I just meant that if someone performs a private build with their own TEST_S
I wonder if that's on purpose, would you like me to ask on dev@?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 5
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 13 Feb 2018 18:07:25 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

[Lars Volker (Code Review)]

Hello Michael Ho, Michael Brown, Sailesh Mukil,

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

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

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

Change subject: IMPALA-6508: add KRPC test flag
..

IMPALA-6508: add KRPC test flag

This change adds a flag "--use_krpc" to start-impala-cluster.py. The
flag is currently passed as an argument to the impalad daemon. In the
future it will also enable KRPC for the catalogd and statestored
daemons.

This change also adds a flag "--test_krpc" to pytest. When running tests
using "impala-py.test --test_krpc", the test cluster will be started
by passing "--use_krpc" to start-impala-cluster.py (see above).

This change also adds a SkipIf to skip tests based on whether the
cluster was started with KRPC support or not.

- SkipIf.not_krpc can be used to mark a test that depends on KRPC.
- SkipIf.not_thrift can be used to mark a test that depends on Thrift
  RPC.

This change adds a meta test to make sure that the new SkipIf decorators
work correctly. The test should be removed as soon as real tests have
been added with the new decorators.

Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
---
M bin/run-all-tests.sh
M bin/start-impala-cluster.py
M tests/common/custom_cluster_test_suite.py
M tests/common/skip.py
A tests/common/test_skip.py
M tests/conftest.py
M tests/custom_cluster/test_exchange_delays.py
M tests/custom_cluster/test_krpc_mem_usage.py
8 files changed, 82 insertions(+), 6 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 6
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-5269: Fix issue with final line of query followed by a comment

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9191 )

Change subject: IMPALA-5269: Fix issue with final line of query followed by a 
comment
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9191/5/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/9191/5/tests/shell/test_shell_interactive.py@444
PS5, Line 444: queries = ['select 1 + 1; --comment',
Additional tests:

1. Test where the last line contains part of the SQL statement, the delimiter, 
and a comment. This is to check that sqlparse can remove the comment from a 
line with an incomplete SQL statement.

Example:

select c from (
  select count(*) from t
) v; -- Incomplete SQL statement in this line


2. Test multi-line comments

Example:

select c1 from t
order by c2; /*
* Multi-line comment
*/



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I54f9a8f65214023520eaa010fc462a663d02d258
Gerrit-Change-Number: 9191
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Tue, 13 Feb 2018 17:51:33 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6258: Uninitialized tuple pointers in row batch for empty rows

[Zoltan Borok-Nagy (Code Review)]

Hello Thomas Tauber-Marshall, Tim Armstrong,

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

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

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

Change subject: IMPALA-6258: Uninitialized tuple pointers in row batch for 
empty rows
..

IMPALA-6258: Uninitialized tuple pointers in row batch for empty rows

Tuple pointers in the generated row batches may not be initialized
if a tuple has byte size 0. There are some codes which compare these
uninitialized pointers against nullptr so having them uninitialized
may return wrong (and non-deterministic) results, e.g.:
impala::TupleIsNullPredicate::GetBooleanVal()

The following query produces non-deterministic results currently:

SELECT count(v.x) FROM functional.alltypestiny t3 LEFT OUTER JOIN (
  SELECT true AS x FROM functional.alltypestiny t1 LEFT OUTER JOIN
  functional.alltypestiny t2 ON (true)) v ON (v.x = t3.bool_col)
WHERE t3.bool_col = true;

The alltypestiny table has 8 records, 4 records of them has the true
boolean value for bool_col. Therefore, the above query is a fancy
way of saying "8 * 8 * 4", i.e. it should return 256.

The solution is that scanners initialize tuple pointers to a non-null
value when there are no materialized slots. This non-null value is
provided by the new static member Tuple::POISON.

I extended QueryTest/scanners.test with the above query. This test
executes the query against all table formats.

This change has the biggest performance impact on count(*) queries on
large kudu tables. For my quick benchmark I copied tpch_kudu.lineitem
and doubled its data. The resulting table has 12,002,430 rows.

Without this patch 'select count(*) from biglineitem' runs for ~0.12s.
With the patch applied, the overhead is around a dozens of ms. I measured
the query on my desktop PC using a relase build of Impala. On debug builds,
the execution time of the patched version is around 160% of the original
version.

Without this patch:
+--++--+--+++---+---+-+
| Operator | #Hosts | Avg Time | Max Time | #Rows  | Est. #Rows | Peak Mem  
| Est. Peak Mem | Detail  |
+--++--+--+++---+---+-+
| 03:AGGREGATE | 1  | 127.50us | 127.50us | 1  | 1  | 28.00 KB  
| 10.00 MB  | FINALIZE|
| 02:EXCHANGE  | 1  | 22.32ms  | 22.32ms  | 3  | 1  | 0 B   
| 0 B   | UNPARTITIONED   |
| 01:AGGREGATE | 3  | 1.78ms   | 1.89ms   | 3  | 1  | 16.00 KB  
| 10.00 MB  | |
| 00:SCAN KUDU | 3  | 8.00ms   | 8.28ms   | 12.00M | -1 | 512.00 KB 
| 0 B   | default.biglineitem |
+--++--+--+++---+---+-+

With this patch:
+--++--+--+++---+---+-+
| Operator | #Hosts | Avg Time | Max Time | #Rows  | Est. #Rows | Peak Mem  
| Est. Peak Mem | Detail  |
+--++--+--+++---+---+-+
| 03:AGGREGATE | 1  | 129.01us | 129.01us | 1  | 1  | 28.00 KB  
| 10.00 MB  | FINALIZE|
| 02:EXCHANGE  | 1  | 33.00ms  | 33.00ms  | 3  | 1  | 0 B   
| 0 B   | UNPARTITIONED   |
| 01:AGGREGATE | 3  | 1.99ms   | 2.13ms   | 3  | 1  | 16.00 KB  
| 10.00 MB  | |
| 00:SCAN KUDU | 3  | 13.13ms  | 13.97ms  | 12.00M | -1 | 512.00 KB 
| 0 B   | default.biglineitem |
+--++--+--+++---+---+-+

Change-Id: I2981227e62eb5971508e0698e189519755de
---
M be/src/exec/hdfs-scanner.cc
M be/src/exec/kudu-scanner.cc
M be/src/runtime/tuple.cc
M be/src/runtime/tuple.h
M testdata/workloads/functional-query/queries/QueryTest/scanners.test
5 files changed, 27 insertions(+), 2 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2981227e62eb5971508e0698e189519755de
Gerrit-Change-Number: 9239
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 

[Impala-ASF-CR] IMPALA-4835: Part 2: Allocate scan range buffers upfront

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8707 )

Change subject: IMPALA-4835: Part 2: Allocate scan range buffers upfront
..


Patch Set 24:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8707/24/be/src/runtime/io/disk-io-mgr.h
File be/src/runtime/io/disk-io-mgr.h:

http://gerrit.cloudera.org:8080/#/c/8707/24/be/src/runtime/io/disk-io-mgr.h@308
PS24, Line 308:   int64_t max_bytes);
> It felt awkward to force the caller to specify the amount of memory to use
But given that a scanner will be scanning multiple scan ranges, don't we have 
to assume the worse case when assigning reservations? So what does it matter 
how big the current scan range is?

I can see the argument for not hiding the memory allocation. But on the flip 
side, that's kind of the benefit of reservations -- the policy logic is just 
divvying up the reservation and that is inside the scan node and scanner.

Also, a different question relating to how things are organized given we have 
reservations: why do we need to queue the buffers per range rather than just 
allocate them from the buffer pool, given that the reservations should ensure 
they are available. i.e .why isn't it a matter of reimplementing 
TryAllocateNextBufferForRange() to use the assigned reservation to get the 
buffer from the buffer pool?

It's a bit hard to understand how this all comes together with the patches 
divided given that the reservation logic is not a trivial addition. Could you 
generate a CR that has part 2&3 combined?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia243bf8b62feeea602b122e0503ea4ba7d3ee70f
Gerrit-Change-Number: 8707
Gerrit-PatchSet: 24
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 13 Feb 2018 17:39:33 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6258: Uninitialized tuple pointers in row batch for empty rows

[Zoltan Borok-Nagy (Code Review)]

Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9239 )

Change subject: IMPALA-6258: Uninitialized tuple pointers in row batch for 
empty rows
..


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/9239/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9239/2//COMMIT_MSG@57
PS2, Line 57: | 03:AGGREGATE | 1  | 129.01us | 129.01us | 1  | 1
  | 28.00 KB  | 10.00 MB  | FINALIZE|
> With the new numbers, I think that this seems okay.
Done, Tim filed IMPALA-6501


http://gerrit.cloudera.org:8080/#/c/9239/3/be/src/exec/hdfs-scanner.cc
File be/src/exec/hdfs-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/9239/3/be/src/exec/hdfs-scanner.cc@228
PS3, Line 228: // Let's not leave tuple ptrs uninitialized.
> Can you add the JIRA to this comment to better explain the consequences of
Added the JIRA number


http://gerrit.cloudera.org:8080/#/c/9239/3/be/src/exec/kudu-scanner.cc
File be/src/exec/kudu-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/9239/3/be/src/exec/kudu-scanner.cc@259
PS3, Line 259: // Initialize tuple ptrs to a dummy non-null value
> Can you add the JIRA there too?
yes, added it


http://gerrit.cloudera.org:8080/#/c/9239/3/be/src/runtime/tuple.h
File be/src/runtime/tuple.h:

http://gerrit.cloudera.org:8080/#/c/9239/3/be/src/runtime/tuple.h@89
PS3, Line 89: Poison
> This should either be lower-case if we're treating it as a variable or uppe
Renamed it to all upper-case, I also think it is more of a constant. And we'll 
never modify the pointed object since it would raise an error instantly.


http://gerrit.cloudera.org:8080/#/c/9239/3/be/src/runtime/tuple.h@91
PS3, Line 91:   void Init(int size) {
> Unnecessary formatting change here.
Oops, it became multi-line when I added those DCHECKs everywhere.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2981227e62eb5971508e0698e189519755de
Gerrit-Change-Number: 9239
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 13 Feb 2018 17:39:20 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6479: Update DESCRIBE to respect column privileges

[Adam Holley (Code Review)]

Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9276 )

Change subject: IMPALA-6479: Update DESCRIBE to respect column privileges
..


Patch Set 1:

Original patch build: 
https://jenkins.impala.io/job/gerrit-verify-dryrun-external/80/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic96ae184fccdc88ba970b5adcd501da1966accb9
Gerrit-Change-Number: 9276
Gerrit-PatchSet: 1
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Comment-Date: Tue, 13 Feb 2018 17:36:09 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 5:

(2 comments)

> This means we will have automated test runs with both krpc enabled and 
> disabled for the foreseeable future

I'm convinced, then. Thanks.

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/custom_cluster_test_suite.py@137
PS5, Line 137: if pytest.config.option.test_krpc:
 :   cmd.append("--impalad_args=-use_krpc")
> I'm not sure I understand what you mean. Should they use $TEST_START_CLUSTE
I just meant that if someone performs a private build with their own 
TEST_START_CLUSTER_ARGS, then the TEST_START_CLUSTER_ARGS will not be applied 
in custom cluster tests. This is something I noticed while poking around in the 
code. You don't have to fix that in this patch.


http://gerrit.cloudera.org:8080/#/c/9291/5/tests/custom_cluster/test_krpc_mem_usage.py
File tests/custom_cluster/test_krpc_mem_usage.py:

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/custom_cluster/test_krpc_mem_usage.py@77
PS5, Line 77:   
@CustomClusterTestSuite.with_args("--stress_datastream_recvr_delay_ms=1000")
> I'll reply on the change itself.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 5
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 13 Feb 2018 17:34:41 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 5:

(3 comments)

> Patch Set 5:
>
> (3 comments)
>
> I was trying to think about how this could be done without yet another 
> "global" environment variable, and an unenforceable flag between pytest and 
> the cluster, and more skip logic.
>
> Do you think it's possible to achieve the same effect by:
>
> 1. Creating CI that sets TEST_START_CLUSTER_ARGS appropriately for enabling 
> KRPC
>
> 2. Ensuring $TEST_START_CLUSTER_ARGS is properly reconciled any time the 
> cluster starts, even in custom cluster tests.
>
> 3. Running the 3 tests that are KRPC specific with --use-krpc always set, 
> unconditionally.
>
> I have a feeling this would reduce some of the churn that's introduced in 
> this patch and then would be gutted again later. What do you think?

I think we plan on keeping the option to turn KRPC off, even after we turn it 
on by default. This means we will have automated test runs with both krpc 
enabled and disabled for the foreseeable future, until we completely remove the 
old Thrift RPC code. It would be very convenient to add tests into the 
appropriate test files, e.g. add a test for KRPC metrics showing up on the 
/rpcz page (IMPALA-6269) to test_web_pages.py. In these files we would need a 
way to decorate tests that require KRPC and should be skipped for runs with 
KRPC disabled. Similarly I anticipate that we will have tests that require 
Thrift RPC at some time. Our e2e tests should also be faster than the custom 
cluster tests.

Not having a SkipIf means we would have to look at the environment to skip 
those tests from within the test methods, but wrapping that in a function then 
seems to be equivalent of having a SkipIf.

Hopefully we will also have a lot more than 3 tests that require KRPC to be 
turned on. :)

We could try to interfere the SkipIf from the command line argument to reduce 
the amount of config variables. Alternatively we could unhide -use_krpc and get 
the value from the /varz page.

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/custom_cluster_test_suite.py@137
PS5, Line 137: if pytest.config.option.test_krpc:
 :   cmd.append("--impalad_args=-use_krpc")
> It seems like a bug that custom cluster tests don't prepend $TEST_START_CLU
I'm not sure I understand what you mean. Should they use 
$TEST_START_CLUSTER_ARGS instead of appending to the command line themselves?


http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/test_skip.py
File tests/common/test_skip.py:

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/test_skip.py@18
PS5, Line 18: import os
> Unused import.
Done


http://gerrit.cloudera.org:8080/#/c/9291/5/tests/custom_cluster/test_krpc_mem_usage.py
File tests/custom_cluster/test_krpc_mem_usage.py:

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/custom_cluster/test_krpc_mem_usage.py@77
PS5, Line 77:   
@CustomClusterTestSuite.with_args("--stress_datastream_recvr_delay_ms=1000")
> An alternative to skipping would be to just force --use-krpc always to on i
I'll reply on the change itself.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 5
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 13 Feb 2018 17:25:59 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 5:

(3 comments)

I was trying to think about how this could be done without yet another "global" 
environment variable, and an unenforceable flag between pytest and the cluster, 
and more skip logic.

Do you think it's possible to achieve the same effect by:

1. Creating CI that sets TEST_START_CLUSTER_ARGS appropriately for enabling KRPC

2. Ensuring $TEST_START_CLUSTER_ARGS is properly reconciled any time the 
cluster starts, even in custom cluster tests.

3. Running the 3 tests that are KRPC specific with --use-krpc always set, 
unconditionally.

I have a feeling this would reduce some of the churn that's introduced in this 
patch and then would be gutted again later. What do you think?

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/custom_cluster_test_suite.py@137
PS5, Line 137: if pytest.config.option.test_krpc:
 :   cmd.append("--impalad_args=-use_krpc")
It seems like a bug that custom cluster tests don't prepend 
$TEST_START_CLUSTER_ARGS with other custom args.


http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/test_skip.py
File tests/common/test_skip.py:

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/test_skip.py@18
PS5, Line 18: import os
Unused import.


http://gerrit.cloudera.org:8080/#/c/9291/5/tests/custom_cluster/test_krpc_mem_usage.py
File tests/custom_cluster/test_krpc_mem_usage.py:

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/custom_cluster/test_krpc_mem_usage.py@77
PS5, Line 77:   
@CustomClusterTestSuite.with_args("--stress_datastream_recvr_delay_ms=1000")
An alternative to skipping would be to just force --use-krpc always to on in 
these tests.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 5
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 13 Feb 2018 17:05:45 +
Gerrit-HasComments: Yes

[native-toolchain-CR] Quiet down file transfers, rename target platform variable

[Laszlo Gaal (Code Review)]

Laszlo Gaal has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9149 )

Change subject: Quiet down file transfers, rename target platform variable
..


Patch Set 3: Verified+1


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa4349a8ae0ab5bde2dabf5ff65db0f2db4b9ba9
Gerrit-Change-Number: 9149
Gerrit-PatchSet: 3
Gerrit-Owner: Laszlo Gaal 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 13 Feb 2018 14:05:23 +
Gerrit-HasComments: No

[native-toolchain-CR] Quiet down file transfers, rename target platform variable

[Laszlo Gaal (Code Review)]

Laszlo Gaal has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9149 )

Change subject: Quiet down file transfers, rename target platform variable
..

Quiet down file transfers, rename target platform variable

1. File download with wget and maven upload all have verbose progress
indicators. This increases noise in the build logs making it hard
to navigate to relevant details.

This patch:
- changes wget to use a less chatty progress bar
- changes maven upload to batch mode and bumps log level to warnings
  or worse
- tells "aws s3 cp" to report errors only, adding an explicit echo to
  report file copy source and estination.

2. When the toolchain is built in an automated environment (e.g. using
a Jenkins job), the build target platform identifier is communicated
in an environment variable. This patch renames this variable to
BUILD_TARGET_LABEL to untangle it from Jenkins-defined variables and
to make its role more explicit.

Tested by rebuilding the toolchain with the new script, using
a conforming Jenkins job; all artifacts were prodiced with correct
package name suffixes. Results are stored at:
s3://impala-toolchain-test/build/33-a25ad7b25c/

Change-Id: Iaa4349a8ae0ab5bde2dabf5ff65db0f2db4b9ba9
---
M functions.sh
M init.sh
2 files changed, 17 insertions(+), 10 deletions(-)

Approvals:
  Laszlo Gaal: Looks good to me, approved; Verified

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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iaa4349a8ae0ab5bde2dabf5ff65db0f2db4b9ba9
Gerrit-Change-Number: 9149
Gerrit-PatchSet: 3
Gerrit-Owner: Laszlo Gaal 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Tim Armstrong 

[native-toolchain-CR] Quiet down file transfers, rename target platform variable

[Laszlo Gaal (Code Review)]

Laszlo Gaal has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9149 )

Change subject: Quiet down file transfers, rename target platform variable
..


Patch Set 3: Code-Review+2

Rebased, unchanged, carrying Tim's +2


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa4349a8ae0ab5bde2dabf5ff65db0f2db4b9ba9
Gerrit-Change-Number: 9149
Gerrit-PatchSet: 3
Gerrit-Owner: Laszlo Gaal 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 13 Feb 2018 13:58:02 +
Gerrit-HasComments: No