[Impala-ASF-CR] IMPALA-8629: (part 2) Adjust new KuduStorageHandler package

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13541 )

Change subject: IMPALA-8629: (part 2) Adjust new KuduStorageHandler package
..


Patch Set 7: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d0c505643247498383472704a37d27c9e1ce473
Gerrit-Change-Number: 13541
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Wed, 12 Jun 2019 05:47:41 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8629: (part 2) Adjust new KuduStorageHandler package

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13541 )

Change subject: IMPALA-8629: (part 2) Adjust new KuduStorageHandler package
..


Patch Set 7:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d0c505643247498383472704a37d27c9e1ce473
Gerrit-Change-Number: 13541
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Wed, 12 Jun 2019 05:27:05 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8629: (part 2) Adjust new KuduStorageHandler package

2019-06-11 Thread Grant Henke (Code Review)
Hello Thomas Marshall, Hao Hao, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-8629: (part 2) Adjust new KuduStorageHandler package
..

IMPALA-8629: (part 2) Adjust new KuduStorageHandler package

This patch changes the new KuduStorageHandler
package from “org.apache.kudu.hive” to
“org.apache.hadoop.hive.kudu”.

This is being done to ensure the stand-in storage handler
can be a real storage handler when a Hive integration
is added in the future. The “org.apache.hadoop.hive”
package is the standard package all Hive storage
handlers lives under.

Additionally this patch updates the stand-in InputFormat,
OutputFormat, and SerDe entries for Kudu. This allows a
future Hive integration to read HMS tables/entries created
by Impala.

Change-Id: I4d0c505643247498383472704a37d27c9e1ce473
---
M bin/impala-config.sh
M fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
M fe/src/main/java/org/apache/impala/catalog/KuduTable.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M tests/custom_cluster/test_kudu.py
M tests/query_test/test_kudu.py
7 files changed, 26 insertions(+), 34 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d0c505643247498383472704a37d27c9e1ce473
Gerrit-Change-Number: 13541
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 


[Impala-ASF-CR] IMPALA-8629: (part 2) Adjust new KuduStorageHandler package

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13541 )

Change subject: IMPALA-8629: (part 2) Adjust new KuduStorageHandler package
..


Patch Set 5:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d0c505643247498383472704a37d27c9e1ce473
Gerrit-Change-Number: 13541
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Wed, 12 Jun 2019 04:23:09 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8629: (part 2) Adjust new KuduStorageHandler package

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13541 )

Change subject: IMPALA-8629: (part 2) Adjust new KuduStorageHandler package
..


Patch Set 4:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d0c505643247498383472704a37d27c9e1ce473
Gerrit-Change-Number: 13541
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Wed, 12 Jun 2019 04:14:36 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8629: (part 2) Adjust new KuduStorageHandler package

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13541 )

Change subject: IMPALA-8629: (part 2) Adjust new KuduStorageHandler package
..


Patch Set 6: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d0c505643247498383472704a37d27c9e1ce473
Gerrit-Change-Number: 13541
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Wed, 12 Jun 2019 03:52:48 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8629: (part 2) Adjust new KuduStorageHandler package

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13541 )

Change subject: IMPALA-8629: (part 2) Adjust new KuduStorageHandler package
..


Patch Set 6:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/3577/ : Initial code 
review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d0c505643247498383472704a37d27c9e1ce473
Gerrit-Change-Number: 13541
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Wed, 12 Jun 2019 03:41:51 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8629: (part 2) Adjust new KuduStorageHandler package

2019-06-11 Thread Grant Henke (Code Review)
Hello Thomas Marshall, Hao Hao, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-8629: (part 2) Adjust new KuduStorageHandler package
..

IMPALA-8629: (part 2) Adjust new KuduStorageHandler package

This patch changes the new KuduStorageHandler
package from “org.apache.kudu.hive” to
“org.apache.hadoop.hive.kudu”.

This is being done to ensure the stand-in storage handler
can be a real storage handler when a Hive integration
is added in the future. The “org.apache.hadoop.hive”
package is the standard package all Hive storage
handlers lives under.

Additionally this patch updates the stand-in InputFormat,
OutputFormat, and SerDe entries for Kudu. This allows a future Hive integration 
to read HMS tables/entries created by Impala.

Change-Id: I4d0c505643247498383472704a37d27c9e1ce473
---
M bin/impala-config.sh
M fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
M fe/src/main/java/org/apache/impala/catalog/KuduTable.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M tests/custom_cluster/test_kudu.py
M tests/query_test/test_kudu.py
7 files changed, 26 insertions(+), 34 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d0c505643247498383472704a37d27c9e1ce473
Gerrit-Change-Number: 13541
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 


[Impala-ASF-CR] IMPALA-8629: (part 2) Adjust new KuduStorageHandler package

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13541 )

Change subject: IMPALA-8629: (part 2) Adjust new KuduStorageHandler package
..


Patch Set 6:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d0c505643247498383472704a37d27c9e1ce473
Gerrit-Change-Number: 13541
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Wed, 12 Jun 2019 03:37:37 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8629: (part 2) Adjust new KuduStorageHandler package

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13541 )

Change subject: IMPALA-8629: (part 2) Adjust new KuduStorageHandler package
..


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d0c505643247498383472704a37d27c9e1ce473
Gerrit-Change-Number: 13541
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Wed, 12 Jun 2019 03:35:06 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8629: (part 2) Adjust new KuduStorageHandler package

2019-06-11 Thread Grant Henke (Code Review)
Hello Thomas Marshall, Hao Hao, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-8629: (part 2) Adjust new KuduStorageHandler package
..

IMPALA-8629: (part 2) Adjust new KuduStorageHandler package

This patch changes the new KuduStorageHandler
package from “org.apache.kudu.hive” to
“org.apache.hadoop.hive.kudu”.

This is being done to ensure the stand-in storage handler
can be a real storage handler when a Hive integration
is added in the future. The “org.apache.hadoop.hive”
package is the standard package all Hive storage
handlers lives under.

Additionally this patch updates the stand-in InputFormat,
OutputFormat, and SerDe entries for Kudu. This allows a future Hive integration 
to read HMS tables/entries created by Impala.

Change-Id: I4d0c505643247498383472704a37d27c9e1ce473
---
M fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
M fe/src/main/java/org/apache/impala/catalog/KuduTable.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M tests/custom_cluster/test_kudu.py
M tests/query_test/test_kudu.py
6 files changed, 24 insertions(+), 32 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d0c505643247498383472704a37d27c9e1ce473
Gerrit-Change-Number: 13541
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 


[Impala-ASF-CR] IMPALA-8629: (part 2) Adjust new KuduStorageHandler package

2019-06-11 Thread Grant Henke (Code Review)
Hello Thomas Marshall, Hao Hao, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-8629: (part 2) Adjust new KuduStorageHandler package
..

IMPALA-8629: (part 2) Adjust new KuduStorageHandler package

This patch changes the new KuduStorageHandler
package from “org.apache.kudu.hive” to
“org.apache.hadoop.hive.kudu”.

This is being done to ensure the stand-in storage handler
can be a real storage handler when a Hive integration
is added in the future. The “org.apache.hadoop.hive”
package is the standard package all Hive storage
handlers lives under.

Additionally this patch updates the stand-in InputFormat,
OutputFormat, and SerDe entries for Kudu. This allows a future Hive integration 
to read HMS tables/entries created by Impala.

Change-Id: I4d0c505643247498383472704a37d27c9e1ce473
---
M fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
M fe/src/main/java/org/apache/impala/catalog/KuduTable.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M tests/custom_cluster/test_kudu.py
M tests/query_test/test_kudu.py
6 files changed, 24 insertions(+), 25 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d0c505643247498383472704a37d27c9e1ce473
Gerrit-Change-Number: 13541
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 


[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile

2019-06-11 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13353 )

Change subject: IMPALA-8443: Record time spent in authorization in the runtime 
profile
..


Patch Set 3:

> Patch Set 3: Patch Set 2 was rebased

Hi Tamas, I saw that you just rebased the patch. Are you going to make a change 
to make sure the authorization time logged here: 
https://github.com/apache/impala/blob/ab908d54c22861967f693428ec7d9f6d7008607f/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java#L89
 is the same as the one in timeline.markEvent("Authorization finished")?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Austin Nobis 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Comment-Date: Wed, 12 Jun 2019 01:23:36 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8654: Log the SQL statement in the Ranger audit log

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13590 )

Change subject: IMPALA-8654: Log the SQL statement in the Ranger audit log
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id9f584ac4209604675eb13b6d6f349c6cbb1a387
Gerrit-Change-Number: 13590
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 12 Jun 2019 01:20:29 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8654: Log the SQL statement in the Ranger audit log

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13590 )

Change subject: IMPALA-8654: Log the SQL statement in the Ranger audit log
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id9f584ac4209604675eb13b6d6f349c6cbb1a387
Gerrit-Change-Number: 13590
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 12 Jun 2019 01:20:29 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8536: Add Scalable Pool Configuration to Admission Controller.

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13307 )

Change subject: IMPALA-8536: Add Scalable Pool Configuration to Admission 
Controller.
..


Patch Set 9:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If47508728124076f3b9200c27cffc989f7a4f188
Gerrit-Change-Number: 13307
Gerrit-PatchSet: 9
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 12 Jun 2019 01:16:17 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7608: Estimate row count from file size when no stats available

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12974 )

Change subject: IMPALA-7608: Estimate row count from file size when no stats 
available
..


Patch Set 14:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic414121c8df0d5222e4aeea096b5365beb04568a
Gerrit-Change-Number: 12974
Gerrit-PatchSet: 14
Gerrit-Owner: Fang-Yu Rao 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 12 Jun 2019 01:14:25 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7290: part 2: Add HS2 support to Impala shell

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12884 )

Change subject: IMPALA-7290: part 2: Add HS2 support to Impala shell
..


Patch Set 20:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d5cc83d545aacc659523f29b1d6feed672e2a12
Gerrit-Change-Number: 12884
Gerrit-PatchSet: 20
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 12 Jun 2019 00:59:49 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7290: part 2: Add HS2 support to Impala shell

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12884 )

Change subject: IMPALA-7290: part 2: Add HS2 support to Impala shell
..


Patch Set 19:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d5cc83d545aacc659523f29b1d6feed672e2a12
Gerrit-Change-Number: 12884
Gerrit-PatchSet: 19
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 12 Jun 2019 00:50:02 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8536: Add Scalable Pool Configuration to Admission Controller.

