[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13744 ) Change subject: IMPALA-8716: Log a group of privileges into a single audit event. .. IMPALA-8716: Log a group of privileges into a single audit event. This patch updates the audit log handler to group a privilege that consists of multiple privileges into a single audit event. For example if we run "show partitions foo.bar" and we have SELECT privilege on table "foo.bar", before this patch, we would be creating 2 audit events: - Attempt to check if there's INSERT privilege on table "foo.bar" Result: denied, access type: insert, resource: foo.bar - Attempt to check if there's SELECT privilege on table "foo.bar" Result: allowed, access type: select, resource: foo.bar After this patch, we will only create a single audit event, e.g. Result: allowed, access type: view_metadata, resource: foo.bar Testing: - Updated tests in RangerAuditLogTest - Ran FE tests Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca Reviewed-on: http://gerrit.cloudera.org:8080/13744 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- 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/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java 3 files changed, 101 insertions(+), 24 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/13744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca Gerrit-Change-Number: 13744 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 ) Change subject: IMPALA-8716: Log a group of privileges into a single audit event. .. Patch Set 6: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/13744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca Gerrit-Change-Number: 13744 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 02 Jul 2019 03:26:07 + Gerrit-HasComments: No
[Impala-ASF-CR] Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13768 ) Change subject: Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95 .. Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95 This updates to the latest build of squeasel, which fixes a few small issues and also improves support for keepalive. This patch doesn't itself enable keepalive, but switches to the new APIs. Change-Id: I17f90561e0ea6b0917fff51b055225060a4fa549 Reviewed-on: http://gerrit.cloudera.org:8080/13768 Reviewed-by: Todd Lipcon Tested-by: Impala Public Jenkins --- M be/src/thirdparty/squeasel/squeasel.c M be/src/thirdparty/squeasel/squeasel.h M be/src/util/webserver.cc M be/src/util/webserver.h 4 files changed, 77 insertions(+), 31 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/13768 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I17f90561e0ea6b0917fff51b055225060a4fa549 Gerrit-Change-Number: 13768 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13768 ) Change subject: Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95 .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/13768 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17f90561e0ea6b0917fff51b055225060a4fa549 Gerrit-Change-Number: 13768 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 02 Jul 2019 02:58:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8572: improve QueryEventHook execution
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13748 ) Change subject: IMPALA-8572: improve QueryEventHook execution .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3800/ : 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/13748 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb88422f7cfe86947d11ce57d2b4c63e57d1b643 Gerrit-Change-Number: 13748 Gerrit-PatchSet: 7 Gerrit-Owner: radford nguyen Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Tue, 02 Jul 2019 01:39:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13776 ) Change subject: IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration .. Patch Set 4: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/367/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/13776 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieec79ac0bbb860c6394a3bf0617b285a7d23ca9e Gerrit-Change-Number: 13776 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 02 Jul 2019 01:32:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13776 ) Change subject: IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration .. Patch Set 3: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/366/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/13776 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieec79ac0bbb860c6394a3bf0617b285a7d23ca9e Gerrit-Change-Number: 13776 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 02 Jul 2019 01:30:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8427: [DOCS] Document the new startup flag IMPALA-7800 introduced
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/13762 ) Change subject: IMPALA-8427: [DOCS] Document the new startup flag IMPALA-7800 introduced .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a4c1975721c32a78a003d91babc5d2bb90f3949 Gerrit-Change-Number: 13762 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Tue, 02 Jul 2019 01:25:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration
Hello Thomas Tauber-Marshall, Andrew Wong, Grant Henke, Hao Hao, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13776 to look at the new patch set (#4). Change subject: IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration .. IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration Change-Id: Ieec79ac0bbb860c6394a3bf0617b285a7d23ca9e --- M docs/shared/impala_common.xml M docs/topics/impala_kudu.xml M docs/topics/impala_tables.xml 3 files changed, 60 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/13776/4 -- To view, visit http://gerrit.cloudera.org:8080/13776 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ieec79ac0bbb860c6394a3bf0617b285a7d23ca9e Gerrit-Change-Number: 13776 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13776 ) Change subject: IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration .. Patch Set 4: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/367/ Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/13776 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieec79ac0bbb860c6394a3bf0617b285a7d23ca9e Gerrit-Change-Number: 13776 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 02 Jul 2019 01:22:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13776 ) Change subject: IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration .. Patch Set 3: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/366/ Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/13776 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieec79ac0bbb860c6394a3bf0617b285a7d23ca9e Gerrit-Change-Number: 13776 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 02 Jul 2019 01:18:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration
Hello Thomas Tauber-Marshall, Andrew Wong, Grant Henke, Hao Hao, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13776 to look at the new patch set (#3). Change subject: IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration .. IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration Change-Id: Ieec79ac0bbb860c6394a3bf0617b285a7d23ca9e --- M docs/shared/impala_common.xml M docs/topics/impala_kudu.xml M docs/topics/impala_tables.xml 3 files changed, 56 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/13776/3 -- To view, visit http://gerrit.cloudera.org:8080/13776 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ieec79ac0bbb860c6394a3bf0617b285a7d23ca9e Gerrit-Change-Number: 13776 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-8519: [DOCS] Doc the limitation in insert events from SparkSQL
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13777 ) Change subject: IMPALA-8519: [DOCS] Doc the limitation in insert events from SparkSQL .. Patch Set 1: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/365/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/13777 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I36cfc7e592ed2588a8c1f8375033d60492b27a4f Gerrit-Change-Number: 13777 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 02 Jul 2019 01:18:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8572: improve QueryEventHook execution
Hello Fredy Wijaya, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13748 to look at the new patch set (#8). Change subject: IMPALA-8572: improve QueryEventHook execution .. IMPALA-8572: improve QueryEventHook execution WIP Testing: TBD Change-Id: Ibb88422f7cfe86947d11ce57d2b4c63e57d1b643 --- M be/src/service/impala-server.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift A fe/src/main/java/org/apache/impala/hooks/FixedCapacityQueryHookExecutor.java M fe/src/main/java/org/apache/impala/hooks/QueryEventHookManager.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/Frontend.java A fe/src/test/java/org/apache/impala/hooks/FixedCapacityQueryHookExecutorTest.java M fe/src/test/java/org/apache/impala/hooks/QueryEventHookManagerTest.java M query-event-hook-api/src/main/java/org/apache/impala/hooks/QueryEventHook.java 10 files changed, 658 insertions(+), 109 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/48/13748/8 -- To view, visit http://gerrit.cloudera.org:8080/13748 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibb88422f7cfe86947d11ce57d2b4c63e57d1b643 Gerrit-Change-Number: 13748 Gerrit-PatchSet: 8 Gerrit-Owner: radford nguyen Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8572: improve QueryEventHook execution
radford nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/13748 )
Change subject: IMPALA-8572: improve QueryEventHook execution
..
Patch Set 7:
(2 comments)
add comment where you should probably start reviewing
http://gerrit.cloudera.org:8080/#/c/13748/7/fe/src/main/java/org/apache/impala/hooks/FixedCapacityQueryHookExecutor.java
File
fe/src/main/java/org/apache/impala/hooks/FixedCapacityQueryHookExecutor.java:
http://gerrit.cloudera.org:8080/#/c/13748/7/fe/src/main/java/org/apache/impala/hooks/FixedCapacityQueryHookExecutor.java@161
PS7, Line 161: Future submitOnQueryComplete(
Entry point for executing hook onQueryComplete asynchronously
http://gerrit.cloudera.org:8080/#/c/13748/7/fe/src/main/java/org/apache/impala/hooks/QueryEventHookManager.java
File fe/src/main/java/org/apache/impala/hooks/QueryEventHookManager.java:
http://gerrit.cloudera.org:8080/#/c/13748/7/fe/src/main/java/org/apache/impala/hooks/QueryEventHookManager.java@133
PS7, Line 133: final String msg = String.format("hook timeout should be
positive but was {}. " +
might allow negative value to signify block indefinitely
--
To view, visit http://gerrit.cloudera.org:8080/13748
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb88422f7cfe86947d11ce57d2b4c63e57d1b643
Gerrit-Change-Number: 13748
Gerrit-PatchSet: 7
Gerrit-Owner: radford nguyen
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: radford nguyen
Gerrit-Comment-Date: Tue, 02 Jul 2019 01:12:20 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8519: [DOCS] Doc the limitation in insert events from SparkSQL
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/13777 ) Change subject: IMPALA-8519: [DOCS] Doc the limitation in insert events from SparkSQL .. Patch Set 1: Append and Overwrite look to be fixed in 3.3, so I am only documented save to location. -- To view, visit http://gerrit.cloudera.org:8080/13777 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I36cfc7e592ed2588a8c1f8375033d60492b27a4f Gerrit-Change-Number: 13777 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 02 Jul 2019 01:09:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8519: [DOCS] Doc the limitation in insert events from SparkSQL
Alex Rodoni has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13777 Change subject: IMPALA-8519: [DOCS] Doc the limitation in insert events from SparkSQL .. IMPALA-8519: [DOCS] Doc the limitation in insert events from SparkSQL Change-Id: I36cfc7e592ed2588a8c1f8375033d60492b27a4f --- M docs/topics/impala_metadata.xml 1 file changed, 6 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/13777/1 -- To view, visit http://gerrit.cloudera.org:8080/13777 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I36cfc7e592ed2588a8c1f8375033d60492b27a4f Gerrit-Change-Number: 13777 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni
[Impala-ASF-CR] IMPALA-8519: [DOCS] Doc the limitation in insert events from SparkSQL
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13777 ) Change subject: IMPALA-8519: [DOCS] Doc the limitation in insert events from SparkSQL .. Patch Set 1: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/365/ Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/13777 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I36cfc7e592ed2588a8c1f8375033d60492b27a4f Gerrit-Change-Number: 13777 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 02 Jul 2019 01:08:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8572: improve QueryEventHook execution
radford nguyen has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/13748 ) Change subject: IMPALA-8572: improve QueryEventHook execution .. IMPALA-8572: improve QueryEventHook execution Description goes here Testing: TBD Change-Id: Ibb88422f7cfe86947d11ce57d2b4c63e57d1b643 --- M be/src/service/impala-server.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift A fe/src/main/java/org/apache/impala/hooks/FixedCapacityQueryHookExecutor.java M fe/src/main/java/org/apache/impala/hooks/QueryEventHookManager.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/Frontend.java A fe/src/test/java/org/apache/impala/hooks/FixedCapacityQueryHookExecutorTest.java M fe/src/test/java/org/apache/impala/hooks/QueryEventHookManagerTest.java M query-event-hook-api/src/main/java/org/apache/impala/hooks/QueryEventHook.java 10 files changed, 658 insertions(+), 109 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/48/13748/7 -- To view, visit http://gerrit.cloudera.org:8080/13748 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibb88422f7cfe86947d11ce57d2b4c63e57d1b643 Gerrit-Change-Number: 13748 Gerrit-PatchSet: 7 Gerrit-Owner: radford nguyen
[Impala-ASF-CR] Support SPNEGO for Impala webserver
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13774 ) Change subject: Support SPNEGO for Impala webserver .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3798/ : 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/13774 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife2b04310e1571d231bf8ee1bcfd3b7afc2edd8f Gerrit-Change-Number: 13774 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 02 Jul 2019 00:42:34 + Gerrit-HasComments: No
[Impala-ASF-CR] build: use thin static archives
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13775 ) Change subject: build: use thin static archives .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3799/ : 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/13775 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3b54b5be8658c951914758c406cca55d4cc1756e Gerrit-Change-Number: 13775 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 02 Jul 2019 00:42:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13776 ) Change subject: IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration .. Patch Set 2: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/364/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/13776 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieec79ac0bbb860c6394a3bf0617b285a7d23ca9e Gerrit-Change-Number: 13776 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 02 Jul 2019 00:31:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13776 ) Change subject: IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration .. Patch Set 1: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/363/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/13776 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieec79ac0bbb860c6394a3bf0617b285a7d23ca9e Gerrit-Change-Number: 13776 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 02 Jul 2019 00:31:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8673: Add query option to force plan hints for insert queries
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/13753 )
Change subject: IMPALA-8673: Add query option to force plan hints for insert
queries
..
Patch Set 2:
(3 comments)
Nice test coverage. Can +2 once the nits are addressed.
http://gerrit.cloudera.org:8080/#/c/13753/2/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
File fe/src/main/java/org/apache/impala/analysis/InsertStmt.java:
http://gerrit.cloudera.org:8080/#/c/13753/2/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@854
PS2, Line 854: planHints_.add(new PlanHint(hint));
nit: planHints_.add(new PlanHint(hint.trim()));
http://gerrit.cloudera.org:8080/#/c/13753/2/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@842
PS2, Line 842: if (planHints_.isEmpty() &&
: ((table_ instanceof FeHBaseTable) ||
:
!(analyzer.getQueryOptions().isSetDefault_hints_insert_statement( return;
:
: // Set up the plan hints from query option
DEFAULT_HINTS_INSERT_STATEMENT.
: // Default hint is ignored, if the original statement had
hints.
: if (planHints_.isEmpty() &&
:
analyzer.getQueryOptions().isSetDefault_hints_insert_statement()) {
: String defaultHints =
:
analyzer.getQueryOptions().getDefault_hints_insert_statement();
: for (String hint: defaultHints.trim().split(":")) {
: hint = hint.trim();
: planHints_.add(new PlanHint(hint));
: }
: }
:
: if (table_ instanceof FeHBaseTable) {
: throw new AnalysisException(String.format("INSERT hints
are only supported for " +
: "inserting into Hdfs and Kudu tables: %s",
getTargetTableName()));
: }
I think this can be formatted to be more readable. The checks on the hbase
table seem scattered. How about something like?
if (table_ instanceof HBaseTable) {
if (!planHints.isEmpty()) throw AnalysisException()
return; // Don't care about default hints, even if they are set.
}
// Now we know it is not an HBase table, populate the hints
if (planHints_isEmpty() && defaultOptionsSet()) {
parseDefaultOptions()..
}
http://gerrit.cloudera.org:8080/#/c/13753/2/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java:
http://gerrit.cloudera.org:8080/#/c/13753/2/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@2038
PS2, Line 2038: AnalyzesOk(String.format("insert into table
functional_kudu.alltypes " +
Mind adding some checks that
- verify that the query level hints override the default hints
- default hints are actually applied during analysis when no query hints are
provided.
In either case you can do AnalysisCtx.getInsertStmt().getHints().contains() ..
once you analyze the query.
(I see that there is some e-e coverage in the test files for this, but probably
good to add some asserts in the unit test)
--
To view, visit http://gerrit.cloudera.org:8080/13753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1c3f213402b8e4d1940f96738ad21edf800fa43a
Gerrit-Change-Number: 13753
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Rawat
Gerrit-Reviewer: Abhishek Rawat
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Tue, 02 Jul 2019 00:24:02 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13776 ) Change subject: IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration .. Patch Set 2: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/364/ Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/13776 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieec79ac0bbb860c6394a3bf0617b285a7d23ca9e Gerrit-Change-Number: 13776 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 02 Jul 2019 00:12:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13776 to look at the new patch set (#2). Change subject: IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration .. IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration Change-Id: Ieec79ac0bbb860c6394a3bf0617b285a7d23ca9e --- M docs/shared/impala_common.xml M docs/topics/impala_kudu.xml M docs/topics/impala_tables.xml 3 files changed, 48 insertions(+), 28 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/13776/2 -- To view, visit http://gerrit.cloudera.org:8080/13776 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ieec79ac0bbb860c6394a3bf0617b285a7d23ca9e Gerrit-Change-Number: 13776 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13776 ) Change subject: IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration .. Patch Set 1: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/363/ Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/13776 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieec79ac0bbb860c6394a3bf0617b285a7d23ca9e Gerrit-Change-Number: 13776 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 02 Jul 2019 00:06:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration
Alex Rodoni has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13776 Change subject: IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration .. IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration Change-Id: Ieec79ac0bbb860c6394a3bf0617b285a7d23ca9e --- M docs/shared/impala_common.xml M docs/topics/impala_kudu.xml M docs/topics/impala_tables.xml 3 files changed, 41 insertions(+), 32 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/13776/1 -- To view, visit http://gerrit.cloudera.org:8080/13776 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ieec79ac0bbb860c6394a3bf0617b285a7d23ca9e Gerrit-Change-Number: 13776 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni
[Impala-ASF-CR] Support SPNEGO for Impala webserver
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13774 ) Change subject: Support SPNEGO for Impala webserver .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/13774/1/be/src/util/webserver.cc File be/src/util/webserver.cc: http://gerrit.cloudera.org:8080/#/c/13774/1/be/src/util/webserver.cc@203 PS1, Line 203: // is invalid, a bad Status will be returned (and the other out-parameters left untouched). line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/13774/1/be/src/util/webserver.cc@214 PS1, Line 214: RETURN_NOT_OK(kudu::gssapi::SpnegoStep(neg_token, &resp_token_b64, &is_complete, authn_user)); line too long (96 > 90) -- To view, visit http://gerrit.cloudera.org:8080/13774 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife2b04310e1571d231bf8ee1bcfd3b7afc2edd8f Gerrit-Change-Number: 13774 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 02 Jul 2019 00:03:16 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Support SPNEGO for Impala webserver
Todd Lipcon has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13774 Change subject: Support SPNEGO for Impala webserver .. Support SPNEGO for Impala webserver This ports over changes from kudu commit 1f291b77ef0868ac888a850678adc2d7cce65529 which implemented SPNEGO for the Kudu webserver. Unfortunately, thorough testing of this is difficult given that curl isn't currently in the toolchain. I was able to manually test this by adding a 'sleep(1000)' call into the newly added test case, then setting up $KRB5_CONFIG in my shell to point to the temporary KDC's environment, and using 'curl -u : --negotiate http://...' to authenticate. Strangely, using the version of curl on el7 didn't seem to work properly (perhaps an el7 curl bug) but using curl on my Ubuntu 18 laptop I was able to authenticate with SPNEGO. Change-Id: Ife2b04310e1571d231bf8ee1bcfd3b7afc2edd8f --- M be/src/exec/kudu-util.h M be/src/gutil/strings/escaping.cc M be/src/util/CMakeLists.txt A be/src/util/kudu-status-util.h M be/src/util/webserver-test.cc M be/src/util/webserver.cc M be/src/util/webserver.h 7 files changed, 263 insertions(+), 42 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/13774/1 -- To view, visit http://gerrit.cloudera.org:8080/13774 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ife2b04310e1571d231bf8ee1bcfd3b7afc2edd8f Gerrit-Change-Number: 13774 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon
[Impala-ASF-CR] build: use thin static archives
Todd Lipcon has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13775 Change subject: build: use thin static archives .. build: use thin static archives This enables thin static archives for our internal libraries during the build. This makes linking much faster since the static archives just point to object files instead of copying them. Change-Id: I3b54b5be8658c951914758c406cca55d4cc1756e --- M CMakeLists.txt 1 file changed, 9 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/13775/1 -- To view, visit http://gerrit.cloudera.org:8080/13775 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I3b54b5be8658c951914758c406cca55d4cc1756e Gerrit-Change-Number: 13775 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon
[Impala-ASF-CR] IMPALA-6159 / KUDU-2192: Enable TCP keepalive for all outbound connections
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13764 ) Change subject: IMPALA-6159 / KUDU-2192: Enable TCP keepalive for all outbound connections .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/13764 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa1d66d83aea1cc82d07fc6217be5fc1306695bc Gerrit-Change-Number: 13764 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 01 Jul 2019 23:28:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6159 / KUDU-2192: Enable TCP keepalive for all outbound connections
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13764 ) Change subject: IMPALA-6159 / KUDU-2192: Enable TCP keepalive for all outbound connections .. IMPALA-6159 / KUDU-2192: Enable TCP keepalive for all outbound connections This change enables TCP keepalive for all outbound connections. This aims to handle cases in which the remote peer may have dropped off the network without sending a TCP RST. For instance, a remote host could have hit a kernel panic and got power cycled. In which case, the existing TCP connection to that host may be stale. In an idle cluster, this stale connection may not be detected until the next use of it, in which case it will result in a RPC failure due to TCP RST sent from the restarted peer. By enabling TCP keepalive, we ensure that stale TCP connections in an idle cluster will be detected and closed within a time bound so a new connection will be created on the next use. This change introduces 3 different flags: --tcp_keepalive_probe_period_s: the duration in seconds a TCP connection has to be idle before keepalive probes started to be sent. --tcp_keepalive_retry_period_s: the duration in seconds between successive keepalive probes if previous probes didn't get an ACK from remote peer. --tcp_keepalive_retry_count: the maximum number of TCP keepalive probes sent without an ACK before declaring the remote peer as dead. Testing: - Used TCP dump to verify that keepalive probes are being sent periodically. - Verified that blocking all incoming traffic to a server's port via an iptable rule caused the TCP connection to be closed and the keepalive probes to stop eventually. Change-Id: Iaa1d66d83aea1cc82d07fc6217be5fc1306695bc Reviewed-on: http://gerrit.cloudera.org:8080/13702 Reviewed-by: Alexey Serbin Reviewed-by: Todd Lipcon Tested-by: Kudu Jenkins Reviewed-on: http://gerrit.cloudera.org:8080/13764 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/kudu/rpc/connection.cc M be/src/kudu/rpc/connection.h M be/src/kudu/rpc/reactor.cc M be/src/kudu/rpc/reactor.h M be/src/kudu/rpc/rpc-test.cc M be/src/kudu/util/net/socket.cc M be/src/kudu/util/net/socket.h 7 files changed, 112 insertions(+), 2 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/13764 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Iaa1d66d83aea1cc82d07fc6217be5fc1306695bc Gerrit-Change-Number: 13764 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8673: Add query option to force plan hints for insert queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13753 ) Change subject: IMPALA-8673: Add query option to force plan hints for insert queries .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3797/ : 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/13753 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1c3f213402b8e4d1940f96738ad21edf800fa43a Gerrit-Change-Number: 13753 Gerrit-PatchSet: 2 Gerrit-Owner: Abhishek Rawat Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 01 Jul 2019 22:45:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8663 : FileMetadataLoader should skip hidden and tmp directories
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/13665 )
Change subject: IMPALA-8663 : FileMetadataLoader should skip hidden and tmp
directories
..
Patch Set 11:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13665/8/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
File fe/src/main/java/org/apache/impala/common/FileSystemUtil.java:
http://gerrit.cloudera.org:8080/#/c/13665/8/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java@545
PS8, Line 545:
:* If 'recursive' is true, all
> Actually, I added this line in the doc since I noticed that the RecursingIt
hrm, were the semantics already broken then? See the unit test
AcidUtilsTest.testAcidStateNoBase:
assertFiltering(new String[]{
"base_01.txt",
"post_upgrade.txt",
"delta_05_05_/",
"delta_05_05_/lmn.txt",
"base_10/",
"delta_012_012_/",
"delta_012_012_/_0",
"delta_012_012_/_1"},
"", // writeIdList that accepts all transactions as valid
new String[]{
// Post upgrade files are ignored if there is a valid base.
"delta_012_012_/_0",
"delta_012_012_/_1"});
ie here the empty base directory has important semantics. Maybe we don't have a
valid e2e test case to reproduce this yet because of this Hive bug:
https://issues.apache.org/jira/browse/HIVE-21750 but it certainly seems like
this is an issue. Let's file a JIRA to make sure we validate this case before
releasing this feature.
http://gerrit.cloudera.org:8080/#/c/13665/11/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
File fe/src/main/java/org/apache/impala/common/FileSystemUtil.java:
http://gerrit.cloudera.org:8080/#/c/13665/11/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java@731
PS11, Line 731: // we use listFiles API to natively recurse on S3 for
performance reasons, so we
Are we really gaining much from using this RecursiveIteratorWithFilter class
for the S3 case now? It seems like this is getting to be a bit of spaghetti,
with this being named "Recursive" but having a flag that makes it non-recursive.
What about just having a separate class like 'FilteringIterator' which wraps
RemoteIterator and does filtering, and keep this one as always-recursive, so
the code path is easier to follow?
http://gerrit.cloudera.org:8080/#/c/13665/11/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java@736
PS11, Line 736: LOG.info("Ignoring {} since its contained in a ignored
directory",
This probably shouldn't be at INFO level
--
To view, visit http://gerrit.cloudera.org:8080/13665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2c4a22908304fe9e377d77d6c18d401c3f3294aa
Gerrit-Change-Number: 13665
Gerrit-PatchSet: 11
Gerrit-Owner: Vihang Karajgaonkar
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Mon, 01 Jul 2019 22:40:45 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 ) Change subject: IMPALA-8716: Log a group of privileges into a single audit event. .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3796/ : 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/13744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca Gerrit-Change-Number: 13744 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 01 Jul 2019 22:21:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8673: Add query option to force plan hints for insert queries
Abhishek Rawat has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/13753 ) Change subject: IMPALA-8673: Add query option to force plan hints for insert queries .. IMPALA-8673: Add query option to force plan hints for insert queries IMPALA-5293 enabled the pre-insert clustering by default. This could cause performance regression and this change provides a query option for setting default hints for INSERT statement. New query option 'DEFAULT_HINTS_INSERT_STATEMENT' was added. It also supports adding multiple supported hints when separated by ':' set DEFAULT_HINTS_INSERT_STATEMENT=[clustered|noclustered]; set DEFAULT_HINTS_INSERT_STATEMENT=[shuffle|noshuffle]; set DEFAULT_HINTS_INSERT_STATEMENT= [clustered|noclustered]:[shuffle|noshuffle]; If a given insert statement already has plan hints in the query text, the default hints, if any, are all ignored. This is because, if a query has plan hints specified by the user, we don't want to override it. When a default hint is set, and there is an INSERT statement without any plan hints in the query text, the default hints have the same affect as they would have had, if they were applied as plan hints in the query text. So these default hints have the same application and restrictions as the existing plan hints for INSERT statement. The default hints apply to HDFS and Kudu table formats and are ignored for HBase table format. Testing: - Added unit tests in AnalyzeDDLTest for CTAS. - Added unit tests in AnalyzeStmtsTest for insert statements. - Added unit tests in PlannerTest validating the plan for various scenarios involving different combinations of default hints. Change-Id: I1c3f213402b8e4d1940f96738ad21edf800fa43a --- 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/analysis/InsertStmt.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java A testdata/workloads/functional-planner/queries/PlannerTest/insert-default-clustered-noshuffle.test A testdata/workloads/functional-planner/queries/PlannerTest/insert-default-clustered-shuffle.test A testdata/workloads/functional-planner/queries/PlannerTest/insert-default-clustered.test A testdata/workloads/functional-planner/queries/PlannerTest/insert-default-noclustered-noshuffle.test A testdata/workloads/functional-planner/queries/PlannerTest/insert-default-noclustered-shuffle.test A testdata/workloads/functional-planner/queries/PlannerTest/insert-default-noclustered.test A testdata/workloads/functional-planner/queries/PlannerTest/insert-default-noshuffle.test A testdata/workloads/functional-planner/queries/PlannerTest/insert-default-shuffle.test 16 files changed, 1,299 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/13753/2 -- To view, visit http://gerrit.cloudera.org:8080/13753 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1c3f213402b8e4d1940f96738ad21edf800fa43a Gerrit-Change-Number: 13753 Gerrit-PatchSet: 2 Gerrit-Owner: Abhishek Rawat Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8673: Add query option to force plan hints for insert queries
Abhishek Rawat has posted comments on this change. (
http://gerrit.cloudera.org:8080/13753 )
Change subject: IMPALA-8673: Add query option to force plan hints for insert
queries
..
Patch Set 1:
(8 comments)
Thanks for review comments.
http://gerrit.cloudera.org:8080/#/c/13753/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13753/1//COMMIT_MSG@28
PS1, Line 28:
> Can you mention which table formats this applies to - HDFS/Kudu/HBase?
Done
http://gerrit.cloudera.org:8080/#/c/13753/1/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
File fe/src/main/java/org/apache/impala/analysis/InsertStmt.java:
http://gerrit.cloudera.org:8080/#/c/13753/1/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@841
PS1, Line 841: ((table_ instanceof FeHBaseTable) ||
> I think the HBase bit deserves some explanation. The idea is that the hints
Yeah the insert hints are not supported for HBase table. And if the new query
option is set and we come across a HBase table we should ignore the default
hints.
Added a comment.
http://gerrit.cloudera.org:8080/#/c/13753/1/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@844
PS1, Line 844: Setup
> Set up
Done
http://gerrit.cloudera.org:8080/#/c/13753/1/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@850
PS1, Line 850: toks
> nit: could skip the intermediate value and inline the RHS on the below line
Done
http://gerrit.cloudera.org:8080/#/c/13753/1/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@852
PS1, Line 852: hint.trim();
> Is there a test to make sure that whitespace is ignored?
Updated some tests to have whitespaces.
http://gerrit.cloudera.org:8080/#/c/13753/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java:
http://gerrit.cloudera.org:8080/#/c/13753/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@2042
PS1, Line 2042:
insertCtx.getQueryOptions().setDefault_hints_insert_statement("NOCLUSTERED:noshuffle");
> line too long (91 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/13753/1/fe/src/test/java/org/apache/impala/planner/PlannerTest.java
File fe/src/test/java/org/apache/impala/planner/PlannerTest.java:
http://gerrit.cloudera.org:8080/#/c/13753/1/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@190
PS1, Line 190: @Test
> The tests are pretty comprehensive, but can you add a sanity test for HBase
Done
http://gerrit.cloudera.org:8080/#/c/13753/1/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@193
PS1, Line 193: options.setDefault_hints_insert_statement("clustered");
> You added the .trim() to make this whitespace-insensitive. Maybe add some v
Added white spaces.
--
To view, visit http://gerrit.cloudera.org:8080/13753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1c3f213402b8e4d1940f96738ad21edf800fa43a
Gerrit-Change-Number: 13753
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Rawat
Gerrit-Reviewer: Abhishek Rawat
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Mon, 01 Jul 2019 22:02:22 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP] IMPALA-8732: Prototype for serialized descriptor table
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13772 ) Change subject: [WIP] IMPALA-8732: Prototype for serialized descriptor table .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3795/ : 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/13772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I458aa62dd4d1e4e4a7b1869a604623a69f3b2d9a Gerrit-Change-Number: 13772 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 01 Jul 2019 22:03:17 + Gerrit-HasComments: No
[Impala-ASF-CR] Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13768 ) Change subject: Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95 .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3794/ : 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/13768 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17f90561e0ea6b0917fff51b055225060a4fa549 Gerrit-Change-Number: 13768 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 01 Jul 2019 21:57:45 + Gerrit-HasComments: No
[Impala-ASF-CR] [DOCS] \N needs to be escaped in Sqoop for Impala to recognize as null
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13773 ) Change subject: [DOCS] \N needs to be escaped in Sqoop for Impala to recognize as null .. Patch Set 1: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/362/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/13773 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I581f532c2f865e93d8922661c389791f41805b64 Gerrit-Change-Number: 13773 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Mon, 01 Jul 2019 21:54:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 ) Change subject: IMPALA-8716: Log a group of privileges into a single audit event. .. Patch Set 5: Code-Review+2 Carry Bharath's +2. -- To view, visit http://gerrit.cloudera.org:8080/13744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca Gerrit-Change-Number: 13744 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 01 Jul 2019 21:41:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 ) Change subject: IMPALA-8716: Log a group of privileges into a single audit event. .. Patch Set 6: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4573/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/13744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca Gerrit-Change-Number: 13744 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 01 Jul 2019 21:41:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 ) Change subject: IMPALA-8716: Log a group of privileges into a single audit event. .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca Gerrit-Change-Number: 13744 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 01 Jul 2019 21:41:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 ) Change subject: IMPALA-8716: Log a group of privileges into a single audit event. .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java: http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@319 PS2, Line 319: // so that we know all the audit events generated by the temporary audit : // handler are contained to the given resource. : RangerBufferAuditHandler tmpAuditHandler = originalAuditHandler == null ? : null : new RangerBufferAuditHandler(originalAuditHandler); : for (Privilege impliedPrivilege: privile > I think you missed the comment. Done -- To view, visit http://gerrit.cloudera.org:8080/13744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca Gerrit-Change-Number: 13744 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 01 Jul 2019 21:41:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.
Fredy Wijaya has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/13744 ) Change subject: IMPALA-8716: Log a group of privileges into a single audit event. .. IMPALA-8716: Log a group of privileges into a single audit event. This patch updates the audit log handler to group a privilege that consists of multiple privileges into a single audit event. For example if we run "show partitions foo.bar" and we have SELECT privilege on table "foo.bar", before this patch, we would be creating 2 audit events: - Attempt to check if there's INSERT privilege on table "foo.bar" Result: denied, access type: insert, resource: foo.bar - Attempt to check if there's SELECT privilege on table "foo.bar" Result: allowed, access type: select, resource: foo.bar After this patch, we will only create a single audit event, e.g. Result: allowed, access type: view_metadata, resource: foo.bar Testing: - Updated tests in RangerAuditLogTest - Ran FE tests Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca --- 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/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java 3 files changed, 101 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/13744/5 -- To view, visit http://gerrit.cloudera.org:8080/13744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca Gerrit-Change-Number: 13744 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] [DOCS] \N needs to be escaped in Sqoop for Impala to recognize as null
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13773 ) Change subject: [DOCS] \N needs to be escaped in Sqoop for Impala to recognize as null .. Patch Set 1: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/362/ Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/13773 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I581f532c2f865e93d8922661c389791f41805b64 Gerrit-Change-Number: 13773 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 01 Jul 2019 21:32:05 + Gerrit-HasComments: No
[Impala-ASF-CR] [DOCS] \N needs to be escaped in Sqoop for Impala to recognize as null
Alex Rodoni has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13773 Change subject: [DOCS] \N needs to be escaped in Sqoop for Impala to recognize as null .. [DOCS] \N needs to be escaped in Sqoop for Impala to recognize as null Change-Id: I581f532c2f865e93d8922661c389791f41805b64 --- M docs/topics/impala_txtfile.xml 1 file changed, 21 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/13773/1 -- To view, visit http://gerrit.cloudera.org:8080/13773 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I581f532c2f865e93d8922661c389791f41805b64 Gerrit-Change-Number: 13773 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni
[Impala-ASF-CR] [WIP] IMPALA-8732: Prototype for serialized descriptor table
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13772 ) Change subject: [WIP] IMPALA-8732: Prototype for serialized descriptor table .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/13772/1/common/thrift/ImpalaInternalService.thrift File common/thrift/ImpalaInternalService.thrift: http://gerrit.cloudera.org:8080/#/c/13772/1/common/thrift/ImpalaInternalService.thrift@469 PS1, Line 469: // with incomplete structures (e.g. THdfsTable without the required nullPartitionKeyValue line too long (91 > 90) -- To view, visit http://gerrit.cloudera.org:8080/13772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I458aa62dd4d1e4e4a7b1869a604623a69f3b2d9a Gerrit-Change-Number: 13772 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 01 Jul 2019 21:24:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP] IMPALA-8732: Prototype for serialized descriptor table
Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13772 Change subject: [WIP] IMPALA-8732: Prototype for serialized descriptor table .. [WIP] IMPALA-8732: Prototype for serialized descriptor table In IMPALA-8732, there is contention in tcmalloc when sending the ExecQueryFInstances messages for a query referencing a large number of partitions. This is because each thread in the ExecEnv::exec_rpc_thread_pool_ is making a copy of the TQueryCtx, which contains the TDescriptorTable and a large map of THdfsPartition objects. Every thread in the exec_rpc_thread_pool_ is doing this simultaneously. The threads are copying this structure, but this structure is the same across all messages for this query. Copying a large map of THdfsPartition objects is wasteful, especially considering that the coordinator does not need to access any of this information before sending it out to executors. This changes TQueryCtx to contain a TDescriptorTableSerialized, which is a binary blob containing the serialized form of TDescriptorTable. This is serialized in the frontend and passed directly through to executors. The old unserialized TDescriptorTable form is maintained to enable frontend planner tests (which use incomplete structures lacking some required fields and cannot be serialized). Testing: - Core end to end tests pass - Running an exhaustive set of tests Change-Id: I458aa62dd4d1e4e4a7b1869a604623a69f3b2d9a --- M be/src/runtime/coordinator.cc M be/src/runtime/descriptors.cc M be/src/runtime/descriptors.h M be/src/runtime/query-state.cc M common/thrift/Descriptors.thrift M common/thrift/ImpalaInternalService.thrift M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java 9 files changed, 90 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/13772/1 -- To view, visit http://gerrit.cloudera.org:8080/13772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I458aa62dd4d1e4e4a7b1869a604623a69f3b2d9a Gerrit-Change-Number: 13772 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell
[Impala-ASF-CR] Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13768 ) Change subject: Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95 .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4572/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/13768 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17f90561e0ea6b0917fff51b055225060a4fa549 Gerrit-Change-Number: 13768 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 01 Jul 2019 21:18:40 + Gerrit-HasComments: No
[Impala-ASF-CR] Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13768 ) Change subject: Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95 .. Patch Set 3: Code-Review+2 ps2 fixes the whitespace, ps3 is just a rebase. Carrying +2 -- To view, visit http://gerrit.cloudera.org:8080/13768 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17f90561e0ea6b0917fff51b055225060a4fa549 Gerrit-Change-Number: 13768 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 01 Jul 2019 21:18:23 + Gerrit-HasComments: No
[Impala-ASF-CR] Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95
Hello Michael Ho, Lars Volker, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13768 to look at the new patch set (#2). Change subject: Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95 .. Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95 This updates to the latest build of squeasel, which fixes a few small issues and also improves support for keepalive. This patch doesn't itself enable keepalive, but switches to the new APIs. Change-Id: I17f90561e0ea6b0917fff51b055225060a4fa549 --- M be/src/thirdparty/squeasel/squeasel.c M be/src/thirdparty/squeasel/squeasel.h M be/src/util/webserver.cc M be/src/util/webserver.h 4 files changed, 77 insertions(+), 31 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/13768/2 -- To view, visit http://gerrit.cloudera.org:8080/13768 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I17f90561e0ea6b0917fff51b055225060a4fa549 Gerrit-Change-Number: 13768 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/13768 ) Change subject: Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95 .. Patch Set 1: Code-Review+2 Didn't look too much into the diff in squeasel files as they are mostly copy-and-paste from upstream I assume. LGTM otherwise. Please fix the formatting nit in webserver.cc -- To view, visit http://gerrit.cloudera.org:8080/13768 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17f90561e0ea6b0917fff51b055225060a4fa549 Gerrit-Change-Number: 13768 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Mon, 01 Jul 2019 21:09:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/13744 )
Change subject: IMPALA-8716: Log a group of privileges into a single audit
event.
..
Patch Set 3: Code-Review+2
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
File
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java:
http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@319
PS2, Line 319: null : new RangerBufferAuditHandler(originalAuditHandler);
: for (Privilege impliedPrivilege:
privilege.getImpliedPrivileges()) {
: if (authorizeResource(authzCtx, resource, user,
impliedPrivilege,
: tmpAuditHandler)) {
: authorized = true;
> Good idea about copy constructor. Done.
I think you missed the comment.
http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
File
fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java:
http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java@141
PS2, Line 141: 1
> Those test cases have already been been covered:
Ah, i didn't see the other new additions in this file below, my bad.
--
To view, visit http://gerrit.cloudera.org:8080/13744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Mon, 01 Jul 2019 20:07:03 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 ) Change subject: IMPALA-8716: Log a group of privileges into a single audit event. .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3793/ : 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/13744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca Gerrit-Change-Number: 13744 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 01 Jul 2019 20:05:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.
Fredy Wijaya has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/13744 ) Change subject: IMPALA-8716: Log a group of privileges into a single audit event. .. IMPALA-8716: Log a group of privileges into a single audit event. This patch updates the audit log handler to group a privilege that consists of multiple privileges into a single audit event. For example if we run "show partitions foo.bar" and we have SELECT privilege on table "foo.bar", before this patch, we would be creating 2 audit events: - Attempt to check if there's INSERT privilege on table "foo.bar" Result: denied, access type: insert, resource: foo.bar - Attempt to check if there's SELECT privilege on table "foo.bar" Result: allowed, access type: select, resource: foo.bar After this patch, we will only create a single audit event, e.g. Result: allowed, access type: view_metadata, resource: foo.bar Testing: - Updated tests in RangerAuditLogTest - Ran FE tests Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca --- 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/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java 3 files changed, 95 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/13744/3 -- To view, visit http://gerrit.cloudera.org:8080/13744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca Gerrit-Change-Number: 13744 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/13744 )
Change subject: IMPALA-8716: Log a group of privileges into a single audit
event.
..
Patch Set 3:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
File
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java:
http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@318
PS2, Line 318: fferAu
> nit: authorized ? (same below)
Done
http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@319
PS2, Line 319: null : new RangerBufferAuditHandler(originalAuditHandler);
: for (Privilege impliedPrivilege:
privilege.getImpliedPrivileges()) {
: if (authorizeResource(authzCtx, resource, user,
impliedPrivilege,
: tmpAuditHandler)) {
: authorized = true;
> I think this could use a comment. Also, create a copy c'tor? (same below)
Good idea about copy constructor. Done.
http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@385
PS2, Line 385: RangerBufferAuditHandler auditHandler) throws
InternalException {
> just checking, is the accessResult automatically inferred from newAuditEven
Yeah since we're assigning newAuditEvent to an existing instance and mutating
it, so it will have the accessResult.
http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
File
fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java:
http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java@141
PS2, Line 141: 1
> Mind adding a couple more tests?
Those test cases have already been been covered:
accessResult = 0, isAny = true in L193-L199
accessResult = 1, isAny = false in L63-L136 --> isAny = false is basically all
privileges in
https://github.com/apache/impala/blob/c353cf7a648651244ac39677d0cb028e704281d0/fe/src/main/java/org/apache/impala/authorization/Privilege.java#L28-L35
(anyOf_ = false)
--
To view, visit http://gerrit.cloudera.org:8080/13744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Mon, 01 Jul 2019 19:23:20 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 ) Change subject: IMPALA-8716: Log a group of privileges into a single audit event. .. Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java: http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@318 PS2, Line 318: result nit: authorized ? (same below) http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@319 PS2, Line 319: RangerBufferAuditHandler originalAuditHandler = authzCtx.getAuditHandler(); : RangerBufferAuditHandler tmpAuditHandler = : originalAuditHandler == null ? null : new RangerBufferAuditHandler( : originalAuditHandler.getSqlStmt(), originalAuditHandler.getClusterName(), : originalAuditHandler.getClientIp()); I think this could use a comment. Also, create a copy c'tor? (same below) http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@385 PS2, Line 385: dest.getAuthzEvents().add(newAuditEvent); just checking, is the accessResult automatically inferred from newAuditEvent? http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@408 PS2, Line 408: RangerAccessResult result = plugin_.isAccessAllowed(request, auditHandler); > Tthe plugin._isAccessAllowed() does some logic that passes the correct Rang I see, ok! http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java File fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java: http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java@141 PS2, Line 141: 1 Mind adding a couple more tests? - accessResult = 0 isAny = true - accessResult = 1 isAny = false -- To view, visit http://gerrit.cloudera.org:8080/13744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca Gerrit-Change-Number: 13744 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 01 Jul 2019 18:24:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6159 / KUDU-2192: Enable TCP keepalive for all outbound connections
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13764 ) Change subject: IMPALA-6159 / KUDU-2192: Enable TCP keepalive for all outbound connections .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4571/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/13764 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa1d66d83aea1cc82d07fc6217be5fc1306695bc Gerrit-Change-Number: 13764 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 01 Jul 2019 17:42:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6159 / KUDU-2192: Enable TCP keepalive for all outbound connections
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13764 ) Change subject: IMPALA-6159 / KUDU-2192: Enable TCP keepalive for all outbound connections .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13764 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa1d66d83aea1cc82d07fc6217be5fc1306695bc Gerrit-Change-Number: 13764 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 01 Jul 2019 17:42:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
Attila Jeges has posted comments on this change. (
http://gerrit.cloudera.org:8080/13722 )
Change subject: IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
..
Patch Set 2:
(24 comments)
Another bunch of comments.
http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/runtime/datetime-iso-sql-format-parser.cc
File be/src/runtime/datetime-iso-sql-format-parser.cc:
http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/runtime/datetime-iso-sql-format-parser.cc@184
PS2, Line 184: DCHECK(result->year >= 1400 && result->year <= );
: int month = 1;
: for (int i = 0; i < 12; ++i) {
: int month_length = MONTH_LENGTHS[i];
: if (i == 1 &&
boost::gregorian::gregorian_calendar::is_leap_year(result->year)) {
: month_length = 29;
: }
: if (month_length >= day_in_year) {
: result->month = month;
: result->day = day_in_year;
: break;
: }
: ++month;
: day_in_year -= month_length;
: }
: DCHECK(result->month <= 12);
: DCHECK(result->day <= 31);
> Move this to a separate function.
This can be done more efficiently if you use hard-coded month ranges, like
MONTH_RANGES and LEAP_YEAR_MONTH_RANGES in be/src/runtime/date-value.cc.
See be/src/runtime/date-value.cc, L193-L208 for an example of how day_in_year
-> (month, day) conversion is done.
http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/runtime/datetime-iso-sql-format-tokenizer.h
File be/src/runtime/datetime-iso-sql-format-tokenizer.h:
http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/runtime/datetime-iso-sql-format-tokenizer.h@42
PS2, Line 42: int
bool?
http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/runtime/datetime-iso-sql-format-tokenizer.h@45
PS2, Line 45: accept_time_toks_ = time_toks;
Shouldn't 'used_tokens_' be cleared when tokenizer is reset?
http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/runtime/datetime-iso-sql-format-tokenizer.cc
File be/src/runtime/datetime-iso-sql-format-tokenizer.cc:
http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@87
PS2, Line 87: used_tokens_.clear();
Any reason 'used_tokens_' is cleared here and not in Reset()?
http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@93
PS2, Line 93: current_pos
In some functions this is called 'current' while in others it is called
'current_pos'. Please use the same name everywhere.
http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@94
PS2, Line 94: current_pos != str_end
current_pos < str_end
http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@96
PS2, Line 96: return SUCCESS
This should be break, otherwise the check in L101 won't be executed.
http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@104
PS2, Line 104: void DateTimeIsoSqlFormatTokenizer::ProcessSeparators(const
char** current) {
DCHECK(*current != nullptr);
http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@107
PS2, Line 107: IsSeparator(*current) && *current <= str_end
I think the condition should be:
(*current < str_end && IsSeparator(*current))
to avoid buffer over-read.
http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@115
PS2, Line 115: const char** current_pos) {
DCHECK(*current_pos != nullptr);
http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@120
PS2, Line 120: string curr_token(*current_pos, curr_token_size);
: boost::to_upper(curr_token);
This should be done before the loop starts. No need to convert every
prefix-string to uppercase.
http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@204
PS2, Line 204:
I think, you should also check that no more than one fractional tokens were
specified.
It would be better to do this check more generally: loop through dt_ctx_->toks
and check that there are no duplicate DateTimeFormatTokenType values in the
TokenItem items. You can also do this check in ProcessNextToken() similarly to
L124.
http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@218
PS2, Line 218: const char* c
We can pass the char directly. No need for a pointer.
http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/runtime/datetime-parser-common.h
File be/src/runtime/datetime-parser-common.h:
http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/runtime/datetime-parser-common.h@128
PS2, Line 128: results longer output length than the format string.
nit:
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 ) Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. Patch Set 4: Code-Review+1 (1 comment) The code looks good to me, but I am still trying to grasp the details Hive capabilities concept. For example I saw a line in HMS that suggests that managed tables without ACID will be treated as read only: https://github.com/apache/hive/blob/ec3779978797051fdb345172536aafcd50f1b4ae/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetastoreDefaultTransformer.java#L130 Can't this break some Impala tests? http://gerrit.cloudera.org:8080/#/c/13558/4/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java File fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java: http://gerrit.cloudera.org:8080/#/c/13558/4/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java@599 PS4, Line 599: catlaog typo: catalog -- 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: 4 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 01 Jul 2019 16:06:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 ) Change subject: IMPALA-8716: Log a group of privileges into a single audit event. .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java: http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@408 PS2, Line 408: RangerAccessResult result = plugin_.isAccessAllowed(request, auditHandler); > Instead of modifying entries in a tmpAuditHandler, can we make it simple by Tthe plugin._isAccessAllowed() does some logic that passes the correct RangerAccessResult including populating some default values. Calling it manually means we need to always keep the logic similar to the one implemented by isAccessAllowed. -- To view, visit http://gerrit.cloudera.org:8080/13744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca Gerrit-Change-Number: 13744 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 01 Jul 2019 14:16:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/13670 ) Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory .. Patch Set 6: (3 comments) Thank you for the review Sahil. Please let me know if you would add additional docs/descriptions. http://gerrit.cloudera.org:8080/#/c/13670/6/be/src/util/memusage-path-handlers.h File be/src/util/memusage-path-handlers.h: http://gerrit.cloudera.org:8080/#/c/13670/6/be/src/util/memusage-path-handlers.h@33 PS6, Line 33: /// Adds a set of memory usage metrics to the webserver to display > Should add more docs, like what memory usage metrics are added and when. Added extra docs, the memory metric descriptions were moved to the Add* helper functions. http://gerrit.cloudera.org:8080/#/c/13670/6/be/src/util/memusage-path-handlers.cc File be/src/util/memusage-path-handlers.cc: http://gerrit.cloudera.org:8080/#/c/13670/6/be/src/util/memusage-path-handlers.cc@32 PS6, Line 32: void AddMemTracker(MemTracker* mem_tracker, > would be nice to add some docs to each of these methods Done http://gerrit.cloudera.org:8080/#/c/13670/6/be/src/util/memusage-path-handlers.cc@67 PS6, Line 67: > nit: extra new line? Done -- To view, visit http://gerrit.cloudera.org:8080/13670 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23 Gerrit-Change-Number: 13670 Gerrit-PatchSet: 6 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tamas Mate Gerrit-Comment-Date: Mon, 01 Jul 2019 09:05:19 + Gerrit-HasComments: Yes
