[Impala-ASF-CR] Clean up generation of XML configuration files
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12930 ) Change subject: Clean up generation of XML configuration files .. Patch Set 1: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/2637/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/12930 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47 Gerrit-Change-Number: 12930 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 04 Apr 2019 06:48:08 + Gerrit-HasComments: No
[Impala-ASF-CR] Configure Hive 3's HS2 to execute queries using Tez local mode
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12931 ) Change subject: Configure Hive 3's HS2 to execute queries using Tez local mode .. Patch Set 1: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/2638/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/12931 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I76e47fbd1d6ff5103d81a8de430d5465dba284cd Gerrit-Change-Number: 12931 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 04 Apr 2019 06:35:49 + Gerrit-HasComments: No
[Impala-ASF-CR] Configure Hive 3's HS2 to execute queries using Tez local mode
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12931 ) Change subject: Configure Hive 3's HS2 to execute queries using Tez local mode .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/12931/1/bin/bootstrap_toolchain.py File bin/bootstrap_toolchain.py: http://gerrit.cloudera.org:8080/#/c/12931/1/bin/bootstrap_toolchain.py@443 PS1, Line 443: def download_cdp_component(toolchain_root, dir_name, makedir=False): flake8: E302 expected 2 blank lines, found 1 http://gerrit.cloudera.org:8080/#/c/12931/1/bin/bootstrap_toolchain.py@488 PS1, Line 488: e flake8: E722 do not use bare except' -- To view, visit http://gerrit.cloudera.org:8080/12931 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I76e47fbd1d6ff5103d81a8de430d5465dba284cd Gerrit-Change-Number: 12931 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 04 Apr 2019 05:52:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Clean up generation of XML configuration files
Hello Vihang Karajgaonkar, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/12930 to review the following change. Change subject: Clean up generation of XML configuration files .. Clean up generation of XML configuration files hive-site.xml and sentry-site.xml in particular had grown multiple slightly-different variants, differing only in a few small pieces. This was difficult to maintain: in fact, while attempting to clean them up I found a number of places that the MySQL and Postgres versions of hive-site had diverged for no apparent reason. This moves away from using the sed-based templating for these configuration files, and instead uses python as a poor man's template system. That enables much simpler conditional logic. I briefly considered XSLT for this, but decided that Python is probably easier for the average developer to follow, modify, and debug. Along the way, I removed a few flags which appear to be no longer used by Hive 2 or later, and a few items which were already commented out in the previous template. Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47 --- M bin/create-test-configuration.sh A bin/generate_xml_config.py A fe/src/test/resources/hive-site.xml.py D fe/src/test/resources/mysql-hive-site.xml.template D fe/src/test/resources/postgresql-hive-site.xml.cdp.template D fe/src/test/resources/postgresql-hive-site.xml.template A fe/src/test/resources/sentry-site.xml.py D fe/src/test/resources/sentry-site.xml.template D fe/src/test/resources/sentry-site_no_oo.xml.template D fe/src/test/resources/sentry-site_oo.xml.template D fe/src/test/resources/sentry-site_oo_nogrant.xml.template 11 files changed, 280 insertions(+), 997 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/30/12930/1 -- To view, visit http://gerrit.cloudera.org:8080/12930 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47 Gerrit-Change-Number: 12930 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] Clean up generation of XML configuration files
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12930 ) Change subject: Clean up generation of XML configuration files .. Patch Set 1: (19 comments) http://gerrit.cloudera.org:8080/#/c/12930/1/bin/generate_xml_config.py File bin/generate_xml_config.py: http://gerrit.cloudera.org:8080/#/c/12930/1/bin/generate_xml_config.py@18 PS1, Line 18: t flake8: E501 line too long (99 > 90 characters) http://gerrit.cloudera.org:8080/#/c/12930/1/bin/generate_xml_config.py@33 PS1, Line 33: def _substitute_env_vars(s): flake8: E302 expected 2 blank lines, found 1 http://gerrit.cloudera.org:8080/#/c/12930/1/bin/generate_xml_config.py@38 PS1, Line 38: def dump_config(d, source_path, out): flake8: E302 expected 2 blank lines, found 1 http://gerrit.cloudera.org:8080/#/c/12930/1/bin/generate_xml_config.py@46 PS1, Line 46: flake8: W293 blank line contains whitespace http://gerrit.cloudera.org:8080/#/c/12930/1/bin/generate_xml_config.py@46 PS1, Line 46: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/12930/1/bin/generate_xml_config.py@57 PS1, Line 57: ) flake8: E501 line too long (91 > 90 characters) http://gerrit.cloudera.org:8080/#/c/12930/1/bin/generate_xml_config.py@66 PS1, Line 66: def main(): flake8: E302 expected 2 blank lines, found 1 http://gerrit.cloudera.org:8080/#/c/12930/1/bin/generate_xml_config.py@81 PS1, Line 81: e flake8: E722 do not use bare except' http://gerrit.cloudera.org:8080/#/c/12930/1/bin/generate_xml_config.py@86 PS1, Line 86: if __name__ == "__main__": flake8: E305 expected 2 blank lines after class or function definition, found 1 http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/hive-site.xml.py File fe/src/test/resources/hive-site.xml.py: http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/hive-site.xml.py@58 PS1, Line 58: 2 flake8: E501 line too long (96 > 90 characters) http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/hive-site.xml.py@66 PS1, Line 66: # flake8: E131 continuation line unaligned for hanging indent http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/hive-site.xml.py@67 PS1, Line 67: e flake8: E501 line too long (94 > 90 characters) http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/hive-site.xml.py@82 PS1, Line 82: ' flake8: E131 continuation line unaligned for hanging indent http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/hive-site.xml.py@82 PS1, Line 82: i flake8: E501 line too long (119 > 90 characters) http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/hive-site.xml.py@92 PS1, Line 92: f flake8: E501 line too long (108 > 90 characters) http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/hive-site.xml.py@93 PS1, Line 93: . flake8: E501 line too long (117 > 90 characters) http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/hive-site.xml.py@113 PS1, Line 113: t flake8: E501 line too long (112 > 90 characters) http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/hive-site.xml.py@118 PS1, Line 118: flake8: W391 blank line at end of file http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/sentry-site.xml.py File fe/src/test/resources/sentry-site.xml.py: http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/sentry-site.xml.py@49 PS1, Line 49: r flake8: E501 line too long (95 > 90 characters) -- To view, visit http://gerrit.cloudera.org:8080/12930 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47 Gerrit-Change-Number: 12930 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 04 Apr 2019 05:51:45 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Configure Hive 3's HS2 to execute queries using Tez local mode
Hello Vihang Karajgaonkar, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/12931 to review the following change. Change subject: Configure Hive 3's HS2 to execute queries using Tez local mode .. Configure Hive 3's HS2 to execute queries using Tez local mode Hive 3 no longer supports MR execution, so this sets up the appropriate configuration and classpath so that HS2 can run queries using Tez. The bulk of this patch is toolchain changes to download Tez itself. The Tez tarball is slightly odd in that it has no top-level directory, so the patch changes around bootstrap_toolchain a bit to support creating its own top-level directory for a component. The remainder of the patch is some classpath setup and hive-site changes when Hive 3 is enabled. So far I tested this manually by setting up a metastore and impala-config with USE_CDP_HIVE=true, and then connecting to HS2 using hive beeline -u 'jdbc:hive2://localhost:11050' I was able to insert and query data, and was able to verify that queries like 'select count(*)' were executing via Tez local mode. NOTE: this patch relies on a custom build of Tez, based on a private branch. I've submitted a PR to Tez upstream, referenced in the commits here. Will remove this hack once the PR is accepted and makes its way into an official build. Change-Id: I76e47fbd1d6ff5103d81a8de430d5465dba284cd --- M bin/bootstrap_toolchain.py M bin/impala-config.sh M fe/src/test/resources/hive-site.xml.py M testdata/bin/run-hive-server.sh 4 files changed, 65 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/12931/1 -- To view, visit http://gerrit.cloudera.org:8080/12931 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I76e47fbd1d6ff5103d81a8de430d5465dba284cd Gerrit-Change-Number: 12931 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-8309: add user authorization provider flag
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12901 ) Change subject: IMPALA-8309: add user authorization_provider flag .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2636/ : 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/12901 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I001c20505ba4f0562b60fdef73d15308e8500c19 Gerrit-Change-Number: 12901 Gerrit-PatchSet: 4 Gerrit-Owner: radford nguyen Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Thu, 04 Apr 2019 04:33:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8309: add user authorization provider flag
radford nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/12901 ) Change subject: IMPALA-8309: add user authorization_provider flag .. Patch Set 4: (1 comment) Modified existing e2e tests to use new `authorization_provider` flag http://gerrit.cloudera.org:8080/#/c/12901/4/tests/authorization/test_ranger.py File tests/authorization/test_ranger.py: http://gerrit.cloudera.org:8080/#/c/12901/4/tests/authorization/test_ranger.py@38 PS4, Line 38: "--authorization_provider=ranger") I changed these to use the new flag as I imagine this is the preferred way to select internally-supported auth providers. I didn't see any existing e2e tests for the `authorization_factory_class` flag, otherwise I would have duplicated them using the `authorization_provider` flag. Up to this point, looks like we only have full test coverage of authorization selection behavior in the fe, and I'm wondering if this is enough -- To view, visit http://gerrit.cloudera.org:8080/12901 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I001c20505ba4f0562b60fdef73d15308e8500c19 Gerrit-Change-Number: 12901 Gerrit-PatchSet: 4 Gerrit-Owner: radford nguyen Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Thu, 04 Apr 2019 04:30:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7995: part 1: fixes for e2e dockerised impala tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12639 ) Change subject: IMPALA-7995: part 1: fixes for e2e dockerised impala tests .. Patch Set 11: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3980/ -- To view, visit http://gerrit.cloudera.org:8080/12639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee86cbd2c4631a014af1e8cef8e1cd523a812755 Gerrit-Change-Number: 12639 Gerrit-PatchSet: 11 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 04 Apr 2019 04:07:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8309: add user authorization provider flag
Hello Austin Nobis, Fredy Wijaya, Todd Lipcon, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12901 to look at the new patch set (#4). Change subject: IMPALA-8309: add user authorization_provider flag .. IMPALA-8309: add user authorization_provider flag This commit adds a `authorization_provider` user-facing flag in order to provide a more human-readable alternative to the `authorization_factory_class` for internally-provided authorization strategies. The `authorization_factory_class` flag is retained, but no longer takes a default value if not specified. The default for `authorization_provider` is "sentry" in order to retain backwards-compatibility. If specified, `authorization_factory_class` will take precedence. Testing: Manually started minicluster with each of following flags and verified correct authorization strategy chosen: - provider='' factory='' => sentry - provider=sentry factory='' => sentry - provider=ranger factory='' => ranger - provider='' factory=sentry => sentry - provider='' factory=ranger => ranger - provider=sentry factory=sentry => sentry - provider=ranger factory=sentry => sentry - provider=sentry factory=ranger => ranger - provider=ranger factory=ranger => ranger Wrote unit tests to capture above assertions Ran fe unit and e2e tests Change-Id: I001c20505ba4f0562b60fdef73d15308e8500c19 --- M be/src/service/frontend.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/authorization/AuthorizationConfig.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationProvider.java M fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationConfig.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationConfig.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/service/JniFrontendTest.java M tests/authorization/test_ranger.py 15 files changed, 229 insertions(+), 61 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/12901/4 -- To view, visit http://gerrit.cloudera.org:8080/12901 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I001c20505ba4f0562b60fdef73d15308e8500c19 Gerrit-Change-Number: 12901 Gerrit-PatchSet: 4 Gerrit-Owner: radford nguyen Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: radford nguyen
[Impala-ASF-CR] IMPALA-8359: Fix coverage data generation for impalads
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/12858 ) Change subject: IMPALA-8359: Fix coverage data generation for impalads .. Patch Set 4: Code-Review+2 Thanks for fixing this! -- To view, visit http://gerrit.cloudera.org:8080/12858 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9be1e1e73b6cfc3557077f763aee4dbfcc7a2d27 Gerrit-Change-Number: 12858 Gerrit-PatchSet: 4 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 04 Apr 2019 03:37:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8101: Thrift 11 and ext-data-source compilation are always run
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/12290 ) Change subject: IMPALA-8101: Thrift 11 and ext-data-source compilation are always run .. Patch Set 1: Don't think so. I think it just needs to be merged. I should probably rebase it though since its so old. -- To view, visit http://gerrit.cloudera.org:8080/12290 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I52520e4b099c7bac5d088b4ba5d8a335495f727d Gerrit-Change-Number: 12290 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 04 Apr 2019 03:23:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8309: add user authorization provider flag
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12901 ) Change subject: IMPALA-8309: add user authorization_provider flag .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2635/ : 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/12901 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I001c20505ba4f0562b60fdef73d15308e8500c19 Gerrit-Change-Number: 12901 Gerrit-PatchSet: 3 Gerrit-Owner: radford nguyen Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Thu, 04 Apr 2019 02:52:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8309: add user authorization provider flag
Hello Austin Nobis, Fredy Wijaya, Todd Lipcon, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12901 to look at the new patch set (#3). Change subject: IMPALA-8309: add user authorization_provider flag .. IMPALA-8309: add user authorization_provider flag This commit adds a `authorization_provider` user-facing flag in order to provide a more human-readable alternative to the `authorization_factory_class` for internally-provided authorization strategies. The `authorization_factory_class` flag is retained, but no longer takes a default value if not specified. The default for `authorization_provider` is "sentry" in order to retain backwards-compatibility. If specified, `authorization_factory_class` will take precedence. Testing: Manually started minicluster with each of following flags and verified correct authorization strategy chosen: - provider='' factory='' => sentry - provider=sentry factory='' => sentry - provider=ranger factory='' => ranger - provider='' factory=sentry => sentry - provider='' factory=ranger => ranger - provider=sentry factory=sentry => sentry - provider=ranger factory=sentry => sentry - provider=sentry factory=ranger => ranger - provider=ranger factory=ranger => ranger Wrote unit tests to capture above assertions Ran fe unit and e2e tests Change-Id: I001c20505ba4f0562b60fdef73d15308e8500c19 --- M be/src/service/frontend.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/authorization/AuthorizationConfig.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationProvider.java M fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationConfig.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationConfig.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/service/JniFrontendTest.java 14 files changed, 227 insertions(+), 57 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/12901/3 -- To view, visit http://gerrit.cloudera.org:8080/12901 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I001c20505ba4f0562b60fdef73d15308e8500c19 Gerrit-Change-Number: 12901 Gerrit-PatchSet: 3 Gerrit-Owner: radford nguyen Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: radford nguyen
[Impala-ASF-CR] IMPALA-8309: add user authorization provider flag
radford nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/12901 )
Change subject: IMPALA-8309: add user authorization_provider flag
..
Patch Set 2:
(8 comments)
Addressed nits; adding e2e tests next
http://gerrit.cloudera.org:8080/#/c/12901/2/be/src/service/frontend.cc
File be/src/service/frontend.cc:
http://gerrit.cloudera.org:8080/#/c/12901/2/be/src/service/frontend.cc@41
PS2, Line 41: "sentry",
> nit: move default to line above like other DEFINE
Honest question: why?
I don't really see consistent usage in this file; for example
`DEFINE_string(ranger_app_id` begins the description string on a newline but
`DEFINE_string(server_name` begins the description on the same line... even
though both description strings end up wrapping.
I think the current approach is perfectly readable, so I don't understand the
point of these 3 nits.
http://gerrit.cloudera.org:8080/#/c/12901/2/fe/src/main/java/org/apache/impala/authorization/AuthorizationProvider.java
File
fe/src/main/java/org/apache/impala/authorization/AuthorizationProvider.java:
http://gerrit.cloudera.org:8080/#/c/12901/2/fe/src/main/java/org/apache/impala/authorization/AuthorizationProvider.java@31
PS2, Line 31: factoryClass
> Should this be `factoryClassName`?
Yeah that's a better name. I was waffling back and forth between using the
actual `Class` object vs. the canonical name.
http://gerrit.cloudera.org:8080/#/c/12901/2/fe/src/main/java/org/apache/impala/service/BackendConfig.java
File fe/src/main/java/org/apache/impala/service/BackendConfig.java:
http://gerrit.cloudera.org:8080/#/c/12901/2/fe/src/main/java/org/apache/impala/service/BackendConfig.java@141
PS2, Line 141: public @Nullable String getAuthorizationFactoryClassOrNull() {
> nit: I'm not opposed to adding the @Nullable annotation and adding `orNull`
That's the thing, I wasn't able to find another example in the codebase for
handling optional flags. If you could point one out I'll make sure this
conforms
http://gerrit.cloudera.org:8080/#/c/12901/2/fe/src/main/java/org/apache/impala/service/JniFrontend.java
File fe/src/main/java/org/apache/impala/service/JniFrontend.java:
http://gerrit.cloudera.org:8080/#/c/12901/2/fe/src/main/java/org/apache/impala/service/JniFrontend.java@773
PS2, Line 773: * @param beCfg
:* @return
:* @throws InternalException
> Finish the documentation
Done
http://gerrit.cloudera.org:8080/#/c/12901/2/fe/src/main/java/org/apache/impala/service/JniFrontend.java@778
PS2, Line 778: throws InternalException {
> nit: indent 4
Done
http://gerrit.cloudera.org:8080/#/c/12901/2/fe/src/main/java/org/apache/impala/service/JniFrontend.java@794
PS2, Line 794: +authzProvider
> nit: add space around the `+`
Done
http://gerrit.cloudera.org:8080/#/c/12901/2/fe/src/main/java/org/apache/impala/service/JniFrontend.java@812
PS2, Line 812: throws InternalException {
> nit: indent 4
Done
http://gerrit.cloudera.org:8080/#/c/12901/2/fe/src/main/java/org/apache/impala/service/JniFrontend.java@822
PS2, Line 822: +authzFactoryClassName
> nit: add space around the `+`
Done
--
To view, visit http://gerrit.cloudera.org:8080/12901
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I001c20505ba4f0562b60fdef73d15308e8500c19
Gerrit-Change-Number: 12901
Gerrit-PatchSet: 2
Gerrit-Owner: radford nguyen
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: radford nguyen
Gerrit-Comment-Date: Thu, 04 Apr 2019 02:08:02 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8226: Add grant/revoke to/from group for Ranger
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12914 ) Change subject: IMPALA-8226: Add grant/revoke to/from group for Ranger .. Patch Set 7: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3977/ -- To view, visit http://gerrit.cloudera.org:8080/12914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96 Gerrit-Change-Number: 12914 Gerrit-PatchSet: 7 Gerrit-Owner: Austin Nobis Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Thu, 04 Apr 2019 01:55:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8380: Bump Postgres JDBC driver version to 9.4
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12894 ) Change subject: IMPALA-8380: Bump Postgres JDBC driver version to 9.4 .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica5423c18a9f8346dda7dae617b1764638b57b6c Gerrit-Change-Number: 12894 Gerrit-PatchSet: 2 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Laszlo Gaal Gerrit-Comment-Date: Thu, 04 Apr 2019 01:58:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12179 ) Change subject: IMPALA-7640: RENAME on managed Kudu table should rename Kudu table .. Patch Set 4: Thomas, do you have time to look at this change? -- To view, visit http://gerrit.cloudera.org:8080/12179 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I77e7583ce93cba8f6e743c4bedd9900ae1fae081 Gerrit-Change-Number: 12179 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Thu, 04 Apr 2019 01:07:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7087: Read Parquet decimal columns with lower precision/scale than table metadata
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12163 ) Change subject: IMPALA-7087: Read Parquet decimal columns with lower precision/scale than table metadata .. Patch Set 3: Zoltan, any plans to take another look at this one? -- To view, visit http://gerrit.cloudera.org:8080/12163 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iafc8efd12379a39756e3e70f022a81a636dadb61 Gerrit-Change-Number: 12163 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 04 Apr 2019 01:05:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8383 Bump toolchain version
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12923 ) Change subject: IMPALA-8383 Bump toolchain version .. Patch Set 1: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3975/ -- To view, visit http://gerrit.cloudera.org:8080/12923 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If221f001bc106530c5faf08b38385028c056983f Gerrit-Change-Number: 12923 Gerrit-PatchSet: 1 Gerrit-Owner: Hector Acosta Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 04 Apr 2019 01:05:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8101: Thrift 11 and ext-data-source compilation are always run
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12290 ) Change subject: IMPALA-8101: Thrift 11 and ext-data-source compilation are always run .. Patch Set 1: Anything blocking this from moving forward? -- To view, visit http://gerrit.cloudera.org:8080/12290 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I52520e4b099c7bac5d088b4ba5d8a335495f727d Gerrit-Change-Number: 12290 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Thu, 04 Apr 2019 00:57:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8380: Bump Postgres JDBC driver version to 9.4
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12894 ) Change subject: IMPALA-8380: Bump Postgres JDBC driver version to 9.4 .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2634/ : 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/12894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica5423c18a9f8346dda7dae617b1764638b57b6c Gerrit-Change-Number: 12894 Gerrit-PatchSet: 3 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Laszlo Gaal Gerrit-Comment-Date: Thu, 04 Apr 2019 00:20:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7995: part 1: fixes for e2e dockerised impala tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12639 ) Change subject: IMPALA-7995: part 1: fixes for e2e dockerised impala tests .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2633/ : 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/12639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee86cbd2c4631a014af1e8cef8e1cd523a812755 Gerrit-Change-Number: 12639 Gerrit-PatchSet: 10 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 04 Apr 2019 00:10:17 + Gerrit-HasComments: No
[native-toolchain-CR] Remove '-static-libstdc++' from CXXFLAGS
Thomas Marshall has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12929 ) Change subject: Remove '-static-libstdc++' from CXXFLAGS .. Remove '-static-libstdc++' from CXXFLAGS CXXFLAGS used to be ignored by the Kudu build but a change upstream, which was pulled in when the toolchain Kudu version was bumped to 1.9.0, makes it take effect, which caused problems - impalads crash immediately upon startup when linked against a libkudu_client.so built with these CXXFLAGS. This patch removes the flag '-static-libstdc++' so that we link to libstdc++.so we ship rather than having two versions of the symbols in two libraries. Change-Id: Icb95520f98ee9ff46c4a64e3aec1c26eee144f5b Reviewed-on: http://gerrit.cloudera.org:8080/12929 Reviewed-by: Lars Volker Tested-by: Thomas Marshall --- M init-compiler.sh 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Lars Volker: Looks good to me, approved Thomas Marshall: Verified -- To view, visit http://gerrit.cloudera.org:8080/12929 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Icb95520f98ee9ff46c4a64e3aec1c26eee144f5b Gerrit-Change-Number: 12929 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Hector Acosta Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[native-toolchain-CR] Remove '-static-libstdc++' from CXXFLAGS
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/12929 ) Change subject: Remove '-static-libstdc++' from CXXFLAGS .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12929 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icb95520f98ee9ff46c4a64e3aec1c26eee144f5b Gerrit-Change-Number: 12929 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Hector Acosta Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 04 Apr 2019 00:04:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8377: bump toolchain version to 107-acaeac961d
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12928 ) Change subject: IMPALA-8377: bump toolchain version to 107-acaeac961d .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2632/ : 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/12928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4ac25aa230b9d2559cd4eb6166ab985b18ef7e2a Gerrit-Change-Number: 12928 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Hector Acosta Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 04 Apr 2019 00:03:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7031: Cancel action of debug page should not unregister query
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12926 ) Change subject: IMPALA-7031: Cancel action of debug page should not unregister query .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2630/ : 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/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 2 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 03 Apr 2019 23:34:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6826: Extend bootstrap system.sh to Ubuntu 18.04
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12893 ) Change subject: IMPALA-6826: Extend bootstrap_system.sh to Ubuntu 18.04 .. IMPALA-6826: Extend bootstrap_system.sh to Ubuntu 18.04 Tweak bin/bootstrap_system.sh to automate the preparation of an Impala development environment on Ubuntu 18.04. The following changes were required: - extend the OS recognition logic to Ubuntu 18.04 - add 'ant' to the list of installed packages - request OpenJDK 8 as the default Java environment (Ubuntu 18.04 defaults to OpenJDK 11) These changes enable bootstrap_system.sh to set up an Impala development environment where Impala can be successfully built. Note that the patch does not attempt to pass the tests yet; this change prepares only the environment. Bugs specific to Ubuntu 18 will be fixed by follow-up commits. Tested in the following environments: - in a Docker container, using "docker/test-with-docker.py --base-image:ubuntu:18.04" - on an AWS EC2 m5.4xlarge instance Change-Id: Iad790f72ea6b62258aed2225eb7bdf79590c350f Reviewed-on: http://gerrit.cloudera.org:8080/12893 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M bin/bootstrap_system.sh 1 file changed, 40 insertions(+), 10 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/12893 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Iad790f72ea6b62258aed2225eb7bdf79590c350f Gerrit-Change-Number: 12893 Gerrit-PatchSet: 5 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Laszlo Gaal
[Impala-ASF-CR] IMPALA-8363: Deny access when column masking or row filtering is enabled in Ranger
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12927 ) Change subject: IMPALA-8363: Deny access when column masking or row filtering is enabled in Ranger .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2631/ : 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/12927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If46b4bf24d916e4a4ea8a36ff4acfd95d5f45c8e Gerrit-Change-Number: 12927 Gerrit-PatchSet: 1 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 03 Apr 2019 23:48:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6826: Extend bootstrap system.sh to Ubuntu 18.04
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12893 ) Change subject: IMPALA-6826: Extend bootstrap_system.sh to Ubuntu 18.04 .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12893 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iad790f72ea6b62258aed2225eb7bdf79590c350f Gerrit-Change-Number: 12893 Gerrit-PatchSet: 4 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Laszlo Gaal Gerrit-Comment-Date: Wed, 03 Apr 2019 23:42:06 + Gerrit-HasComments: No
[native-toolchain-CR] Remove '-static-libstdc++' from CXXFLAGS
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12929 ) Change subject: Remove '-static-libstdc++' from CXXFLAGS .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12929 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icb95520f98ee9ff46c4a64e3aec1c26eee144f5b Gerrit-Change-Number: 12929 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 03 Apr 2019 23:48:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8380: Bump Postgres JDBC driver version to 9.4
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/12894 )
Change subject: IMPALA-8380: Bump Postgres JDBC driver version to 9.4
..
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12894/3/bin/impala-config.sh
File bin/impala-config.sh:
http://gerrit.cloudera.org:8080/#/c/12894/3/bin/impala-config.sh@567
PS3, Line 567: export
POSTGRES_JDBC_DRIVER="${IMPALA_FE_DIR}/target/dependency/postgresql-${IMPALA_POSTGRES_JDBC_DRIVER_VERSION}.jar"
line too long (118 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/12894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ica5423c18a9f8346dda7dae617b1764638b57b6c
Gerrit-Change-Number: 12894
Gerrit-PatchSet: 3
Gerrit-Owner: Laszlo Gaal
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Laszlo Gaal
Gerrit-Comment-Date: Wed, 03 Apr 2019 23:47:49 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7995: part 1: fixes for e2e dockerised impala tests
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12639 ) Change subject: IMPALA-7995: part 1: fixes for e2e dockerised impala tests .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/12639/11/tests/query_test/test_hdfs_caching.py File tests/query_test/test_hdfs_caching.py: http://gerrit.cloudera.org:8080/#/c/12639/11/tests/query_test/test_hdfs_caching.py@77 PS11, Line 77: LOG.info(cached_bytes_after) This is debugging code which I should remove -- To view, visit http://gerrit.cloudera.org:8080/12639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee86cbd2c4631a014af1e8cef8e1cd523a812755 Gerrit-Change-Number: 12639 Gerrit-PatchSet: 11 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 03 Apr 2019 23:47:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8380: Bump Postgres JDBC driver version to 9.4
Lars Volker has uploaded a new patch set (#3) to the change originally created by Laszlo Gaal. ( http://gerrit.cloudera.org:8080/12894 ) Change subject: IMPALA-8380: Bump Postgres JDBC driver version to 9.4 .. IMPALA-8380: Bump Postgres JDBC driver version to 9.4 Testing on Ubuntu 18.04 with PostgreSQL 10 (the default for the OS) revealed that HMS fails to start with the existing v9.0 Postgres JDBC driver. The patch bumps the driver version to 42.2.5, which allows HMS to start with PostreSQL 10. The updated driver turned out to be pickier about the JDBC connection strings, so these had to be updated in the various sentry-site-*.xml templates. The old connection strings had two problems: - trailing slashes in the URIs, - a ';create=true' additional parameter, which seems to be a remnant of a Derby connection string. The previous driver accepted (and probably ignored) this, but the new one rejects any extension after the database name. To ensure that existing platforms are not broken, core tests were run: - in Docker for CentOS 6, CentOS 7 and Ubuntu 16.04 - on Amazon VMs for CentOS 6.4 and CentOS 7.4 - Ubuntu 18.04 was tested on a VM and in Docker as well. Change-Id: Ica5423c18a9f8346dda7dae617b1764638b57b6c --- M bin/create-test-configuration.sh M bin/impala-config.sh M fe/pom.xml M fe/src/test/resources/sentry-site.xml.template M fe/src/test/resources/sentry-site_no_oo.xml.template M fe/src/test/resources/sentry-site_oo.xml.template M fe/src/test/resources/sentry-site_oo_nogrant.xml.template M impala-parent/pom.xml 8 files changed, 14 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/94/12894/3 -- To view, visit http://gerrit.cloudera.org:8080/12894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ica5423c18a9f8346dda7dae617b1764638b57b6c Gerrit-Change-Number: 12894 Gerrit-PatchSet: 3 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Laszlo Gaal
[Impala-ASF-CR] IMPALA-7995: part 1: fixes for e2e dockerised impala tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12639 ) Change subject: IMPALA-7995: part 1: fixes for e2e dockerised impala tests .. Patch Set 11: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3980/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee86cbd2c4631a014af1e8cef8e1cd523a812755 Gerrit-Change-Number: 12639 Gerrit-PatchSet: 11 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 03 Apr 2019 23:45:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7995: part 1: fixes for e2e dockerised impala tests
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12639 to look at the new patch set (#10). Change subject: IMPALA-7995: part 1: fixes for e2e dockerised impala tests .. IMPALA-7995: part 1: fixes for e2e dockerised impala tests This fixes all core e2e tests running on my local dockerised minicluster build. I do not yet have a CI job or script running but I wanted to get feedback on these changes sooner. The second part of the change will include the CI script and any follow-on fixes required for the exhaustive tests. The following fixes were required: * Detect docker_network from TEST_START_CLUSTER_ARGS * get_webserver_port() does not depend on the caller passing in the default webserver port. It failed previously because it relied on start-impala-cluster.py setting -webserver_port for *all* processes. * Add SkipIf markers for tests that don't make sense or are non-trivial to fix for containerised Impala. * Support loading Impala-lzo plugin from host for tests that depend on it. * Fix some tests that had 'localhost' hardcoded - instead it should be $INTERNAL_LISTEN_HOST, which defaults to localhost. * Fix bug with sorting impala daemons by backend port, which is the same for all dockerised impalads. Testing: I ran tests locally as follows after having set up a docker network and starting other services: ./buildall.sh -noclean -notests -ninja ninja -j $IMPALA_BUILD_THREADS docker_images export TEST_START_CLUSTER_ARGS="--docker_network=impala-cluster" export FE_TEST=false export BE_TEST=false export JDBC_TEST=false export CLUSTER_TEST=false ./bin/run-all-tests.sh Change-Id: Iee86cbd2c4631a014af1e8cef8e1cd523a812755 --- M bin/start-impala-cluster.py M docker/daemon_entrypoint.sh M docker/impala_base/Dockerfile M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test M testdata/workloads/functional-query/queries/QueryTest/kudu_create.test M tests/catalog_service/test_catalog_service_client.py M tests/common/custom_cluster_test_suite.py M tests/common/environ.py M tests/common/impala_cluster.py M tests/common/impala_test_suite.py M tests/common/skip.py M tests/conftest.py M tests/custom_cluster/test_jvm_mem_tracking.py M tests/custom_cluster/test_krpc_mem_usage.py M tests/custom_cluster/test_rpc_timeout.py M tests/custom_cluster/test_udf_concurrency.py M tests/hs2/test_fetch_first.py M tests/hs2/test_hs2.py M tests/hs2/test_json_endpoints.py M tests/metadata/test_compute_stats.py M tests/metadata/test_ddl.py M tests/observability/test_log_fragments.py M tests/query_test/test_hash_join_timer.py M tests/query_test/test_hdfs_caching.py M tests/query_test/test_insert.py M tests/query_test/test_insert_behaviour.py M tests/query_test/test_kudu.py M tests/query_test/test_lifecycle.py M tests/query_test/test_local_fs.py M tests/query_test/test_mem_usage_scaling.py M tests/query_test/test_queries.py M tests/query_test/test_udfs.py M tests/run-tests.py M tests/statestore/test_statestore.py M tests/stress/test_mini_stress.py M tests/webserver/test_web_pages.py 36 files changed, 183 insertions(+), 80 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/12639/10 -- To view, visit http://gerrit.cloudera.org:8080/12639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iee86cbd2c4631a014af1e8cef8e1cd523a812755 Gerrit-Change-Number: 12639 Gerrit-PatchSet: 10 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins
[native-toolchain-CR] Remove '-static-libstdc++' from CXXFLAGS
Thomas Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12929 Change subject: Remove '-static-libstdc++' from CXXFLAGS .. Remove '-static-libstdc++' from CXXFLAGS CXXFLAGS used to be ignored by the Kudu build but a change upstream, which was pulled in when the toolchain Kudu version was bumped to 1.9.0, makes it take effect, which caused problems - impalads crash immediately upon startup when linked against a libkudu_client.so built with these CXXFLAGS. This patch removes the flag '-static-libstdc++' so that we link to libstdc++.so we ship rather than having two versions of the symbols in two libraries. Change-Id: Icb95520f98ee9ff46c4a64e3aec1c26eee144f5b --- M init-compiler.sh 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/29/12929/1 -- To view, visit http://gerrit.cloudera.org:8080/12929 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Icb95520f98ee9ff46c4a64e3aec1c26eee144f5b Gerrit-Change-Number: 12929 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall
[Impala-ASF-CR] IMPALA-8377: bump toolchain version to 107-acaeac961d
Thomas Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12928 Change subject: IMPALA-8377: bump toolchain version to 107-acaeac961d .. IMPALA-8377: bump toolchain version to 107-acaeac961d This fixes an issue with the previous toolchain version where the Kudu client was broken and caused all binaries to crash on startup due to an issue with linked libstdc++ Change-Id: I4ac25aa230b9d2559cd4eb6166ab985b18ef7e2a --- M bin/impala-config.sh 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/12928/1 -- To view, visit http://gerrit.cloudera.org:8080/12928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I4ac25aa230b9d2559cd4eb6166ab985b18ef7e2a Gerrit-Change-Number: 12928 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall
[Impala-ASF-CR] IMPALA-7031: Cancel action of debug page should not unregister query
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12926 ) Change subject: IMPALA-7031: Cancel action of debug page should not unregister query .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2629/ : 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/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 1 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 03 Apr 2019 23:37:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8363: Deny access when column masking or row filtering is enabled in Ranger
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12927 Change subject: IMPALA-8363: Deny access when column masking or row filtering is enabled in Ranger .. IMPALA-8363: Deny access when column masking or row filtering is enabled in Ranger This patch updates the Ranger authorization checker code to deny access when column masking and row filtering is enabled in Ranger for queries that that have columns/tables specified in column mask and row filter policies. This is to prevent data leak, such that the data that is masked/filtered in Hive should not be visible at all in Impala until Impala has full support for column masking and row filtering. Testing: - Added tests in AuthorizationStmtTest to test queries with column masking and row filtering enabled. - Ran all FE tests - Ran all E2E tests Change-Id: If46b4bf24d916e4a4ea8a36ff4acfd95d5f45c8e --- M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationProvider.java R fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationFactory.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java M fe/src/test/java/org/apache/impala/catalog/AlterDatabaseTest.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M fe/src/test/java/org/apache/impala/common/FrontendFixture.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/common/QueryFixture.java M fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java M fe/src/test/java/org/apache/impala/testutil/PlannerTestCaseLoader.java 23 files changed, 436 insertions(+), 81 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/12927/1 -- To view, visit http://gerrit.cloudera.org:8080/12927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: If46b4bf24d916e4a4ea8a36ff4acfd95d5f45c8e Gerrit-Change-Number: 12927 Gerrit-PatchSet: 1 Gerrit-Owner: Fredy Wijaya
[Impala-ASF-CR] IMPALA-7031: Cancel action of debug page should not unregister query
Hello Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12926 to look at the new patch set (#2). Change subject: IMPALA-7031: Cancel action of debug page should not unregister query .. IMPALA-7031: Cancel action of debug page should not unregister query When a running query is cancelled from impalad WebUI (debug page), the client will receive an invalid query handle exception on the next fetch/getOperationStatus attempts. The patch modified ImpalaHttpHandler, "Cancel" button will only cancel the query instead of unregister it. Testing: - Added test_cancel_query_from_debug_page at test_web_pages.py Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 --- M be/src/service/impala-http-handler.cc M tests/webserver/test_web_pages.py 2 files changed, 21 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/12926/2 -- To view, visit http://gerrit.cloudera.org:8080/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 2 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7031: Cancel action of debug page should not unregister query
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12926 ) Change subject: IMPALA-7031: Cancel action of debug page should not unregister query .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/12926/1/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/12926/1/tests/webserver/test_web_pages.py@543 PS1, Line 543: 0 flake8: E501 line too long (93 > 90 characters) -- To view, visit http://gerrit.cloudera.org:8080/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 1 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 03 Apr 2019 22:57:15 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7031: Cancel action of debug page should not unregister query
Alice Fan has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12926 Change subject: IMPALA-7031: Cancel action of debug page should not unregister query .. IMPALA-7031: Cancel action of debug page should not unregister query When a running query is cancelled from impalad WebUI (debug page), the client will receive an invalid query handle exception on the next fetch/getOperationStatus attempts. The patch modified ImpalaHttpHandler, "Cancel" button will only cancel the query instead of unregister it. Testing: - Added test_cancel_query_from_debug_page at test_web_pages.py Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 --- M be/src/service/impala-http-handler.cc M tests/webserver/test_web_pages.py 2 files changed, 21 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/12926/1 -- To view, visit http://gerrit.cloudera.org:8080/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 1 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8380: Bump Postgres JDBC driver version to 9.4
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12894 ) Change subject: IMPALA-8380: Bump Postgres JDBC driver version to 9.4 .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2628/ : 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/12894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica5423c18a9f8346dda7dae617b1764638b57b6c Gerrit-Change-Number: 12894 Gerrit-PatchSet: 2 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Laszlo Gaal Gerrit-Comment-Date: Wed, 03 Apr 2019 22:01:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5843: Use page index in Parquet files to skip pages
Lars Volker has posted comments on this change. (
http://gerrit.cloudera.org:8080/12065 )
Change subject: IMPALA-5843: Use page index in Parquet files to skip pages
..
Patch Set 8:
(37 comments)
http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/hdfs-scan-node-base.h
File be/src/exec/hdfs-scan-node-base.h:
http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/hdfs-scan-node-base.h@272
PS8, Line 272: io::ScanRange* AllocateScanRange(hdfsFS fs, const char* file,
nit: same line wrapping for both methods
http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/hdfs-parquet-scanner.h
File be/src/exec/parquet/hdfs-parquet-scanner.h:
http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/hdfs-parquet-scanner.h@360
PS8, Line 360: parquet::ColumnOrder*
nit: make this a const& or non-const ptr. If a const* is required to be null,
mention in the comment.
http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/hdfs-parquet-scanner.h@428
PS8, Line 428: Page
nit: lower case
http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/hdfs-parquet-scanner.h@429
PS8, Line 429: eliminated
nit: missing comma
http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/hdfs-parquet-scanner.h@552
PS8, Line 552: /// Reads statistics data from the page index.
This looks like a candidate to explain the various parameters in some details,
in particular fn_name
http://gerrit.cloudera.org:8080/#/c/12065/7/be/src/exec/parquet/hdfs-parquet-scanner.cc
File be/src/exec/parquet/hdfs-parquet-scanner.cc:
http://gerrit.cloudera.org:8080/#/c/12065/7/be/src/exec/parquet/hdfs-parquet-scanner.cc@803
PS7, Line 803: nReader* scalar_reader :
> I only did it this way to not make the signature awkward. If I change 'skip
That's a good point. One could pass skip_ranges by value and then let the
compiler elide the copies but consistency with out style is important, too.
To improve the quality of the abstraction and readability, I'd also consider
making a copy of skip_ranges inside the method and then sorting it there. If it
is small, the extra copy won't matter, and if it is large, the sorting will
dominate the runtime, anyways.
Passing by ptr would also be OK I think, the order could be (num_rows,
&skip_ranges, &candidate_ranges), which I think is fine.
http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/hdfs-parquet-scanner.cc
File be/src/exec/parquet/hdfs-parquet-scanner.cc:
http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/hdfs-parquet-scanner.cc@526
PS8, Line 526:
nit: double space
http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/parquet-common-test.cc
File be/src/exec/parquet/parquet-common-test.cc:
http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/parquet-common-test.cc@32
PS8, Line 32: TEST(ParquetCommon, CalculateCandidateRanges) {
Please add some comments to the TEST functions that briefly explain what the
test does.
http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/parquet-common-test.cc@32
PS8, Line 32: CalculateCandidateRanges
rename
http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/parquet-common-test.cc@49
PS8, Line 49: CalculateCandidateRanges({{1, 8}, {3, 8}, {7, 8}}, 10),
{{0, 0}, {9, 9}});
Let's add some tests about error conditions, either here or elsewhere (this
method might DCHECK). Currently the ranges are populated from
page_locations.first_row_index which could have corrupted data.
GetRowRangesForPage() could have some error handling but
ComputeCandidateRanges() feels far away enough to warrant its own error
checking.
* number of rows smaller than the beginning or end of one of the ranges
* Negative boundaries
* Last < first
http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/parquet-common-test.cc@52
PS8, Line 52: int
It would be good to have some tests that validate that values > INT_MAX don't
break stuff for all places where they can occur.
http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/parquet-common-test.cc@61
PS8, Line 61: CalculateCandidatePages
rename
http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/parquet-common-test.cc@77
PS8, Line 77: ValidatePages({0, 10, 20, 50, 70}, {{9, 9}, {10, 10}, {50,
50}}, 100, {0, 1, 3});
Some tests for error conditions would be good here, too.
http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/parquet-common.h
File be/src/exec/parquet/parquet-common.h:
http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/parquet-common.h@72
PS8, Line 72: don't want to skip.
"want to read" or "want to scan"?
http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/parquet-common.h@74
PS8, Line 74: const
nit: I think we usually omit this when passing by value
http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/parquet-common.cc
File be/src/exec/parquet/parquet-
[Impala-ASF-CR] IMPALA-8226: Add grant/revoke to/from group for Ranger
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12914 ) Change subject: IMPALA-8226: Add grant/revoke to/from group for Ranger .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2627/ : 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/12914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96 Gerrit-Change-Number: 12914 Gerrit-PatchSet: 6 Gerrit-Owner: Austin Nobis Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Wed, 03 Apr 2019 21:50:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5843: Use page index in Parquet files to skip pages
Lars Volker has posted comments on this change. (
http://gerrit.cloudera.org:8080/12065 )
Change subject: IMPALA-5843: Use page index in Parquet files to skip pages
..
Patch Set 8:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/12065/7/be/src/exec/parquet/hdfs-parquet-scanner.cc
File be/src/exec/parquet/hdfs-parquet-scanner.cc:
http://gerrit.cloudera.org:8080/#/c/12065/7/be/src/exec/parquet/hdfs-parquet-scanner.cc@119
PS7, Line 119: num_stats_filtered_row_groups_by_page_index_counter_ =
> I am not sure how to track this, but it is also possible that some columns
Should these be NumStatsFilteredRowGroups and NumStatsFilteredPages?
http://gerrit.cloudera.org:8080/#/c/12065/7/be/src/exec/parquet/hdfs-parquet-scanner.cc@813
PS7, Line 813:
> Now that ComputeCandidatePages() has an output parameter, setting scalar_re
You could pass the scalar reader into ComputeCandidatePages() to call the
setter and move the result there to avoid the copy. If you don't want to have
that dependency, you could pass a callback lambda to ComputeCandidatePages
which you can define here and call the setter from there.
http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/hdfs-parquet-scanner.cc
File be/src/exec/parquet/hdfs-parquet-scanner.cc:
http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/hdfs-parquet-scanner.cc@644
PS8, Line 644: if (filter_pages && candidate_ranges_.empty()) {
Do we expect this to happen in reality if the row group also has stats? If so,
can we add it to NumStatsFilteredRowGroups and change that counter's meaning to
"Row group that has not been read because of stats"?
http://gerrit.cloudera.org:8080/#/c/12065/7/be/src/exec/parquet/parquet-common.cc
File be/src/exec/parquet/parquet-common.cc:
http://gerrit.cloudera.org:8080/#/c/12065/7/be/src/exec/parquet/parquet-common.cc@123
PS7, Line 123: }
> I take it as a parameter now, and call it 'candidate_pages'.
I think "candidate_pages" is ok as long as it's only ever used as a positive
list, i.e. candidates to be read.
--
To view, visit http://gerrit.cloudera.org:8080/12065
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0cc99f129f2048dbafbe7f5a51d1ea3a5005731a
Gerrit-Change-Number: 12065
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Borok-Nagy
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Pooja Nilangekar
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Wed, 03 Apr 2019 21:52:31 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8226: Add grant/revoke to/from group for Ranger
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12914 ) Change subject: IMPALA-8226: Add grant/revoke to/from group for Ranger .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3977/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96 Gerrit-Change-Number: 12914 Gerrit-PatchSet: 7 Gerrit-Owner: Austin Nobis Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Wed, 03 Apr 2019 21:17:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8226: Add grant/revoke to/from group for Ranger
Austin Nobis has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/12914 ) Change subject: IMPALA-8226: Add grant/revoke to/from group for Ranger .. IMPALA-8226: Add grant/revoke to/from group for Ranger This patch adds fupport for GRANT privilege statements to GROUP and REVOKE privilege statements from GROUP. The grammar has been updated to support FROM GROUP and TO GROUP for GRANT/REVOKE statements, i.e: GRANT ON TO GROUP REVOKE ON FROM GROUP Currently, only Ranger's authorization implementation supports GROUP based privileges. Sentry will throw an UnsupportedOperationException if it is the enabled authorization provider and this new grammar is used. Testing: - AuthorizationStmtTest was updated to also test for GROUP authorization. - ToSqlTest was updated to test for GROUP changes to the grammar. - A GROUP based E2E test was added to test_ranger.py - ParserTest was updated to test combinations for GrantRevokePrivilege - AnalyzeAuthStmtsTest was updated to test for USER and GROUP identities - Ran all FE tests - Ran authorization E2E tests Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96 --- M common/thrift/CatalogObjects.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/resources/ranger-hive-security.xml M testdata/bin/create-load-data.sh A testdata/cluster/ranger/setup/impala_group.json.template M testdata/cluster/ranger/setup/impala_user.json.template M tests/authorization/test_ranger.py 17 files changed, 468 insertions(+), 323 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/12914/6 -- To view, visit http://gerrit.cloudera.org:8080/12914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96 Gerrit-Change-Number: 12914 Gerrit-PatchSet: 6 Gerrit-Owner: Austin Nobis Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: radford nguyen
[Impala-ASF-CR] IMPALA-8226: Add grant/revoke to/from group for Ranger
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12914 ) Change subject: IMPALA-8226: Add grant/revoke to/from group for Ranger .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96 Gerrit-Change-Number: 12914 Gerrit-PatchSet: 6 Gerrit-Owner: Austin Nobis Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Wed, 03 Apr 2019 21:17:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8226: Add grant/revoke to/from group for Ranger
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12914 ) Change subject: IMPALA-8226: Add grant/revoke to/from group for Ranger .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96 Gerrit-Change-Number: 12914 Gerrit-PatchSet: 7 Gerrit-Owner: Austin Nobis Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Wed, 03 Apr 2019 21:17:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8380: Bump Postgres JDBC driver version to 9.4
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12894 ) Change subject: IMPALA-8380: Bump Postgres JDBC driver version to 9.4 .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3976/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/12894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica5423c18a9f8346dda7dae617b1764638b57b6c Gerrit-Change-Number: 12894 Gerrit-PatchSet: 2 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Laszlo Gaal Gerrit-Comment-Date: Wed, 03 Apr 2019 21:17:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8380: Bump Postgres JDBC driver version to 9.4
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/12894 )
Change subject: IMPALA-8380: Bump Postgres JDBC driver version to 9.4
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12894/2/bin/impala-config.sh
File bin/impala-config.sh:
http://gerrit.cloudera.org:8080/#/c/12894/2/bin/impala-config.sh@567
PS2, Line 567: export
POSTGRES_JDBC_DRIVER="${IMPALA_FE_DIR}/target/dependency/postgresql-${IMPALA_POSTGRES_JDBC_DRIVER_VERSION}.jar"
line too long (118 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/12894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ica5423c18a9f8346dda7dae617b1764638b57b6c
Gerrit-Change-Number: 12894
Gerrit-PatchSet: 2
Gerrit-Owner: Laszlo Gaal
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Laszlo Gaal
Gerrit-Comment-Date: Wed, 03 Apr 2019 21:16:09 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8380: Bump Postgres JDBC driver version to 9.4
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12894 ) Change subject: IMPALA-8380: Bump Postgres JDBC driver version to 9.4 .. Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/12894/1/bin/impala-config.sh File bin/impala-config.sh: http://gerrit.cloudera.org:8080/#/c/12894/1/bin/impala-config.sh@567 PS1, Line 567: jre7 > I'm curious why we chose the one with jre7 instead of the default one. Possibly for compatibility versions but I'll give it a try with the default one for now. http://gerrit.cloudera.org:8080/#/c/12894/1/fe/pom.xml File fe/pom.xml: http://gerrit.cloudera.org:8080/#/c/12894/1/fe/pom.xml@451 PS1, Line 451: > nit: this comment is not necessary I actually found it helpful, but that's because I didn't know that location. Happy to remove it though. http://gerrit.cloudera.org:8080/#/c/12894/1/fe/pom.xml@453 PS1, Line 453: > nit: use 2 spaces Done http://gerrit.cloudera.org:8080/#/c/12894/1/fe/pom.xml@455 PS1, Line 455: 9.4.1212.jre7 > shouldn't we be getting it from the IMPALA_POSTGRES_JDBC_DRIVER_VERSION env Done -- To view, visit http://gerrit.cloudera.org:8080/12894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica5423c18a9f8346dda7dae617b1764638b57b6c Gerrit-Change-Number: 12894 Gerrit-PatchSet: 1 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Laszlo Gaal Gerrit-Comment-Date: Wed, 03 Apr 2019 21:15:03 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8380: Bump Postgres JDBC driver version to 9.4
Lars Volker has uploaded a new patch set (#2) to the change originally created by Laszlo Gaal. ( http://gerrit.cloudera.org:8080/12894 ) Change subject: IMPALA-8380: Bump Postgres JDBC driver version to 9.4 .. IMPALA-8380: Bump Postgres JDBC driver version to 9.4 Testing on Ubuntu 18.04 with PostgreSQL 10 (the default for the OS) revealed that HMS fails to start with the existing v9.0 Postgres JDBC driver. The patch bumps the driver version to 42.2.5, which allows HMS to start with PostreSQL 10. The updated driver turned out to be pickier about the JDBC connection strings, so these had to be updated in the various sentry-site-*.xml templates. The old connection strings had two problems: - trailing slashes in the URIs, - a ';create=true' additional parameter, which seems to be a remnant of a Derby connection string. The previous driver accepted (and probably ignored) this, but the new one rejects any extension after the database name. To ensure that existing platforms are not broken, core tests were run: - in Docker for CentOS 6, CentOS 7 and Ubuntu 16.04 - on Amazon VMs for CentOS 6.4 and CentOS 7.4 - Ubuntu 18.04 was tested on a VM and in Docker as well. Change-Id: Ica5423c18a9f8346dda7dae617b1764638b57b6c --- M bin/create-test-configuration.sh M bin/impala-config.sh M fe/pom.xml M fe/src/test/resources/sentry-site.xml.template M fe/src/test/resources/sentry-site_no_oo.xml.template M fe/src/test/resources/sentry-site_oo.xml.template M fe/src/test/resources/sentry-site_oo_nogrant.xml.template M impala-parent/pom.xml 8 files changed, 14 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/94/12894/2 -- To view, visit http://gerrit.cloudera.org:8080/12894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ica5423c18a9f8346dda7dae617b1764638b57b6c Gerrit-Change-Number: 12894 Gerrit-PatchSet: 2 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Laszlo Gaal
[Impala-ASF-CR] IMPALA-8226: Add grant/revoke to/from group for Ranger
Austin Nobis has posted comments on this change. (
http://gerrit.cloudera.org:8080/12914 )
Change subject: IMPALA-8226: Add grant/revoke to/from group for Ranger
..
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12914/5/common/thrift/CatalogObjects.thrift
File common/thrift/CatalogObjects.thrift:
http://gerrit.cloudera.org:8080/#/c/12914/5/common/thrift/CatalogObjects.thrift@485
PS5, Line 485: // Represents a type of principal.
: enum TPrincipalType {
> nit: remove this comment i don't think this is specific to Sentry anymore.
Done
--
To view, visit http://gerrit.cloudera.org:8080/12914
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96
Gerrit-Change-Number: 12914
Gerrit-PatchSet: 6
Gerrit-Owner: Austin Nobis
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: radford nguyen
Gerrit-Comment-Date: Wed, 03 Apr 2019 21:14:48 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8383 Bump toolchain version
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12923 ) Change subject: IMPALA-8383 Bump toolchain version .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2626/ : 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/12923 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If221f001bc106530c5faf08b38385028c056983f Gerrit-Change-Number: 12923 Gerrit-PatchSet: 1 Gerrit-Owner: Hector Acosta Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 03 Apr 2019 21:07:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8226: Add grant/revoke to/from group for Ranger
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12914 ) Change subject: IMPALA-8226: Add grant/revoke to/from group for Ranger .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/12914/5/common/thrift/CatalogObjects.thrift File common/thrift/CatalogObjects.thrift: http://gerrit.cloudera.org:8080/#/c/12914/5/common/thrift/CatalogObjects.thrift@485 PS5, Line 485: // Represents a principal type that maps to Sentry principal type. : // https://github.com/apache/sentry/blob/3d062f39ce6a047138660a7b3d0024bde916c5b4/sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrincipalType.java nit: remove this comment i don't think this is specific to Sentry anymore. -- To view, visit http://gerrit.cloudera.org:8080/12914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96 Gerrit-Change-Number: 12914 Gerrit-PatchSet: 5 Gerrit-Owner: Austin Nobis Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Wed, 03 Apr 2019 20:54:24 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8226: Add grant/revoke to/from group for Ranger
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12914 ) Change subject: IMPALA-8226: Add grant/revoke to/from group for Ranger .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2625/ : 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/12914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96 Gerrit-Change-Number: 12914 Gerrit-PatchSet: 5 Gerrit-Owner: Austin Nobis Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Wed, 03 Apr 2019 20:54:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8383 Bump toolchain version
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12923 ) Change subject: IMPALA-8383 Bump toolchain version .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3975/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/12923 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If221f001bc106530c5faf08b38385028c056983f Gerrit-Change-Number: 12923 Gerrit-PatchSet: 1 Gerrit-Owner: Hector Acosta Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 03 Apr 2019 20:26:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8383 Bump toolchain version
Hector Acosta has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12923 Change subject: IMPALA-8383 Bump toolchain version .. IMPALA-8383 Bump toolchain version Thrift's python bindings in the current toolchain don't include the fastbinary shared object due to a bug in the native-toolchain code. A fix for that has been submitted to the native-toolchain and the newly generated toolchain builds should include fastbinary.so Testing: Compared thrift's tarballs using: diff <(curl -s $URL1| tar tzf - |sort) \ <(curl -s $URL2 | tar tzf -|sort) 147d146 < thrift-0.9.3-p5/python/lib/python2.7/site-packages/thrift/protocol/fastbinary.so Change-Id: If221f001bc106530c5faf08b38385028c056983f --- M bin/impala-config.sh 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/12923/1 -- To view, visit http://gerrit.cloudera.org:8080/12923 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: If221f001bc106530c5faf08b38385028c056983f Gerrit-Change-Number: 12923 Gerrit-PatchSet: 1 Gerrit-Owner: Hector Acosta
[Impala-ASF-CR] IMPALA-8226: Add grant/revoke to/from group for Ranger
Austin Nobis has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/12914 ) Change subject: IMPALA-8226: Add grant/revoke to/from group for Ranger .. IMPALA-8226: Add grant/revoke to/from group for Ranger This patch adds fupport for GRANT privilege statements to GROUP and REVOKE privilege statements from GROUP. The grammar has been updated to support FROM GROUP and TO GROUP for GRANT/REVOKE statements, i.e: GRANT ON TO GROUP REVOKE ON FROM GROUP Currently, only Ranger's authorization implementation supports GROUP based privileges. Sentry will throw an UnsupportedOperationException if it is the enabled authorization provider and this new grammar is used. Testing: - AuthorizationStmtTest was updated to also test for GROUP authorization. - ToSqlTest was updated to test for GROUP changes to the grammar. - A GROUP based E2E test was added to test_ranger.py - ParserTest was updated to test combinations for GrantRevokePrivilege - AnalyzeAuthStmtsTest was updated to test for USER and GROUP identities - Ran all FE tests - Ran authorization E2E tests Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96 --- M common/thrift/CatalogObjects.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/resources/ranger-hive-security.xml M testdata/bin/create-load-data.sh A testdata/cluster/ranger/setup/impala_group.json.template M testdata/cluster/ranger/setup/impala_user.json.template M tests/authorization/test_ranger.py 17 files changed, 467 insertions(+), 321 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/12914/5 -- To view, visit http://gerrit.cloudera.org:8080/12914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96 Gerrit-Change-Number: 12914 Gerrit-PatchSet: 5 Gerrit-Owner: Austin Nobis Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: radford nguyen
[Impala-ASF-CR] Bump Postgres JDBC driver version to 9.4, update connection strings
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12894 ) Change subject: Bump Postgres JDBC driver version to 9.4, update connection strings .. Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/12894/1/bin/impala-config.sh File bin/impala-config.sh: http://gerrit.cloudera.org:8080/#/c/12894/1/bin/impala-config.sh@567 PS1, Line 567: jre7 I'm curious why we chose the one with jre7 instead of the default one. http://gerrit.cloudera.org:8080/#/c/12894/1/fe/pom.xml File fe/pom.xml: http://gerrit.cloudera.org:8080/#/c/12894/1/fe/pom.xml@451 PS1, Line 451: nit: this comment is not necessary http://gerrit.cloudera.org:8080/#/c/12894/1/fe/pom.xml@453 PS1, Line 453: nit: use 2 spaces http://gerrit.cloudera.org:8080/#/c/12894/1/fe/pom.xml@455 PS1, Line 455: 9.4.1212.jre7 shouldn't we be getting it from the IMPALA_POSTGRES_JDBC_DRIVER_VERSION environment variable? -- To view, visit http://gerrit.cloudera.org:8080/12894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica5423c18a9f8346dda7dae617b1764638b57b6c Gerrit-Change-Number: 12894 Gerrit-PatchSet: 1 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Laszlo Gaal Gerrit-Comment-Date: Wed, 03 Apr 2019 19:55:09 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8226: Add grant/revoke to/from group for Ranger
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12914 ) Change subject: IMPALA-8226: Add grant/revoke to/from group for Ranger .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2624/ : 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/12914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96 Gerrit-Change-Number: 12914 Gerrit-PatchSet: 4 Gerrit-Owner: Austin Nobis Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Wed, 03 Apr 2019 19:34:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8226: Add grant/revoke to/from group for Ranger
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/12914 )
Change subject: IMPALA-8226: Add grant/revoke to/from group for Ranger
..
Patch Set 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/12914/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java:
http://gerrit.cloudera.org:8080/#/c/12914/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java@165
PS4, Line 165: String[] idents = {"myRole", "ROLE myRole", "GROUP myGroup",
"USER myUser"};
: boolean[] isGrantVals = {true, false};
do we have tests for bad idents?
http://gerrit.cloudera.org:8080/#/c/12914/4/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java:
http://gerrit.cloudera.org:8080/#/c/12914/4/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@3153
PS4, Line 3153: withPrincipals.add((isUser_) ? new WithRangerUser() : new
WithRangerGroup());
shouldn't we be running both user and ranger?
http://gerrit.cloudera.org:8080/#/c/12914/4/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
File fe/src/test/java/org/apache/impala/analysis/ParserTest.java:
http://gerrit.cloudera.org:8080/#/c/12914/4/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@3592
PS4, Line 3592: String[] resources = {"SERVER", "SERVER foo", "DATABASE
foo", "TABLE foo",
: "URI 'foo'"};
: String[] badResources = {"DATABASE", "TABLE", "URI", "URI
foo", "TABLE 'foo'",
: "SERVER 'foo'", "DATABASE 'foo'"};
: String[] privileges = {"SELECT", "INSERT", "ALL",
"REFRESH", "CREATE", "ALTER",
: "DROP"};
: String[] badPrivileges = {"UPDATE", "DELETE", "UPSERT",
"FAKE"};
: String[] columnPrivResource = {"SELECT (a, b) ON TABLE
foo", "SELECT () on TABLE foo",
: "INSERT (a, b) ON TABLE foo", "ALL (a, b) ON TABLE
foo"};
: String[] badColumnPrivResource = {"SELECT (a,) ON TABLE
foo",
: "SELECT (*) ON TABLE foo", "SELECT (a), b ON TABLE foo",
: "SELECT ((a)) ON TABLE foo", "SELECT (a, b) ON URI foo",
: "SELECT ON TABLE (a, b) foo",};
: String[][] grantRevoke = {{"GRANT", "TO"}, {"REVOKE",
"FROM"}};
: String[] idents = {"myRole", "GROUP myGroup", "USER user",
"ROLE myRole"};
: String[] badIdents = {"GROUP", "ROLE", "GROUP group",
"GROUP role", "USER role",
: "FOOBAR foobar", ""};
this is a bit hard to read, maybe put each element in a new line where it makes
sense?
--
To view, visit http://gerrit.cloudera.org:8080/12914
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96
Gerrit-Change-Number: 12914
Gerrit-PatchSet: 4
Gerrit-Owner: Austin Nobis
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: radford nguyen
Gerrit-Comment-Date: Wed, 03 Apr 2019 19:06:36 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6826: Extend bootstrap system.sh to Ubuntu 18.04
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12893 ) Change subject: IMPALA-6826: Extend bootstrap_system.sh to Ubuntu 18.04 .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12893 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iad790f72ea6b62258aed2225eb7bdf79590c350f Gerrit-Change-Number: 12893 Gerrit-PatchSet: 4 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Laszlo Gaal Gerrit-Comment-Date: Wed, 03 Apr 2019 19:01:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6826: Extend bootstrap system.sh to Ubuntu 18.04
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12893 ) Change subject: IMPALA-6826: Extend bootstrap_system.sh to Ubuntu 18.04 .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3974/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12893 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iad790f72ea6b62258aed2225eb7bdf79590c350f Gerrit-Change-Number: 12893 Gerrit-PatchSet: 4 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Laszlo Gaal Gerrit-Comment-Date: Wed, 03 Apr 2019 19:01:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8226: Add grant/revoke to/from group for Ranger
Austin Nobis has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/12914 ) Change subject: IMPALA-8226: Add grant/revoke to/from group for Ranger .. IMPALA-8226: Add grant/revoke to/from group for Ranger This patch adds fupport for GRANT privilege statements to GROUP and REVOKE privilege statements from GROUP. The grammar has been updated to support FROM GROUP and TO GROUP for GRANT/REVOKE statements, i.e: GRANT ON TO GROUP REVOKE ON FROM GROUP Currently, only Ranger's authorization implementation supports GROUP based privileges. Sentry will throw an UnsupportedOperationException if it is the enabled authorization provider and this new grammar is used. Testing: - AuthorizationStmtTest was updated to also test for GROUP authorization. - ToSqlTest was updated to test for GROUP changes to the grammar. - A GROUP based E2E test was added to test_ranger.py - ParserTest was updated to test combinations for GrantRevokePrivilege - AnalyzeAuthStmtsTest was updated to test for USER and GROUP identities - Ran all FE tests - Ran authorization E2E tests Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96 --- M common/thrift/CatalogObjects.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/resources/ranger-hive-security.xml M testdata/bin/create-load-data.sh A testdata/cluster/ranger/setup/impala_group.json.template M testdata/cluster/ranger/setup/impala_user.json.template M tests/authorization/test_ranger.py 17 files changed, 470 insertions(+), 325 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/12914/4 -- To view, visit http://gerrit.cloudera.org:8080/12914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96 Gerrit-Change-Number: 12914 Gerrit-PatchSet: 4 Gerrit-Owner: Austin Nobis Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: radford nguyen
[Impala-ASF-CR] IMPALA-7971: Add support for insert events in event processor.
Vihang Karajgaonkar has posted comments on this change. (
http://gerrit.cloudera.org:8080/12889 )
Change subject: IMPALA-7971: Add support for insert events in event processor.
..
Patch Set 5:
(15 comments)
http://gerrit.cloudera.org:8080/#/c/12889/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12889/5//COMMIT_MSG@14
PS5, Line 14: Also, renamed
: TableInvalidatingEvent class to TableInvalidatingOrRefreshingEvent
: to reflect new behaviour.
This may not be applicable anymore
http://gerrit.cloudera.org:8080/#/c/12889/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:
http://gerrit.cloudera.org:8080/#/c/12889/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@413
PS5, Line 413: getOrLoadTable
I think this call is not correct since this will be a no-op if the table is
loaded already. You should probably use reloadTable() method here to force the
refresh.
I also see that there is a reloadPartition() method in CatalogServiceCatalog.
Can we use that method to refresh only the Partition when the insert_event is
for a given partition in case of partitioned tables.
http://gerrit.cloudera.org:8080/#/c/12889/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/12889/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3575
PS4, Line 3575: // Attempt to remove this partition name from from
partsToCreate. If remove
> This is a good suggestion for partitioned tables. However, for unpartitione
I see. Thanks for that info. using nulls are keys is error-prone since the
methods where this collection is passed need to be aware of this to avoid NPE.
Its better to just use a separate collection like you seemed to have started
doing that by having filesForUnpartitionedTable before.
http://gerrit.cloudera.org:8080/#/c/12889/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/12889/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@38
PS5, Line 38: import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
unused?
http://gerrit.cloudera.org:8080/#/c/12889/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3557
PS5, Line 3557: existingFilesForInsertedPartitions
may be a better name would be to suggest that this contains files before
insert.. so may something like filesBeforeInsertForPartitions?
http://gerrit.cloudera.org:8080/#/c/12889/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3558
PS5, Line 3558: filesForUnpartitionedTable
is this unused?
http://gerrit.cloudera.org:8080/#/c/12889/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3580
PS5, Line 3580: HdfsPartition) partition)
Is there a concern here of running into CastException? I see that FeFsPartition
has two implementations HdfsPartition and LocalFsPartition. Do we need to
handle LocalFsPartition as well?
Also, can we do this only when event processing is enabled?
http://gerrit.cloudera.org:8080/#/c/12889/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3673
PS5, Line 3673: else {
may be do a else if(catalog.isExternalEventProcessingEnabled()) here so that
this code is only triggered when event processing is enabled.
http://gerrit.cloudera.org:8080/#/c/12889/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3674
PS5, Line 3674: FeFsPartition singlePart =
Iterables.getOnlyElement((List) parts);
May be add a Preconditions.checkState(parts.size == 1); here to make sure that
the assumption in the code below are always true
http://gerrit.cloudera.org:8080/#/c/12889/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3676
PS5, Line 3676: (HdfsPartition) singlePart
same as above, Do we need to handle LocalFsPartition as well?
http://gerrit.cloudera.org:8080/#/c/12889/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3699
PS5, Line 3699: createInsertEvent
May be you can create 2 methods, one for partitioned case and another for
non-partitioned case. The former will take in Map> as an
argument for filesBeforeInsertInPartitions and the later will take Set
as an argument for filesBeforeInsertInTable
you can then call them using the
if (table is partitioned)
createInsertEventForTable()
else
createInsertEventsForPartitions()
http://gerrit.cloudera.org:8080/#/c/12889/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3716
PS5, Line 3716:* fireInsertEvent() if external event processing is enabled.
Add to the description that this method is a no-op if event processing is
d
[Impala-ASF-CR] IMPALA-8382 Add support for SLES 12 SP3
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12922 ) Change subject: IMPALA-8382 Add support for SLES 12 SP3 .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2623/ : 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/12922 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia3cb1311b15226f1130be7e1d79110d16e3287ef Gerrit-Change-Number: 12922 Gerrit-PatchSet: 1 Gerrit-Owner: Hector Acosta Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 03 Apr 2019 17:51:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8382 Add support for SLES 12 SP3
Hector Acosta has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12922 Change subject: IMPALA-8382 Add support for SLES 12 SP3 .. IMPALA-8382 Add support for SLES 12 SP3 Change-Id: Ia3cb1311b15226f1130be7e1d79110d16e3287ef Testing: Ran a build, reployed a cluster on sles 12 sp3. --- M bin/bootstrap_toolchain.py 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/12922/1 -- To view, visit http://gerrit.cloudera.org:8080/12922 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ia3cb1311b15226f1130be7e1d79110d16e3287ef Gerrit-Change-Number: 12922 Gerrit-PatchSet: 1 Gerrit-Owner: Hector Acosta
[Impala-ASF-CR] IMPALA-8143: Enhance DoRpcWithRetry().
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12672 ) Change subject: IMPALA-8143: Enhance DoRpcWithRetry(). .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2622/ : 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/12672 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia9693151c35e02235665b3c285a48c585973d390 Gerrit-Change-Number: 12672 Gerrit-PatchSet: 9 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 03 Apr 2019 16:55:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8359: Fix coverage data generation for impalads
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12858 ) Change subject: IMPALA-8359: Fix coverage data generation for impalads .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2621/ : 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/12858 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9be1e1e73b6cfc3557077f763aee4dbfcc7a2d27 Gerrit-Change-Number: 12858 Gerrit-PatchSet: 4 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 03 Apr 2019 16:35:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8143: Enhance DoRpcWithRetry().
Andrew Sherman has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/12672 ) Change subject: IMPALA-8143: Enhance DoRpcWithRetry(). .. IMPALA-8143: Enhance DoRpcWithRetry(). Allow callers of RpcMgr::DoRpcWithRetry to specify a time to sleep if the remote service is busy. DoRpcWithRetry now only sleeps if the remote service is busy. TESTING: Ran all end-to-end tests. Add a new test to rpc-mgr-test.cc which tests RpcMgr::DoRpcWithRetry in two ways. Firstly we test the case where the remote service is busy (which we force by filling up the queue), and secondly we exercise DoRpcWithRetry by using a fake Proxy that simulates failures. Clean up unused values of FaultInjectionUtil.RpcCallType and match values with test_rpc_timeout.py. Change-Id: Ia9693151c35e02235665b3c285a48c585973d390 --- M be/src/rpc/impala-service-pool.cc M be/src/rpc/impala-service-pool.h M be/src/rpc/rpc-mgr-test.cc M be/src/rpc/rpc-mgr-test.h M be/src/rpc/rpc-mgr.h M be/src/rpc/rpc-mgr.inline.h M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/query-state.h M be/src/service/client-request-state.cc M be/src/service/control-service.cc M be/src/service/control-service.h M be/src/testutil/fault-injection-util.h M tests/custom_cluster/test_rpc_timeout.py 13 files changed, 274 insertions(+), 62 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/12672/9 -- To view, visit http://gerrit.cloudera.org:8080/12672 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia9693151c35e02235665b3c285a48c585973d390 Gerrit-Change-Number: 12672 Gerrit-PatchSet: 9 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8226: Add grant/revoke to/from group for Ranger
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12914 ) Change subject: IMPALA-8226: Add grant/revoke to/from group for Ranger .. Patch Set 3: (2 comments) Can you add tests in ParserTest and AnalyzeAuthStmtTest? http://gerrit.cloudera.org:8080/#/c/12914/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java: http://gerrit.cloudera.org:8080/#/c/12914/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@3002 PS3, Line 3002: private class WithRanger implements WithPrincipal Instead of having a boolean flag, let's use WithRangerGroup and rename this class with WithRangerUser. It think it's much cleaner. http://gerrit.cloudera.org:8080/#/c/12914/3/tests/authorization/test_ranger.py File tests/authorization/test_ranger.py: http://gerrit.cloudera.org:8080/#/c/12914/3/tests/authorization/test_ranger.py@68 PS3, Line 68: time.sleep(35) since you changed the polling interval to 5 seconds, we no longer need to wait that long. -- To view, visit http://gerrit.cloudera.org:8080/12914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96 Gerrit-Change-Number: 12914 Gerrit-PatchSet: 3 Gerrit-Owner: Austin Nobis Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Wed, 03 Apr 2019 15:58:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8359: Fix coverage data generation for impalads
Hello Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12858 to look at the new patch set (#4). Change subject: IMPALA-8359: Fix coverage data generation for impalads .. IMPALA-8359: Fix coverage data generation for impalads impala::InitCommonRuntime() sets a signal handler for SIGTERM. It calls _exit(0) which causes normal program termination without cleaning up, i.e. no destructors are called etc. Gcov writes the coverage data in this cleanup phase, so calling _exit() prevents flushing coverage data. Now the '-codecoverage' flag also defines a macro named CODE_COVERAGE_ENABLED. If this macro is defined we explicitly call __gcov_flush() before calling _exit(). I tested manually. Change-Id: I9be1e1e73b6cfc3557077f763aee4dbfcc7a2d27 --- M be/CMakeLists.txt M be/src/common/init.cc 2 files changed, 17 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/12858/4 -- To view, visit http://gerrit.cloudera.org:8080/12858 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9be1e1e73b6cfc3557077f763aee4dbfcc7a2d27 Gerrit-Change-Number: 12858 Gerrit-PatchSet: 4 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8359: Fix coverage data generation for impalads
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12858 ) Change subject: IMPALA-8359: Fix coverage data generation for impalads .. Patch Set 3: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/2620/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/12858 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9be1e1e73b6cfc3557077f763aee4dbfcc7a2d27 Gerrit-Change-Number: 12858 Gerrit-PatchSet: 3 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 03 Apr 2019 15:27:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5843: Use page index in Parquet files to skip pages
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/12065 ) Change subject: IMPALA-5843: Use page index in Parquet files to skip pages .. Patch Set 8: Code-Review+1 (12 comments) I have only minor comments, mainly about naming and line breaking. http://gerrit.cloudera.org:8080/#/c/12065/7/be/src/exec/parquet/hdfs-parquet-scanner.cc File be/src/exec/parquet/hdfs-parquet-scanner.cc: http://gerrit.cloudera.org:8080/#/c/12065/7/be/src/exec/parquet/hdfs-parquet-scanner.cc@119 PS7, Line 119: num_stats_filtered_row_groups_by_page_index_counter_ = > Good idea! Added counter for it. I am not sure how to track this, but it is also possible that some columns have page indexes, while some do not. For example Parquet-mr does not write stats for int96 timestamps, and I think that it is also not written for double/float if there is a NaN value. http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/hdfs-parquet-scanner.cc File be/src/exec/parquet/hdfs-parquet-scanner.cc: http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/hdfs-parquet-scanner.cc@117 PS8, Line 117: NumStatsFilteredRowGroups This name is not precise anymore, as it only means row groups filtered by column index. I am not sure how we should handle this, as keeping the old name is probably useful for compatibility reasons. A possibility is to count both cases in this stat, and have two separate stats for the two reasons. http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/hdfs-parquet-scanner.cc@120 PS8, Line 120: NumStatsFilteredRowGroupsByPageIndex Maybe "completely" could be added to the name to make its meaning more exact. The fact that row groups that survived column chunk stats can still be completely eliminated by page filtering may not be obvious for the reader at first, and the current name could be interpreted as the number of row groups where page filtering was applied. http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/hdfs-parquet-scanner.cc@517 PS8, Line 517: const parquet::ColumnOrder* col_order = col_idx < col_orders.size() ? : &col_orders[col_idx] : nullptr; nit: I prefer breaking after = http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/hdfs-parquet-scanner.cc@525 PS8, Line 525: ColumnStatsReader stat_reader = CreateColumnStatsReader(col_chunk, col_type, nit: I prefer breaking after = http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/hdfs-parquet-scanner.cc@752 PS8, Line 752: = nit: I prefer breaking after = http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/hdfs-parquet-scanner.cc@755 PS8, Line 755: col_order, *node->element); nit: I prefer breaking after = http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/hdfs-parquet-scanner.cc@773 PS8, Line 773: skip_ranges.push_back(GetRowRangeForPage(row_group, scalar_reader->offset_index_, nit: I would prefer breaking before GetRowRangeForPage http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/parquet-column-readers.h File be/src/exec/parquet/parquet-column-readers.h: http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/parquet-column-readers.h@503 PS8, Line 503: IsLastCandidateRange I think that adding "InRowGroup" to the name would make the call site clearer. http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/parquet-column-readers.h@526 PS8, Line 526: CandidateRowsRemainingInCurrentPage The name mislead me a bit, I have assumed that it returns an integer. Maybe something like PageHasRemainingCandidateRows would be better? http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/parquet-column-readers.cc File be/src/exec/parquet/parquet-column-readers.cc: http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/parquet-column-readers.cc@1732 PS8, Line 1732: num_values_read_ -= num_buffered_values_; This seems to be the only place where num_values_read_ is decreased, so it looks like num_values_read_ = total_number_of_values_in_row_group - number_of_values_in_completely_skipped_pages - number_of_values_skipped_at_the_end_of_pages. It seems that we only use this variable in cases that are irrelevant with PageFiltering(), so I would remove this subtraction, and add some comments to num_values_read_ about its role if there is no page filtering, and its irrelevance if there is page filtering. http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/parquet-common.h File be/src/exec/parquet/parquet-common.h: http://gerrit.cloudera.org:8080/#/c/12065/8/be/src/exec/parquet/parquet-common.h@329 PS8, Line 329: str_len += sizeof(int32_t); : if (UNLIKELY(str_len < 0 || buffer_end - buffer < str_len)) return -1; I am not sure if this matters but str_len of -4 is accepted as OK. -- To view, visit http://gerrit.cloudera.org:8080/12065 To
[Impala-ASF-CR] IMPALA-8359: Fix coverage data generation for impalads
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12858 ) Change subject: IMPALA-8359: Fix coverage data generation for impalads .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/12858/2/be/src/common/init.cc File be/src/common/init.cc: http://gerrit.cloudera.org:8080/#/c/12858/2/be/src/common/init.cc@199 PS2, Line 199: sys_write(STDOUT_FILENO, msg, strlen(msg)); : #if CODE_COVERAGE_ENABLED : // On so > Yeah, I knew about __gcov_flush() and tried it out already, but it only gen In an offline discussion we agreed that calling __gcov_flush() is the proper way. Unfortunately __gcov_flush() only flushes a small fraction of the coverage data on some systems, so I added a comment about the workaround for users who run into this issue. -- To view, visit http://gerrit.cloudera.org:8080/12858 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9be1e1e73b6cfc3557077f763aee4dbfcc7a2d27 Gerrit-Change-Number: 12858 Gerrit-PatchSet: 3 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 03 Apr 2019 14:46:16 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8359: Fix coverage data generation for impalads
Hello Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12858 to look at the new patch set (#3). Change subject: IMPALA-8359: Fix coverage data generation for impalads .. IMPALA-8359: Fix coverage data generation for impalads impala::InitCommonRuntime() sets a signal handler for SIGTERM. It calls _exit(0) which causes normal program termination without cleaning up, i.e. no destructors are called etc. Gcov writes the coverage data in this cleanup phase, so calling _exit() prevents flushing coverage data. Now the '-codecoverage' flag also defines a macro named CODE_COVERAGE_ENABLED. If this macro is defined we explicitly call __gcov_flush() before calling _exit(). I tested manually. Change-Id: I9be1e1e73b6cfc3557077f763aee4dbfcc7a2d27 --- M be/CMakeLists.txt M be/src/common/init.cc 2 files changed, 19 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/12858/3 -- To view, visit http://gerrit.cloudera.org:8080/12858 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9be1e1e73b6cfc3557077f763aee4dbfcc7a2d27 Gerrit-Change-Number: 12858 Gerrit-PatchSet: 3 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Zoltan Borok-Nagy