2019-06-11 Thread Andrew Sherman (Code Review)
Andrew Sherman has uploaded a new patch set (#9). ( 
http://gerrit.cloudera.org:8080/13307 )

Change subject: IMPALA-8536: Add Scalable Pool Configuration to Admission 
Controller.
..

IMPALA-8536: Add Scalable Pool Configuration to Admission Controller.

Add 3 configuration parameters to Admission Controller that scale with
the number of hosts in the resource pool. These parameters are specified
to the Impalad through the -llama_site_path flag which points to a Llama
XML configuration file.

The new configuration parameters are:
+ Max Running Queries Multiple - this floating point number is
  multiplied by the current total number of executors at runtime to give
  the maximum number of concurrently running queries allowed in the
  pool. This calculation is rounded up to the nearest integer so the
  result will always be at least one as long as the parameter is
  non-zero.
+ Max Queued Queries Multiple - this floating point number is multiplied
  by the current total number of executors at runtime to give the
  maximum number of queries that can be queued in the pool. This
  calculation is rounded up to the nearest integer so the result will
  always be at least one as long as the parameter is non-zero.
+ Max Memory Multiple - this number of bytes is multiplied by the
  current total number of executors at runtime to give the maximum
  memory available across the cluster for the pool.
If any of these parameters have zero value then they will be ignored.
In this case the corresponding non-scalable parameters will be used, if
they are set.

The new parameters are exposed through the webui.

At various points in the code Admission Controller looks at the Pool
Config objects to find non-scalable parameters such as the max number of
queries that can run in the pool. These access have been encapsulated in
functions that return the scalable version of the configuration value if
the new scalable parameters are being used. Diagnostic messages are
enhanced to show the origin of the encapsulated parameters.

TESTING

All end-to-end tests are running clean with ASAN.

The unit test admission-controller-test.cc has been expanded to test the
newly added code.

Added an end-to-end test that adds and removes Impalads from a
minicluster.

Change-Id: If47508728124076f3b9200c27cffc989f7a4f188
---
M be/src/runtime/exec-env.cc
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/generate_metrics.py
M common/thrift/metrics.json
M fe/src/main/java/org/apache/impala/util/RequestPoolService.java
M fe/src/test/java/org/apache/impala/util/TestRequestPoolService.java
M fe/src/test/resources/fair-scheduler-test.xml
M fe/src/test/resources/fair-scheduler-test2.xml
M fe/src/test/resources/llama-site-test.xml
M fe/src/test/resources/llama-site-test2.xml
M 
testdata/workloads/functional-query/queries/QueryTest/admission-max-min-mem-limits.test
M 
testdata/workloads/functional-query/queries/QueryTest/admission-reject-mem-estimate.test
M 
testdata/workloads/functional-query/queries/QueryTest/admission-reject-min-reservation.test
M tests/common/impala_cluster.py
M tests/custom_cluster/test_admission_controller.py
M tests/webserver/test_web_pages.py
M www/admission_controller.tmpl
20 files changed, 1,408 insertions(+), 305 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/13307/9
--
To view, visit http://gerrit.cloudera.org:8080/13307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If47508728124076f3b9200c27cffc989f7a4f188
Gerrit-Change-Number: 13307
Gerrit-PatchSet: 9
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-7608: Estimate row count from file size when no stats available

2019-06-11 Thread Fang-Yu Rao (Code Review)
Fang-Yu Rao has uploaded a new patch set (#14). ( 
http://gerrit.cloudera.org:8080/12974 )

Change subject: IMPALA-7608: Estimate row count from file size when no stats 
available
..

IMPALA-7608: Estimate row count from file size when no stats available

Added the feature that computes an estimated number of rows in the current
hdfs table if the statistics for the cardinality of the current hdfs table is 
not
available.

Also added an additional query option to revert the change in case of 
regression.

Testing:
(1) In CardinalityTest.java, replaced the original statement
"verifyCardinality("SELECT a FROM functional.tinytable", -1);" in
the method testBasicsWithoutStats() with
"verifyCardinality("SELECT a FROM functional.tinytable", 2);".
(2) In CarginalityTest.java, added more tests to check the cardinality
of most PlanNode implementations. For each tested PlanNode, the behaviors
before and after we disable the feature are both tested.
(3) In set.test, modified three related test cases to make sure that
the added query option is included after executing "set all" in various
scenarios.
(4) There are 8 JUnit tests in PlannerTest.java that would produce different
distributed query plans when this feature is enabled. Added an additional
JUnit test for each of those 8 affected JUnit tests when this feature is
enabled. Specifically, each tested query in a newly added test files involves
at least one hdfs table without available statistics.

Change-Id: Ic414121c8df0d5222e4aeea096b5365beb04568a
---
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M fe/src/main/java/org/apache/impala/catalog/HdfsCompression.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/test/java/org/apache/impala/planner/CardinalityTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
A 
testdata/workloads/functional-planner/queries/PlannerTest/default-join-distr-mode-shuffle-hdfs-num-rows-est-enabled.test
A 
testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection-hdfs-num-rows-est-enabled.test
A 
testdata/workloads/functional-planner/queries/PlannerTest/joins-hdfs-num-rows-est-enabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
A 
testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters-hdfs-num-rows-est-enabled.test
A 
testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation-hdfs-num-rows-est-enabled.test
A 
testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements-hdfs-num-rows-est-enabled.test
A 
testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing-hdfs-num-rows-est-enabled.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
A 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite-hdfs-num-rows-est-enabled.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test
M testdata/workloads/functional-query/queries/QueryTest/set.test
21 files changed, 2,940 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/12974/14
--
To view, visit http://gerrit.cloudera.org:8080/12974
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic414121c8df0d5222e4aeea096b5365beb04568a
Gerrit-Change-Number: 12974
Gerrit-PatchSet: 14
Gerrit-Owner: Fang-Yu Rao 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 


[Impala-ASF-CR] IMPALA-8617: Add support for lz4 in parquet

2019-06-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13582 )

Change subject: IMPALA-8617: Add support for lz4 in parquet
..


Patch Set 5:

I probably won't have a chance to review this, but it looks like other 
reviewers should be able to cover this. Please let me know if I should take a 
look.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6850a39ef3f1e0e7ba48e08eef1d4f7cbb74d0c
Gerrit-Change-Number: 13582
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 12 Jun 2019 00:17:39 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7290: part 2: Add HS2 support to Impala shell

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12884 )

Change subject: IMPALA-7290: part 2: Add HS2 support to Impala shell
..


Patch Set 20:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12884/20/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/12884/20/shell/impala_shell.py@1658
PS20, Line 1658: def impala_shell_main():
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/12884/20/shell/impala_shell.py@1852
PS20, Line 1852: if __name__ == "__main__":
flake8: E305 expected 2 blank lines after class or function definition, found 1



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d5cc83d545aacc659523f29b1d6feed672e2a12
Gerrit-Change-Number: 12884
Gerrit-PatchSet: 20
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 12 Jun 2019 00:13:39 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7290: part 2: Add HS2 support to Impala shell

2019-06-11 Thread Tim Armstrong (Code Review)
Hello Thomas Marshall, Fredy Wijaya, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7290: part 2: Add HS2 support to Impala shell
..

IMPALA-7290: part 2: Add HS2 support to Impala shell

HS2 is the new default and the user-visible differences
are minimal. Beeswax is still supported via --protocol=beeswax
but will be deprecated. Differences are abstracted into
ImpalaClient subclasses.

This support requires Impala-specific extensions to
the HS2 interface, similar to the existing extensions
to Beeswax. Thus the HS2 shell is only
forwards-compatible with newer Impala versions.
I considered trying to gracefully degrade when the
new extensions weren't present, but it didn't seem to be
worth the ongoing testing effort.

Here are the changes required to make it work:
* Switch to TBinaryProtocolAccelerated to avoid perf
  regression. The HS2 protocol requires decoding
  more primitive values (because its not a string-per-row),
  which was slow with the pure python implementation of
  TBinaryProtocol.
* Added bitarray module to efficiently unpack null indicators
* Minimise invasiveness of changes by transposing and stringifying
  the columnar results into rows in impala_client.py. The transposition
  needs to happen before display anyway.
* Add PingImpalaHS2Service() to get back version string and webserver
  address.
* Add CloseImpalaOperation() extension to return DML row counts. This
  possibly addresses IMPALA-1789, although we need to confirm that
  this is a sufficient solution.
* Add is_closed member to query handles to avoid shell independently
  tracking whether the query handle was closed or not.
* Include query status in HS2 log to match beeswax.
* HS2 GetLog() command now includes query status error message for
  consistency with beeswax.
* "set"/"set all" uses the client requests options, not the session
  default. This captures the effective value of TIMEZONE, which
  was previously missing. This also requires test changes where
  the tests set non-default values, e.g. for ABORT_ON_ERROR.
* "set all" on the server side returns REMOVED query options - the
  shell needs to know these so it can correctly ignore them.
* Clean up self.orig_cmd/self.last_leading comment argument
  passing to avoid implicit parameter passing through multiple
  function calls.
* Clean up argument handling in shell tests to consistently pass
  around lists of arguments instead of strings that are subject
  to shell tokenisation rules.
* Consistently close connections in the shell to avoid leaking
  HS2 sessions. This is enforced by making ImpalaShell a context
  manager and also eliminating all sys.exit() calls that would
  bypass the explicit connection closing.

Testing:
* Shell tests can run with both protocols
* Add tests for formatting of all types and NULL values
* Added testing for floating point output formatting, which does
  change as a result of switching to server-side vs client-side
  formatting.
* Verified that newly-added tests were actually going through HS2
  by disabling hs2 on the minicluster and running tests.
* Add checks to test_verify_metrics.py to ensure that no sessions
  are left open at the end of tests.

Performance:
Baseline from beeswax shell for large extract is as follows:

  $ time impala-shell.sh -B -q 'select * from tpch_parquet.orders' > /dev/null
  real0m6.708s
  user0m5.132s
  sys 0m0.204s

After this change it is somewhat slower, but we generally don't consider
bulk extract performance through the shell to be perf-critical:
  real0m7.625s
  user0m6.436s
  sys 0m0.256s

Change-Id: I6d5cc83d545aacc659523f29b1d6feed672e2a12
---
M LICENSE.txt
M be/src/runtime/dml-exec-state.cc
M be/src/runtime/dml-exec-state.h
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M bin/rat_exclude_files.txt
M common/thrift/ImpalaService.thrift
M infra/python/deps/compiled-requirements.txt
A shell/ext-py/bitarray-0.9.0/PKG-INFO
A shell/ext-py/bitarray-0.9.0/bitarray/__init__.py
A shell/ext-py/bitarray-0.9.0/bitarray/_bitarray.c
A shell/ext-py/bitarray-0.9.0/bitarray/test_bitarray.py
A shell/ext-py/bitarray-0.9.0/setup.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/make_shell_tarball.sh
M shell/option_parser.py
M shell/thrift_sasl.py
M testdata/workloads/functional-query/queries/QueryTest/set.test
M tests/common/impala_test_suite.py
M tests/custom_cluster/test_shell_interactive.py
M tests/custom_cluster/test_shell_interactive_reconnect.py
M tests/hs2/test_hs2.py
M tests/query_test/test_observability.py
M tests/shell/test_shell_commandline.py
M 

[Impala-ASF-CR] IMPALA-7290: part 2: Add HS2 support to Impala shell

