[Impala-ASF-CR] IMPALA-8974: Fixed a bug when create kudu managed table without HMS config
wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/14398 ) Change subject: IMPALA-8974: Fixed a bug when create kudu managed table without HMS config .. Patch Set 19: > (6 comments) > > I forgot to get back to this review. Thanks for cleaning up the > test. I have a few style nits about the test, then I'm OK to +1. > > Would be good if quanlong could also confirm that you addressed his > comments Hi Tim, thanks for review again, I've already adjusted the above issues, and submit fe tests on Jenkins. I also reply quanlong about his comments, waiting for his review! @Quanlong Huang -- To view, visit http://gerrit.cloudera.org:8080/14398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iacc53801a660c033869cb4747910c98a80e08297 Gerrit-Change-Number: 14398 Gerrit-PatchSet: 19 Gerrit-Owner: wangsheng Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Wed, 18 Dec 2019 09:22:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9195: Using multithreaded execution to accelerate 'show tables/databases'
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/14846 ) Change subject: IMPALA-9195: Using multithreaded execution to accelerate 'show tables/databases' .. Patch Set 9: (3 comments) http://gerrit.cloudera.org:8080/#/c/14846/7/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/14846/7/fe/src/main/java/org/apache/impala/service/Frontend.java@859 PS7, Line 859: d(dbNa > Using MoreExecutors.sameThreadExecutor seems much better. About tests: yes, tests/authorization/test_authorization.py is a good place to add a test for this new flag. All the tests there are "custom cluster tests", so they will restart the impala deamons with flags specified in "impalad_args". http://gerrit.cloudera.org:8080/#/c/14846/9/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/14846/9/fe/src/main/java/org/apache/impala/service/Frontend.java@866 PS9, Line 866: new CheckAuthorization(dbName, tblName, tableOwner, user))); nit: +2 indentation http://gerrit.cloudera.org:8080/#/c/14846/9/fe/src/main/java/org/apache/impala/service/Frontend.java@997 PS9, Line 997: new CheckAuthorization(db.getName(), null, db.getOwnerUser(), user))); nit: +2 indentation -- To view, visit http://gerrit.cloudera.org:8080/14846 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I860e0d18afa0421665f8b3b1c5561d6bdacc5e96 Gerrit-Change-Number: 14846 Gerrit-PatchSet: 9 Gerrit-Owner: Zhou Xu Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Zhou Xu Gerrit-Comment-Date: Wed, 18 Dec 2019 13:35:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9009: Core support for Ranger column masking
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/14894 ) Change subject: IMPALA-9009: Core support for Ranger column masking .. Patch Set 7: (11 comments) Some initial comments, I still didn't digest thing 100%. http://gerrit.cloudera.org:8080/#/c/14894/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14894/7//COMMIT_MSG@10 PS7, Line 10: to specific users when reading specific columns. This patch add support nit: adds http://gerrit.cloudera.org:8080/#/c/14894/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: http://gerrit.cloudera.org:8080/#/c/14894/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@784 PS7, Line 784: return new CollectionTableRef(tableRef, resolvedPath); Can we mask values inside collections? http://gerrit.cloudera.org:8080/#/c/14894/6/fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java File fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java: http://gerrit.cloudera.org:8080/#/c/14894/6/fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java@153 PS6, Line 153: in typo: if? http://gerrit.cloudera.org:8080/#/c/14894/7/fe/src/main/java/org/apache/impala/authorization/TableMask.java File fe/src/main/java/org/apache/impala/authorization/TableMask.java: http://gerrit.cloudera.org:8080/#/c/14894/7/fe/src/main/java/org/apache/impala/authorization/TableMask.java@67 PS7, Line 67: getColumnMask nit, naming: I think that this function does too many things for a "get" function, I would rename it to createColumnMask or something similar http://gerrit.cloudera.org:8080/#/c/14894/7/fe/src/main/java/org/apache/impala/authorization/TableMask.java@69 PS7, Line 69: getColumnMask same as line 67 http://gerrit.cloudera.org:8080/#/c/14894/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java: http://gerrit.cloudera.org:8080/#/c/14894/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@278 PS7, Line 278: result nit, naming: result suggests to me that the function will return this variable http://gerrit.cloudera.org:8080/#/c/14894/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@297 PS7, Line 297: else if Is it ok to just pass through and return the original column? Shouldn't we return in line 280 in that case? http://gerrit.cloudera.org:8080/#/c/14894/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@298 PS7, Line 298: maskedColumn This branch seems a bit weird to me - what is the difference between a custom mask and a transformer? http://gerrit.cloudera.org:8080/#/c/14894/7/tests/authorization/test_ranger.py File tests/authorization/test_ranger.py: http://gerrit.cloudera.org:8080/#/c/14894/7/tests/authorization/test_ranger.py@605 PS7, Line 605: "{0}/service/public/v2/api/policy?servicename=test_impala&policyname={1}".format( nit, here and at many other places: please try to use +4 indentation for broken lines http://gerrit.cloudera.org:8080/#/c/14894/7/tests/authorization/test_ranger.py@796 PS7, Line 796: don nit: doesn't http://gerrit.cloudera.org:8080/#/c/14894/7/tests/authorization/test_ranger.py@832 PS7, Line 832: self.try_execute_query(admin_client, "drop table if exists functional.masked_tbl") : self.try_execute_query(admin_client, "drop view if exists functional.masked_view") I would prefer to create a unique database for these to avoid the possibility of making functional "dirty". -- To view, visit http://gerrit.cloudera.org:8080/14894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4cad60e0e69ea573b7ecfc011b142c46ef52ed61 Gerrit-Change-Number: 14894 Gerrit-PatchSet: 7 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Wed, 18 Dec 2019 16:33:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Add --impalad args to single node perf run.py
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14923 Change subject: Add --impalad_args to single_node_perf_run.py .. Add --impalad_args to single_node_perf_run.py This is useful for benchmarking non-standard configurations, e.g. with mt_dop enabled. Testing: Ran the script, confirmed manually that the arguments took effect. single_node_perf_run.py \ --impalad_args=--default_query_options=mt_dop=4 \ --impalad_args=--unlock_mt_dop=true Change-Id: Ib903f0eabb06a7e8981c874c8fe1cec0936b1a64 --- M bin/single_node_perf_run.py 1 file changed, 8 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/14923/1 -- To view, visit http://gerrit.cloudera.org:8080/14923 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ib903f0eabb06a7e8981c874c8fe1cec0936b1a64 Gerrit-Change-Number: 14923 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] IMPALA-9197: make HashTable lookups thread-safe
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/14917 ) Change subject: IMPALA-9197: make HashTable lookups thread-safe .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/14917/4/be/src/exec/hash-table.h File be/src/exec/hash-table.h: http://gerrit.cloudera.org:8080/#/c/14917/4/be/src/exec/hash-table.h@71 PS4, Line 71: /// Operations that mutate the hash table are thread-safe. Read-only access to the hash Maybe "not" is missing here? http://gerrit.cloudera.org:8080/#/c/14917/4/be/src/exec/hash-table.cc File be/src/exec/hash-table.cc: http://gerrit.cloudera.org:8080/#/c/14917/4/be/src/exec/hash-table.cc@164 PS4, Line 164: / Shouldn't we handle division by zero here? -- To view, visit http://gerrit.cloudera.org:8080/14917 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I92fbfa8cc000477b8e01975a102d818f9fa27c61 Gerrit-Change-Number: 14917 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 18 Dec 2019 17:32:12 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Add --impalad args to single node perf run.py
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14923 ) Change subject: Add --impalad_args to single_node_perf_run.py .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5307/ : 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/14923 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib903f0eabb06a7e8981c874c8fe1cec0936b1a64 Gerrit-Change-Number: 14923 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Comment-Date: Wed, 18 Dec 2019 17:34:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9257: Last event id should be advanced if all events are skipped
Vihang Karajgaonkar has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14916 ) Change subject: IMPALA-9257: Last event id should be advanced if all events are skipped .. IMPALA-9257: Last event id should be advanced if all events are skipped Events processor implements a filtering method which skips certain unnecessary events (eg. events on blacklisted dbs and tables). However, if the received batch has all the events which are filtered out, it fails to update its lastSyncedEventId. This causes unnecessary logs being printed in catalog and the same event batch being fetched repeatedly. Testing: Modified existing test to compare event id after events on blacklisted dbs and tables. Change-Id: I7f94c1a8e8c221f504262d5591cda8c3a25c0c32 Reviewed-on: http://gerrit.cloudera.org:8080/14916 Reviewed-by: Anurag Mantripragada Reviewed-by: Quanlong Huang Tested-by: Impala Public Jenkins --- M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java M tests/custom_cluster/test_event_processing.py 2 files changed, 9 insertions(+), 3 deletions(-) Approvals: Anurag Mantripragada: Looks good to me, but someone else must approve Quanlong Huang: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/14916 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I7f94c1a8e8c221f504262d5591cda8c3a25c0c32 Gerrit-Change-Number: 14916 Gerrit-PatchSet: 3 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Fang-Yu Rao has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/14798 ) Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests .. IMPALA-9149: part 1: Re-enabe Ranger-related FE tests In IMPALA-9047, we disabled some Ranger-related FE and BE tests due to changes in Ranger's behavior after upgrading Ranger from 1.2 to 2.0. This patch aims to re-enable those disabled FE tests in AuthorizationStmtTest.java and RangerAuditLogTest.java to increase Impala's test coverage of authorization via Ranger. There are at least two major changes in Ranger's behavior in the newer versions. 1. The first is that the owner of the requested resource no longer has to be explicitly granted privileges in order to access the resource. 2. The second is that a user not explicitly granted the privilege of creating a database is able to do so. Due to these changes, some of previous Ranger authorization requests that were expected to be rejected are now granted after the upgrade. To re-enable the tests affected by the first change described above, we modify AuthorizationTestBase.java to allow our FE Ranger authorization tests to specify the requesting user in an authorization test. Those tests failed after the upgrade because the default requesting user in Impala's AuthorizationTestBase.java happens to be the owner of the resources involved in our FE authorization tests. After this patch, a requesting user can be either a non-owner user or an owner user in a Ranger authorization test and the requesting user would correspond to a non-owner user if it is not explicitly specified. Note that in a Sentry authorization test, we do not use the non-owner user as the requesting user by default as we do in the Ranger authorization tests. Instead, we set the name of the requesting user to a name that is the same as the owner user in Ranger authorization tests to avoid the need for providing a customized group mapping service when instantiating a Sentry ResourceAuthorizationProvider as we do in AuthorizationTest.java, our FE tests specifically for testing authorization via Sentry. On the other hand, to re-enable the tests affected by the second change, we remove from the Ranger policy for all databases the allowed codition that grants any user the privilege of creating a database, which is not by default granted by Sentry. After the removal of the allowed codition, those tests in AuthorizationStmtTest.java and RangerAuditLogTest.java affected by the second change now result in the same authorization errors before the upgrade of Ranger. Testing: - Passed AuthorizationStmtTest.java - Passed RangerAuditLogTest.java Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2 --- M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java M fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java M fe/src/test/java/org/apache/impala/common/FrontendFixture.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M testdata/bin/create-load-data.sh A testdata/cluster/ranger/setup/impala_group_non_owner.json R testdata/cluster/ranger/setup/impala_group_owner.json.template A testdata/cluster/ranger/setup/impala_user_non_owner.json.template R testdata/cluster/ranger/setup/impala_user_owner.json.template A testdata/cluster/ranger/setup/policy_4_revised.json 12 files changed, 395 insertions(+), 212 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/14798/11 -- To view, visit http://gerrit.cloudera.org:8080/14798 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2 Gerrit-Change-Number: 14798 Gerrit-PatchSet: 11 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR] Add --impalad args to single node perf run.py
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14923 ) Change subject: Add --impalad_args to single_node_perf_run.py .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5357/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/14923 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib903f0eabb06a7e8981c874c8fe1cec0936b1a64 Gerrit-Change-Number: 14923 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Comment-Date: Wed, 18 Dec 2019 18:21:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 ) Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5308/ : 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/14798 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2 Gerrit-Change-Number: 14798 Gerrit-PatchSet: 11 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Wed, 18 Dec 2019 18:51:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9240: add HTTP code handling to THttpClient.
Andrew Sherman has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14924 Change subject: IMPALA-9240: add HTTP code handling to THttpClient. .. IMPALA-9240: add HTTP code handling to THttpClient. Before this change Impala Shell is not checking HTTP return codes when using the hs2-http protocol. The shell is sending a request message (e.g. send_CloseOperation) but the HTTP call to send this message may fail. This will result in a failure when reading the reply (e.g. in recv_CloseOperation) as there is no reply data to read. This will typically result in an 'EOFError'. In THttpClient.flush() check the HTTP code that is returned after the HTTP call is made. If the code is not 1XX (informational response) or 2XX (successful) then throw an RPCException. This change does not contain any attempt to recover from an HTTP failures but it does allow the failure to be detected and a message to be printed. In future it may be possible to retry after certain HTTP errors. Testing: - Add a new test for impala-shell that tries to connect to an HTTP server that always returns a 503 error. Check that an appropriate error message is printed. Change-Id: I3c105f4b8237b87695324d75981821c08c43 --- M shell/impala_client.py M tests/shell/test_shell_interactive.py 2 files changed, 62 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/24/14924/1 -- To view, visit http://gerrit.cloudera.org:8080/14924 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I3c105f4b8237b87695324d75981821c08c43 Gerrit-Change-Number: 14924 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] Fix bug in report benchmark results.py
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14925 Change subject: Fix bug in report_benchmark_results.py .. Fix bug in report_benchmark_results.py This was introduced by IMPALA-4618. Exception was: Traceback (most recent call last): File "tests/benchmark/report_benchmark_results.py", line 1135, in report = Report(grouped, ref_grouped) File "tests/benchmark/report_benchmark_results.py", line 494, in __init__ self.__analyze() File "tests/benchmark/report_benchmark_results.py", line 517, in __analyze query_variability_row = Report.QueryVariabilityRow(results, ref_results) File "tests/benchmark/report_benchmark_results.py", line 480, in __init__ results, ref_results, for_variability = True) File "tests/benchmark/report_benchmark_results.py", line 1095, in build_exec_summary_str return str(comparison) + '\n' File "tests/benchmark/report_benchmark_results.py", line 844, in __str__ return str(self.__build_table_variability()) File "tests/benchmark/report_benchmark_results.py", line 891, in __build_table_variability output += str(self.combined_summary) + '\n' File "tests/benchmark/report_benchmark_results.py", line 713, in __str__ table.add_row(table_row) File "/home/tarmstrong/Impala/incubator-impala/infra/python/env/local/lib/python2.7/site-packages/prettytable.py", line 818, in add_row raise Exception("Row has incorrect number of values, (actual) %d!=%d (expected)" %(len(row),len(self._field_names))) Exception: Row has incorrect number of values, (actual) 8!=7 (expected) Change-Id: I6a163e74fefc896464e35a6a1b91ce57de592f1a --- M tests/benchmark/report_benchmark_results.py 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/25/14925/1 -- To view, visit http://gerrit.cloudera.org:8080/14925 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I6a163e74fefc896464e35a6a1b91ce57de592f1a Gerrit-Change-Number: 14925 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] IMPALA-9240: add HTTP code handling to THttpClient.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14924 ) Change subject: IMPALA-9240: add HTTP code handling to THttpClient. .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5309/ : 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/14924 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3c105f4b8237b87695324d75981821c08c43 Gerrit-Change-Number: 14924 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 18 Dec 2019 19:43:04 + Gerrit-HasComments: No
[Impala-ASF-CR] Fix bug in report benchmark results.py
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14925 ) Change subject: Fix bug in report_benchmark_results.py .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5310/ : 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/14925 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6a163e74fefc896464e35a6a1b91ce57de592f1a Gerrit-Change-Number: 14925 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Comment-Date: Wed, 18 Dec 2019 20:05:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9197: make HashTable lookups thread-safe
Hello Csaba Ringhofer, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14917 to look at the new patch set (#5). Change subject: IMPALA-9197: make HashTable lookups thread-safe .. IMPALA-9197: make HashTable lookups thread-safe This makes it possible to do HashTable lookups from multiple threads without any data races. This requires moving statistics that are updated during probing to the HashTableCtx object. There are some small changes to the hash table stat logging behaviour as a result of the stats being moved to the context. I don't believe these logs are used much, if at all. Testing: Ran exhaustive tests. Manually inspected some aggregation and join query profiles to check that hash table stats looked reasonable. Perf: Ran TPC-H scale factor 30 on a single node. No significant change in perf. Change-Id: I92fbfa8cc000477b8e01975a102d818f9fa27c61 --- M be/src/exec/grouping-aggregator.cc M be/src/exec/grouping-aggregator.h M be/src/exec/hash-table.cc M be/src/exec/hash-table.h M be/src/exec/hash-table.inline.h M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/partitioned-hash-join-builder.h M be/src/exec/partitioned-hash-join-node.cc 8 files changed, 135 insertions(+), 71 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/17/14917/5 -- To view, visit http://gerrit.cloudera.org:8080/14917 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I92fbfa8cc000477b8e01975a102d818f9fa27c61 Gerrit-Change-Number: 14917 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-9197: make HashTable lookups thread-safe
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14917 ) Change subject: IMPALA-9197: make HashTable lookups thread-safe .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/14917/5/be/src/exec/hash-table.cc File be/src/exec/hash-table.cc: http://gerrit.cloudera.org:8080/#/c/14917/5/be/src/exec/hash-table.cc@595 PS5, Line 595: double curr_fill_factor = num_buckets_ == 0 ? 0 : (double)num_filled_buckets_/(double)num_buckets_; line too long (101 > 90) -- To view, visit http://gerrit.cloudera.org:8080/14917 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I92fbfa8cc000477b8e01975a102d818f9fa27c61 Gerrit-Change-Number: 14917 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 18 Dec 2019 20:14:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9197: make HashTable lookups thread-safe
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14917 ) Change subject: IMPALA-9197: make HashTable lookups thread-safe .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/14917/4/be/src/exec/hash-table.h File be/src/exec/hash-table.h: http://gerrit.cloudera.org:8080/#/c/14917/4/be/src/exec/hash-table.h@71 PS4, Line 71: /// Operations that mutate the hash table are thread-safe. Read-only access to the hash > Maybe "not" is missing here? Done http://gerrit.cloudera.org:8080/#/c/14917/4/be/src/exec/hash-table.cc File be/src/exec/hash-table.cc: http://gerrit.cloudera.org:8080/#/c/14917/4/be/src/exec/hash-table.cc@164 PS4, Line 164: / > Shouldn't we handle division by zero here? It's floating point so you end up with inf. It's subtle though, so I added special handling for that case. https://gist.github.com/timarmstrong/ad5096ce220ba86313e32f73aecef6b6 -- To view, visit http://gerrit.cloudera.org:8080/14917 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I92fbfa8cc000477b8e01975a102d818f9fa27c61 Gerrit-Change-Number: 14917 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 18 Dec 2019 20:14:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9197: make HashTable lookups thread-safe
Hello Csaba Ringhofer, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14917 to look at the new patch set (#6). Change subject: IMPALA-9197: make HashTable lookups thread-safe .. IMPALA-9197: make HashTable lookups thread-safe This makes it possible to do HashTable lookups from multiple threads without any data races. This requires moving statistics that are updated during probing to the HashTableCtx object. There are some small changes to the hash table stat logging behaviour as a result of the stats being moved to the context. I don't believe these logs are used much, if at all. Testing: Ran exhaustive tests. Manually inspected some aggregation and join query profiles to check that hash table stats looked reasonable. Perf: Ran TPC-H scale factor 30 on a single node. No significant change in perf. Change-Id: I92fbfa8cc000477b8e01975a102d818f9fa27c61 --- M be/src/exec/grouping-aggregator.cc M be/src/exec/grouping-aggregator.h M be/src/exec/hash-table.cc M be/src/exec/hash-table.h M be/src/exec/hash-table.inline.h M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/partitioned-hash-join-builder.h M be/src/exec/partitioned-hash-join-node.cc 8 files changed, 136 insertions(+), 71 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/17/14917/6 -- To view, visit http://gerrit.cloudera.org:8080/14917 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I92fbfa8cc000477b8e01975a102d818f9fa27c61 Gerrit-Change-Number: 14917 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-9197: make HashTable lookups thread-safe
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14917 ) Change subject: IMPALA-9197: make HashTable lookups thread-safe .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5311/ : 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/14917 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I92fbfa8cc000477b8e01975a102d818f9fa27c61 Gerrit-Change-Number: 14917 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 18 Dec 2019 20:43:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9197: make HashTable lookups thread-safe
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14917 ) Change subject: IMPALA-9197: make HashTable lookups thread-safe .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5312/ : 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/14917 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I92fbfa8cc000477b8e01975a102d818f9fa27c61 Gerrit-Change-Number: 14917 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 18 Dec 2019 20:47:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7117: Lower debug level for HDFS S3 connector back to INFO
Sahil Takiar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14926 Change subject: IMPALA-7117: Lower debug level for HDFS S3 connector back to INFO .. IMPALA-7117: Lower debug level for HDFS S3 connector back to INFO This reverts the changes done in https://gerrit.cloudera.org/#/c/10596/ to set the HDFS S3 connector logs to DEBUG. DEBUG logs drastically increase the size of the Impala logs during test runs and are no longer necessary given recent stability improvements to the S3A code and the Impala-S3Guard integration. Change-Id: I4b8bf91fbf06e7609e3f92205e71557723163ecf --- M fe/src/test/resources/log4j.properties.template 1 file changed, 0 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/14926/1 -- To view, visit http://gerrit.cloudera.org:8080/14926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I4b8bf91fbf06e7609e3f92205e71557723163ecf Gerrit-Change-Number: 14926 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar
[Impala-ASF-CR] IMPALA-8974: Fixed a bug when create kudu managed table without HMS config
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14398 ) Change subject: IMPALA-8974: Fixed a bug when create kudu managed table without HMS config .. Patch Set 19: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/14398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iacc53801a660c033869cb4747910c98a80e08297 Gerrit-Change-Number: 14398 Gerrit-PatchSet: 19 Gerrit-Owner: wangsheng Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Wed, 18 Dec 2019 21:07:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8974: Fixed a bug when create kudu managed table without HMS config
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14398 ) Change subject: IMPALA-8974: Fixed a bug when create kudu managed table without HMS config .. Patch Set 20: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5358/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iacc53801a660c033869cb4747910c98a80e08297 Gerrit-Change-Number: 14398 Gerrit-PatchSet: 20 Gerrit-Owner: wangsheng Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Wed, 18 Dec 2019 21:07:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7117: Lower debug level for HDFS S3 connector back to INFO
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14926 ) Change subject: IMPALA-7117: Lower debug level for HDFS S3 connector back to INFO .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5313/ : 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/14926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4b8bf91fbf06e7609e3f92205e71557723163ecf Gerrit-Change-Number: 14926 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Wed, 18 Dec 2019 21:19:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9240: add HTTP code handling to THttpClient.
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/14924 ) Change subject: IMPALA-9240: add HTTP code handling to THttpClient. .. Patch Set 1: (1 comment) Looks good to me, overall. Just one small comment from me. http://gerrit.cloudera.org:8080/#/c/14924/1/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/14924/1/tests/shell/test_shell_interactive.py@868 PS1, Line 868: return I feel like this should be pytest.skip(). I think using return here will mean that this shows up as a passing test, even if it didn't run. -- To view, visit http://gerrit.cloudera.org:8080/14924 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3c105f4b8237b87695324d75981821c08c43 Gerrit-Change-Number: 14924 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 18 Dec 2019 21:23:27 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7117: Lower debug level for HDFS S3 connector back to INFO
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/14926 ) Change subject: IMPALA-7117: Lower debug level for HDFS S3 connector back to INFO .. Patch Set 1: Code-Review+2 Makes sense to change this. -- To view, visit http://gerrit.cloudera.org:8080/14926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4b8bf91fbf06e7609e3f92205e71557723163ecf Gerrit-Change-Number: 14926 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Wed, 18 Dec 2019 21:34:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9240: add HTTP code handling to THttpClient.
Andrew Sherman has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/14924 ) Change subject: IMPALA-9240: add HTTP code handling to THttpClient. .. IMPALA-9240: add HTTP code handling to THttpClient. Before this change Impala Shell is not checking HTTP return codes when using the hs2-http protocol. The shell is sending a request message (e.g. send_CloseOperation) but the HTTP call to send this message may fail. This will result in a failure when reading the reply (e.g. in recv_CloseOperation) as there is no reply data to read. This will typically result in an 'EOFError'. In code that overrides THttpClient.flush(), check the HTTP code that is returned after the HTTP call is made. If the code is not 1XX (informational response) or 2XX (successful) then throw an RPCException. This change does not contain any attempt to recover from an HTTP failures but it does allow the failure to be detected and a message to be printed. In future it may be possible to retry after certain HTTP errors. Testing: - Add a new test for impala-shell that tries to connect to an HTTP server that always returns a 503 error. Check that an appropriate error message is printed. Change-Id: I3c105f4b8237b87695324d75981821c08c43 --- M shell/impala_client.py M tests/shell/test_shell_interactive.py 2 files changed, 62 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/24/14924/2 -- To view, visit http://gerrit.cloudera.org:8080/14924 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3c105f4b8237b87695324d75981821c08c43 Gerrit-Change-Number: 14924 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-9240: add HTTP code handling to THttpClient.
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/14924 ) Change subject: IMPALA-9240: add HTTP code handling to THttpClient. .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/14924/1/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/14924/1/tests/shell/test_shell_interactive.py@868 PS1, Line 868: return > I feel like this should be pytest.skip(). I think using return here will me Thanks for the pythonic suggestion -- To view, visit http://gerrit.cloudera.org:8080/14924 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3c105f4b8237b87695324d75981821c08c43 Gerrit-Change-Number: 14924 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 18 Dec 2019 22:28:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Add --impalad args to single node perf run.py
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14923 ) Change subject: Add --impalad_args to single_node_perf_run.py .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14923 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib903f0eabb06a7e8981c874c8fe1cec0936b1a64 Gerrit-Change-Number: 14923 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Comment-Date: Wed, 18 Dec 2019 22:51:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9231: support customized privilege checks for visibility
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14904 ) Change subject: IMPALA-9231: support customized privilege checks for visibility .. Patch Set 3: (8 comments) Thanks for making the suggested change. I have some minor suggestions below and its good to go from my side. http://gerrit.cloudera.org:8080/#/c/14904/3/be/src/common/global-flags.cc File be/src/common/global-flags.cc: http://gerrit.cloudera.org:8080/#/c/14904/3/be/src/common/global-flags.cc@282 PS3, Line 282: visibility_privilege_set may be rename to min_privilege_set_for_show_stmts? http://gerrit.cloudera.org:8080/#/c/14904/3/be/src/common/global-flags.cc@283 PS3, Line 283: Any o Any one of them.. http://gerrit.cloudera.org:8080/#/c/14904/3/be/src/common/global-flags.cc@284 PS3, Line 284: "table. Default to \"any\" which means if the user has any privilege (ALL, SELECT, " s/Default/Defaults http://gerrit.cloudera.org:8080/#/c/14904/3/be/src/common/global-flags.cc@288 PS3, Line 288: practise s/practise/practice http://gerrit.cloudera.org:8080/#/c/14904/3/be/src/common/global-flags.cc@291 PS3, Line 291: nit, additional space http://gerrit.cloudera.org:8080/#/c/14904/3/be/src/common/global-flags.cc@292 PS3, Line 292: Because a lot of privilege checks on invisible dbs/tables can " : "be bypassed I think this is redundant information and can be skipped. http://gerrit.cloudera.org:8080/#/c/14904/3/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java File fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java: http://gerrit.cloudera.org:8080/#/c/14904/3/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java@93 PS3, Line 93: hasAccess nit, not a blocker for this patch. I think sentry already does this for us. http://gerrit.cloudera.org:8080/#/c/14904/3/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/14904/3/fe/src/main/java/org/apache/impala/service/Frontend.java@326 PS3, Line 326: LOG.error("Ignored illegal privilege name '{}'", pStr, e); Do you think we should throw ImpalaException here instead of silently logging ignoring the user configuration? -- To view, visit http://gerrit.cloudera.org:8080/14904 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I631fc5c386a52f0a1f62182473be15fcc3dd8609 Gerrit-Change-Number: 14904 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Wed, 18 Dec 2019 22:55:45 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9240: add HTTP code handling to THttpClient.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14924 ) Change subject: IMPALA-9240: add HTTP code handling to THttpClient. .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5314/ : 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/14924 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3c105f4b8237b87695324d75981821c08c43 Gerrit-Change-Number: 14924 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 18 Dec 2019 22:59:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Fang-Yu Rao has uploaded a new patch set (#14). ( http://gerrit.cloudera.org:8080/14798 ) Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests .. IMPALA-9149: part 1: Re-enabe Ranger-related FE tests In IMPALA-9047, we disabled some Ranger-related FE and BE tests due to changes in Ranger's behavior after upgrading Ranger from 1.2 to 2.0. This patch aims to re-enable those disabled FE tests in AuthorizationStmtTest.java and RangerAuditLogTest.java to increase Impala's test coverage of authorization via Ranger. There are at least two major changes in Ranger's behavior in the newer versions. 1. The first is that the owner of the requested resource no longer has to be explicitly granted privileges in order to access the resource. 2. The second is that a user not explicitly granted the privilege of creating a database is able to do so. Due to these changes, some of previous Ranger authorization requests that were expected to be rejected are now granted after the upgrade. To re-enable the tests affected by the first change described above, we modify AuthorizationTestBase.java to allow our FE Ranger authorization tests to specify the requesting user in an authorization test. Those tests failed after the upgrade because the default requesting user in Impala's AuthorizationTestBase.java happens to be the owner of the resources involved in our FE authorization tests. After this patch, a requesting user can be either a non-owner user or an owner user in a Ranger authorization test and the requesting user would correspond to a non-owner user if it is not explicitly specified. Note that in a Sentry authorization test, we do not use the non-owner user as the requesting user by default as we do in the Ranger authorization tests. Instead, we set the name of the requesting user to a name that is the same as the owner user in Ranger authorization tests to avoid the need for providing a customized group mapping service when instantiating a Sentry ResourceAuthorizationProvider as we do in AuthorizationTest.java, our FE tests specifically for testing authorization via Sentry. On the other hand, to re-enable the tests affected by the second change, we remove from the Ranger policy for all databases the allowed condition that grants any user the privilege of creating a database, which is not by default granted by Sentry. After the removal of the allowed codition, those tests in AuthorizationStmtTest.java and RangerAuditLogTest.java affected by the second change now result in the same authorization errors before the upgrade of Ranger. Testing: - Passed AuthorizationStmtTest.java in a local dev environment - Passed RangerAuditLogTest.java in a local dev environment Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2 --- M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java M fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java M fe/src/test/java/org/apache/impala/common/FrontendFixture.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M testdata/bin/create-load-data.sh A testdata/cluster/ranger/setup/impala_group_non_owner.json R testdata/cluster/ranger/setup/impala_group_owner.json.template A testdata/cluster/ranger/setup/impala_user_non_owner.json.template R testdata/cluster/ranger/setup/impala_user_owner.json.template A testdata/cluster/ranger/setup/policy_4_revised.json 12 files changed, 392 insertions(+), 212 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/14798/14 -- To view, visit http://gerrit.cloudera.org:8080/14798 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2 Gerrit-Change-Number: 14798 Gerrit-PatchSet: 14 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR] IMPALA-6393 live summary and live progress are not supported in impalarc
Wenzhe Zhou has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14927 Change subject: IMPALA-6393 live_summary and live_progress are not supported in impalarc .. IMPALA-6393 live_summary and live_progress are not supported in impalarc impala_shell main function call function get_config_from_file() to read impalarc file. The function get_config_from_file() save and return the shell options in one dictionary 'shell_options' with the option name as key. impala_shell then update the default option values in 'impala_shell_defaults' with the returned shell options dictionary. But the 'impala_shell_defaults' is defined with option 'dest' as key, instead of option name itself. There are some options for which option name and option dest are different. For example, dest of 'live_summary' is defined as 'print_summary', dest of 'live_progress' is defined as 'print_progress'. This cause option values in impala_shell_defaults are not updated with the values specified in config file if option name and option dest are different. This is the root cause why live_summary and live_progress in impalarc do not take effect in impala_shell. To fix the issue, modify function get_config_from_file() to save the options with 'dest' as key. Add some unit-test cases in test_shell_commandline.py and test_shell_interactive.py. Change-Id: If4549b775a7966ad89d661d0349cc78754e13a86 --- M bin/rat_exclude_files.txt M shell/option_parser.py A tests/shell/good_impalarc3 A tests/shell/good_impalarc4 A tests/shell/impalarc_with_error2 M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py 7 files changed, 96 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/14927/1 -- To view, visit http://gerrit.cloudera.org:8080/14927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: If4549b775a7966ad89d661d0349cc78754e13a86 Gerrit-Change-Number: 14927 Gerrit-PatchSet: 1 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Bikramjeet Vig
[Impala-ASF-CR] IMPALA-6393 live summary and live progress are not supported in impalarc
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14927 ) Change subject: IMPALA-6393 live_summary and live_progress are not supported in impalarc .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/14927/1/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/14927/1/tests/shell/test_shell_interactive.py@489 PS1, Line 489: # flake8: E265 block comment should start with '# ' -- To view, visit http://gerrit.cloudera.org:8080/14927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If4549b775a7966ad89d661d0349cc78754e13a86 Gerrit-Change-Number: 14927 Gerrit-PatchSet: 1 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 18 Dec 2019 23:10:58 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 ) Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests .. Patch Set 14: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5315/ : 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/14798 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2 Gerrit-Change-Number: 14798 Gerrit-PatchSet: 14 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Wed, 18 Dec 2019 23:35:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6393 live summary and live progress are not supported in impalarc
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14927 ) Change subject: IMPALA-6393 live_summary and live_progress are not supported in impalarc .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5316/ : 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/14927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If4549b775a7966ad89d661d0349cc78754e13a86 Gerrit-Change-Number: 14927 Gerrit-PatchSet: 1 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 18 Dec 2019 23:41:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6393 live summary and live progress are not supported in impalarc
Hello Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14927 to look at the new patch set (#2). Change subject: IMPALA-6393 live_summary and live_progress are not supported in impalarc .. IMPALA-6393 live_summary and live_progress are not supported in impalarc impala_shell main function call function get_config_from_file() to read impalarc file. The function get_config_from_file() save and return the shell options in one dictionary 'shell_options' with the option name as key. impala_shell then update the default option values in 'impala_shell_defaults' with the returned shell options dictionary. But the 'impala_shell_defaults' is defined with option 'dest' as key, instead of option name itself. There are some options for which option name and option dest are different. For example, dest of 'live_summary' is defined as 'print_summary', dest of 'live_progress' is defined as 'print_progress'. This cause option values in impala_shell_defaults are not updated with the values specified in config file if option name and option dest are different. This is the root cause why live_summary and live_progress in impalarc do not take effect in impala_shell. To fix the issue, modify function get_config_from_file() to save the options with 'dest' as key. Add some unit-test cases in test_shell_commandline.py and test_shell_interactive.py. Change-Id: If4549b775a7966ad89d661d0349cc78754e13a86 --- M bin/rat_exclude_files.txt M shell/option_parser.py A tests/shell/good_impalarc3 A tests/shell/good_impalarc4 A tests/shell/impalarc_with_error2 M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py 7 files changed, 95 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/14927/2 -- To view, visit http://gerrit.cloudera.org:8080/14927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If4549b775a7966ad89d661d0349cc78754e13a86 Gerrit-Change-Number: 14927 Gerrit-PatchSet: 2 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-9240: add HTTP code handling to THttpClient.
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/14924 ) Change subject: IMPALA-9240: add HTTP code handling to THttpClient. .. Patch Set 2: Code-Review+1 Looks good to me. I'll give a +1 for now so others have a chance to comment. -- To view, visit http://gerrit.cloudera.org:8080/14924 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3c105f4b8237b87695324d75981821c08c43 Gerrit-Change-Number: 14924 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Thu, 19 Dec 2019 00:09:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7117: Lower debug level for HDFS S3 connector back to INFO
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14926 ) Change subject: IMPALA-7117: Lower debug level for HDFS S3 connector back to INFO .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4b8bf91fbf06e7609e3f92205e71557723163ecf Gerrit-Change-Number: 14926 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Thu, 19 Dec 2019 00:24:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7117: Lower debug level for HDFS S3 connector back to INFO
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14926 ) Change subject: IMPALA-7117: Lower debug level for HDFS S3 connector back to INFO .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5359/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4b8bf91fbf06e7609e3f92205e71557723163ecf Gerrit-Change-Number: 14926 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Thu, 19 Dec 2019 00:24:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6393 live summary and live progress are not supported in impalarc
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14927 ) Change subject: IMPALA-6393 live_summary and live_progress are not supported in impalarc .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5317/ : 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/14927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If4549b775a7966ad89d661d0349cc78754e13a86 Gerrit-Change-Number: 14927 Gerrit-PatchSet: 2 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 19 Dec 2019 00:25:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8974: Fixed a bug when create kudu managed table without HMS config
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14398 ) Change subject: IMPALA-8974: Fixed a bug when create kudu managed table without HMS config .. Patch Set 20: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iacc53801a660c033869cb4747910c98a80e08297 Gerrit-Change-Number: 14398 Gerrit-PatchSet: 20 Gerrit-Owner: wangsheng Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Thu, 19 Dec 2019 01:42:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9231: support customized privilege checks for SHOW visibility
Hello Vihang Karajgaonkar, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14904 to look at the new patch set (#4). Change subject: IMPALA-9231: support customized privilege checks for SHOW visibility .. IMPALA-9231: support customized privilege checks for SHOW visibility In IMPALA-9002 we introduce a flag simplify_check_on_show_tables which simplifies privilege checks for SHOW TABLES. Only tables with privileges implying SELECT privilege will be shown. This patch provides the same mechanism for SHOW DATABASES. Also augment the flag to be a list of privilege names and rename it to min_privilege_set_for_show_stmts. The default value is "any" which remains the default behavior. If set to "select", only dbs/tables on which the user has SELECT privilege will be shown. If set to "select,insert", only dbs/tables on which the user has SELECT or INSERT privilege will be shown. Tests: - Add tests in test_authorization.py Change-Id: I631fc5c386a52f0a1f62182473be15fcc3dd8609 --- M be/src/common/global-flags.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/Frontend.java M tests/authorization/test_authorization.py 9 files changed, 217 insertions(+), 52 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/14904/4 -- To view, visit http://gerrit.cloudera.org:8080/14904 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I631fc5c386a52f0a1f62182473be15fcc3dd8609 Gerrit-Change-Number: 14904 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9231: support customized privilege checks for SHOW visibility
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14904 ) Change subject: IMPALA-9231: support customized privilege checks for SHOW visibility .. Patch Set 4: (10 comments) Thanks for the review! Rename the flag and throw exception for illegal flag value. http://gerrit.cloudera.org:8080/#/c/14904/3/be/src/common/global-flags.cc File be/src/common/global-flags.cc: http://gerrit.cloudera.org:8080/#/c/14904/3/be/src/common/global-flags.cc@282 PS3, Line 282: min_privilege_set_for_sh > may be rename to min_privilege_set_for_show_stmts? Done. Feel hard to find a good name for this... http://gerrit.cloudera.org:8080/#/c/14904/3/be/src/common/global-flags.cc@283 PS3, Line 283: Any o > Any one of them.. Done http://gerrit.cloudera.org:8080/#/c/14904/3/be/src/common/global-flags.cc@284 PS3, Line 284: "or table. Defaults to \"any\" which means if the user has any privilege (CREATE, " > s/Default/Defaults Done http://gerrit.cloudera.org:8080/#/c/14904/3/be/src/common/global-flags.cc@288 PS3, Line 288: be shown > s/practise/practice Done http://gerrit.cloudera.org:8080/#/c/14904/3/be/src/common/global-flags.cc@291 PS3, Line 291: ec > nit, additional space Done http://gerrit.cloudera.org:8080/#/c/14904/3/be/src/common/global-flags.cc@292 PS3, Line 292: e of privileges. No significant performance gain when using " : "Ranger"); > I think this is redundant information and can be skipped. Done http://gerrit.cloudera.org:8080/#/c/14904/3/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java File fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java: http://gerrit.cloudera.org:8080/#/c/14904/3/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java@93 PS3, Line 93: hasAccess > nit, not a blocker for this patch. I think sentry already does this for us. Sorry, I'm not quite clear... This is the base class for both Sentry and Ranger authorization. Do you mean we can override the 'hasAnyAccess' function in SentryAuthorizationChecker using Sentry's API directly? http://gerrit.cloudera.org:8080/#/c/14904/3/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/14904/3/fe/src/main/java/org/apache/impala/service/Frontend.java@326 PS3, Line 326: LOG.error("Illegal privilege name '{}'", pStr, e); > Do you think we should throw ImpalaException here instead of silently loggi Yes, that's better for careless users that even don't read logs. http://gerrit.cloudera.org:8080/#/c/14904/2/tests/authorization/test_authorization.py File tests/authorization/test_authorization.py: http://gerrit.cloudera.org:8080/#/c/14904/2/tests/authorization/test_authorization.py@574 PS2, Line 574: > flake8: E131 continuation line unaligned for hanging indent Done http://gerrit.cloudera.org:8080/#/c/14904/2/tests/authorization/test_authorization.py@600 PS2, Line 600: > flake8: E131 continuation line unaligned for hanging indent Done -- To view, visit http://gerrit.cloudera.org:8080/14904 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I631fc5c386a52f0a1f62182473be15fcc3dd8609 Gerrit-Change-Number: 14904 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 19 Dec 2019 02:09:03 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9231: support customized privilege checks for SHOW visibility
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14904 ) Change subject: IMPALA-9231: support customized privilege checks for SHOW visibility .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5318/ : 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/14904 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I631fc5c386a52f0a1f62182473be15fcc3dd8609 Gerrit-Change-Number: 14904 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 19 Dec 2019 02:36:48 + Gerrit-HasComments: No
[Impala-ASF-CR] Add --impalad args to single node perf run.py
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/14923 ) Change subject: Add --impalad_args to single_node_perf_run.py .. Patch Set 1: Code-Review+2 (1 comment) Fine as-is, but one question you might want to clarify in the help text. http://gerrit.cloudera.org:8080/#/c/14923/1/bin/single_node_perf_run.py File bin/single_node_perf_run.py: http://gerrit.cloudera.org:8080/#/c/14923/1/bin/single_node_perf_run.py@286 PS1, Line 286: help="Additional arguments to pass to each Impalad during startup") Does it only work like --impalad_args=--foo --impalad_args=--bar or will the following also work: --impalad_args="--foo --bar" -- To view, visit http://gerrit.cloudera.org:8080/14923 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib903f0eabb06a7e8981c874c8fe1cec0936b1a64 Gerrit-Change-Number: 14923 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Jim Apple Gerrit-Comment-Date: Thu, 19 Dec 2019 03:42:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9137: Blacklist node if a DataStreamService RPC to the node fails
Hello Michael Ho, Thomas Tauber-Marshall, Lars Volker, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14677 to look at the new patch set (#13). Change subject: IMPALA-9137: Blacklist node if a DataStreamService RPC to the node fails .. IMPALA-9137: Blacklist node if a DataStreamService RPC to the node fails Introduces a new optional field to FragmentInstanceExecStatusPB: AuxErrorInfoPB. AuxErrorInfoPB contains optional metadata associated with a failed fragment instance. Currently, AuxErrorInfoPB only contains one field: RPCErrorInfoPB, which is only set if the fragment failed because a RPC to another impalad failed. The RPCErrorInfoPB contains the destination node of the failed RPC and the posix error code of the failed RPC. Coordinator::UpdateBackendExecStatus(ReportExecStatusRequestPB, ...) uses the information in RPCErrorInfoPB (if one is set) to blacklist the target node. While RPCErrorInfoPB::dest_node can be set to the address of the Coordinator, the Coordinator will not blacklist itself. The Coordinator only blacklists the node if the RPC failed with a specific error code (currently either ENOTCONN, ECONNREFUSED, ESHUTDOWN). Testing: * Ran core tests * Added new test to test_blacklist.py Change-Id: I733cca13847fde43c8ea2ae574d3ae04bd06419c --- M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/runtime/fragment-instance-state.cc M be/src/runtime/krpc-data-stream-sender.cc M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M be/src/util/network-util.cc M be/src/util/network-util.h M common/protobuf/common.proto M common/protobuf/control_service.proto M tests/custom_cluster/test_blacklist.py 11 files changed, 230 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/14677/13 -- To view, visit http://gerrit.cloudera.org:8080/14677 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I733cca13847fde43c8ea2ae574d3ae04bd06419c Gerrit-Change-Number: 14677 Gerrit-PatchSet: 13 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-9137: Blacklist node if a DataStreamService RPC to the node fails
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14677 ) Change subject: IMPALA-9137: Blacklist node if a DataStreamService RPC to the node fails .. Patch Set 12: (7 comments) http://gerrit.cloudera.org:8080/#/c/14677/12/be/src/runtime/coordinator.h File be/src/runtime/coordinator.h: http://gerrit.cloudera.org:8080/#/c/14677/12/be/src/runtime/coordinator.h@152 PS12, Line 152: void UpdateBlacklistWithAuxErrorInfo(const ReportExecStatusRequestPB& request); > This can be private Done http://gerrit.cloudera.org:8080/#/c/14677/12/be/src/runtime/coordinator.h@263 PS12, Line 263: boost::unordered_map addr_to_backend_state_; > I think we might already have this mapping in QuerySchedule::per_backend_ex Yeah, but that mapping uses Thrift address rather than KRPC addresses. Updated the docs to make this more clear. http://gerrit.cloudera.org:8080/#/c/14677/12/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/14677/12/be/src/runtime/coordinator.cc@862 PS12, Line 862: LOG(WARNING) << "Query failed due to a failed RPC to an unknown target address " > Doesn't seem like this case should be possible, since 'addr_to_backend_stat I added a DCHECK(false) and changed the log level to ERROR. http://gerrit.cloudera.org:8080/#/c/14677/12/be/src/runtime/coordinator.cc@879 PS12, Line 879: static const set blacklistable_rpc_error_codes = { > Do you examples of posix codes a NetworkError could generate that we wouldn Discussed offline. There are a bunch of error codes (https://www-numi.fnal.gov/offline_software/srt_public_context/WebDocs/Errors/unix_system_errors.html) that don't seem useful to blacklist. I'm not sure what set of error codes KRPC is even possible of setting for network errors. Regardless, its probably safer to selectively add error codes to this list as we see fit. http://gerrit.cloudera.org:8080/#/c/14677/12/be/src/runtime/coordinator.cc@882 PS12, Line 882: ECONNREFUSED > I think you can specify these directly instead of having to use the actual Done http://gerrit.cloudera.org:8080/#/c/14677/12/be/src/runtime/coordinator.cc@892 PS12, Line 892: break; > Was it your intention to not continue through the loop if the first RPCErro Done http://gerrit.cloudera.org:8080/#/c/14677/12/be/src/runtime/runtime-state.h File be/src/runtime/runtime-state.h: http://gerrit.cloudera.org:8080/#/c/14677/12/be/src/runtime/runtime-state.h@439 PS12, Line 439: struct AuxErrorInfo { > Why not just use AuxErrorInfoPB directly? Done -- To view, visit http://gerrit.cloudera.org:8080/14677 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I733cca13847fde43c8ea2ae574d3ae04bd06419c Gerrit-Change-Number: 14677 Gerrit-PatchSet: 12 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Thu, 19 Dec 2019 03:51:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9137: Blacklist node if a DataStreamService RPC to the node fails
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14677 ) Change subject: IMPALA-9137: Blacklist node if a DataStreamService RPC to the node fails .. Patch Set 13: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5319/ : 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/14677 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I733cca13847fde43c8ea2ae574d3ae04bd06419c Gerrit-Change-Number: 14677 Gerrit-PatchSet: 13 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Thu, 19 Dec 2019 04:20:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7117: Lower debug level for HDFS S3 connector back to INFO
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14926 ) Change subject: IMPALA-7117: Lower debug level for HDFS S3 connector back to INFO .. IMPALA-7117: Lower debug level for HDFS S3 connector back to INFO This reverts the changes done in https://gerrit.cloudera.org/#/c/10596/ to set the HDFS S3 connector logs to DEBUG. DEBUG logs drastically increase the size of the Impala logs during test runs and are no longer necessary given recent stability improvements to the S3A code and the Impala-S3Guard integration. Change-Id: I4b8bf91fbf06e7609e3f92205e71557723163ecf Reviewed-on: http://gerrit.cloudera.org:8080/14926 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M fe/src/test/resources/log4j.properties.template 1 file changed, 0 insertions(+), 3 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/14926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I4b8bf91fbf06e7609e3f92205e71557723163ecf Gerrit-Change-Number: 14926 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell
[Impala-ASF-CR] IMPALA-7117: Lower debug level for HDFS S3 connector back to INFO
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14926 ) Change subject: IMPALA-7117: Lower debug level for HDFS S3 connector back to INFO .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4b8bf91fbf06e7609e3f92205e71557723163ecf Gerrit-Change-Number: 14926 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Thu, 19 Dec 2019 04:53:51 + Gerrit-HasComments: No