2019-06-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12884 )

Change subject: IMPALA-7290: part 2: Add HS2 support to Impala shell
..


Patch Set 19:

Rebased. Had to make some changes for IMPALA-8605.

Rebasing onto IMPALA-1653 (don't close sessions when connection closed) was 
more interesting - it turns out the shell was really bad about closing 
connections, so I had to rework the ImpalaShell lifecycle to make sure it 
reliably closes connections before exiting the process. Added some checks to 
tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d5cc83d545aacc659523f29b1d6feed672e2a12
Gerrit-Change-Number: 12884
Gerrit-PatchSet: 19
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 12 Jun 2019 00:08:58 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7290: part 2: Add HS2 support to Impala shell

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12884 )

Change subject: IMPALA-7290: part 2: Add HS2 support to Impala shell
..


Patch Set 19:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12884/19/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/12884/19/shell/impala_shell.py@1656
PS19, Line 1656: def impala_shell_main():
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/12884/19/shell/impala_shell.py@1850
PS19, Line 1850: if __name__ == "__main__":
flake8: E305 expected 2 blank lines after class or function definition, found 1



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d5cc83d545aacc659523f29b1d6feed672e2a12
Gerrit-Change-Number: 12884
Gerrit-PatchSet: 19
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 12 Jun 2019 00:07:05 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7290: part 2: Add HS2 support to Impala shell

2019-06-11 Thread Tim Armstrong (Code Review)
Hello Thomas Marshall, Fredy Wijaya, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7290: part 2: Add HS2 support to Impala shell
..

IMPALA-7290: part 2: Add HS2 support to Impala shell

HS2 is the new default and the user-visible differences
are minimal. Beeswax is still supported via --protocol=beeswax
but will be deprecated. Differences are abstracted into
ImpalaClient subclasses.

This support requires Impala-specific extensions to
the HS2 interface, similar to the existing extensions
to Beeswax. Thus the HS2 shell is only
forwards-compatible with newer Impala versions.
I considered trying to gracefully degrade when the
new extensions weren't present, but it didn't seem to be
worth the ongoing testing effort.

Here are the changes required to make it work:
* Switch to TBinaryProtocolAccelerated to avoid perf
  regression. The HS2 protocol requires decoding
  more primitive values (because its not a string-per-row),
  which was slow with the pure python implementation of
  TBinaryProtocol.
* Added bitarray module to efficiently unpack null indicators
* Minimise invasiveness of changes by transposing and stringifying
  the columnar results into rows in impala_client.py. The transposition
  needs to happen before display anyway.
* Add PingImpalaHS2Service() to get back version string and webserver
  address.
* Add CloseImpalaOperation() extension to return DML row counts. This
  possibly addresses IMPALA-1789, although we need to confirm that
  this is a sufficient solution.
* Add is_closed member to query handles to avoid shell independently
  tracking whether the query handle was closed or not.
* Include query status in HS2 log to match beeswax.
* HS2 GetLog() command now includes query status error message for
  consistency with beeswax.
* "set"/"set all" uses the client requests options, not the session
  default. This captures the effective value of TIMEZONE, which
  was previously missing. This also requires test changes where
  the tests set non-default values, e.g. for ABORT_ON_ERROR.
* "set all" on the server side returns REMOVED query options - the
  shell needs to know these so it can correctly ignore them.
* Clean up self.orig_cmd/self.last_leading comment argument
  passing to avoid implicit parameter passing through multiple
  function calls.
* Clean up argument handling in shell tests to consistently pass
  around lists of arguments instead of strings that are subject
  to shell tokenisation rules.
* Consistently close connections in the shell to avoid leaking
  HS2 sessions. This is enforced by making ImpalaShell a context
  manager and also eliminating all sys.exit() calls that would
  bypass the explicit connection closing.

Testing:
* Shell tests can run with both protocols
* Add tests for formatting of all types and NULL values
* Added testing for floating point output formatting, which does
  change as a result of switching to server-side vs client-side
  formatting.
* Verified that newly-added tests were actually going through HS2
  by disabling hs2 on the minicluster and running tests.
* Add checks to test_verify_metrics.py to ensure that no sessions
  are left open at the end of tests.

Performance:
Baseline from beeswax shell for large extract is as follows:

  $ time impala-shell.sh -B -q 'select * from tpch_parquet.orders' > /dev/null
  real0m6.708s
  user0m5.132s
  sys 0m0.204s

After this change it is somewhat slower, but we generally don't consider
bulk extract performance through the shell to be perf-critical:
  real0m7.625s
  user0m6.436s
  sys 0m0.256s

Change-Id: I6d5cc83d545aacc659523f29b1d6feed672e2a12
---
M LICENSE.txt
M be/src/runtime/dml-exec-state.cc
M be/src/runtime/dml-exec-state.h
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M bin/rat_exclude_files.txt
M common/thrift/ImpalaService.thrift
M infra/python/deps/compiled-requirements.txt
A shell/ext-py/bitarray-0.9.0/PKG-INFO
A shell/ext-py/bitarray-0.9.0/bitarray/__init__.py
A shell/ext-py/bitarray-0.9.0/bitarray/_bitarray.c
A shell/ext-py/bitarray-0.9.0/bitarray/test_bitarray.py
A shell/ext-py/bitarray-0.9.0/setup.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/make_shell_tarball.sh
M shell/option_parser.py
M shell/thrift_sasl.py
M testdata/workloads/functional-query/queries/QueryTest/set.test
M tests/common/impala_test_suite.py
M tests/custom_cluster/test_shell_interactive.py
M tests/custom_cluster/test_shell_interactive_reconnect.py
M tests/hs2/test_hs2.py
M tests/query_test/test_observability.py
M tests/shell/test_shell_commandline.py
M 

[Impala-ASF-CR] IMPALA-8654: Log the SQL statement in the Ranger audit log

2019-06-11 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13590 )

Change subject: IMPALA-8654: Log the SQL statement in the Ranger audit log
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id9f584ac4209604675eb13b6d6f349c6cbb1a387
Gerrit-Change-Number: 13590
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 11 Jun 2019 23:36:40 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8617: Add support for lz4 in parquet

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13582 )

Change subject: IMPALA-8617: Add support for lz4 in parquet
..


Patch Set 5:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/3569/ : Initial code 
review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6850a39ef3f1e0e7ba48e08eef1d4f7cbb74d0c
Gerrit-Change-Number: 13582
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 11 Jun 2019 23:36:05 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8654: Log the SQL statement in the Ranger audit log

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13590 )

Change subject: IMPALA-8654: Log the SQL statement in the Ranger audit log
..


Patch Set 1:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/3570/ : Initial code 
review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id9f584ac4209604675eb13b6d6f349c6cbb1a387
Gerrit-Change-Number: 13590
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 11 Jun 2019 23:36:04 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8617: Add support for lz4 in parquet

2019-06-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13582 )

Change subject: IMPALA-8617: Add support for lz4 in parquet
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13582/2/common/thrift/generate_error_codes.py
File common/thrift/generate_error_codes.py:

http://gerrit.cloudera.org:8080/#/c/13582/2/common/thrift/generate_error_codes.py@421
PS2, Line 421:"LZ4Block: Decompressed size is not correct."),
> it seems we're missing the underlying HDFS path which had the bad block, th
Yes! We should include the file path and column name in the error message. E.g. 
see PARQUET_DATE_OUT_OF_RANGE just above



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6850a39ef3f1e0e7ba48e08eef1d4f7cbb74d0c
Gerrit-Change-Number: 13582
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 11 Jun 2019 23:34:56 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] Update mustache to commit b290952d8eb93d085214d8c8c9eab8559df9f606

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/13563 )

Change subject: Update mustache to commit 
b290952d8eb93d085214d8c8c9eab8559df9f606
..

Update mustache to commit b290952d8eb93d085214d8c8c9eab8559df9f606

This fixes some issues with regard to template variable scoping that
will be useful for Knox integration.

Change-Id: If26f3aaa2a3279a1f6a300c4f4cee7ec899e22ed
Reviewed-on: http://gerrit.cloudera.org:8080/13563
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/thirdparty/mustache/README
M be/src/thirdparty/mustache/mustache.cc
M be/src/thirdparty/mustache/mustache.h
M be/src/util/webserver.cc
4 files changed, 184 insertions(+), 102 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If26f3aaa2a3279a1f6a300c4f4cee7ec899e22ed
Gerrit-Change-Number: 13563
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] Update mustache to commit b290952d8eb93d085214d8c8c9eab8559df9f606

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13563 )

Change subject: Update mustache to commit 
b290952d8eb93d085214d8c8c9eab8559df9f606
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If26f3aaa2a3279a1f6a300c4f4cee7ec899e22ed
Gerrit-Change-Number: 13563
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 11 Jun 2019 23:24:59 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8654: Log the SQL statement in the Ranger audit log

2019-06-11 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/13590


Change subject: IMPALA-8654: Log the SQL statement in the Ranger audit log
..

IMPALA-8654: Log the SQL statement in the Ranger audit log

This patch logs the SQL statement to be authorized in the Ranger audit
log since it was a required field prior to the fix in RANGER-2463 to
avoid a NullPointerException in the Ranger admin that could cause the
Ranger audit logs to not show up in the Ranger web UI.

Logging the SQL statement requires proper implementation of
StatementBase::toSql() which unfortunately is not available in all SQL
statements. See IMPALA-8655.

Testing:
- Updated the RangerAuditLogTest
- Tested the changes against Solr cluster

Change-Id: Id9f584ac4209604675eb13b6d6f349c6cbb1a387
---
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java
M 
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M 
fe/src/main/java/org/apache/impala/authorization/ranger/RangerBufferAuditHandler.java
M 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java
M 
fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
10 files changed, 84 insertions(+), 12 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id9f584ac4209604675eb13b6d6f349c6cbb1a387
Gerrit-Change-Number: 13590
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya 


[Impala-ASF-CR] IMPALA-8649: Fix confusing SHOW GRANT error messages

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13587 )

Change subject: IMPALA-8649: Fix confusing SHOW GRANT error messages
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb88bdc19cd1223902b44e3634f756d086332266
Gerrit-Change-Number: 13587
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Austin Nobis 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 11 Jun 2019 23:11:34 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8649: Fix confusing SHOW GRANT error messages

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13587 )

Change subject: IMPALA-8649: Fix confusing SHOW GRANT error messages
..


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb88bdc19cd1223902b44e3634f756d086332266
Gerrit-Change-Number: 13587
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Austin Nobis 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 11 Jun 2019 23:11:35 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8617: Add support for lz4 in parquet

2019-06-11 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13582 )

Change subject: IMPALA-8617: Add support for lz4 in parquet
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13582/2/common/thrift/generate_error_codes.py
File common/thrift/generate_error_codes.py:

http://gerrit.cloudera.org:8080/#/c/13582/2/common/thrift/generate_error_codes.py@421
PS2, Line 421:"LZ4Block: Decompressed size is not correct."),
> I think this is probably a result of having abort_on_error=0 by default. Fo
it seems we're missing the underlying HDFS path which had the bad block, 
though. Is it possible to ensure that this gets propagated up? If that's a lot 
of scope creep for this commit maybe we can file it as a follow-up? My fear is 
that someone hits this issue and we can't tell them which file is broken.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6850a39ef3f1e0e7ba48e08eef1d4f7cbb74d0c
Gerrit-Change-Number: 13582
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 11 Jun 2019 23:08:11 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8649: Fix confusing SHOW GRANT error messages

2019-06-11 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13587 )

Change subject: IMPALA-8649: Fix confusing SHOW GRANT error messages
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb88bdc19cd1223902b44e3634f756d086332266
Gerrit-Change-Number: 13587
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Austin Nobis 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 11 Jun 2019 23:06:51 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8617: Add support for lz4 in parquet

2019-06-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13582 )

Change subject: IMPALA-8617: Add support for lz4 in parquet
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13582/2/common/thrift/generate_error_codes.py
File common/thrift/generate_error_codes.py:

http://gerrit.cloudera.org:8080/#/c/13582/2/common/thrift/generate_error_codes.py@421
PS2, Line 421:"LZ4Block: Decompressed size is not correct."),
> impala-shell is pretty weird since it is treating these errors as warnings!
I think this is probably a result of having abort_on_error=0 by default. For 
historical reasons there has been behaviour of skipping over files with parse 
errors.

There's two mechanisms for returning errors - the query status and the query 
error log. The shell prefixes the query error log with "WARNING" if the query 
doesn't have an error status.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6850a39ef3f1e0e7ba48e08eef1d4f7cbb74d0c
Gerrit-Change-Number: 13582
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 11 Jun 2019 23:06:30 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8617: Add support for lz4 in parquet

2019-06-11 Thread Abhishek Rawat (Code Review)
Abhishek Rawat has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/13582 )

Change subject: IMPALA-8617: Add support for lz4 in parquet
..

IMPALA-8617: Add support for lz4 in parquet

A new enum value LZ4_BLOCKED was added to the THdfsCompression enum, to
distinguish it from the existing LZ4 codec. LZ4_BLOCKED codec represents
the block compression scheme used by Hadoop. Its similar to
SNAPPY_BLOCKED as far as the block format is concerned, with the only
difference being the codec used for compression and decompression.

Added Lz4BlockCompressor and Lz4BlockDecompressor classes for
compressing and decompressing parquet data using Hadoop's
lz4 block compression scheme.

The Lz4BlockCompressor treats the input
as a single block and generates a compressed block with following layout
  <4 byte big endian uncompressed size>
  <4 byte big endian compressed size>
  
The hdfs parquet table writer should call the Lz4BlockCompressor
using the ideal input size (unit of compression in parquet is a page),
and so the Lz4BlockCompressor does not further break down the input
into smaller blocks.

The Lz4BlockDecompressor on the other hand should be compatible with
blocks written by Impala and other engines in Hadoop ecosystem. It can
decompress compressed data in following format
  <4 byte big endian uncompressed size>
  <4 byte big endian compressed size>
  
  ...
  <4 byte big endian compressed size>
  
  ...
  

Externally users can now set the lz4 codec for parquet using:
  set COMPRESSION_CODEC=lz4
This gets translated into LZ4_BLOCKED codec for the
HdfsParquetTableWriter. Similarly, when reading lz4 compressed parquet
data, the LZ4_BLOCKED codec is used.

Testing:
 - Added unit tests for LZ4_BLOCKED in decompress-test.cc
 - Added unit tests for Hadoop compatibility in decompress-test.cc,
   basically being able to decompress an outer block with multiple inner
   blocks (the Lz4BlockDecompressor description above)
 - Added interoperability tests for Hive and Impala for all parquet
   codecs. New test added to
   tests/custom_cluster/test_hive_parquet_codec_interop.py

Change-Id: Ia6850a39ef3f1e0e7ba48e08eef1d4f7cbb74d0c
---
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/parquet-common.cc
M be/src/exec/parquet/parquet-metadata-utils.cc
M be/src/service/query-options-test.cc
M be/src/util/codec.cc
M be/src/util/compress.cc
M be/src/util/compress.h
M be/src/util/decompress-test.cc
M be/src/util/decompress.cc
M be/src/util/decompress.h
M common/thrift/CatalogObjects.thrift
M common/thrift/generate_error_codes.py
M testdata/workloads/functional-query/queries/QueryTest/set.test
M tests/common/test_dimensions.py
A tests/custom_cluster/test_hive_parquet_codec_interop.py
M tests/query_test/test_insert.py
M tests/query_test/test_insert_parquet.py
17 files changed, 349 insertions(+), 13 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia6850a39ef3f1e0e7ba48e08eef1d4f7cbb74d0c
Gerrit-Change-Number: 13582
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 


[Impala-ASF-CR] IMPALA-7608: Estimate row count from file size when no stats available

2019-06-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12974 )

Change subject: IMPALA-7608: Estimate row count from file size when no stats 
available
..


Patch Set 12:

(3 comments)

I have a few more nits, then I'm happy.

http://gerrit.cloudera.org:8080/#/c/12974/11/fe/src/test/java/org/apache/impala/planner/CardinalityTest.java
File fe/src/test/java/org/apache/impala/planner/CardinalityTest.java:

http://gerrit.cloudera.org:8080/#/c/12974/11/fe/src/test/java/org/apache/impala/planner/CardinalityTest.java@567
PS11, Line 567:   // TODO: It seems that the cardinality of the SelectNode 
should be 1 instead
TODO(IMPALA-8647) is a bit more standard.


http://gerrit.cloudera.org:8080/#/c/12974/11/fe/src/test/java/org/apache/impala/planner/CardinalityTest.java@768
PS11, Line 768:  // The cardinality check performed by this method
Should be a javadoc comment - can you check the rest of the methods in this 
file to make sure they're all javadoc comments.


http://gerrit.cloudera.org:8080/#/c/12974/11/fe/src/test/java/org/apache/impala/planner/CardinalityTest.java@779
PS11, Line 779:   // This method allows us to inspect the cardinality of
Should be a javadoc comment



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic414121c8df0d5222e4aeea096b5365beb04568a
Gerrit-Change-Number: 12974
Gerrit-PatchSet: 12
Gerrit-Owner: Fang-Yu Rao 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 11 Jun 2019 23:00:16 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8617: Add support for lz4 in parquet

2019-06-11 Thread Abhishek Rawat (Code Review)
Abhishek Rawat has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13582 )

Change subject: IMPALA-8617: Add support for lz4 in parquet
..


Patch Set 2:

(8 comments)

Thanks for the review comments. Please see my responses below. I have addressed 
the comments and also fixed some test case errors hit during pre-review tests.

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

http://gerrit.cloudera.org:8080/#/c/13582/2//COMMIT_MSG@21
PS2, Line 21:  <4 byte big endian uncompressed size>
> nit: some weird formatting here and below
Hoping it looks better now.


http://gerrit.cloudera.org:8080/#/c/13582/2//COMMIT_MSG@25
PS2, Line 25: parquer
> typo
Done


http://gerrit.cloudera.org:8080/#/c/13582/2/be/src/util/compress.cc
File be/src/util/compress.cc:

http://gerrit.cloudera.org:8080/#/c/13582/2/be/src/util/compress.cc@355
PS2, Line 355: length = sizeof (int32_t);
> style nit: I don't think we usually have a space between sizeof and '('
Done


http://gerrit.cloudera.org:8080/#/c/13582/2/be/src/util/compress.cc@381
PS2, Line 381: int64_t size = LZ4_compress_default(reinterpret_cast(input),
> lz4.h indicates that this can return 0 to indicate failure of some kind. I
Added a CHECK() to ensure compressed length returned is > 0.


http://gerrit.cloudera.org:8080/#/c/13582/2/be/src/util/decompress-test.cc
File be/src/util/decompress-test.cc:

http://gerrit.cloudera.org:8080/#/c/13582/2/be/src/util/decompress-test.cc@559
PS2, Line 559:   EXPECT_EQ(Ubsan::MemCmp(input_, output, input_len), 0);
> do we ever expect input_ or output_ to be null? if not, we can use regular
input_/output should not be null and so using memcmp instead.


http://gerrit.cloudera.org:8080/#/c/13582/2/common/thrift/generate_error_codes.py
File common/thrift/generate_error_codes.py:

http://gerrit.cloudera.org:8080/#/c/13582/2/common/thrift/generate_error_codes.py@421
PS2, Line 421:"LZ4Block: Decompressed size is not correct."),
> when these errors get propagated up, do we get the context of which file ha
impala-shell is pretty weird since it is treating these errors as warnings!

```
Query: select * from t1_lz4
Query submitted at: 2019-06-11 15:07:48 (Coordinator: 
http://optimus-prime:25000)
Query progress can be monitored at: 
http://optimus-prime:25000/query_plan?query_id=4a4fa37d756cade3:644f63aa
+--+--+--+
| c1   | c2   | c3   |
+--+--+--+
| NULL | NULL | NULL |
+--+--+--+
WARNINGS: LZ4Block: Invalid input length. (1 of 2 similar)
```


But, we do get the stack trace and error message in the log file.

```
I0611 15:07:48.827903 28994 status.cc:55] 4a4fa37d756cade3:644f63aa0002] 
LZ4Block: Invalid input length.
@  0x1af8e99  impala::Status::Status()
@  0x1b004c8  impala::Status::Status()
@  0x225fa43  impala::Lz4BlockDecompressor::ProcessBlock()
@  0x2235540  impala::Codec::ProcessBlock32()
@  0x2931acf  impala::BaseScalarColumnReader::InitDictionary()
@  0x29324d1  impala::BaseScalarColumnReader::InitDictionaries()
@  0x28df386  impala::HdfsParquetScanner::NextRowGroup()
@  0x28dd366  impala::HdfsParquetScanner::GetNextInternal()
@  0x28db77a  impala::HdfsParquetScanner::ProcessSplit()
@  0x23cf573  impala::HdfsScanNode::ProcessSplit()
@  0x23ce748  impala::HdfsScanNode::ScannerThread()
@  0x23cdad1  
_ZZN6impala12HdfsScanNode22ThreadTokenAvailableCbEPNS_18ThreadResourcePoolEENKUlvE_clEv
@  0x23d0099  
_ZN5boost6detail8function26void_function_obj_invoker0IZN6impala12HdfsScanNode22ThreadTokenAvailableCbEPNS3_18ThreadResourcePoolEEUlvE_vE6invokeERNS1_15function_bufferE
@  0x1e14fdd  boost::function0<>::operator()()
@  0x231c010  impala::Thread::SuperviseThread()
@  0x2324394  boost::_bi::list5<>::operator()<>()
@  0x23242b8  boost::_bi::bind_t<>::operator()()
@  0x232427b  boost::detail::thread_data<>::run()
@  0x39e2439  thread_proxy
@ 0x7fb5a30ce6b9  start_thread
@ 0x7fb59f8ef41c  clone
I0611 15:07:48.827935 28994 runtime-state.cc:199]
...
Error from query 4a4fa37d756cade3:644f63aa: LZ4Block: Invalid input 
length.
```


http://gerrit.cloudera.org:8080/#/c/13582/2/tests/custom_cluster/test_hive_parquet_codec_interop.py
File tests/custom_cluster/test_hive_parquet_codec_interop.py:

http://gerrit.cloudera.org:8080/#/c/13582/2/tests/custom_cluster/test_hive_parquet_codec_interop.py@20
PS2, Line 20: import os
> flake8: F401 'os' imported but unused
Done


http://gerrit.cloudera.org:8080/#/c/13582/2/tests/custom_cluster/test_hive_parquet_codec_interop.py@32
PS2, Line 32: class TestParquetInterop(CustomClusterTestSuite):
> flake8: E302 expected 2 blank lines, found 1
Done



--
To view, visit http://gerrit.cloudera.org:8080/13582

[Impala-ASF-CR] IMPALA-8649: Fix confusing SHOW GRANT error messages

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13587 )

Change subject: IMPALA-8649: Fix confusing SHOW GRANT error messages
..


Patch Set 2:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb88bdc19cd1223902b44e3634f756d086332266
Gerrit-Change-Number: 13587
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Austin Nobis 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 11 Jun 2019 22:49:45 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8605: clean up HS2/beeswax session management

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/13585 )

Change subject: IMPALA-8605: clean up HS2/beeswax session management
..

IMPALA-8605: clean up HS2/beeswax session management

Session/operation secrets are part of the HS2 handle
but we haven't made use of them up until now. This
patch checks the value and treats it as part of the
session key, as originally intended. I.e. if the
secret is missing, the session lookup fails.

The operation secret is the same as the session secret
to save having to generate and store extra secrets
(there's no real benefit).

A secret is added to each Beeswax session. This secret is
internal to the server and not exposed. Adds validation
that client requests accessed via Beeswax belong to the
same user as the session.

We switch uuid_generator_ to use boost::random_device, which
uses /dev/urandom as its source of randomness to be more robust -
otherwise it's hard to be sure that we won't have collisions,
although it doesn't seem to be a problem in practice.

For requests - GetRuntimeProfile() and GetExecSummary()
that provide both a session and query ID, the code already
checks that the session's user matches the query.

An exception to the validation mechanisms above is added
for Close() and Cancel() beeswax operations, because impala-shell
and some administrative tools allow cancellation of
queries on different threads and from different tools.

We skip validating the session secret when cancelling queries
from the web UI, since web UI users don't have the secret.

Testing:
* Ran exhaustive tests.
* Add tests for all HS2 RPCs that provide invalid session secrets.
* Add tests for HS2 RPCs that provide both session and query
  ID to ensure that query belongs to the session.
* Add basic test for beeswax testing accessing a query from
  different connections.

Change-Id: I4c014d1a32e273275a773f842b9ed9793dbdba6b
Reviewed-on: http://gerrit.cloudera.org:8080/13585
Reviewed-by: Lars Volker 
Reviewed-by: Thomas Marshall 
Tested-by: Impala Public Jenkins 
---
M CMakeLists.txt
M be/src/service/child-query.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-server.cc
M be/src/service/impala-server.h
M tests/beeswax/impala_beeswax.py
M tests/common/impala_connection.py
M tests/hs2/hs2_test_suite.py
M tests/hs2/test_fetch.py
M tests/hs2/test_hs2.py
A tests/query_test/test_beeswax.py
13 files changed, 688 insertions(+), 133 deletions(-)

Approvals:
  Lars Volker: Looks good to me, approved
  Thomas Marshall: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I4c014d1a32e273275a773f842b9ed9793dbdba6b
Gerrit-Change-Number: 13585
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Thomas Marshall 


[Impala-ASF-CR] IMPALA-8605: clean up HS2/beeswax session management

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13585 )

Change subject: IMPALA-8605: clean up HS2/beeswax session management
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c014d1a32e273275a773f842b9ed9793dbdba6b
Gerrit-Change-Number: 13585
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Tue, 11 Jun 2019 22:38:23 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7154 : Error making 'dropDatabase' RPC to Hive Metastore. NoSuchObjectException thrown

2019-06-11 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13586 )

Change subject: IMPALA-7154 : Error making 'dropDatabase' RPC to Hive 
Metastore. NoSuchObjectException thrown
..


Patch Set 1: Code-Review+1

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/13586/1//COMMIT_MSG@14
PS1, Line 14: timedout
typo: timed out



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I253fc00618b28d8d6e626e1b6bf9c5c8290006d7
Gerrit-Change-Number: 13586
Gerrit-PatchSet: 1
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Tue, 11 Jun 2019 22:26:23 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8561: Eliminate mtime=-1 for HDFS scan ranges (part 1)

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13522 )

Change subject: IMPALA-8561: Eliminate mtime=-1 for HDFS scan ranges (part 1)
..


Patch Set 6: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I48b7ed60d6ab9104b993237b4fe23de5dc058672
Gerrit-Change-Number: 13522
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 11 Jun 2019 22:22:02 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7154 : Error making 'dropDatabase' RPC to Hive Metastore. NoSuchObjectException thrown

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13586 )

Change subject: IMPALA-7154 : Error making 'dropDatabase' RPC to Hive 
Metastore. NoSuchObjectException thrown
..


Patch Set 1:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I253fc00618b28d8d6e626e1b6bf9c5c8290006d7
Gerrit-Change-Number: 13586
Gerrit-PatchSet: 1
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Tue, 11 Jun 2019 22:20:38 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8649: Fix confusing SHOW GRANT error messages

2019-06-11 Thread Austin Nobis (Code Review)
Austin Nobis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13587 )

Change subject: IMPALA-8649: Fix confusing SHOW GRANT error messages
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb88bdc19cd1223902b44e3634f756d086332266
Gerrit-Change-Number: 13587
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Austin Nobis 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 11 Jun 2019 22:09:32 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7154 : Error making 'dropDatabase' RPC to Hive Metastore. NoSuchObjectException thrown

2019-06-11 Thread Vihang Karajgaonkar (Code Review)
Vihang Karajgaonkar has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/13586


Change subject: IMPALA-7154 : Error making 'dropDatabase' RPC to Hive 
Metastore. NoSuchObjectException thrown
..

IMPALA-7154 : Error making 'dropDatabase' RPC to Hive Metastore. 
NoSuchObjectException thrown

Some of the e2e tests create tables and databases on S3. HMS operations
like dropDatabaseCascade can take significant time (3-5 min) in such
tests. If the HMS Client times out, it retries for a configurable number
of times. Unfortunately, HMS client is not smart enough to understand
which operations should be retried. Eg, a drop database operation which
is timedout on the client side, should not be retried since the server
thread may have succeeded before the next retry. This would cause the
client to receive a NoSuchObjectException which would look like a flaky
test failure on the Impala side.

This patch increases the metastore client.socket.timeout for
the tests so that such flaky test failures are reduced.

Note:
1. The timeout value of 5 min was chosen based on looking at logs for
one such failure where the HMS API took about 3.5 min. Once this patch
is deployed we should continue to see if such failures reoccur and
readjust the client timeout value accordingly.

Change-Id: I253fc00618b28d8d6e626e1b6bf9c5c8290006d7
---
M fe/src/test/resources/hive-site.xml.py
1 file changed, 5 insertions(+), 2 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I253fc00618b28d8d6e626e1b6bf9c5c8290006d7
Gerrit-Change-Number: 13586
Gerrit-PatchSet: 1
Gerrit-Owner: Vihang Karajgaonkar 


[Impala-ASF-CR] IMPALA-8436: Prohibit write/alter operations on materialized view

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13503 )

Change subject: IMPALA-8436: Prohibit write/alter operations on materialized 
view
..


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idcd619303e19b5a2551876a63d67569c76bd22f0
Gerrit-Change-Number: 13503
Gerrit-PatchSet: 5
Gerrit-Owner: Sudhanshu Arora 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sudhanshu Arora 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Reviewer: Yongzhi Chen 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 11 Jun 2019 21:04:50 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8617: Add support for lz4 in parquet

2019-06-11 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13582 )

Change subject: IMPALA-8617: Add support for lz4 in parquet
..


Patch Set 2:

(6 comments)

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

http://gerrit.cloudera.org:8080/#/c/13582/2//COMMIT_MSG@21
PS2, Line 21:  <4 byte big endian uncompressed size>
nit: some weird formatting here and below


http://gerrit.cloudera.org:8080/#/c/13582/2//COMMIT_MSG@25
PS2, Line 25: parquer
typo


http://gerrit.cloudera.org:8080/#/c/13582/2/be/src/util/compress.cc
File be/src/util/compress.cc:

http://gerrit.cloudera.org:8080/#/c/13582/2/be/src/util/compress.cc@355
PS2, Line 355: length = sizeof (int32_t);
style nit: I don't think we usually have a space between sizeof and '('


http://gerrit.cloudera.org:8080/#/c/13582/2/be/src/util/compress.cc@381
PS2, Line 381: int64_t size = LZ4_compress_default(reinterpret_cast(input),
lz4.h indicates that this can return 0 to indicate failure of some kind. I 
can't think of any reasons it would happen aside from OOM but it's probably 
worth a CHECK


http://gerrit.cloudera.org:8080/#/c/13582/2/be/src/util/decompress-test.cc
File be/src/util/decompress-test.cc:

http://gerrit.cloudera.org:8080/#/c/13582/2/be/src/util/decompress-test.cc@559
PS2, Line 559:   EXPECT_EQ(Ubsan::MemCmp(input_, output, input_len), 0);
do we ever expect input_ or output_ to be null? if not, we can use regular 
memcmp here


http://gerrit.cloudera.org:8080/#/c/13582/2/common/thrift/generate_error_codes.py
File common/thrift/generate_error_codes.py:

http://gerrit.cloudera.org:8080/#/c/13582/2/common/thrift/generate_error_codes.py@421
PS2, Line 421:"LZ4Block: Decompressed size is not correct."),
when these errors get propagated up, do we get the context of which file had 
the issue?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6850a39ef3f1e0e7ba48e08eef1d4f7cbb74d0c
Gerrit-Change-Number: 13582
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 11 Jun 2019 20:51:21 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8633 : Insert event should not error when table does not exists

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13548 )

Change subject: IMPALA-8633 : Insert event should not error when table does not 
exists
..


Patch Set 5:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I961cd7cbede4c248dba538c7fabb4bc708e49693
Gerrit-Change-Number: 13548
Gerrit-PatchSet: 5
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Anurag Mantripragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Tue, 11 Jun 2019 20:50:57 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8633 : Insert event should not error when table does not exists

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13548 )

Change subject: IMPALA-8633 : Insert event should not error when table does not 
exists
..


Patch Set 6:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I961cd7cbede4c248dba538c7fabb4bc708e49693
Gerrit-Change-Number: 13548
Gerrit-PatchSet: 6
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Anurag Mantripragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Tue, 11 Jun 2019 20:47:27 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8633 : Insert event should not error when table does not exists

2019-06-11 Thread Vihang Karajgaonkar (Code Review)
Vihang Karajgaonkar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13548 )

Change subject: IMPALA-8633 : Insert event should not error when table does not 
exists
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13548/4/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
File 
fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java:

http://gerrit.cloudera.org:8080/#/c/13548/4/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@755
PS4, Line 755: BLE event is processe
> I see you are creating the insert events by calling the HMS API. You can pr
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I961cd7cbede4c248dba538c7fabb4bc708e49693
Gerrit-Change-Number: 13548
Gerrit-PatchSet: 6
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Anurag Mantripragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Tue, 11 Jun 2019 20:05:19 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8633 : Insert event should not error when table does not exists

2019-06-11 Thread Vihang Karajgaonkar (Code Review)
Vihang Karajgaonkar has uploaded a new patch set (#6). ( 
http://gerrit.cloudera.org:8080/13548 )

Change subject: IMPALA-8633 : Insert event should not error when table does not 
exists
..

IMPALA-8633 : Insert event should not error when table does not exists

When a insert event is received by the events processor it is possible
that the table has already been removed from metastore. In such cases,
the reloadTable method throws a NoSuchObjectException which is
unhandled. This causes EventProcessor to go in NEEDS_INVALIDATE state.
The patch adds exception handling for such cases so that the error can
be ignored and a warning is logged.

Ideally, if the table has been removed from metastore, this insert event
should be followed by a drop-table event and hence the table is cleaned
up from the catalog subsequently. Hence Event processor does not need to
remove the table from the catalog.

Also, the patch adds the exception trace to the error messages during
processing of Insert events to improve debugging ability in case of
such errors.

Testing:

Refactored the existing test for insert events into util methods
which can be reused for other tests. Added a new test case which
generates a insert event on a table which has been dropped. This test
reproduces the scenario seen by debugging test failures in IMPALA-8567.
The newly added test succeeds after the patch.

Change-Id: I961cd7cbede4c248dba538c7fabb4bc708e49693
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M 
fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
5 files changed, 185 insertions(+), 153 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I961cd7cbede4c248dba538c7fabb4bc708e49693
Gerrit-Change-Number: 13548
Gerrit-PatchSet: 6
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Anurag Mantripragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 


[Impala-ASF-CR] IMPALA-8633 : Insert event should not error when table does not exists

2019-06-11 Thread Vihang Karajgaonkar (Code Review)
Vihang Karajgaonkar has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/13548 )

Change subject: IMPALA-8633 : Insert event should not error when table does not 
exists
..

IMPALA-8633 : Insert event should not error when table does not exists

When a insert event is received by the events processor it is possible
that the table has already been removed from metastore. In such cases,
the reloadTable method throws a NoSuchObjectException which is
unhandled. This causes EventProcessor to go in NEEDS_INVALIDATE state.
The patch adds exception handling for such cases so that the error can
be ignored and a warning is logged.

Ideally, if the table has been removed from metastore, this insert event
should be followed by a drop-table event and hence the table is cleaned
up from the catalog subsequently. Hence Event processor does not need to
remove the table from the catalog.

Also, the patch adds the exception trace to the error messages during
processing of Insert events to improve debugging ability in case of
such errors.

Testing:

Refactored the existing test for insert events into util methods
which can be reused for other tests. Added a new test case which
generates a insert event on a table which has been dropped. This test
reproduces the scenario seen by debugging test failures in IMPALA-8567.
The newly added test succeeds after the patch.

Change-Id: I961cd7cbede4c248dba538c7fabb4bc708e49693
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M 
fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
5 files changed, 184 insertions(+), 134 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I961cd7cbede4c248dba538c7fabb4bc708e49693
Gerrit-Change-Number: 13548
Gerrit-PatchSet: 5
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Anurag Mantripragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 


[Impala-ASF-CR] IMPALA-7290: part 2: Add HS2 support to Impala shell

2019-06-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12884 )

Change subject: IMPALA-7290: part 2: Add HS2 support to Impala shell
..


Patch Set 18:

I got back to this. It turns out that the shell is very bad about explicitly 
closing the client so it leaves lots of dangling HS2 sessions. Looking at 
fixing that.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d5cc83d545aacc659523f29b1d6feed672e2a12
Gerrit-Change-Number: 12884
Gerrit-PatchSet: 18
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 11 Jun 2019 19:59:10 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

2019-06-11 Thread Joe McDonnell (Code Review)
Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
..


Patch Set 1:

(1 comment)

Drive-by comment. Hope this helps!

http://gerrit.cloudera.org:8080/#/c/13558/1/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:

http://gerrit.cloudera.org:8080/#/c/13558/1/testdata/datasets/functional/functional_schema_template.sql@2183
PS1, Line 2183:  DEPENDENT_LOAD_HIVE
  : INSERT INTO TABLE {db_name}{db_suffix}.{table_name}
  : SELECT id, int_col from functional.alltypes;
This will only run for the non-text versions of the tables. If you want the 
same thing for text, you would want a LOAD section (which always runs in Hive). 
See, for example, complextypes_fileformat for a table that has an insert in the 
LOAD section.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 1
Gerrit-Owner: Yongzhi Chen 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Sudhanshu Arora 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Reviewer: Yongzhi Chen 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 11 Jun 2019 19:49:32 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8588: Fix revoke grant option with Ranger

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13450 )

Change subject: IMPALA-8588: Fix revoke grant option with Ranger
..


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iddfccb442c3be3c266dbc2d8ae85c5674c534d7c
Gerrit-Change-Number: 13450
Gerrit-PatchSet: 3
Gerrit-Owner: Austin Nobis 
Gerrit-Reviewer: Austin Nobis 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 11 Jun 2019 19:06:52 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8588: Fix revoke grant option with Ranger

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13450 )

Change subject: IMPALA-8588: Fix revoke grant option with Ranger
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iddfccb442c3be3c266dbc2d8ae85c5674c534d7c
Gerrit-Change-Number: 13450
Gerrit-PatchSet: 3
Gerrit-Owner: Austin Nobis 
Gerrit-Reviewer: Austin Nobis 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 11 Jun 2019 19:06:51 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8588: Fix revoke grant option with Ranger

2019-06-11 Thread Austin Nobis (Code Review)
Austin Nobis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13450 )

Change subject: IMPALA-8588: Fix revoke grant option with Ranger
..


Patch Set 2:

(1 comment)

> Patch Set 2:
>
> (1 comment)

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

http://gerrit.cloudera.org:8080/#/c/13450/2//COMMIT_MSG@20
PS2, Line 20: REVOKE GRANT OPTION FOR SELECT ON DATABASE  FROM USER 

:
: This will revoke the grant option for all privileges on this 
database
: resource. It will not revoke the SELECT privilege on the resource.
> i have some concern with this, especially since Ranger behaves differently
I think Option 1 makes more sense as it offers better functionality to the 
user.  That being said, I don't think it is required for this CR. I created a 
follow up JIRA, IMPALA-8651, for this work.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iddfccb442c3be3c266dbc2d8ae85c5674c534d7c
Gerrit-Change-Number: 13450
Gerrit-PatchSet: 2
Gerrit-Owner: Austin Nobis 
Gerrit-Reviewer: Austin Nobis 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 11 Jun 2019 18:56:00 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8612: Fix sporadic NPE when dropping an authorized table

2019-06-11 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13508 )

Change subject: IMPALA-8612: Fix sporadic NPE when dropping an authorized table
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13508/1/fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
File fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java:

http://gerrit.cloudera.org:8080/#/c/13508/1/fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java@124
PS1, Line 124:   analyzer.addAccessEvent(new TAccessEvent(
> Should we log the exception?
We should have an E2E test here by having a table that is dropped externally.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70bd7ca4796b24920ee156436bf8bbc682e7d952
Gerrit-Change-Number: 13508
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 11 Jun 2019 18:30:32 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8633 : Insert event should not error when table does not exists

2019-06-11 Thread Vihang Karajgaonkar (Code Review)
Vihang Karajgaonkar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13548 )

Change subject: IMPALA-8633 : Insert event should not error when table does not 
exists
..


Patch Set 4:

(1 comment)

> (1 comment)
 >
 > Is there any e2e test we could write to stress these paths? eg if
 > you were to run the metastore poller with a 5 second frequency, and
 > you did some random sequence of hive operations, would it end up
 > triggering it? I'm wondering whether we have similar issues in
 > other events. For example, if you ALTER TABLE followed by DROP,
 > will Impala get desynchronized?

For all the other events the event processor takes action if the 
table/partition exists. For example in case of alter table, drop table event 
sequence, the alter event does a invalidateTableIfExists so in this particular 
case it will be no-op.

I have been thinking of what is the best way to add a test stress these code 
paths. I was thinking may be writing a junit test which creates random batches 
of the events for the eventProcessor is simpler and faster way. An equivalent 
e2e test would be to run random hive queries for a set of given queries but its 
needs to have a more complex logic of all the various ways you can write a 
alter query (change col, add/drop partition, rename etc). Any thoughts? I can 
create a separate JIRA to do that in a separate patch if that is okay with you.

http://gerrit.cloudera.org:8080/#/c/13548/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:

http://gerrit.cloudera.org:8080/#/c/13548/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@829
PS4, Line 829: }
> should this have an else { throw e }?
Yes, Thanks for catching that.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I961cd7cbede4c248dba538c7fabb4bc708e49693
Gerrit-Change-Number: 13548
Gerrit-PatchSet: 4
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Anurag Mantripragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Tue, 11 Jun 2019 18:24:30 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8617: Add support for lz4 in parquet

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13582 )

Change subject: IMPALA-8617: Add support for lz4 in parquet
..


Patch Set 2:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6850a39ef3f1e0e7ba48e08eef1d4f7cbb74d0c
Gerrit-Change-Number: 13582
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 11 Jun 2019 18:18:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] fe: improve logging for metadata loading

2019-06-11 Thread Vihang Karajgaonkar (Code Review)
Vihang Karajgaonkar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13463 )

Change subject: fe: improve logging for metadata loading
..


Patch Set 1:

(5 comments)

Patch looks good to me. left some non-blocking nits and naming suggestions.

http://gerrit.cloudera.org:8080/#/c/13463/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/13463/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@515
PS1, Line 515: needed to fetch partition stats
Can we create constants for these reasons. Seems they never change.


http://gerrit.cloudera.org:8080/#/c/13463/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1954
PS1, Line 1954: why
Nit, I think have nouns as the arguments are more readable. eg: reload(Table 
tbl, String reason)

Same comment for the other method arg names where the patch uses "why"


http://gerrit.cloudera.org:8080/#/c/13463/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:

http://gerrit.cloudera.org:8080/#/c/13463/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@103
PS1, Line 103: hiveexec
Is there a particular reason to use the shaded version of Joiner?


http://gerrit.cloudera.org:8080/#/c/13463/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@628
PS1, Line 628: new Exception()
This will potentially add a lot of traces in the log. Don't we generally log 
the trace for errors. Could be confusing to the users when they see these in 
the log. Perhaps change this to debug?


http://gerrit.cloudera.org:8080/#/c/13463/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:

http://gerrit.cloudera.org:8080/#/c/13463/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@813
PS1, Line 813: insert
would be used to say processing table-level insert event from HMS for 
consistency with the other message. Also, might want to change it to INSERT 
event similar to other messages.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8
Gerrit-Change-Number: 13463
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Tue, 11 Jun 2019 18:16:47 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8617: Add support for lz4 in parquet

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13582 )

Change subject: IMPALA-8617: Add support for lz4 in parquet
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13582/2/tests/custom_cluster/test_hive_parquet_codec_interop.py
File tests/custom_cluster/test_hive_parquet_codec_interop.py:

http://gerrit.cloudera.org:8080/#/c/13582/2/tests/custom_cluster/test_hive_parquet_codec_interop.py@20
PS2, Line 20: import os
flake8: F401 'os' imported but unused


http://gerrit.cloudera.org:8080/#/c/13582/2/tests/custom_cluster/test_hive_parquet_codec_interop.py@32
PS2, Line 32: class TestParquetInterop(CustomClusterTestSuite):
flake8: E302 expected 2 blank lines, found 1



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6850a39ef3f1e0e7ba48e08eef1d4f7cbb74d0c
Gerrit-Change-Number: 13582
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 11 Jun 2019 18:02:57 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8617: Add support for lz4 in parquet

2019-06-11 Thread Abhishek Rawat (Code Review)
Abhishek Rawat has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/13582


Change subject: IMPALA-8617: Add support for lz4 in parquet
..

IMPALA-8617: Add support for lz4 in parquet

A new enum value LZ4_BLOCKED was added to the THdfsCompression enum, to
distinguish it from the existing LZ4 codec. LZ4_BLOCKED codec represents
the block compression scheme used by Hadoop. Its similar to
SNAPPY_BLOCKED as far as the block format is concerned, with the only
difference being the codec used for compression and decompression.

Added Lz4BlockCompressor and Lz4BlockDecompressor classes for
compressing and decompressing parquet data using Hadoop's
lz4 block compression scheme.

The Lz4BlockCompressor treats the input
as a single block and generates a compressed block with following layout
 <4 byte big endian uncompressed size>
   <4 byte big endian compressed size>
   
The hdfs parquet table writer should call the Lz4BlockCompressor
using the ideal input size (unit of compression in parquer is a page),
and so the Lz4BlockCompressor does not further break down the input
into smaller blocks.

The Lz4BlockDecompressor on the other hand should be compatible with
blocks written by Impala and other engines in Hadoop ecosystem. It can
decompress compressed data in following format
  <4 byte big endian uncompressed size>
<4 byte big endian compressed size>

...
<4 byte big endian compressed size>

...


Externally users can now set the lz4 codec for parquet using:
  set COMPRESSION_CODEC=lz4
This gets translated into LZ4_BLOCKED codec for the
HdfsParquetTableWriter. Similarly, when reading lz4 compressed parquet
data, the LZ4_BLOCKED codec is used.

Testing:
 - Added unit tests for LZ4_BLOCKED in decompress-test.cc
 - Added unit tests for Hadoop compatibility in decompress-test.cc,
   basically being able to decompress an outer block with multiple inner
   blocks (the Lz4BlockDecompressor description above)
 - Added interoperability tests for Hive and Impala for all parquet
   codecs. New test added to
   tests/custom_cluster/test_hive_parquet_codec_interop.py

Change-Id: Ia6850a39ef3f1e0e7ba48e08eef1d4f7cbb74d0c
---
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/parquet-common.cc
M be/src/exec/parquet/parquet-metadata-utils.cc
M be/src/util/codec.cc
M be/src/util/compress.cc
M be/src/util/compress.h
M be/src/util/decompress-test.cc
M be/src/util/decompress.cc
M be/src/util/decompress.h
M common/thrift/CatalogObjects.thrift
M common/thrift/generate_error_codes.py
M tests/common/test_dimensions.py
A tests/custom_cluster/test_hive_parquet_codec_interop.py
M tests/query_test/test_insert.py
M tests/query_test/test_insert_parquet.py
15 files changed, 341 insertions(+), 11 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia6850a39ef3f1e0e7ba48e08eef1d4f7cbb74d0c
Gerrit-Change-Number: 13582
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 


[Impala-ASF-CR] IMPALA-7608: Estimate row count from file size when no stats available

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12974 )

Change subject: IMPALA-7608: Estimate row count from file size when no stats 
available
..


Patch Set 12:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic414121c8df0d5222e4aeea096b5365beb04568a
Gerrit-Change-Number: 12974
Gerrit-PatchSet: 12
Gerrit-Owner: Fang-Yu Rao 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 11 Jun 2019 17:58:51 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8605: clean up HS2/beeswax session management

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13585 )

Change subject: IMPALA-8605: clean up HS2/beeswax session management
..


Patch Set 1:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c014d1a32e273275a773f842b9ed9793dbdba6b
Gerrit-Change-Number: 13585
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Tue, 11 Jun 2019 17:52:28 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8561: Eliminate mtime=-1 for HDFS scan ranges (part 1)

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13522 )

Change subject: IMPALA-8561: Eliminate mtime=-1 for HDFS scan ranges (part 1)
..


Patch Set 6:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I48b7ed60d6ab9104b993237b4fe23de5dc058672
Gerrit-Change-Number: 13522
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 11 Jun 2019 17:51:10 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7467: Ported ExecQueryFInstances over to krpc

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13583 )

Change subject: IMPALA-7467: Ported ExecQueryFInstances over to krpc
..


Patch Set 1:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3f1c6099109bd8e5361188005a7d0e892147570
Gerrit-Change-Number: 13583
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Tue, 11 Jun 2019 17:37:30 +
Gerrit-HasComments: No


[Impala-ASF-CR] Update mustache to commit b290952d8eb93d085214d8c8c9eab8559df9f606

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13563 )

Change subject: Update mustache to commit 
b290952d8eb93d085214d8c8c9eab8559df9f606
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If26f3aaa2a3279a1f6a300c4f4cee7ec899e22ed
Gerrit-Change-Number: 13563
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 11 Jun 2019 17:36:56 +
Gerrit-HasComments: No


[Impala-ASF-CR] Update mustache to commit b290952d8eb93d085214d8c8c9eab8559df9f606

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13563 )

Change subject: Update mustache to commit 
b290952d8eb93d085214d8c8c9eab8559df9f606
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If26f3aaa2a3279a1f6a300c4f4cee7ec899e22ed
Gerrit-Change-Number: 13563
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 11 Jun 2019 17:36:55 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8143: Enhance DoRpcWithRetry().

2019-06-11 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12672 )

Change subject: IMPALA-8143: Enhance DoRpcWithRetry().
..


Patch Set 12: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9693151c35e02235665b3c285a48c585973d390
Gerrit-Change-Number: 12672
Gerrit-PatchSet: 12
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Tue, 11 Jun 2019 17:30:39 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8561: Eliminate mtime=-1 for HDFS scan ranges (part 1)

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13522 )

Change subject: IMPALA-8561: Eliminate mtime=-1 for HDFS scan ranges (part 1)
..


Patch Set 6:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I48b7ed60d6ab9104b993237b4fe23de5dc058672
Gerrit-Change-Number: 13522
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 11 Jun 2019 17:13:26 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7608: Estimate row count from file size when no stats available

2019-06-11 Thread Fang-Yu Rao (Code Review)
Fang-Yu Rao has uploaded a new patch set (#12). ( 
http://gerrit.cloudera.org:8080/12974 )

Change subject: IMPALA-7608: Estimate row count from file size when no stats 
available
..

IMPALA-7608: Estimate row count from file size when no stats available

Added the feature that computes an estimated number of rows in the current
hdfs table if the statistics for the cardinality of the current hdfs table is 
not
available.

Also added an additional query option to revert the change in case of 
regression.

Testing:
(1) In CardinalityTest.java, replaced the original statement
"verifyCardinality("SELECT a FROM functional.tinytable", -1);" in
the method testBasicsWithoutStats() with
"verifyCardinality("SELECT a FROM functional.tinytable", 2);".
(2) In CarginalityTest.java, added more tests to check the cardinality
of most PlanNode implementations. For each tested PlanNode, the behaviors
before and after we disable the feature are both tested.
(3) In set.test, modified three related test cases to make sure that
the added query option is included after executing "set all" in various
scenarios.
(4) There are 8 JUnit tests in PlannerTest.java that would produce different
distributed query plans when this feature is enabled. Added an additional
JUnit test for each of those 8 affected JUnit tests when this feature is
enabled. Specifically, each tested query in a newly added test files involves
at least one hdfs table without available statistics.

Change-Id: Ic414121c8df0d5222e4aeea096b5365beb04568a
---
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M fe/src/main/java/org/apache/impala/catalog/HdfsCompression.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/test/java/org/apache/impala/planner/CardinalityTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
A 
testdata/workloads/functional-planner/queries/PlannerTest/default-join-distr-mode-shuffle-hdfs-num-rows-est-enabled.test
A 
testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection-hdfs-num-rows-est-enabled.test
A 
testdata/workloads/functional-planner/queries/PlannerTest/joins-hdfs-num-rows-est-enabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
A 
testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters-hdfs-num-rows-est-enabled.test
A 
testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation-hdfs-num-rows-est-enabled.test
A 
testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements-hdfs-num-rows-est-enabled.test
A 
testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing-hdfs-num-rows-est-enabled.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
A 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite-hdfs-num-rows-est-enabled.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test
M testdata/workloads/functional-query/queries/QueryTest/set.test
21 files changed, 2,927 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/12974/12
--
To view, visit http://gerrit.cloudera.org:8080/12974
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic414121c8df0d5222e4aeea096b5365beb04568a
Gerrit-Change-Number: 12974
Gerrit-PatchSet: 12
Gerrit-Owner: Fang-Yu Rao 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 


[Impala-ASF-CR] IMPALA-8561: Eliminate mtime=-1 for HDFS scan ranges (part 1)

2019-06-11 Thread Joe McDonnell (Code Review)
Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13522 )

Change subject: IMPALA-8561: Eliminate mtime=-1 for HDFS scan ranges (part 1)
..


Patch Set 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/13522/4/be/src/runtime/io/handle-cache.inline.h
File be/src/runtime/io/handle-cache.inline.h:

http://gerrit.cloudera.org:8080/#/c/13522/4/be/src/runtime/io/handle-cache.inline.h@93
PS4, Line 93: DCHECK_GT(mtime, 0);
> super minor nit: is 0 invalid ?
I was thinking about this, and I decided that I would want to know if it is 0. 
Modification time is defined to be UNIX time, so a value of 0 indicates 
something unexpected with a chance that it is always 0.


http://gerrit.cloudera.org:8080/#/c/13522/4/be/src/runtime/io/request-ranges.h
File be/src/runtime/io/request-ranges.h:

http://gerrit.cloudera.org:8080/#/c/13522/4/be/src/runtime/io/request-ranges.h@170
PS4, Line 170: 'mtime' matches
 :   // the modified time of the cached file in the HDFS cache
> stale
Good point, fixed.


http://gerrit.cloudera.org:8080/#/c/13522/4/be/src/runtime/io/request-ranges.h@244
PS4, Line 244: int64_t mtime,
> Nice to have one comment about it.
Added comments for mtime and is_erasure_coded.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I48b7ed60d6ab9104b993237b4fe23de5dc058672
Gerrit-Change-Number: 13522
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 11 Jun 2019 17:09:40 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8605: clean up HS2/beeswax session management

2019-06-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/13585


Change subject: IMPALA-8605: clean up HS2/beeswax session management
..

IMPALA-8605: clean up HS2/beeswax session management

Session/operation secrets are part of the HS2 handle
but we haven't made use of them up until now. This
patch checks the value and treats it as part of the
session key, as originally intended. I.e. if the
secret is missing, the session lookup fails.

The operation secret is the same as the session secret
to save having to generate and store extra secrets
(there's no real benefit).

A secret is added to each Beeswax session. This secret is
internal to the server and not exposed. Adds validation
that client requests accessed via Beeswax belong to the
same user as the session.

We switch uuid_generator_ to use boost::random_device, which
uses /dev/urandom as its source of randomness to be more robust -
otherwise it's hard to be sure that we won't have collisions,
although it doesn't seem to be a problem in practice.

For requests - GetRuntimeProfile() and GetExecSummary()
that provide both a session and query ID, the code already
checks that the session's user matches the query.

An exception to the validation mechanisms above is added
for Close() and Cancel() beeswax operations, because impala-shell
and some administrative tools allow cancellation of
queries on different threads and from different tools.

We skip validating the session secret when cancelling queries
from the web UI, since web UI users don't have the secret.

Testing:
* Ran exhaustive tests.
* Add tests for all HS2 RPCs that provide invalid session secrets.
* Add tests for HS2 RPCs that provide both session and query
  ID to ensure that query belongs to the session.
* Add basic test for beeswax testing accessing a query from
  different connections.

Change-Id: I4c014d1a32e273275a773f842b9ed9793dbdba6b
---
M CMakeLists.txt
M be/src/service/child-query.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-server.cc
M be/src/service/impala-server.h
M tests/beeswax/impala_beeswax.py
M tests/common/impala_connection.py
M tests/hs2/hs2_test_suite.py
M tests/hs2/test_fetch.py
M tests/hs2/test_hs2.py
A tests/query_test/test_beeswax.py
13 files changed, 688 insertions(+), 133 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4c014d1a32e273275a773f842b9ed9793dbdba6b
Gerrit-Change-Number: 13585
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8605: clean up HS2/beeswax session management

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13585 )

Change subject: IMPALA-8605: clean up HS2/beeswax session management
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c014d1a32e273275a773f842b9ed9793dbdba6b
Gerrit-Change-Number: 13585
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 11 Jun 2019 17:08:31 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7467: Ported ExecQueryFInstances over to krpc

2019-06-11 Thread Thomas Marshall (Code Review)
Thomas Marshall has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/13583


Change subject: IMPALA-7467: Ported ExecQueryFInstances over to krpc
..

IMPALA-7467: Ported ExecQueryFInstances over to krpc

This patch ports the ExecQuerryFInstances rpc to use KRPC. The
parameters for this call contain a huge number of Thrift structs
(eg. everything related to TPlanNode and TExpr), so to avoid
converting all of these to protobuf and the resulting effect that
would have on the FE and catalog, this patch stores most of the
parameters in a sidecar (in particular the TQueryCtx,
TPlanFragmentCtx's, and TPlanFragmentInstanceCtx's).

Testing:
- Passed a full exhaustive run on the minicluster.
Set up a ten node cluster with tpch 500:
- Ran perf tests: 3 iterations per tpch query, 4 concurrent streams,
  no perf change.
- Ran the stress test for 1000 queries, passed.

Change-Id: Id3f1c6099109bd8e5361188005a7d0e892147570
---
M be/src/runtime/backend-client.h
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/query-exec-mgr.cc
M be/src/runtime/query-exec-mgr.h
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/runtime/test-env.cc
M be/src/service/control-service.cc
M be/src/service/control-service.h
M be/src/service/impala-internal-service.cc
M be/src/service/impala-internal-service.h
M common/protobuf/control_service.proto
M common/thrift/ImpalaInternalService.thrift
M tests/custom_cluster/test_rpc_timeout.py
15 files changed, 247 insertions(+), 171 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id3f1c6099109bd8e5361188005a7d0e892147570
Gerrit-Change-Number: 13583
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall 


[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

2019-06-11 Thread Sudhanshu Arora (Code Review)
Sudhanshu Arora has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:

http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@78
PS1, Line 78:   private static final String CONNECTORREAD = 
"CONNECTORREAD".intern();
> These are hive defined strings for capabilities, they only recognize these
Just to be clear I was saying to do the following
public enum Capability {
  CONNECTOREAD("CONNECTTOREAD")
  private final String capabilityName;

  private final getName() {
 return capabilityName;
  }
  }

And we can use the getName to get the String. If you still feel using String 
constants is better, please go ahead with that.

Also what's the benefit of calling intern on the string constants?


http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java:

http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java@93
PS1, Line 93: analyzer.ensureTableWriteSupported(table_);
> Hive defines access types as byte,  we combine the access type check in Met
1) If Hive does not define Metadata_modification and we want to fold this 
operation under write. We can do that, i.e. we could still define
@OperationType(WRITE)

2) What is the advantage of using bitwise operators?

I do think annotations are introducing slightly more work. And before we make 
the change we should agree that it offers more clarity than the current changes.


http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
File fe/src/test/java/org/apache/impala/analysis/ParserTest.java:

http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@4021
PS1, Line 4021: ParserError("Create table bucketed_tbl(order_id int, 
order_name string)"
> Parsing happens before analysis. Impala does not recognize clustered by, so
My bad. I don't know what I was thinking :).



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 1
Gerrit-Owner: Yongzhi Chen 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sudhanshu Arora 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Reviewer: Yongzhi Chen 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 11 Jun 2019 16:04:26 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8436: Prohibit write/alter operations on materialized view

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13503 )

Change subject: IMPALA-8436: Prohibit write/alter operations on materialized 
view
..


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idcd619303e19b5a2551876a63d67569c76bd22f0
Gerrit-Change-Number: 13503
Gerrit-PatchSet: 5
Gerrit-Owner: Sudhanshu Arora 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sudhanshu Arora 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Reviewer: Yongzhi Chen 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 11 Jun 2019 15:29:18 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

2019-06-11 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
..


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:

http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@92
PS1, Line 92:   private static final long MAJOR_VERSION = 3;
nit: HIVE_MAJOR_VERSION?


http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@437
PS1, Line 437:* Set impala capabilities to hive client
nit: could you elaborate a little? E.g. why is it needed, what are these 
capabilities?


http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:

http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@129
PS1, Line 129:   public static final byte REQUIRE_READ = (byte)2;
> They are consistent with hive thrift defined constant for access type, and
Why not import hive_metastoreConstants access types?


http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@150
PS1, Line 150: "Table %s not supported. Bucketed tables are only supported " +
 :   "for read."
nit: How about "%s is a bucketed table. Only read operations are supported on 
such tables."


http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java:

http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java@876
PS1, Line 876: if (MetastoreShim.getMajorVersion() > 2) {
Maybe you could add tests for the other statements as well, e.g. COMPUTE STATS, 
CREATE TABLE LIKE, DROP, etc.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 1
Gerrit-Owner: Yongzhi Chen 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sudhanshu Arora 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Reviewer: Yongzhi Chen 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 11 Jun 2019 15:12:18 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

2019-06-11 Thread Yongzhi Chen (Code Review)
Yongzhi Chen has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:

http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@78
PS1, Line 78:   private static final String CONNECTORREAD = 
"CONNECTORREAD".intern();
> How about using enum for this?
These are hive defined strings for capabilities, they only recognize these 
strings.


http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java:

http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java@93
PS1, Line 93: analyzer.ensureTableWriteSupported(table_);
> Nit: Another way to do this is to introduce another enum say OperationType.
Hive defines access types as byte,  we combine the access type check in 
MetastoreShim and try not to spread the definition to so many places and take 
advantage of bitwise operations.  There is no METADATA_MODIFICATION in Hive, 
only read, write, read/write: We need the write operation(write or read/write) 
for it, this method includes both.


http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:

http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@129
PS1, Line 129:   public static final byte REQUIRE_READ = (byte)2;
> If we create an enum then these values can go into enum itself
They are consistent with hive thrift defined constant for access type, and byte 
type has one more advantage: it can  run bitwise operations like  |  (bitwise 
logical or)


http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
File fe/src/test/java/org/apache/impala/analysis/ParserTest.java:

http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@4021
PS1, Line 4021: ParserError("Create table bucketed_tbl(order_id int, 
order_name string)"
> Wouldn't this fail during analysis?
Parsing happens before analysis. Impala does not recognize clustered by, so  it 
failed with the Syntax error



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 1
Gerrit-Owner: Yongzhi Chen 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sudhanshu Arora 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Reviewer: Yongzhi Chen 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 11 Jun 2019 12:02:32 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8551: Make the grant/revoke error messages to be more user friendly

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13525 )

Change subject: IMPALA-8551: Make the grant/revoke error messages to be more 
user friendly
..


Patch Set 8: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8995f5dc88b211cd3af415713802cfeac44fe576
Gerrit-Change-Number: 13525
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 11 Jun 2019 06:20:07 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8551: Make the grant/revoke error messages to be more user friendly

2019-06-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/13525 )

Change subject: IMPALA-8551: Make the grant/revoke error messages to be more 
user friendly
..

IMPALA-8551: Make the grant/revoke error messages to be more user friendly

This patch updates the grant/revoke error messages to be more user
friendly, especially when granting/revoking with an invalid principal.

Testing:
- Added E2E test to test grant/revoke with invalid principal

Change-Id: I8995f5dc88b211cd3af415713802cfeac44fe576
Reviewed-on: http://gerrit.cloudera.org:8080/13525
Reviewed-by: Fredy Wijaya 
Tested-by: Impala Public Jenkins 
---
M 
fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
M tests/authorization/test_ranger.py
2 files changed, 64 insertions(+), 2 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8995f5dc88b211cd3af415713802cfeac44fe576
Gerrit-Change-Number: 13525
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon