[Impala-ASF-CR] IMPALA-8486: fix stale libCache entries in LocalCatalog mode
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13849 ) Change subject: IMPALA-8486: fix stale libCache entries in LocalCatalog mode .. Patch Set 3: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/13849/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java: http://gerrit.cloudera.org:8080/#/c/13849/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@1215 PS3, Line 1215: // Will refresh the cached file if the mtime of its source file in HDFS changes Comment seems out of place. Instead say something like "invalidate the libcache entry for this function location" ? -- To view, visit http://gerrit.cloudera.org:8080/13849 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie4812fb8737de3ba6074ffeb9007927bfbbbaf9b Gerrit-Change-Number: 13849 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Sat, 13 Jul 2019 00:46:10 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/13801 )
Change subject: IMPALA-5149: Provide query profile in JSON format
..
Patch Set 6:
(17 comments)
Some more minor comments, but generally lgtm. I'll let other reviewers take a
look too.
http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/service/impala-http-handler.cc
File be/src/service/impala-http-handler.cc:
http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/service/impala-http-handler.cc@270
PS6, Line 270: if (format != TRuntimeProfileFormat::JSON){
nit: Add a quick comment why?
http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-counters.h
File be/src/util/runtime-profile-counters.h:
http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-counters.h@126
PS6, Line 126: {
nit: space const { .
http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-counters.h@128
PS6, Line 128: rapidjson::Value::MemberIterator type_itr =
val->FindMember("kind");
Add DHCECK (type_itr != val->MemberEnd())? (multiple places below too)
http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-counters.h@164
PS6, Line 164: {
same (multiple places below)
http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-counters.h@289
PS6, Line 289: val->RemoveMember("kind");
why this?
http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-counters.h@403
PS6, Line 403: void ToJson(rapidjson::Document& document, rapidjson::Value*
value) {
move to cc file?
http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-counters.h@421
PS6, Line 421: *value = event_sequence_rapid;
instantiate value = ...directly in L407?
http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-test.cc
File be/src/util/runtime-profile-test.cc:
http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-test.cc@1173
PS6, Line 1173: counter_a = profile_a->AddCounter("A", TUnit::UNIT);
check other units too and assert the unit data?
http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-test.cc@1187
PS6, Line 1187:
EXPECT_TRUE(content.FindMember("info_strings")==content.MemberEnd());
some tests for info_strings and event_sequences too?
http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile-test.cc@1202
PS6, Line 1202: TEST(ToJson, TimeSeriesCounterToJsonTest){
check other TimeSeriesCounter implementations too?
http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile.cc
File be/src/util/runtime-profile.cc:
http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile.cc@705
PS6, Line 705: rapidjson::
remove rapidjson classifiers with using...
http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile.cc@713
PS6, Line 713: allocator
use d->GetAllocator() (avoid temp variable)
http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile.cc@735
PS6, Line 735: child_counter,
counter_map,child_counter_map);
formatting
http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile.cc@1590
PS6, Line 1590: document.GetAllocator());
formatting
http://gerrit.cloudera.org:8080/#/c/13801/6/be/src/util/runtime-profile.cc@1600
PS6, Line 1600: document.GetAllocator());
formatting
http://gerrit.cloudera.org:8080/#/c/13801/6/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:
http://gerrit.cloudera.org:8080/#/c/13801/6/tests/hs2/test_hs2.py@659
PS6, Line 659: def test_get_profile_json(self):
why not do this in the above test after L654?
http://gerrit.cloudera.org:8080/#/c/13801/6/tests/webserver/test_web_pages.py
File tests/webserver/test_web_pages.py:
http://gerrit.cloudera.org:8080/#/c/13801/6/tests/webserver/test_web_pages.py@585
PS6, Line 585: def test_download_json_profile(self):
merge with test_download_text_profile? You can do something like
for format in ("text", "json"):
...
--
To view, visit http://gerrit.cloudera.org:8080/13801
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1
Gerrit-Change-Number: 13801
Gerrit-PatchSet: 6
Gerrit-Owner: Jiawei Wang
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: David Knupp
Gerrit-Reviewer: David Rorke
Gerrit-Reviewer: Greg Rahn
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jiawei Wang
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Sahil Takiar
Gerrit-Comment-Date: Sat, 13 Jul 2019 04:00:03 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8486: fix stale libCache entries in LocalCatalog mode
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13849 ) Change subject: IMPALA-8486: fix stale libCache entries in LocalCatalog mode .. Patch Set 4: Yep, agreed. Dedicated executor libcache invalidation seems broken. -- To view, visit http://gerrit.cloudera.org:8080/13849 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie4812fb8737de3ba6074ffeb9007927bfbbbaf9b Gerrit-Change-Number: 13849 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 15 Jul 2019 16:51:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8486: fix stale libCache entries in LocalCatalog mode coordinators
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13849 ) Change subject: IMPALA-8486: fix stale libCache entries in LocalCatalog mode coordinators .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13849 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie4812fb8737de3ba6074ffeb9007927bfbbbaf9b Gerrit-Change-Number: 13849 Gerrit-PatchSet: 5 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 15 Jul 2019 17:57:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8489: Partitions created by RECOVER PARTITIONS fail to create insert events with IllegalStateException.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13860 ) Change subject: IMPALA-8489: Partitions created by RECOVER PARTITIONS fail to create insert events with IllegalStateException. .. Patch Set 1: Do you have the full IllegalStateException stack trace? (Don't see it in the jira). I'm trying to understand what the problem is here. -- To view, visit http://gerrit.cloudera.org:8080/13860 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idef7f6aadff2868047c861ebfcc05d65f080eab9 Gerrit-Change-Number: 13860 Gerrit-PatchSet: 1 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Mon, 15 Jul 2019 20:39:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/13801 )
Change subject: IMPALA-5149: Provide query profile in JSON format
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-hs2-server.cc
File be/src/service/impala-hs2-server.cc:
http://gerrit.cloudera.org:8080/#/c/13801/2/be/src/service/impala-hs2-server.cc@979
PS2, Line 979: if (request.format == TRuntimeProfileFormat::THRIFT) {
: return_val.__set_thrift_profile(thrift_profile);
: } else {
: DCHECK(request.format == TRuntimeProfileFormat::STRING
: || request.format == TRuntimeProfileFormat::BASE64);
: return_val.__set_profile(ss.str());
: }
> not sure; I'm not too familiar with this part of the code, I guess it depen
I don't think we need to fix this for beeswax server as it will be deprecated
in the future, especially given that shell now supports HS2 protocol.
--
To view, visit http://gerrit.cloudera.org:8080/13801
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1
Gerrit-Change-Number: 13801
Gerrit-PatchSet: 2
Gerrit-Owner: Jiawei Wang
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: David Knupp
Gerrit-Reviewer: David Rorke
Gerrit-Reviewer: Greg Rahn
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jiawei Wang
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Sahil Takiar
Gerrit-Comment-Date: Mon, 15 Jul 2019 22:43:44 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8606: Don't load table meta for GET TABLES in local catalog mode
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/13874 )
Change subject: IMPALA-8606: Don't load table meta for GET_TABLES in local
catalog mode
..
Patch Set 5:
(4 comments)
Patch makes sense to me. Some comments.
http://gerrit.cloudera.org:8080/#/c/13874/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13874/5//COMMIT_MSG@23
PS5, Line 23: - Testing in a HMS with 100 dbs and 3000 tables, without this
patch it
Mind deploying this jar on a cluster with Hue and quickly check that nothing
breaks?
http://gerrit.cloudera.org:8080/#/c/13874/5/fe/src/main/java/org/apache/impala/catalog/Catalog.java
File fe/src/main/java/org/apache/impala/catalog/Catalog.java:
http://gerrit.cloudera.org:8080/#/c/13874/5/fe/src/main/java/org/apache/impala/catalog/Catalog.java@167
PS5, Line 167: Catalog v1
I think the code never refers to Catalog "v1" and Catalog "v2". So this could
be confusing and better to get rid of them (multiple places).
http://gerrit.cloudera.org:8080/#/c/13874/5/fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java:
http://gerrit.cloudera.org:8080/#/c/13874/5/fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java@169
PS5, Line 169: newMap.put(tableName.toLowerCase(), null);
Should we instead load LocalIncompleteTable here?
http://gerrit.cloudera.org:8080/#/c/13874/5/fe/src/test/java/org/apache/impala/service/JdbcTest.java
File fe/src/test/java/org/apache/impala/service/JdbcTest.java:
http://gerrit.cloudera.org:8080/#/c/13874/5/fe/src/test/java/org/apache/impala/service/JdbcTest.java@390
PS5, Line 390: public void testMetaDataGetColumnComments() throws Exception {
Mind adding a test to LocalCatalogTest#testGetTables()? Check that the table
remains in an incomplete state after GET_TABLES request?
--
To view, visit http://gerrit.cloudera.org:8080/13874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8bbab7efdf8e629abe09d89ae3bd770e3feaccb
Gerrit-Change-Number: 13874
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Comment-Date: Thu, 18 Jul 2019 17:28:36 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8606: Don't load table meta for GET TABLES in local catalog mode
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13874 ) Change subject: IMPALA-8606: Don't load table meta for GET_TABLES in local catalog mode .. Patch Set 5: (2 comments) Couple of nits, but can +2 once fixed. http://gerrit.cloudera.org:8080/#/c/13874/5/fe/src/main/java/org/apache/impala/catalog/Catalog.java File fe/src/main/java/org/apache/impala/catalog/Catalog.java: http://gerrit.cloudera.org:8080/#/c/13874/5/fe/src/main/java/org/apache/impala/catalog/Catalog.java@167 PS5, Line 167: Catalog v1 > I'm trying to explain why the implementation here is just delegating to get Right, I'm talking about the terminology. In the code, we never see these terms like v1 and v2. Maybe say legacy implementation vs LocalCatalog implementation or some such thing? http://gerrit.cloudera.org:8080/#/c/13874/6/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java File fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java: http://gerrit.cloudera.org:8080/#/c/13874/6/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java@385 PS6, Line 385: Why this -- To view, visit http://gerrit.cloudera.org:8080/13874 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia8bbab7efdf8e629abe09d89ae3bd770e3feaccb Gerrit-Change-Number: 13874 Gerrit-PatchSet: 5 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Fri, 19 Jul 2019 16:16:52 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8489: Partitions created by RECOVER PARTITIONS fail to create insert events with IllegalStateException.
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/13860 )
Change subject: IMPALA-8489: Partitions created by RECOVER PARTITIONS fail to
create insert events with IllegalStateException.
..
Patch Set 2:
(3 comments)
I won't be able to review this, I'll let Vihang +2 it.
http://gerrit.cloudera.org:8080/#/c/13860/2/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/13860/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3936
PS2, Line 3936: private Map
getPartitionNamesMap(Collectionhttp://gerrit.cloudera.org:8080/#/c/13860/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3939
PS2, Line 3939: for (FeFsPartition part: partitions) {
you could simply this (and below) with stream() and collect()
http://gerrit.cloudera.org:8080/#/c/13860/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3948
PS2, Line 3948: private Map>
getPartitionNameFilesMap(Collectionhttp://gerrit.cloudera.org:8080/13860
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idef7f6aadff2868047c861ebfcc05d65f080eab9
Gerrit-Change-Number: 13860
Gerrit-PatchSet: 2
Gerrit-Owner: Anurag Mantripragada
Gerrit-Reviewer: Anurag Mantripragada
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Fri, 19 Jul 2019 16:44:20 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5149: Provide query profile in JSON format
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/13801 )
Change subject: IMPALA-5149: Provide query profile in JSON format
..
Patch Set 8: Code-Review+1
(5 comments)
Some minor comments, but generally looks good to me. I spoke to Michael Ho
offline who is going to take another look into the change and take it to a +2.
Thanks for your patience so far.
http://gerrit.cloudera.org:8080/#/c/13801/8//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13801/8//COMMIT_MSG@22
PS8, Line 22:
Add a section on future compatibility guarantees?
I think the idea is that this can evolve over time until we standardize the
counters and the profile structure. So, until then if there are some Impal a
changes, clients can expect the JSON structure to change too (keys could be
added, deleted etc.)
http://gerrit.cloudera.org:8080/#/c/13801/8/be/src/util/runtime-profile-counters.h
File be/src/util/runtime-profile-counters.h:
http://gerrit.cloudera.org:8080/#/c/13801/8/be/src/util/runtime-profile-counters.h@292
PS8, Line 292: val->RemoveMember("kind");
Should we not update the kind ? Why remove it?
http://gerrit.cloudera.org:8080/#/c/13801/8/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:
http://gerrit.cloudera.org:8080/#/c/13801/8/tests/hs2/test_hs2.py@668
PS8, Line 668: json_res["contents"]["child_profiles"][0]["info_strings"]:
This looks flaky. Instead assert that they exist before you can loop through
them?
http://gerrit.cloudera.org:8080/#/c/13801/8/tests/hs2/test_hs2.py@670
PS8, Line 670: assert statement in info_string["value"]
dump the json_res if the assert fails, helps with debugging test failures.
http://gerrit.cloudera.org:8080/#/c/13801/8/tests/webserver/test_web_pages.py
File tests/webserver/test_web_pages.py:
http://gerrit.cloudera.org:8080/#/c/13801/8/tests/webserver/test_web_pages.py@594
PS8, Line 594: self.fail("Download JSON format query profile cannot
be parsed.")
dump json
--
To view, visit http://gerrit.cloudera.org:8080/13801
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8181ac818bf22207ca1deabd9220c397ae723ec1
Gerrit-Change-Number: 13801
Gerrit-PatchSet: 8
Gerrit-Owner: Jiawei Wang
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: David Knupp
Gerrit-Reviewer: David Rorke
Gerrit-Reviewer: Greg Rahn
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jiawei Wang
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Sahil Takiar
Gerrit-Comment-Date: Fri, 19 Jul 2019 17:20:25 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8717: impala-shell support for HS2 HTTP endpoint
Hello Thomas Tauber-Marshall, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13746 to look at the new patch set (#6). Change subject: IMPALA-8717: impala-shell support for HS2 HTTP endpoint .. IMPALA-8717: impala-shell support for HS2 HTTP endpoint Adds impala-shell support to connect to HiveServer2 HTTP endpoint. Relies on toolchain change at https://gerrit.cloudera.org/#/c/13725/. Use --protocol='hs2-http' to enable this behavior. Example usages: --- impala-shell --protocol='hs2-http' (No auth) impala-shell --protocol='hs2-http' --ldap -u. (PLAIN auth) impala-shell --protocol-'hs2-http' --ssl --ca_cert... (TLS) impala-shell --protocol='hs2-http' --ldap --ssl --ca_cert... (LDAP + TLS) Limitations: --- - Does not support Kerberos (-k) due to lack ot SPNEGO support. Testing: - Parameterized existing shell tests to support this combination. - Added shell test coverage for LDAP auth. Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 --- M be/src/service/impala-server.cc M bin/impala-config.sh M fe/src/test/java/org/apache/impala/customcluster/LdapJdbcTest.java M shell/impala_client.py M shell/impala_shell.py M shell/option_parser.py M tests/common/impala_cluster.py M tests/common/impala_service.py M tests/common/impala_test_suite.py M tests/common/test_dimensions.py M tests/conftest.py M tests/custom_cluster/test_client_ssl.py M tests/run-tests.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py M tests/shell/util.py 16 files changed, 209 insertions(+), 44 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/13746/6 -- To view, visit http://gerrit.cloudera.org:8080/13746 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 Gerrit-Change-Number: 13746 Gerrit-PatchSet: 6 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8717: impala-shell support for HS2 HTTP endpoint
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/13746 )
Change subject: IMPALA-8717: impala-shell support for HS2 HTTP endpoint
..
Patch Set 6:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/13746/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13746/5//COMMIT_MSG@29
PS5, Line 29: - Added shell test coverage for LDAP auth.
> If you're interested, it should be possible to write automated tests for th
Done. Added some test coverage.
http://gerrit.cloudera.org:8080/#/c/13746/5/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/13746/5/be/src/service/impala-server.cc@2081
PS5, Line 2081: << " The connection had " <<
disconnected_sessions.size()
> I feel like the "with" ... " sessions closed" makes it sound like the sessi
Fair point. Done.
http://gerrit.cloudera.org:8080/#/c/13746/5/shell/impala_client.py
File shell/impala_client.py:
http://gerrit.cloudera.org:8080/#/c/13746/5/shell/impala_client.py@367
PS5, Line 367: # TODO: Investigate connection reuse in THttpClient and
revisit this.
> What would happen if you set the socket timeout and then just didn't reset
Yes. that is not desirable for longer duration RPCs, say waiting on results etc.
http://gerrit.cloudera.org:8080/#/c/13746/5/shell/impala_client.py@369
PS5, Line 369: print_to_stderr("Warning: --connect_timeout_ms is
currently ignored with" +
switched this to a warning.
http://gerrit.cloudera.org:8080/#/c/13746/5/shell/impala_client.py@392
PS5, Line 392:
> flake8: E501 line too long (93 > 90 characters)
Done
http://gerrit.cloudera.org:8080/#/c/13746/5/shell/impala_shell.py
File shell/impala_shell.py:
http://gerrit.cloudera.org:8080/#/c/13746/5/shell/impala_shell.py@531
PS5, Line 531: self.
> Maybe include the param name here and below, i.e. "use_http_base_transport=
Done
http://gerrit.cloudera.org:8080/#/c/13746/5/shell/impala_shell.py@544
PS5, Line 544: _ms
> nit: comma
Done
http://gerrit.cloudera.org:8080/#/c/13746/5/tests/run-tests.py
File tests/run-tests.py:
http://gerrit.cloudera.org:8080/#/c/13746/5/tests/run-tests.py@233
PS5, Line 233:
> flake8: E502 the backslash is redundant between brackets
Done
http://gerrit.cloudera.org:8080/#/c/13746/5/tests/run-tests.py@234
PS5, Line 234:
webserver_certificate_file=cert).read_debug_webpage('metrics?json'))
> Why is this needed?
This is not related to patch but I ran into this during my local testing. So I
thought it'd nice to fix it while I'm here.
So if you have a mini-cluster with TLS for web endpoints enabled, any
subsequent run-tests.py command gets stuck here in a loop, trying to print
metrics, because it fails to connect to the http end points without the
certificate. Essentially no tests run until you manually kill the cluster or
restart it without TLS for web endpoints.
http://gerrit.cloudera.org:8080/#/c/13746/5/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:
http://gerrit.cloudera.org:8080/#/c/13746/5/tests/shell/test_shell_interactive.py@268
PS5, Line 268: assert protocol == "beeswax"
> assert protocol == "beeswax"
Done
--
To view, visit http://gerrit.cloudera.org:8080/13746
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534
Gerrit-Change-Number: 13746
Gerrit-PatchSet: 6
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Sat, 20 Jul 2019 03:23:38 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8772: Import Testcase failed for SQL without table refs
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13893 ) Change subject: IMPALA-8772: Import Testcase failed for SQL without table refs .. Patch Set 1: Code-Review+2 (4 comments) Thanks for fixing this. http://gerrit.cloudera.org:8080/#/c/13893/1/tests/metadata/test_testcase_builder.py File tests/metadata/test_testcase_builder.py: http://gerrit.cloudera.org:8080/#/c/13893/1/tests/metadata/test_testcase_builder.py@41 PS1, Line 41: execute_query execute_query_expect_success http://gerrit.cloudera.org:8080/#/c/13893/1/tests/metadata/test_testcase_builder.py@43 PS1, Line 43: # Test load testcase works assert len(result.data) == 1 ? http://gerrit.cloudera.org:8080/#/c/13893/1/tests/metadata/test_testcase_builder.py@48 PS1, Line 48: # TODO: Delete testcase file from tmp Implement the TODO? http://gerrit.cloudera.org:8080/#/c/13893/1/tests/metadata/test_testcase_builder.py@49 PS1, Line 49: > flake8: W391 blank line at end of file Yep, remove these blank lines? -- To view, visit http://gerrit.cloudera.org:8080/13893 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I50d77d7de44bb140598a1e8db4da85a5ec87f31e Gerrit-Change-Number: 13893 Gerrit-PatchSet: 1 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 23 Jul 2019 02:46:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8746: [DOCS] Document the DEFAULT HINTS INSERT STATEMENT query option
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13885 ) Change subject: IMPALA-8746: [DOCS] Document the DEFAULT_HINTS_INSERT_STATEMENT query option .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13885 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia376721f46eb507901f9f64b5c3341dc0f36475b Gerrit-Change-Number: 13885 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 23 Jul 2019 02:50:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8606: Don't load table meta for GET TABLES in local catalog mode
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13874 ) Change subject: IMPALA-8606: Don't load table meta for GET_TABLES in local catalog mode .. Patch Set 9: Code-Review+2 Thanks for fixing this. -- To view, visit http://gerrit.cloudera.org:8080/13874 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia8bbab7efdf8e629abe09d89ae3bd770e3feaccb Gerrit-Change-Number: 13874 Gerrit-PatchSet: 9 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 23 Jul 2019 02:52:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8717: impala-shell support for HS2 HTTP endpoint
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13746 ) Change subject: IMPALA-8717: impala-shell support for HS2 HTTP endpoint .. Patch Set 7: (6 comments) http://gerrit.cloudera.org:8080/#/c/13746/6/fe/src/test/java/org/apache/impala/customcluster/LdapJdbcTest.java File fe/src/test/java/org/apache/impala/customcluster/LdapJdbcTest.java: http://gerrit.cloudera.org:8080/#/c/13746/6/fe/src/test/java/org/apache/impala/customcluster/LdapJdbcTest.java@116 PS6, Line 116: > line has trailing whitespace Done http://gerrit.cloudera.org:8080/#/c/13746/6/fe/src/test/java/org/apache/impala/customcluster/LdapJdbcTest.java@133 PS6, Line 133: : > Instead of catching the assertion failure to log and rethrow it, I think yo Done http://gerrit.cloudera.org:8080/#/c/13746/6/fe/src/test/java/org/apache/impala/customcluster/LdapJdbcTest.java@140 PS6, Line 140: > nit: I probably got this wrong in some places in my patches around this, bu removed it here. Basic is specific to http, but here were are testing all the protocols. http://gerrit.cloudera.org:8080/#/c/13746/6/fe/src/test/java/org/apache/impala/customcluster/LdapJdbcTest.java@143 PS6, Line 143: > We don't really want to inherit all of the JDBC related setup stuff here, e ya, I should've seen that. Fixed it now. http://gerrit.cloudera.org:8080/#/c/13746/6/fe/src/test/java/org/apache/impala/customcluster/LdapJdbcTest.java@149 PS6, Line 149: > Could you make the 'select logged_in_user()' and actually verify that its c Done http://gerrit.cloudera.org:8080/#/c/13746/6/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/13746/6/shell/impala_shell.py@804 PS6, Line 804: assert options.protocol.lower() == 'beeswax' : port = str(DEFAULT_BEESWAX_PORT) > Probably only want to log if its actually wrong, and we should probably ret oops, debugging message leaked here. thanks for catching. I don't think anyone would ever hit this assertion unless they are changing something in the code. So I think its ok to keep it as is. -- To view, visit http://gerrit.cloudera.org:8080/13746 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 Gerrit-Change-Number: 13746 Gerrit-PatchSet: 7 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 24 Jul 2019 00:29:24 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8717: impala-shell support for HS2 HTTP endpoint
Hello Thomas Tauber-Marshall, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13746 to look at the new patch set (#7). Change subject: IMPALA-8717: impala-shell support for HS2 HTTP endpoint .. IMPALA-8717: impala-shell support for HS2 HTTP endpoint Adds impala-shell support to connect to HiveServer2 HTTP endpoint. Relies on toolchain change at https://gerrit.cloudera.org/#/c/13725/. Use --protocol='hs2-http' to enable this behavior. Example usages: --- impala-shell --protocol='hs2-http' (No auth) impala-shell --protocol='hs2-http' --ldap -u. (PLAIN auth) impala-shell --protocol-'hs2-http' --ssl --ca_cert... (TLS) impala-shell --protocol='hs2-http' --ldap --ssl --ca_cert... (LDAP + TLS) Limitations: --- - Does not support Kerberos (-k) due to lack ot SPNEGO support. Testing: - Parameterized existing shell tests to support this combination. - Added shell test coverage for LDAP auth. Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 --- M be/src/service/impala-server.cc M bin/impala-config.sh A fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java M shell/impala_client.py M shell/impala_shell.py M shell/option_parser.py M tests/common/impala_cluster.py M tests/common/impala_service.py M tests/common/impala_test_suite.py M tests/common/test_dimensions.py M tests/conftest.py M tests/custom_cluster/test_client_ssl.py M tests/run-tests.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py M tests/shell/util.py 16 files changed, 284 insertions(+), 44 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/13746/7 -- To view, visit http://gerrit.cloudera.org:8080/13746 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 Gerrit-Change-Number: 13746 Gerrit-PatchSet: 7 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8717: impala-shell support for HS2 HTTP endpoint
Hello Thomas Tauber-Marshall, David Knupp, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13746 to look at the new patch set (#8). Change subject: IMPALA-8717: impala-shell support for HS2 HTTP endpoint .. IMPALA-8717: impala-shell support for HS2 HTTP endpoint Adds impala-shell support to connect to HiveServer2 HTTP endpoint. Relies on toolchain change at https://gerrit.cloudera.org/#/c/13725/. Use --protocol='hs2-http' to enable this behavior. Example usages: --- impala-shell --protocol='hs2-http' (No auth) impala-shell --protocol='hs2-http' --ldap -u. (PLAIN auth) impala-shell --protocol-'hs2-http' --ssl --ca_cert... (TLS) impala-shell --protocol='hs2-http' --ldap --ssl --ca_cert... (LDAP + TLS) Limitations: --- - Does not support Kerberos (-k) due to lack ot SPNEGO support. Testing: - Parameterized existing shell tests to support this combination. - Added shell test coverage for LDAP auth. Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 --- M be/src/service/impala-server.cc M bin/impala-config.sh A fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java M shell/impala_client.py M shell/impala_shell.py M shell/option_parser.py M tests/common/impala_cluster.py M tests/common/impala_service.py M tests/common/impala_test_suite.py M tests/common/test_dimensions.py M tests/conftest.py M tests/custom_cluster/test_client_ssl.py M tests/run-tests.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py M tests/shell/util.py 16 files changed, 287 insertions(+), 45 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/13746/8 -- To view, visit http://gerrit.cloudera.org:8080/13746 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 Gerrit-Change-Number: 13746 Gerrit-PatchSet: 8 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8717: impala-shell support for HS2 HTTP endpoint
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/13746 )
Change subject: IMPALA-8717: impala-shell support for HS2 HTTP endpoint
..
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13746/6/shell/impala_shell.py
File shell/impala_shell.py:
http://gerrit.cloudera.org:8080/#/c/13746/6/shell/impala_shell.py@804
PS6, Line 804: print_to_stderr("protocol: " + options.protocol.lower())
: assert options.protocol.lower() == 'beeswax'
> Maybe I'm wrong, but it doesn't look like this value is validated anywhere
Done
--
To view, visit http://gerrit.cloudera.org:8080/13746
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534
Gerrit-Change-Number: 13746
Gerrit-PatchSet: 6
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: David Knupp
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 24 Jul 2019 17:02:15 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8717: impala-shell support for HS2 HTTP endpoint
Hello Thomas Tauber-Marshall, David Knupp, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13746 to look at the new patch set (#9). Change subject: IMPALA-8717: impala-shell support for HS2 HTTP endpoint .. IMPALA-8717: impala-shell support for HS2 HTTP endpoint Adds impala-shell support to connect to HiveServer2 HTTP endpoint. Relies on toolchain change at https://gerrit.cloudera.org/#/c/13725/. Use --protocol='hs2-http' to enable this behavior. Example usages: --- impala-shell --protocol='hs2-http' (No auth) impala-shell --protocol='hs2-http' --ldap -u. (PLAIN auth) impala-shell --protocol-'hs2-http' --ssl --ca_cert... (TLS) impala-shell --protocol='hs2-http' --ldap --ssl --ca_cert... (LDAP + TLS) Limitations: --- - Does not support Kerberos (-k) due to lack ot SPNEGO support. Testing: - Parameterized existing shell tests to support this combination. - Added shell test coverage for LDAP auth. Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 --- M be/src/service/impala-server.cc M bin/impala-config.sh A fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java M shell/impala_client.py M shell/impala_shell.py M shell/option_parser.py M tests/common/impala_cluster.py M tests/common/impala_service.py M tests/common/impala_test_suite.py M tests/common/test_dimensions.py M tests/conftest.py M tests/custom_cluster/test_client_ssl.py M tests/run-tests.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py M tests/shell/util.py 16 files changed, 288 insertions(+), 45 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/13746/9 -- To view, visit http://gerrit.cloudera.org:8080/13746 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 Gerrit-Change-Number: 13746 Gerrit-PatchSet: 9 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8717: impala-shell support for HS2 HTTP endpoint
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/13746 )
Change subject: IMPALA-8717: impala-shell support for HS2 HTTP endpoint
..
Patch Set 8:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13746/8/fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
File fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java:
http://gerrit.cloudera.org:8080/#/c/13746/8/fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java@123
PS8, Line 123: String[] commandWithoutAuth = {"impala-shell.sh", "",
String.format("--query=%s", query)};
> line too long (94 > 90)
Done
--
To view, visit http://gerrit.cloudera.org:8080/13746
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534
Gerrit-Change-Number: 13746
Gerrit-PatchSet: 8
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: David Knupp
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 24 Jul 2019 17:07:00 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8772: Import Testcase failed for SQL without table refs
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/13893 )
Change subject: IMPALA-8772: Import Testcase failed for SQL without table refs
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13893/2/tests/metadata/test_testcase_builder.py
File tests/metadata/test_testcase_builder.py:
http://gerrit.cloudera.org:8080/#/c/13893/2/tests/metadata/test_testcase_builder.py@51
PS2, Line 51: status = call(["hdfs", "dfs", "-rm", "-skipTrash",
testcase_path[1: -1]])
: assert status == 0, "Delete generated testcase file failed
with {0}".format(status)
you can use delete_file_dir from hdfs_util.py . Also please put it in a finally
block. (to make sure the deletion happens regardless of test case export
failures).
--
To view, visit http://gerrit.cloudera.org:8080/13893
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I50d77d7de44bb140598a1e8db4da85a5ec87f31e
Gerrit-Change-Number: 13893
Gerrit-PatchSet: 2
Gerrit-Owner: Jiawei Wang
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jiawei Wang
Gerrit-Comment-Date: Thu, 25 Jul 2019 01:02:22 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8606: Don't load table meta for GET TABLES in local catalog mode
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13874 ) Change subject: IMPALA-8606: Don't load table meta for GET_TABLES in local catalog mode .. Patch Set 11: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13874 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia8bbab7efdf8e629abe09d89ae3bd770e3feaccb Gerrit-Change-Number: 13874 Gerrit-PatchSet: 11 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Thu, 25 Jul 2019 01:03:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8772: Import Testcase failed for SQL without table refs
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13893 ) Change subject: IMPALA-8772: Import Testcase failed for SQL without table refs .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13893 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I50d77d7de44bb140598a1e8db4da85a5ec87f31e Gerrit-Change-Number: 13893 Gerrit-PatchSet: 4 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Comment-Date: Thu, 25 Jul 2019 04:51:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8434: retain tables and functions in altering database
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/13904 )
Change subject: IMPALA-8434: retain tables and functions in altering database
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13904/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
File fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java:
http://gerrit.cloudera.org:8080/#/c/13904/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java@394
PS2, Line 394: CatalogObjectVersionSet.INSTANCE.updateVersions(
: existingDb.getCatalogVersion(), catalogVersion);
:
CatalogObjectVersionSet.INSTANCE.removeAll(existingDb.getTables());
: CatalogObjectVersionSet.INSTANCE.removeAll(
: existingDb.getFunctions(null, new PatternMatcher()));
: // IMPALA-8434: add back the existing tables/functions.
Note that their version
: // counters in CatalogObjectVersionSet have been
decreased by the above removeAll
: // statements, meaning their references from the old db
are deleted since the old
: // db object has been replaced by newDb.
: for (Table tbl: existingDb.getTables()) {
: newDb.addTable(tbl);
: }
This looks buggy to me. What does it mean to have tables in the db, without
having their version number maintained in the CatalogObjectVersionSet ? Aren't
we breaking the invariant that CatalogObjectVersionSet can get the correct min
version across all the objects (invalidate metadata relies on it)? or am I
misunderstanding something?
--
To view, visit http://gerrit.cloudera.org:8080/13904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3dc9857fd2733e20cf10fbe17bb1a4670d7d015
Gerrit-Change-Number: 13904
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Thu, 25 Jul 2019 05:09:09 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8434: retain tables and functions in altering database
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/13904 )
Change subject: IMPALA-8434: retain tables and functions in altering database
..
Patch Set 2: Code-Review+2
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13904/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
File fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java:
http://gerrit.cloudera.org:8080/#/c/13904/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java@394
PS2, Line 394: CatalogObjectVersionSet.INSTANCE.updateVersions(
: existingDb.getCatalogVersion(), catalogVersion);
:
CatalogObjectVersionSet.INSTANCE.removeAll(existingDb.getTables());
: CatalogObjectVersionSet.INSTANCE.removeAll(
: existingDb.getFunctions(null, new PatternMatcher()));
: // IMPALA-8434: add back the existing tables/functions.
Note that their version
: // counters in CatalogObjectVersionSet have been
decreased by the above removeAll
: // statements, meaning their references from the old db
are deleted since the old
: // db object has been replaced by newDb.
: for (Table tbl: existingDb.getTables()) {
: newDb.addTable(tbl);
: }
> Db.addTable and Db.addFunction will finally add back the versions to Catalo
Oops I only went one level deep and checked tableCache_.add(table); Didn't
realize that it was updating the CatalogObjectVersionSet underneath. Sorry for
the false alarm.
http://gerrit.cloudera.org:8080/#/c/13904/2/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:
http://gerrit.cloudera.org:8080/#/c/13904/2/tests/metadata/test_ddl.py@241
PS2, Line 241: self.client.execute("create table {0}.tbl (i
int)".format(unique_database))
nit: how about merging this with the above test?
--
To view, visit http://gerrit.cloudera.org:8080/13904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3dc9857fd2733e20cf10fbe17bb1a4670d7d015
Gerrit-Change-Number: 13904
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Thu, 25 Jul 2019 17:44:52 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8717: impala-shell support for HS2 HTTP endpoint
Hello Thomas Tauber-Marshall, David Knupp, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13746 to look at the new patch set (#11). Change subject: IMPALA-8717: impala-shell support for HS2 HTTP endpoint .. IMPALA-8717: impala-shell support for HS2 HTTP endpoint Adds impala-shell support to connect to HiveServer2 HTTP endpoint. Relies on toolchain change at https://gerrit.cloudera.org/#/c/13725/. Use --protocol='hs2-http' to enable this behavior. Example usages: --- impala-shell --protocol='hs2-http' (No auth) impala-shell --protocol='hs2-http' --ldap -u. (PLAIN auth) impala-shell --protocol-'hs2-http' --ssl --ca_cert... (TLS) impala-shell --protocol='hs2-http' --ldap --ssl --ca_cert... (LDAP + TLS) Limitations: --- - Does not support Kerberos (-k) due to lack ot SPNEGO support. Testing: - Parameterized existing shell tests to support this combination. - Added shell test coverage for LDAP auth. Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 --- M be/src/service/impala-server.cc M bin/impala-config.sh A fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java M shell/impala_client.py M shell/impala_shell.py M shell/option_parser.py M tests/common/impala_cluster.py M tests/common/impala_service.py M tests/common/impala_test_suite.py M tests/common/test_dimensions.py M tests/conftest.py M tests/custom_cluster/test_client_ssl.py M tests/run-tests.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py M tests/shell/util.py 16 files changed, 296 insertions(+), 49 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/13746/11 -- To view, visit http://gerrit.cloudera.org:8080/13746 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 Gerrit-Change-Number: 13746 Gerrit-PatchSet: 11 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8717: impala-shell support for HS2 HTTP endpoint
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13746 ) Change subject: IMPALA-8717: impala-shell support for HS2 HTTP endpoint .. Patch Set 11: Missed a bunch of asserts, I fixed the tests and I'm running the test suite again. Lets see how it goes. -- To view, visit http://gerrit.cloudera.org:8080/13746 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 Gerrit-Change-Number: 13746 Gerrit-PatchSet: 11 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 25 Jul 2019 23:57:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8717: impala-shell support for HS2 HTTP endpoint
Hello Thomas Tauber-Marshall, David Knupp, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13746 to look at the new patch set (#12). Change subject: IMPALA-8717: impala-shell support for HS2 HTTP endpoint .. IMPALA-8717: impala-shell support for HS2 HTTP endpoint Adds impala-shell support to connect to HiveServer2 HTTP endpoint. Relies on toolchain change at https://gerrit.cloudera.org/#/c/13725/. Use --protocol='hs2-http' to enable this behavior. Example usages: --- impala-shell --protocol='hs2-http' (No auth) impala-shell --protocol='hs2-http' --ldap -u. (PLAIN auth) impala-shell --protocol-'hs2-http' --ssl --ca_cert... (TLS) impala-shell --protocol='hs2-http' --ldap --ssl --ca_cert... (LDAP + TLS) Limitations: --- - Does not support Kerberos (-k) due to lack ot SPNEGO support. Testing: - Parameterized existing shell tests to support this combination. - Added shell test coverage for LDAP auth. Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 --- M be/src/service/impala-server.cc M bin/impala-config.sh A fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java M shell/impala_client.py M shell/impala_shell.py M shell/option_parser.py M tests/common/impala_cluster.py M tests/common/impala_service.py M tests/common/impala_test_suite.py M tests/common/test_dimensions.py M tests/conftest.py M tests/custom_cluster/test_client_ssl.py M tests/run-tests.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py M tests/shell/util.py 16 files changed, 296 insertions(+), 49 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/13746/12 -- To view, visit http://gerrit.cloudera.org:8080/13746 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 Gerrit-Change-Number: 13746 Gerrit-PatchSet: 12 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] WIP: Add debugging tools to our docker images
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13895 ) Change subject: WIP: Add debugging tools to our docker images .. Patch Set 1: Code-Review+1 I'll let others weigh in, but I think having these tools makes life much simpler. -- To view, visit http://gerrit.cloudera.org:8080/13895 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I47c7aa7076cebfa3bfad2029fb1da9e64364f0e6 Gerrit-Change-Number: 13895 Gerrit-PatchSet: 1 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Fri, 26 Jul 2019 04:23:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8717: impala-shell support for HS2 HTTP endpoint
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13746 ) Change subject: IMPALA-8717: impala-shell support for HS2 HTTP endpoint .. Patch Set 12: There are quite a few places in the test code that assert protocol is either 'beeswax' or 'hs2'. Interestingly I didn't run into this before. So fixing a few more places in the next run. Hopefully last, sorry for for the spam. -- To view, visit http://gerrit.cloudera.org:8080/13746 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 Gerrit-Change-Number: 13746 Gerrit-PatchSet: 12 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 26 Jul 2019 15:59:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8717: impala-shell support for HS2 HTTP endpoint
Hello Thomas Tauber-Marshall, David Knupp, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13746 to look at the new patch set (#13). Change subject: IMPALA-8717: impala-shell support for HS2 HTTP endpoint .. IMPALA-8717: impala-shell support for HS2 HTTP endpoint Adds impala-shell support to connect to HiveServer2 HTTP endpoint. Relies on toolchain change at https://gerrit.cloudera.org/#/c/13725/. Use --protocol='hs2-http' to enable this behavior. Example usages: --- impala-shell --protocol='hs2-http' (No auth) impala-shell --protocol='hs2-http' --ldap -u. (PLAIN auth) impala-shell --protocol-'hs2-http' --ssl --ca_cert... (TLS) impala-shell --protocol='hs2-http' --ldap --ssl --ca_cert... (LDAP + TLS) Limitations: --- - Does not support Kerberos (-k) due to lack ot SPNEGO support. Testing: - Parameterized existing shell tests to support this combination. - Added shell test coverage for LDAP auth. Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 --- M be/src/service/impala-server.cc M bin/impala-config.sh A fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java M shell/impala_client.py M shell/impala_shell.py M shell/option_parser.py M tests/common/impala_cluster.py M tests/common/impala_connection.py M tests/common/impala_service.py M tests/common/impala_test_suite.py M tests/common/test_dimensions.py M tests/conftest.py M tests/custom_cluster/test_client_ssl.py M tests/custom_cluster/test_shell_interactive.py M tests/custom_cluster/test_shell_interactive_reconnect.py M tests/run-tests.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py M tests/shell/util.py 19 files changed, 321 insertions(+), 62 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/13746/13 -- To view, visit http://gerrit.cloudera.org:8080/13746 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 Gerrit-Change-Number: 13746 Gerrit-PatchSet: 13 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8717: impala-shell support for HS2 HTTP endpoint
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13746 ) Change subject: IMPALA-8717: impala-shell support for HS2 HTTP endpoint .. Patch Set 13: (4 comments) http://gerrit.cloudera.org:8080/#/c/13746/13/tests/common/test_dimensions.py File tests/common/test_dimensions.py: http://gerrit.cloudera.org:8080/#/c/13746/13/tests/common/test_dimensions.py@123 PS13, Line 123: def create_beeswax_hs2_hs2http_dimension(): > flake8: E302 expected 2 blank lines, found 1 Done http://gerrit.cloudera.org:8080/#/c/13746/13/tests/custom_cluster/test_shell_interactive_reconnect.py File tests/custom_cluster/test_shell_interactive_reconnect.py: http://gerrit.cloudera.org:8080/#/c/13746/13/tests/custom_cluster/test_shell_interactive_reconnect.py@44 PS13, Line 44: s > flake8: E501 line too long (98 > 90 characters) Done http://gerrit.cloudera.org:8080/#/c/13746/13/tests/custom_cluster/test_shell_interactive_reconnect.py@61 PS13, Line 61: : > flake8: E501 line too long (91 > 90 characters) Done http://gerrit.cloudera.org:8080/#/c/13746/13/tests/custom_cluster/test_shell_interactive_reconnect.py@88 PS13, Line 88: : > flake8: E501 line too long (91 > 90 characters) Done -- To view, visit http://gerrit.cloudera.org:8080/13746 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 Gerrit-Change-Number: 13746 Gerrit-PatchSet: 13 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 26 Jul 2019 17:04:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8717: impala-shell support for HS2 HTTP endpoint
Hello Thomas Tauber-Marshall, David Knupp, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13746 to look at the new patch set (#14). Change subject: IMPALA-8717: impala-shell support for HS2 HTTP endpoint .. IMPALA-8717: impala-shell support for HS2 HTTP endpoint Adds impala-shell support to connect to HiveServer2 HTTP endpoint. Relies on toolchain change at https://gerrit.cloudera.org/#/c/13725/. Use --protocol='hs2-http' to enable this behavior. Example usages: --- impala-shell --protocol='hs2-http' (No auth) impala-shell --protocol='hs2-http' --ldap -u. (PLAIN auth) impala-shell --protocol-'hs2-http' --ssl --ca_cert... (TLS) impala-shell --protocol='hs2-http' --ldap --ssl --ca_cert... (LDAP + TLS) Limitations: --- - Does not support Kerberos (-k) due to lack ot SPNEGO support. Testing: - Parameterized existing shell tests to support this combination. - Added shell test coverage for LDAP auth. Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 --- M be/src/service/impala-server.cc M bin/impala-config.sh A fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java M shell/impala_client.py M shell/impala_shell.py M shell/option_parser.py M tests/common/impala_cluster.py M tests/common/impala_connection.py M tests/common/impala_service.py M tests/common/impala_test_suite.py M tests/common/test_dimensions.py M tests/conftest.py M tests/custom_cluster/test_client_ssl.py M tests/custom_cluster/test_shell_interactive.py M tests/custom_cluster/test_shell_interactive_reconnect.py M tests/run-tests.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py M tests/shell/util.py 19 files changed, 323 insertions(+), 62 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/13746/14 -- To view, visit http://gerrit.cloudera.org:8080/13746 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 Gerrit-Change-Number: 13746 Gerrit-PatchSet: 14 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8717: impala-shell support for HS2 HTTP endpoint
Hello Thomas Tauber-Marshall, David Knupp, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13746 to look at the new patch set (#15). Change subject: IMPALA-8717: impala-shell support for HS2 HTTP endpoint .. IMPALA-8717: impala-shell support for HS2 HTTP endpoint Adds impala-shell support to connect to HiveServer2 HTTP endpoint. Relies on toolchain change at https://gerrit.cloudera.org/#/c/13725/. Use --protocol='hs2-http' to enable this behavior. Example usages: --- impala-shell --protocol='hs2-http' (No auth) impala-shell --protocol='hs2-http' --ldap -u. (PLAIN auth) impala-shell --protocol-'hs2-http' --ssl --ca_cert... (TLS) impala-shell --protocol='hs2-http' --ldap --ssl --ca_cert... (LDAP + TLS) Limitations: --- - Does not support Kerberos (-k) due to lack ot SPNEGO support. Testing: - Parameterized existing shell tests to support this combination. - Added shell test coverage for LDAP auth. Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 --- M be/src/service/impala-server.cc M bin/impala-config.sh A fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java M shell/impala_client.py M shell/impala_shell.py M shell/option_parser.py M tests/common/impala_cluster.py M tests/common/impala_connection.py M tests/common/impala_service.py M tests/common/impala_test_suite.py M tests/common/test_dimensions.py M tests/conftest.py M tests/custom_cluster/test_client_ssl.py M tests/custom_cluster/test_shell_interactive.py M tests/custom_cluster/test_shell_interactive_reconnect.py M tests/run-tests.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py M tests/shell/util.py 19 files changed, 326 insertions(+), 62 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/13746/15 -- To view, visit http://gerrit.cloudera.org:8080/13746 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 Gerrit-Change-Number: 13746 Gerrit-PatchSet: 15 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8717: impala-shell support for HS2 HTTP endpoint
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13746 ) Change subject: IMPALA-8717: impala-shell support for HS2 HTTP endpoint .. Patch Set 15: Fixed the one final test thats failing. Kicking off another run. -- To view, visit http://gerrit.cloudera.org:8080/13746 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 Gerrit-Change-Number: 13746 Gerrit-PatchSet: 15 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sun, 28 Jul 2019 22:51:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8717: impala-shell support for HS2 HTTP endpoint
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13746 ) Change subject: IMPALA-8717: impala-shell support for HS2 HTTP endpoint .. Patch Set 16: Code-Review+2 Carrying +2. -- To view, visit http://gerrit.cloudera.org:8080/13746 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 Gerrit-Change-Number: 13746 Gerrit-PatchSet: 16 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 29 Jul 2019 05:43:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8717: impala-shell support for HS2 HTTP endpoint
Bharath Vissapragada has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13746 ) Change subject: IMPALA-8717: impala-shell support for HS2 HTTP endpoint .. IMPALA-8717: impala-shell support for HS2 HTTP endpoint Adds impala-shell support to connect to HiveServer2 HTTP endpoint. Relies on toolchain change at https://gerrit.cloudera.org/#/c/13725/. Use --protocol='hs2-http' to enable this behavior. Example usages: --- impala-shell --protocol='hs2-http' (No auth) impala-shell --protocol='hs2-http' --ldap -u. (PLAIN auth) impala-shell --protocol-'hs2-http' --ssl --ca_cert... (TLS) impala-shell --protocol='hs2-http' --ldap --ssl --ca_cert... (LDAP + TLS) Limitations: --- - Does not support Kerberos (-k) due to lack ot SPNEGO support. Testing: - Parameterized existing shell tests to support this combination. - Added shell test coverage for LDAP auth. Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 Reviewed-on: http://gerrit.cloudera.org:8080/13746 Tested-by: Impala Public Jenkins Reviewed-by: Bharath Vissapragada --- M be/src/service/impala-server.cc M bin/impala-config.sh A fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java M shell/impala_client.py M shell/impala_shell.py M shell/option_parser.py M tests/common/impala_cluster.py M tests/common/impala_connection.py M tests/common/impala_service.py M tests/common/impala_test_suite.py M tests/common/test_dimensions.py M tests/conftest.py M tests/custom_cluster/test_client_ssl.py M tests/custom_cluster/test_shell_interactive.py M tests/custom_cluster/test_shell_interactive_reconnect.py M tests/run-tests.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py M tests/shell/util.py 19 files changed, 326 insertions(+), 62 deletions(-) Approvals: Impala Public Jenkins: Verified Bharath Vissapragada: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/13746 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534 Gerrit-Change-Number: 13746 Gerrit-PatchSet: 17 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8661 : Add randomized tests to stress MetastoreEventsProcessor
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/13932 )
Change subject: IMPALA-8661 : Add randomized tests to stress
MetastoreEventsProcessor
..
Patch Set 5:
(14 comments)
Great work, exhaustive coverage. I skimmed through most parts of the patch.
seems reasonable to me. I think it is difficult to review the entire patch in
a single go. We should probably commit it and keep fixing any issues
incrementally. Since most of the patch is isolated to test framework, any bugs
(like connection leaks, if any) shouldn't affect the product itself.
http://gerrit.cloudera.org:8080/#/c/13932/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13932/5//COMMIT_MSG@35
PS5, Line 35: 1. Ran the test with defaults. It generates about 2100 events
Should we consider lower defaults for core tests? 15mins seems like a
considerable increase in test execution time.
http://gerrit.cloudera.org:8080/#/c/13932/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/13932/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@299
PS5, Line 299: infoLog("Notification event received");
nit: isn't this too chatty for info?
http://gerrit.cloudera.org:8080/#/c/13932/5/fe/src/test/java/org/apache/impala/catalog/events/EventsProcessorStressTest.java
File
fe/src/test/java/org/apache/impala/catalog/events/EventsProcessorStressTest.java:
http://gerrit.cloudera.org:8080/#/c/13932/5/fe/src/test/java/org/apache/impala/catalog/events/EventsProcessorStressTest.java@46
PS5, Line 46: refresh
nit: grammar
http://gerrit.cloudera.org:8080/#/c/13932/5/fe/src/test/java/org/apache/impala/catalog/events/EventsProcessorStressTest.java@83
PS5, Line 83: else
do we want to override numClients in 3.x? in which case it should probably be
something like
if (version < 3 && numClients != null)...
http://gerrit.cloudera.org:8080/#/c/13932/5/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
File
fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java:
http://gerrit.cloudera.org:8080/#/c/13932/5/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@138
PS5, Line 138: import org.mockito.Mockito;
remove?
http://gerrit.cloudera.org:8080/#/c/13932/5/fe/src/test/java/org/apache/impala/testutil/HiveJdbcClientPool.java
File fe/src/test/java/org/apache/impala/testutil/HiveJdbcClientPool.java:
http://gerrit.cloudera.org:8080/#/c/13932/5/fe/src/test/java/org/apache/impala/testutil/HiveJdbcClientPool.java@28
PS5, Line 28: import java.util.concurrent.BlockingQueue;
nit: duplicate.
http://gerrit.cloudera.org:8080/#/c/13932/5/fe/src/test/java/org/apache/impala/testutil/HiveJdbcClientPool.java@36
PS5, Line 36: public class HiveJdbcClientPool implements Closeable {
class comment.
http://gerrit.cloudera.org:8080/#/c/13932/5/fe/src/test/java/org/apache/impala/testutil/HiveJdbcClientPool.java@71
PS5, Line 71: if (conn_ == null) {
: throw new RuntimeException("Connection not initialized.");
: } else if (conn_.isClosed()) {
: throw new RuntimeException("Connection not open.");
: }
Make them preconditions?
http://gerrit.cloudera.org:8080/#/c/13932/5/fe/src/test/java/org/apache/impala/testutil/HiveJdbcClientPool.java@94
PS5, Line 94: public boolean executeSql(String sql) throws SQLException {
method doc.
http://gerrit.cloudera.org:8080/#/c/13932/5/fe/src/test/java/org/apache/impala/testutil/HiveJdbcClientPool.java@146
PS5, Line 146: closedCount > 0
!freeClients_.isEmpty() ?
http://gerrit.cloudera.org:8080/#/c/13932/5/fe/src/test/java/org/apache/impala/testutil/TestUtils.java
File fe/src/test/java/org/apache/impala/testutil/TestUtils.java:
http://gerrit.cloudera.org:8080/#/c/13932/5/fe/src/test/java/org/apache/impala/testutil/TestUtils.java@42
PS5, Line 42: import org.apache.commons.lang3.RandomStringUtils;
remove?
http://gerrit.cloudera.org:8080/#/c/13932/5/fe/src/test/java/org/apache/impala/testutil/TestUtils.java@77
PS5, Line 77: DEFAULT_CONNECTION_TEMPLATE
nit:HS2_CONNECTION_TEMPLATE?
http://gerrit.cloudera.org:8080/#/c/13932/5/fe/src/test/java/org/apache/impala/util/RandomHiveQueryRunner.java
File fe/src/test/java/org/apache/impala/util/RandomHiveQueryRunner.java:
http://gerrit.cloudera.org:8080/#/c/13932/5/fe/src/test/java/org/apache/impala/util/RandomHiveQueryRunner.java@117
PS5, Line 117: for
nit: remove
http://gerrit.cloudera.org:8080/#/c/13932/5/fe/src/test/java/org/apache/impala/util/RandomHiveQueryRunner.java@345
PS5, Line 345: exists
exist
--
To view, visit http://gerrit.cloudera.org:8080/13932
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
G
[Impala-ASF-CR] IMPALA-8456: [DOCS] New HTTP protocol for Impala clients
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13960 ) Change subject: IMPALA-8456: [DOCS] New HTTP protocol for Impala clients .. Patch Set 2: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/13960/2/docs/topics/impala_ports.xml File docs/topics/impala_ports.xml: http://gerrit.cloudera.org:8080/#/c/13960/2/docs/topics/impala_ports.xml@401 PS2, Line 401: . ..using HiveServer2 protocol. -- To view, visit http://gerrit.cloudera.org:8080/13960 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3101f8babc77a5a872778499a54ac479a66ad996 Gerrit-Change-Number: 13960 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 31 Jul 2019 19:50:39 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8456: [DOCS] New HTTP protocol for Impala clients
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13960 ) Change subject: IMPALA-8456: [DOCS] New HTTP protocol for Impala clients .. Patch Set 2: Code-Review-1 Forgot about this. I think we should talk about authentication semantics over the new HTTP protocol. We support HTTP Basic auth. SPNEGO is not yet supported. -- To view, visit http://gerrit.cloudera.org:8080/13960 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3101f8babc77a5a872778499a54ac479a66ad996 Gerrit-Change-Number: 13960 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 31 Jul 2019 19:53:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8456: [DOCS] New HTTP protocol for Impala clients
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13960 ) Change subject: IMPALA-8456: [DOCS] New HTTP protocol for Impala clients .. Patch Set 2: Oops, I missed that commit. Thanks for the pointer. I think it is still useful to mention that the server endpoint supports basic and spnego auth. -- To view, visit http://gerrit.cloudera.org:8080/13960 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3101f8babc77a5a872778499a54ac479a66ad996 Gerrit-Change-Number: 13960 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 31 Jul 2019 21:38:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8456: [DOCS] New HTTP protocol for Impala clients
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13960 ) Change subject: IMPALA-8456: [DOCS] New HTTP protocol for Impala clients .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13960 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3101f8babc77a5a872778499a54ac479a66ad996 Gerrit-Change-Number: 13960 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 01 Aug 2019 01:53:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8661 : Add randomized tests to stress MetastoreEventsProcessor
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13932 ) Change subject: IMPALA-8661 : Add randomized tests to stress MetastoreEventsProcessor .. Patch Set 6: (1 comment) lgtm pending the test timings. http://gerrit.cloudera.org:8080/#/c/13932/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13932/5//COMMIT_MSG@35 PS5, Line 35: 1. Ran the test with defaults. It generates about 2100 events > I think the fe tests are not the long pole in the test execution, right? So Doesn't this job run everything serially? be tests -> fe-tests->e-e tests https://jenkins.impala.io/job/ubuntu-16.04-from-scratch/6891/consoleFull (just picked up random job from verify-dry-run) -- To view, visit http://gerrit.cloudera.org:8080/13932 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c85b83efd4f56b5ae0e8d1dc6a2ee2feb6721ce Gerrit-Change-Number: 13932 Gerrit-PatchSet: 6 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 01 Aug 2019 02:24:16 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8668: [DOCS] HS2 Support for Impala-Shell connection
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13961 ) Change subject: IMPALA-8668: [DOCS] HS2 Support for Impala-Shell connection .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/13961/2/docs/topics/impala_connecting.xml File docs/topics/impala_connecting.xml: http://gerrit.cloudera.org:8080/#/c/13961/2/docs/topics/impala_connecting.xml@85 PS2, Line 85: # When you are logged onto a different host, and impalad is listening I think you mean, "connecting to an impalad running on a remote machine".. or something like that. I don't think its logged onto a different host... http://gerrit.cloudera.org:8080/#/c/13961/2/docs/topics/impala_connecting.xml@87 PS2, Line 87: -i -i should be before the http://gerrit.cloudera.org:8080/#/c/13961/2/docs/topics/impala_connecting.xml@87 PS2, Line 87: -p -- (double dash) http://gerrit.cloudera.org:8080/#/c/13961/2/docs/topics/impala_shell_options.xml File docs/topics/impala_shell_options.xml: http://gerrit.cloudera.org:8080/#/c/13961/2/docs/topics/impala_shell_options.xml@573 PS2, Line 573: .< ..protocol. (same below too) -- To view, visit http://gerrit.cloudera.org:8080/13961 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie4e207d247409ac3069ce8a235be3f63e616007e Gerrit-Change-Number: 13961 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 01 Aug 2019 02:41:43 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8817: Fix TestTestcaseBuilder tests on non-HDFS environment
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13963 ) Change subject: IMPALA-8817: Fix TestTestcaseBuilder tests on non-HDFS environment .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13963 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibf6767c3275dc5deec75a36f797d0963f83839cf Gerrit-Change-Number: 13963 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 01 Aug 2019 16:16:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8668: [DOCS] HS2 Support for Impala-Shell connection
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13961 ) Change subject: IMPALA-8668: [DOCS] HS2 Support for Impala-Shell connection .. Patch Set 3: Code-Review+2 (2 comments) http://gerrit.cloudera.org:8080/#/c/13961/3/docs/topics/impala_connecting.xml File docs/topics/impala_connecting.xml: http://gerrit.cloudera.org:8080/#/c/13961/3/docs/topics/impala_connecting.xml@81 PS3, Line 81: When you are Remove http://gerrit.cloudera.org:8080/#/c/13961/3/docs/topics/impala_connecting.xml@87 PS3, Line 87: remove trailing space -- To view, visit http://gerrit.cloudera.org:8080/13961 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie4e207d247409ac3069ce8a235be3f63e616007e Gerrit-Change-Number: 13961 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 02 Aug 2019 20:11:24 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8456: [DOCS] New HTTP protocol for Impala clients
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13960 ) Change subject: IMPALA-8456: [DOCS] New HTTP protocol for Impala clients .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13960 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3101f8babc77a5a872778499a54ac479a66ad996 Gerrit-Change-Number: 13960 Gerrit-PatchSet: 5 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 02 Aug 2019 20:14:45 + Gerrit-HasComments: No
[Impala-ASF-CR] Add krb5 client utilities to the containers
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13997 to look at the new patch set (#2). Change subject: Add krb5 client utilities to the containers .. Add krb5 client utilities to the containers Some components depend on these utils (kinit, kdestroy..) for ticket cache lifecycle management. These are also useful for debugging in general, for example, to test KDC connectivity etc. Local docker image size increased from 820MB to 865MB for a release build (=5.4%). Change-Id: I9c9e9ab5b027ea9d223928280bc94f2ed9f701d3 --- M docker/impala_base/Dockerfile 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/13997/2 -- To view, visit http://gerrit.cloudera.org:8080/13997 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9c9e9ab5b027ea9d223928280bc94f2ed9f701d3 Gerrit-Change-Number: 13997 Gerrit-PatchSet: 2 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] Add krb5 client utilities to the containers
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13997 ) Change subject: Add krb5 client utilities to the containers .. Patch Set 2: Verified+1 Code-Review+2 https://jenkins.impala.io/job/gerrit-verify-dryrun/4723/ -- To view, visit http://gerrit.cloudera.org:8080/13997 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c9e9ab5b027ea9d223928280bc94f2ed9f701d3 Gerrit-Change-Number: 13997 Gerrit-PatchSet: 2 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 03 Aug 2019 23:58:47 + Gerrit-HasComments: No
[Impala-ASF-CR] Add krb5 client utilities to the containers
Bharath Vissapragada has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13997 ) Change subject: Add krb5 client utilities to the containers .. Add krb5 client utilities to the containers Some components depend on these utils (kinit, kdestroy..) for ticket cache lifecycle management. These are also useful for debugging in general, for example, to test KDC connectivity etc. Local docker image size increased from 820MB to 865MB for a release build (=5.4%). Change-Id: I9c9e9ab5b027ea9d223928280bc94f2ed9f701d3 Reviewed-on: http://gerrit.cloudera.org:8080/13997 Reviewed-by: Tim Armstrong Reviewed-by: Bharath Vissapragada Tested-by: Bharath Vissapragada --- M docker/impala_base/Dockerfile 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Tim Armstrong: Looks good to me, approved Bharath Vissapragada: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/13997 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I9c9e9ab5b027ea9d223928280bc94f2ed9f701d3 Gerrit-Change-Number: 13997 Gerrit-PatchSet: 3 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Create ranger cache directory in containers.
Bharath Vissapragada has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14007 Change subject: Create ranger cache directory in containers. .. Create ranger cache directory in containers. Create a ranger cache directory used by ranger clients when ranger is enabled. For simplicity, it is added to the base image. It is used only on the coordinators/catalogd. Change-Id: Iad134636e1566a44acf7b010e6b6067a972798c6 --- M docker/impala_base/Dockerfile 1 file changed, 3 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/14007/1 -- To view, visit http://gerrit.cloudera.org:8080/14007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iad134636e1566a44acf7b010e6b6067a972798c6 Gerrit-Change-Number: 14007 Gerrit-PatchSet: 1 Gerrit-Owner: Bharath Vissapragada
[Impala-ASF-CR] IMPALA-7935: Disable /catalog object in local catalog mode.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12443 ) Change subject: IMPALA-7935: Disable /catalog_object in local catalog mode. .. Patch Set 7: (6 comments) http://gerrit.cloudera.org:8080/#/c/12443/7/be/src/service/impala-http-handler.cc File be/src/service/impala-http-handler.cc: http://gerrit.cloudera.org:8080/#/c/12443/7/be/src/service/impala-http-handler.cc@109 PS7, Line 109: // The /catalog_object endpoint is disabled if local_catalog_mode is used. nit: add why http://gerrit.cloudera.org:8080/#/c/12443/7/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/12443/7/tests/webserver/test_web_pages.py@605 PS7, Line 605: execute_serially why? http://gerrit.cloudera.org:8080/#/c/12443/7/tests/webserver/test_web_pages.py@607 PS7, Line 607: A positive test for /catalog_ob Instead say, check /catalog_obj endpoint is not accessible or something? http://gerrit.cloudera.org:8080/#/c/12443/7/tests/webserver/test_web_pages.py@608 PS7, Line 608: http://localhost:25000 use the URL templates above? http://gerrit.cloudera.org:8080/#/c/12443/7/tests/webserver/test_web_pages.py@609 PS7, Line 609: assert 'No URI handler for '/catalog_object'' \ I'm confused, the description says something else? *not* in catalog mode? http://gerrit.cloudera.org:8080/#/c/12443/7/tests/webserver/test_web_pages.py@610 PS7, Line 610:not in response Can you also scrape the webpage source and make sure it doesn't have a "catalog_object" anywhere? -- To view, visit http://gerrit.cloudera.org:8080/12443 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia04797b32964c2edaa2e860dcf510d6f9cccd81c Gerrit-Change-Number: 12443 Gerrit-PatchSet: 7 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 09 Aug 2019 15:54:24 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8848: fix UNION missing input cardinality bug
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14036 ) Change subject: IMPALA-8848: fix UNION missing input cardinality bug .. Patch Set 2: (4 comments) lgtm, just a bunch of clarifying questions and nits. http://gerrit.cloudera.org:8080/#/c/14036/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14036/2//COMMIT_MSG@9 PS2, Line 9: If a UNION has input of unknown size, we should nit: I got confused by this statement for a minute. Rephrase may be? (to clarify that atleast one child with a valid cardinality is needed) http://gerrit.cloudera.org:8080/#/c/14036/2/fe/src/main/java/org/apache/impala/planner/UnionNode.java File fe/src/main/java/org/apache/impala/planner/UnionNode.java: http://gerrit.cloudera.org:8080/#/c/14036/2/fe/src/main/java/org/apache/impala/planner/UnionNode.java@124 PS2, Line 124: nit:..atleast..? http://gerrit.cloudera.org:8080/#/c/14036/2/fe/src/main/java/org/apache/impala/planner/UnionNode.java@127 PS2, Line 127: cardinality_ = checkedAdd(totalChildCardinality, constExprLists_.size()); nit:Preconditions.check(constExprLists_.size() > 0)? http://gerrit.cloudera.org:8080/#/c/14036/2/fe/src/test/java/org/apache/impala/planner/CardinalityTest.java File fe/src/test/java/org/apache/impala/planner/CardinalityTest.java: http://gerrit.cloudera.org:8080/#/c/14036/2/fe/src/test/java/org/apache/impala/planner/CardinalityTest.java@724 PS2, Line 724: path, UnionNode.class); Should we also add a test to cover something like select * from tbl_has_valid_cards union select * from tbl_without_valid_cards ? -- To view, visit http://gerrit.cloudera.org:8080/14036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic3ed670ffb685d8ff24824933ca303f3219737bb Gerrit-Change-Number: 14036 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 09 Aug 2019 16:25:06 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
..
Patch Set 1:
(4 comments)
lgtm overall. Some general comments.
http://gerrit.cloudera.org:8080/#/c/14012/1/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java:
http://gerrit.cloudera.org:8080/#/c/14012/1/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@462
PS1, Line 462: analysisResult_.analyzer_.checkStmtExprLimit();
nit: Probably worth mentioning that we want to enforce this before the rewrites
because rewrites are costly with long expression chains.
http://gerrit.cloudera.org:8080/#/c/14012/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java:
http://gerrit.cloudera.org:8080/#/c/14012/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@2435
PS1, Line 2435: String repCols20 = getRepeatedColumnReference("int_col",
20, true);
Verify that analyzer.numStmtExprs_ is accounted properly?
http://gerrit.cloudera.org:8080/#/c/14012/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@2493
PS1, Line 2493: StringBuilder inList = new StringBuilder();
what about constant expressions in the IN lists? foo IN (2*3, 3*4...) ? It
looks to me like they are accounted, add tests for a mix of Literal and Const
expressions?
http://gerrit.cloudera.org:8080/#/c/14012/1/tests/query_test/test_exprs.py
File tests/query_test/test_exprs.py:
http://gerrit.cloudera.org:8080/#/c/14012/1/tests/query_test/test_exprs.py@139
PS1, Line 139: # This takes 20+ minutes, so only run it on exhaustive.
Is the intention to test the default limits here? If not, we can probably set a
non-default lower expression limit to save time and memory.
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Quanlong Huang
Gerrit-Comment-Date: Fri, 09 Aug 2019 17:11:34 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7322: Add storage wait time to profile
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13786 ) Change subject: IMPALA-7322: Add storage wait time to profile .. Patch Set 1: Can you rebase? I can submit for a GVO. -- To view, visit http://gerrit.cloudera.org:8080/13786 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7447f8c8e7e50eb71d18643859d2e3de865368d2 Gerrit-Change-Number: 13786 Gerrit-PatchSet: 1 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Fri, 09 Aug 2019 17:14:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8848: fix UNION missing input cardinality bug
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14036 ) Change subject: IMPALA-8848: fix UNION missing input cardinality bug .. Patch Set 3: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/14036/2/fe/src/main/java/org/apache/impala/planner/UnionNode.java File fe/src/main/java/org/apache/impala/planner/UnionNode.java: http://gerrit.cloudera.org:8080/#/c/14036/2/fe/src/main/java/org/apache/impala/planner/UnionNode.java@127 PS2, Line 127: cardinality_ = checkedAdd(totalChildCardinality, constExprLists_.size()); > constExprLists could be empty, this calculation is correct when constExprLi nvm, not sure what I was thinking. -- To view, visit http://gerrit.cloudera.org:8080/14036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic3ed670ffb685d8ff24824933ca303f3219737bb Gerrit-Change-Number: 14036 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 10 Aug 2019 18:28:50 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 ) Change subject: IMPALA-4551: Limit the size of SQL statements .. Patch Set 5: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/14012/5/be/src/service/query-options.h File be/src/service/query-options.h: http://gerrit.cloudera.org:8080/#/c/14012/5/be/src/service/query-options.h@185 PS5, Line 185: static const int64_t SPILLABLE_BUFFER_LIMIT = 1LL << 40; // 1 TB nit: not related to your change but indents off? -- To view, visit http://gerrit.cloudera.org:8080/14012 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c Gerrit-Change-Number: 14012 Gerrit-PatchSet: 5 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Wed, 14 Aug 2019 00:02:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8864: Handle py ssl library incompatibility in http mode
Bharath Vissapragada has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14069 Change subject: IMPALA-8864: Handle py ssl library incompatibility in http mode .. IMPALA-8864: Handle py ssl library incompatibility in http mode Older python versions shipped ssl libraries that did not implement SSLContext class. THttpClient relies on it. This patch, - Fails the shell gracefully when such a python version is used. - Skips the http test dimension when running the test suite on a machine that ships such a python verison (centos 6). Change-Id: I28846bde0b8bb8f787e6330cddf91645dba4160e --- M shell/impala_client.py M tests/common/test_dimensions.py 2 files changed, 12 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/14069/1 -- To view, visit http://gerrit.cloudera.org:8080/14069 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I28846bde0b8bb8f787e6330cddf91645dba4160e Gerrit-Change-Number: 14069 Gerrit-PatchSet: 1 Gerrit-Owner: Bharath Vissapragada
[Impala-ASF-CR] IMPALA-8124: TestWebPage::test catalog should run serially.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14075 ) Change subject: IMPALA-8124: TestWebPage::test_catalog should run serially. .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/14075/1/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/14075/1/tests/webserver/test_web_pages.py@274 PS1, Line 274: @pytest.mark.execute_serially How about using unique db fixture rather than running serially? -- To view, visit http://gerrit.cloudera.org:8080/14075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I341bf25baf8d9316a21a9eff860de84b33afd12f Gerrit-Change-Number: 14075 Gerrit-PatchSet: 1 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 15 Aug 2019 21:43:19 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8661 : Add randomized tests to stress MetastoreEventsProcessor
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13932 ) Change subject: IMPALA-8661 : Add randomized tests to stress MetastoreEventsProcessor .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13932 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c85b83efd4f56b5ae0e8d1dc6a2ee2feb6721ce Gerrit-Change-Number: 13932 Gerrit-PatchSet: 8 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 15 Aug 2019 22:07:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8124: Modify TestWebPage::test catalog to avoid flakiness.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14075 ) Change subject: IMPALA-8124: Modify TestWebPage::test_catalog to avoid flakiness. .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I341bf25baf8d9316a21a9eff860de84b33afd12f Gerrit-Change-Number: 14075 Gerrit-PatchSet: 2 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 16 Aug 2019 18:35:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7935: Disable /catalog object in local catalog mode.
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/12443 )
Change subject: IMPALA-7935: Disable /catalog_object in local catalog mode.
..
Patch Set 8: Code-Review+2
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12443/8/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:
http://gerrit.cloudera.org:8080/#/c/12443/8/tests/custom_cluster/test_local_catalog.py@386
PS8, Line 386: in
impalad.service.read_debug_webpage('/catalog_object')
nit: 4 indent spaces.
http://gerrit.cloudera.org:8080/#/c/12443/8/www/catalog.tmpl
File www/catalog.tmpl:
http://gerrit.cloudera.org:8080/#/c/12443/8/www/catalog.tmpl@42
PS8, Line 42: {{?use_local_catalog}} {{name}} {{/use_local_catalog}}
nit: Add a quick comment that /catalog_object is disabled in local catalog mode?
--
To view, visit http://gerrit.cloudera.org:8080/12443
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia04797b32964c2edaa2e860dcf510d6f9cccd81c
Gerrit-Change-Number: 12443
Gerrit-PatchSet: 8
Gerrit-Owner: Anurag Mantripragada
Gerrit-Reviewer: Anurag Mantripragada
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Fri, 16 Aug 2019 18:50:49 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8872: Override httpcore from libthrift to 4.4.6
Bharath Vissapragada has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14087 Change subject: IMPALA-8872: Override httpcore from libthrift to 4.4.6 .. IMPALA-8872: Override httpcore from libthrift to 4.4.6 When deploying a cluster with Ranger publishing audits to SOLR, we noticed that the SPNEGO request consistently fails with httpcore 4.4.1 pulled in by thrift. The error goes away with httpcore v4.4.6 (manually tested). This patch overrides the dependency. Change-Id: I675356d002354b0aff439f0635e30f4610a96989 --- M fe/pom.xml 1 file changed, 9 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/14087/1 -- To view, visit http://gerrit.cloudera.org:8080/14087 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I675356d002354b0aff439f0635e30f4610a96989 Gerrit-Change-Number: 14087 Gerrit-PatchSet: 1 Gerrit-Owner: Bharath Vissapragada
[Impala-ASF-CR] IMPALA-8872: Override httpcore from libthrift to 4.4.6
Hello Vihang Karajgaonkar, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14087 to look at the new patch set (#2). Change subject: IMPALA-8872: Override httpcore from libthrift to 4.4.6 .. IMPALA-8872: Override httpcore from libthrift to 4.4.6 When deploying a cluster with Ranger publishing audits to SOLR, we noticed that the SPNEGO request consistently fails with httpcore 4.4.1 pulled in by thrift. The error goes away with httpcore v4.4.9 (manually tested). This patch overrides the dependency. Change-Id: I675356d002354b0aff439f0635e30f4610a96989 --- M fe/pom.xml 1 file changed, 16 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/14087/2 -- To view, visit http://gerrit.cloudera.org:8080/14087 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I675356d002354b0aff439f0635e30f4610a96989 Gerrit-Change-Number: 14087 Gerrit-PatchSet: 2 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-7935: Disable /catalog object in local catalog mode.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12443 ) Change subject: IMPALA-7935: Disable /catalog_object in local catalog mode. .. Patch Set 9: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12443 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia04797b32964c2edaa2e860dcf510d6f9cccd81c Gerrit-Change-Number: 12443 Gerrit-PatchSet: 9 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Sat, 17 Aug 2019 02:44:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8872: Override httpcore from libthrift to 4.4.9
Hello Lars Volker, Vihang Karajgaonkar, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14087 to look at the new patch set (#3). Change subject: IMPALA-8872: Override httpcore from libthrift to 4.4.9 .. IMPALA-8872: Override httpcore from libthrift to 4.4.9 When deploying a cluster with Ranger publishing audits to SOLR, we noticed that the SPNEGO request consistently fails with httpcore 4.4.1 pulled in by thrift. The error goes away with httpcore v4.4.9 (manually tested). This patch overrides the dependency. Change-Id: I675356d002354b0aff439f0635e30f4610a96989 --- M fe/pom.xml 1 file changed, 16 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/14087/3 -- To view, visit http://gerrit.cloudera.org:8080/14087 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I675356d002354b0aff439f0635e30f4610a96989 Gerrit-Change-Number: 14087 Gerrit-PatchSet: 3 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-8872: Override httpcore from libthrift to 4.4.9
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14087 ) Change subject: IMPALA-8872: Override httpcore from libthrift to 4.4.9 .. Patch Set 3: Code-Review+2 (2 comments) thanks for the quick review. Carrying +2. (Not rerunning the GVO because I just fixed comments, will carry it once the job finishes). http://gerrit.cloudera.org:8080/#/c/14087/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14087/2//COMMIT_MSG@7 PS2, Line 7: 4.4.9 > Should this say "4.4.9"? Done http://gerrit.cloudera.org:8080/#/c/14087/2/fe/pom.xml File fe/pom.xml: http://gerrit.cloudera.org:8080/#/c/14087/2/fe/pom.xml@361 PS2, Line 361: to ke > nit: double word Done -- To view, visit http://gerrit.cloudera.org:8080/14087 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I675356d002354b0aff439f0635e30f4610a96989 Gerrit-Change-Number: 14087 Gerrit-PatchSet: 3 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Sat, 17 Aug 2019 04:23:25 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8872: Override httpcore from libthrift to 4.4.9
Bharath Vissapragada has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14087 ) Change subject: IMPALA-8872: Override httpcore from libthrift to 4.4.9 .. IMPALA-8872: Override httpcore from libthrift to 4.4.9 When deploying a cluster with Ranger publishing audits to SOLR, we noticed that the SPNEGO request consistently fails with httpcore 4.4.1 pulled in by thrift. The error goes away with httpcore v4.4.9 (manually tested). This patch overrides the dependency. Change-Id: I675356d002354b0aff439f0635e30f4610a96989 Reviewed-on: http://gerrit.cloudera.org:8080/14087 Reviewed-by: Bharath Vissapragada Tested-by: Bharath Vissapragada --- M fe/pom.xml 1 file changed, 16 insertions(+), 0 deletions(-) Approvals: Bharath Vissapragada: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/14087 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I675356d002354b0aff439f0635e30f4610a96989 Gerrit-Change-Number: 14087 Gerrit-PatchSet: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-8872: Override httpcore from libthrift to 4.4.9
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14087 ) Change subject: IMPALA-8872: Override httpcore from libthrift to 4.4.9 .. Patch Set 3: Verified+1 Unrelated flaky test failure, merging. -- To view, visit http://gerrit.cloudera.org:8080/14087 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I675356d002354b0aff439f0635e30f4610a96989 Gerrit-Change-Number: 14087 Gerrit-PatchSet: 3 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Sat, 17 Aug 2019 15:11:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7935: Disable /catalog object in local catalog mode.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12443 ) Change subject: IMPALA-7935: Disable /catalog_object in local catalog mode. .. Patch Set 11: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12443 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia04797b32964c2edaa2e860dcf510d6f9cccd81c Gerrit-Change-Number: 12443 Gerrit-PatchSet: 11 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 20 Aug 2019 05:28:05 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14106
to look at the new patch set (#2).
Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz
..
[WIP] IMPALA-8228: Ownership support for Ranger authz
Without this patch, explicit privileges are needed even
for owners of databases/tables to perform actions on them.
Example: 'user' is the owner of database 'foo'. To create
a table 't' under 'foo', 'user' needs to be granted a CREATE
privilege on 'foo'
That is unintuitive from a user POV since users expect owners
to have ALL privileges on the objects they own. This patch extends
that support to Impala's ranger authorization plugin.
Ranger natively supports the concept of ownership by letting the
callers pass the ownership context to RangerAccessResourceImpl.
This patch plumbs the owner information for the authorizables
(currently only supported for Tables / Databases) which is then
evaulated during authorization.
For the ownership based authorization to work, ranger-admin side
policy on {OWNER} user needs to be defined.
(TODO) Working on tests.
Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/authorization/Authorizable.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableDb.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
M
fe/src/main/java/org/apache/impala/authorization/DefaultAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaResourceBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
14 files changed, 107 insertions(+), 46 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/14106/2
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 2
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Hello Austin Nobis, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14106
to look at the new patch set (#3).
Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz
..
[WIP] IMPALA-8228: Ownership support for Ranger authz
Without this patch, explicit privileges are needed even
for owners of databases/tables to perform actions on them.
Example: 'user' is the owner of database 'foo'. To create
a table 't' under 'foo', 'user' needs to be granted a CREATE
privilege on 'foo'
That is unintuitive from a user POV since users expect owners
to have ALL privileges on the objects they own. This patch extends
that support to Impala's ranger authorization plugin.
Ranger natively supports the concept of ownership by letting the
callers pass the ownership context to RangerAccessResourceImpl.
This patch plumbs the owner information for the authorizables
(currently only supported for Tables / Databases) which is then
evaulated during authorization.
For the ownership based authorization to work, ranger-admin side
policy on {OWNER} user needs to be defined.
(TODO) Working on tests.
Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/authorization/Authorizable.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableColumn.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableDb.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
M
fe/src/main/java/org/apache/impala/authorization/DefaultAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaResourceBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/FeDb.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
25 files changed, 250 insertions(+), 85 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/14106/3
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 3
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 3: Oops I didn't mean to push this out for review, still haven't addressed the comments. -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 3 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 20 Aug 2019 20:52:22 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Hello Austin Nobis, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14106
to look at the new patch set (#4).
Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz
..
[WIP] IMPALA-8228: Ownership support for Ranger authz
Without this patch, explicit privileges are needed even
for owners of databases/tables to perform actions on them.
Example: 'user' is the owner of database 'foo'. To create
a table 't' under 'foo', 'user' needs to be granted a CREATE
privilege on 'foo'
That is unintuitive from a user POV since users expect owners
to have ALL privileges on the objects they own. This patch extends
that support to Impala's ranger authorization plugin.
Ranger natively supports the concept of ownership by letting the
callers pass the ownership context to RangerAccessResourceImpl.
This patch plumbs the owner information for the authorizables
(currently only supported for Tables / Databases) which is then
evaulated during authorization.
For the ownership based authorization to work, ranger-admin side
policy on {OWNER} user needs to be defined.
(TODO) Working on tests.
Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/authorization/Authorizable.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableColumn.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableDb.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
M
fe/src/main/java/org/apache/impala/authorization/DefaultAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaResourceBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/FeDb.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
25 files changed, 233 insertions(+), 85 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/14106/4
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 4
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/14106 )
Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz
..
Patch Set 4:
(13 comments)
Still working on tests. Meanwhile kicking off a test run to see what fails.
http://gerrit.cloudera.org:8080/#/c/14106/2/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/14106/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2673
PS2, Line 2673: Preconditions.checkNotNull(privilege);
> we seem to have lost the "checkNotNull' here? was that intentional?
Done
http://gerrit.cloudera.org:8080/#/c/14106/3/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/14106/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2818
PS3, Line 2818: } else {
> line too long (91 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2819
PS3, Line 2819: // Table does not exist and hence the owner information
cannot be deduced.
> line too long (92 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
File fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java:
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@138
PS2, Line 138:
> this could be null in the case of non-table-specific statements, which seem
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@189
PS2, Line 189: databa
> this should be 'database_' right?
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java:
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java@35
PS2, Line 35:
> mind adding @Nullable annotations here and below, if this is allowed to be
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java:
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java@30
PS2, Line 30: private final String tableName_;
> how about using @Nullable on this?
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java@37
PS2, Line 37: Preconditions.checkArgument(ownerUser == null ||
!ownerUser.isEmpty());
> would an empty owner string be valid? maybe we should be checking that it's
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java@55
PS2, Line 55: @Override
> this is @Override right?
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
File
fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java:
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java@71
PS2, Line 71:
> mind giving this a more explicit name like 'onTableWithUnknownOwner' or som
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java@74
PS2, Line 74: public PrivilegeRequestBuilder onTable(
> typo
Done
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
File
fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java:
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java@79
PS3, Line 79: }
> line too long (93 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/14106/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/14106/3/fe/src/main/java/org/apache/impala/service/Frontend.java@786
PS3, Line 786: String tableOwner = table.getOwnerUser();
> line too long (96 > 90)
Done
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 4
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Bharath Vissap
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Hello Austin Nobis, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14106
to look at the new patch set (#5).
Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz
..
[WIP] IMPALA-8228: Ownership support for Ranger authz
Without this patch, explicit privileges are needed even
for owners of databases/tables to perform actions on them.
Example: 'user' is the owner of database 'foo'. To create
a table 't' under 'foo', 'user' needs to be granted a CREATE
privilege on 'foo'
That is unintuitive from a user POV since users expect owners
to have ALL privileges on the objects they own. This patch extends
that support to Impala's ranger authorization plugin.
Ranger natively supports the concept of ownership by letting the
callers pass the ownership context to RangerAccessResourceImpl.
This patch plumbs the owner information for the authorizables
(currently only supported for Tables / Databases) which is then
evaulated during authorization.
For the ownership based authorization to work, ranger-admin side
policy on {OWNER} user needs to be defined.
(TODO) Working on tests.
Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/authorization/Authorizable.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableColumn.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableDb.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
M
fe/src/main/java/org/apache/impala/authorization/DefaultAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaResourceBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/FeDb.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
25 files changed, 237 insertions(+), 88 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/14106/5
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 5
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8864: Ignore fe http transport tests on older python versions
Bharath Vissapragada has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14116 Change subject: IMPALA-8864: Ignore fe http transport tests on older python versions .. IMPALA-8864: Ignore fe http transport tests on older python versions A follow up commit on IMPALA-8864. A couple of fe junit tests also run shell hs2-http tests that are failing on older python versions. This patch identifies such older python versions and skips the problematic tests. Change-Id: I1449965ba6eb1bca0e64c56065c453ed8c7c46a7 --- M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java 1 file changed, 28 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/14116/1 -- To view, visit http://gerrit.cloudera.org:8080/14116 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I1449965ba6eb1bca0e64c56065c453ed8c7c46a7 Gerrit-Change-Number: 14116 Gerrit-PatchSet: 1 Gerrit-Owner: Bharath Vissapragada
[Impala-ASF-CR] IMPALA-8864: Ignore fe http transport tests on older python versions
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14116 ) Change subject: IMPALA-8864: Ignore fe http transport tests on older python versions .. Patch Set 1: I'm running a private build on centos6. Also kicking off a GVO. -- To view, visit http://gerrit.cloudera.org:8080/14116 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1449965ba6eb1bca0e64c56065c453ed8c7c46a7 Gerrit-Change-Number: 14116 Gerrit-PatchSet: 1 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 21 Aug 2019 17:28:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8864: Ignore fe http transport tests on older python versions
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14116 ) Change subject: IMPALA-8864: Ignore fe http transport tests on older python versions .. Patch Set 1: Code-Review-2 Thanks for the quick review. I'm temporarily marking this as a -2 until my private CentOs6 build finishes. Want to be sure that this fix works as intended. -- To view, visit http://gerrit.cloudera.org:8080/14116 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1449965ba6eb1bca0e64c56065c453ed8c7c46a7 Gerrit-Change-Number: 14116 Gerrit-PatchSet: 1 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 21 Aug 2019 20:48:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8864: Ignore fe http transport tests on older python versions
Bharath Vissapragada has removed a vote on this change. Change subject: IMPALA-8864: Ignore fe http transport tests on older python versions .. Removed Code-Review-2 by Bharath Vissapragada -- To view, visit http://gerrit.cloudera.org:8080/14116 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I1449965ba6eb1bca0e64c56065c453ed8c7c46a7 Gerrit-Change-Number: 14116 Gerrit-PatchSet: 1 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-8864: Ignore fe http transport tests on older python versions
Bharath Vissapragada has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14116 ) Change subject: IMPALA-8864: Ignore fe http transport tests on older python versions .. IMPALA-8864: Ignore fe http transport tests on older python versions A follow up commit on IMPALA-8864. A couple of fe junit tests also run shell hs2-http tests that are failing on older python versions. This patch identifies such older python versions and skips the problematic tests. Change-Id: I1449965ba6eb1bca0e64c56065c453ed8c7c46a7 Reviewed-on: http://gerrit.cloudera.org:8080/14116 Reviewed-by: Thomas Tauber-Marshall Tested-by: Impala Public Jenkins --- M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java 1 file changed, 28 insertions(+), 4 deletions(-) Approvals: Thomas Tauber-Marshall: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/14116 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I1449965ba6eb1bca0e64c56065c453ed8c7c46a7 Gerrit-Change-Number: 14116 Gerrit-PatchSet: 2 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-7604: part 1: tests for agg cardinality
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14131 ) Change subject: IMPALA-7604: part 1: tests for agg cardinality .. Patch Set 2: Code-Review+2 Nice, thanks for adding some test coverage. -- To view, visit http://gerrit.cloudera.org:8080/14131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I59eaddbc5be253793293af064bb2d28a425564e1 Gerrit-Change-Number: 14131 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 26 Aug 2019 16:11:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8797: Support database and table blacklist
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/14134 )
Change subject: IMPALA-8797: Support database and table blacklist
..
Patch Set 4:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/14134/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14134/4//COMMIT_MSG@9
PS4, Line 9: Add catalogd startup option for database and table blacklist.
Blacklist
Can we also add them to the coordinator startup flags and intercept any
references to them in the analysis pass? (we have code for validating table and
db names, so we can include this check too probably)
For example: throw an AnalysisException that user cannot create tables with
names in the blacklisted set?
http://gerrit.cloudera.org:8080/#/c/14134/4/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:
http://gerrit.cloudera.org:8080/#/c/14134/4/be/src/common/global-flags.cc@253
PS4, Line 253:
nit: blacklisted (multiple places)
http://gerrit.cloudera.org:8080/#/c/14134/4/be/src/common/global-flags.cc@256
PS4, Line 256: "Comma separated list for full names of blacklist tables.
Configure which tables to "
nit: Define the format to be clear? . ?
http://gerrit.cloudera.org:8080/#/c/14134/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/14134/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@274
PS4, Line 274: protected final Map> blacklistTables_ =
Maps.newHashMap();
nit: you can also do a Set
http://gerrit.cloudera.org:8080/#/c/14134/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@290
PS4, Line 290: blacklistDbs_, blacklistTables_);
why pass class members? I'm guessing it is for unit tests, mention that?
http://gerrit.cloudera.org:8080/#/c/14134/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@332
PS4, Line 332: public static void loadBlacklist(String blacklistDbsConfig,
nit: call parseBlackListedDbsAndTbls() or something similar?
http://gerrit.cloudera.org:8080/#/c/14134/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@335
PS4, Line 335: for (String db: blacklistDbsConfig.trim().split(",\\s*")) {
nit: Splitter is a neater way of doing the same thing. It has helpers to trim,
omitEmptyStrings etc..
https://guava.dev/releases/23.0/api/docs/com/google/common/base/Splitter.html
http://gerrit.cloudera.org:8080/#/c/14134/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@338
PS4, Line 338: if (LOG.isTraceEnabled()) {
nit: I think this can be info, since this is once per process lifetime and is
critical piece of information.
http://gerrit.cloudera.org:8080/#/c/14134/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@356
PS4, Line 356: continue;
worthy of logging?
http://gerrit.cloudera.org:8080/#/c/14134/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2121
PS4, Line 2121: return null;
Like I mentioned in the commit message, I think we should incorporate checks in
the analysis pass to prevent analysis on any table ref / db matching
blacklists. Thoughts?
--
To view, visit http://gerrit.cloudera.org:8080/14134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I02dbb07f8e08793b57b2a88d09b30fd32cff26dc
Gerrit-Change-Number: 14134
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Comment-Date: Mon, 26 Aug 2019 16:44:53 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8572: Log query events before Unregister() call.
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14143 to look at the new patch set (#4). Change subject: IMPALA-8572: Log query events before Unregister() call. .. IMPALA-8572: Log query events before Unregister() call. Currently, the query events (audits and lineages) are logged as a part of query unregistration. This delays the event logging in cases where the Unregister() is delayed by client for some reason (ex: Hue does not call Unregister until the browser tab is closed) or the client goes away without calling Unregister and the query timeout kicks in. This patch moves this event logging to an earlier stage in the query lifecycle while still preserving the current audit/lineage logging guarantees. Moved the event logging related code into ClientRequestState for easier code refactoring. Added some test coverage for coordinator side code paths for writing lineages. fe specific lineage tests only verified the correctness of lineage created but did not test whether it was being flushed correctly to the disk. Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e --- M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/impala-server.cc M be/src/service/impala-server.h A testdata/workloads/functional-query/queries/QueryTest/lineage.test M tests/common/impala_test_suite.py M tests/common/test_result_verifier.py M tests/custom_cluster/test_lineage.py M tests/unittests/test_file_parser.py M tests/util/test_file_parser.py 10 files changed, 6,106 insertions(+), 219 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/14143/4 -- To view, visit http://gerrit.cloudera.org:8080/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8572: Log query events before Unregister() call.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14143 ) Change subject: IMPALA-8572: Log query events before Unregister() call. .. Patch Set 4: I think this is in a decent shape that is ready for review. It could use some more test coverage (like in query failure cases leading to authorization errors) and some test perf improvements like making the new custom cluster test faster (or) not running it for all vectors (or) running it only in exhaustive mode etc. I'd like to get some initial feedback on the approach before I fix these items. -- To view, visit http://gerrit.cloudera.org:8080/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 27 Aug 2019 18:45:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8797: Support database and table blacklist
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/14134 )
Change subject: IMPALA-8797: Support database and table blacklist
..
Patch Set 7:
(13 comments)
Couple of requests for test coverage.
- Add more fe unit tests on default sys/information schema? (one can't
create/alter/access them).
- Add a GET_SCHEMAS/DBs test that it doesn't return these databases by default
(this is the source of this enhancement request, so just want to be sure there
is some coverage for that).
http://gerrit.cloudera.org:8080/#/c/14134/7/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:
http://gerrit.cloudera.org:8080/#/c/14134/7/be/src/common/global-flags.cc@253
PS7, Line 253: blacklist
nit:blacklisted
http://gerrit.cloudera.org:8080/#/c/14134/7/be/src/common/global-flags.cc@254
PS7, Line 254: Users don't see them when querying the database list.
nit:Instead mention that users cannot query or access these databases? (same
below)
http://gerrit.cloudera.org:8080/#/c/14134/7/be/src/common/global-flags.cc@253
PS7, Line 253: Configure which databases to be "
: "skipped for loading
nit: I think this is obvious, can be skipped.
http://gerrit.cloudera.org:8080/#/c/14134/7/fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java:
http://gerrit.cloudera.org:8080/#/c/14134/7/fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java@36
PS7, Line 36: private final static Set BLACKLISTED_DBS =
BlacklistUtils.parseBlacklistedDbs(
Also consider moving this to Analyzer and you can have a helper
Analyzer#isValidDbName() or something like that.
http://gerrit.cloudera.org:8080/#/c/14134/7/fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java@37
PS7, Line 37: BackendConfig.INSTANCE == null ? "" :
BackendConfig.INSTANCE.getBlacklistedDbs(),
: null
fold this logic into parseBlack... ?
http://gerrit.cloudera.org:8080/#/c/14134/7/fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java@102
PS7, Line 102: Can't use blacklisted database name: " + dbName_
Keep the message consistent with above? "Invalid database name. It has been
blacklisted using --black_" or something like that.
http://gerrit.cloudera.org:8080/#/c/14134/7/fe/src/main/java/org/apache/impala/analysis/TableName.java
File fe/src/main/java/org/apache/impala/analysis/TableName.java:
http://gerrit.cloudera.org:8080/#/c/14134/7/fe/src/main/java/org/apache/impala/analysis/TableName.java@42
PS7, Line 42: private final static Set BLACKLISTED_TABLES =
Consider putting a copy of this in Analyzer rather than using it in multiple
places?
http://gerrit.cloudera.org:8080/#/c/14134/7/fe/src/main/java/org/apache/impala/analysis/TableName.java@87
PS7, Line 87: verify
I think the code generally calls it "analyze()". Keep it consistent?
http://gerrit.cloudera.org:8080/#/c/14134/7/fe/src/main/java/org/apache/impala/analysis/TableName.java@90
PS7, Line 90: throw new AnalysisException("Invalid database name: " +
db_);
Do we need to check if the db_ is not black listed?
http://gerrit.cloudera.org:8080/#/c/14134/7/fe/src/main/java/org/apache/impala/analysis/TableName.java@97
PS7, Line 97: Can't use blacklisted table nam
nit: Keep it consistent with above?
http://gerrit.cloudera.org:8080/#/c/14134/7/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/14134/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@70
PS7, Line 70: import org.apache.impala.catalog.DatabaseNotFoundException;
Convert all the checks in here to Preconditions? If all the right Analyzer
checks are in place, we'd never pass blacklisted db/tables until this point.
http://gerrit.cloudera.org:8080/#/c/14134/7/fe/src/main/java/org/apache/impala/util/BlacklistUtils.java
File fe/src/main/java/org/apache/impala/util/BlacklistUtils.java:
http://gerrit.cloudera.org:8080/#/c/14134/7/fe/src/main/java/org/apache/impala/util/BlacklistUtils.java@22
PS7, Line 22: blacklistedDbsConfig
nit: checkNotNull()? (same below)
http://gerrit.cloudera.org:8080/#/c/14134/7/tests/custom_cluster/test_blacklisted_dbs_and_tables.py
File tests/custom_cluster/test_blacklisted_dbs_and_tables.py:
http://gerrit.cloudera.org:8080/#/c/14134/7/tests/custom_cluster/test_blacklisted_dbs_and_tables.py@46
PS7, Line 46: visable
typo (multiple places)
--
To view, visit http://gerrit.cloudera.org:8080/14134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I02dbb07f8e08793b57b2a88d09b30fd32cff26dc
Gerrit-Change-Number: 14134
Gerrit-PatchSet: 7
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public
[Impala-ASF-CR] IMPALA-8797: Support database and table blacklist
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14134 ) Change subject: IMPALA-8797: Support database and table blacklist .. Patch Set 7: I'm out for a week or so and won't be able to review this change until then. Feel free to find a reviewer who can get this merged if it becomes time sensitive. -- To view, visit http://gerrit.cloudera.org:8080/14134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I02dbb07f8e08793b57b2a88d09b30fd32cff26dc Gerrit-Change-Number: 14134 Gerrit-PatchSet: 7 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 27 Aug 2019 19:17:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7604: part 2: fixes for AggregationNode cardinality
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14132 ) Change subject: IMPALA-7604: part 2: fixes for AggregationNode cardinality .. Patch Set 6: Code-Review+1 (4 comments) Patch and the resulting test file changes make sense to me. I'll let Bikram do another pass if he has any comments. http://gerrit.cloudera.org:8080/#/c/14132/6/fe/src/main/java/org/apache/impala/analysis/Expr.java File fe/src/main/java/org/apache/impala/analysis/Expr.java: http://gerrit.cloudera.org:8080/#/c/14132/6/fe/src/main/java/org/apache/impala/analysis/Expr.java@789 PS6, Line 789: nit: Uses? http://gerrit.cloudera.org:8080/#/c/14132/6/fe/src/main/java/org/apache/impala/planner/AggregationNode.java File fe/src/main/java/org/apache/impala/planner/AggregationNode.java: http://gerrit.cloudera.org:8080/#/c/14132/6/fe/src/main/java/org/apache/impala/planner/AggregationNode.java@210 PS6, Line 210: This is prone to overflow, nit: update? http://gerrit.cloudera.org:8080/#/c/14132/6/fe/src/main/java/org/apache/impala/planner/AggregationNode.java@259 PS6, Line 259: // Note that this will still be -1 if the child's cardinality is unknown. nit: Do we need some trace logging, just in case? http://gerrit.cloudera.org:8080/#/c/14132/6/testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by.test File testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by.test: http://gerrit.cloudera.org:8080/#/c/14132/6/testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by.test@14 PS6, Line 14: HDFS Just curious why did this change everywhere? Someone forgot to update the test files in an unrelated change? -- To view, visit http://gerrit.cloudera.org:8080/14132 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieed41d60c0e0dfeca64035e919cb8c28a054a9ab Gerrit-Change-Number: 14132 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 27 Aug 2019 20:52:56 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8797: Support database and table blacklist
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/14134 )
Change subject: IMPALA-8797: Support database and table blacklist
..
Patch Set 8:
(5 comments)
lgtm barring some code refactoring suggestions. I think the issue with sys and
information_schema not appearing is probably something to do with the version
of hive pinned in the toolchain. Unfortunately we have to complicate the tests
because of that. I think that shouldn't be a blocker given we have custom
cluster tests for it and we can always fix them in the follow up commits.
http://gerrit.cloudera.org:8080/#/c/14134/8/fe/src/main/java/org/apache/impala/analysis/TableName.java
File fe/src/main/java/org/apache/impala/analysis/TableName.java:
http://gerrit.cloudera.org:8080/#/c/14134/8/fe/src/main/java/org/apache/impala/analysis/TableName.java@86
PS8, Line 86: throw new AnalysisException("Invalid table/view name: " +
tbl_);
Is there a reason not to fold the Blacklist.verifyTableName() into
TableName#analyze()? In other words, are there places where we want to skip the
blacklist check for whatever reason?
http://gerrit.cloudera.org:8080/#/c/14134/8/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/14134/8/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@270
PS8, Line 270: // Error string for inconsistent blacklisted dbs/tables
configs between catalogd and
Yea, we don't have any checks that validate the configs across roles. That is
likely to be a bigger change.
http://gerrit.cloudera.org:8080/#/c/14134/8/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1519
PS8, Line 1519: addSummary(resp, "Can't drop blacklisted database: " +
dbName);
Add a test for this?
http://gerrit.cloudera.org:8080/#/c/14134/8/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1644
PS8, Line 1644:
Similar check here?
http://gerrit.cloudera.org:8080/#/c/14134/8/fe/src/main/java/org/apache/impala/util/BlacklistUtils.java
File fe/src/main/java/org/apache/impala/util/BlacklistUtils.java:
http://gerrit.cloudera.org:8080/#/c/14134/8/fe/src/main/java/org/apache/impala/util/BlacklistUtils.java@32
PS8, Line 32: BlacklistUtils
nit: Call this CatalogUtils or something? BlackList seems out of place, does
not convey what is being blacklisted.
--
To view, visit http://gerrit.cloudera.org:8080/14134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I02dbb07f8e08793b57b2a88d09b30fd32cff26dc
Gerrit-Change-Number: 14134
Gerrit-PatchSet: 8
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Comment-Date: Tue, 03 Sep 2019 21:57:02 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8797: Support database and table blacklist
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/14134 )
Change subject: IMPALA-8797: Support database and table blacklist
..
Patch Set 9:
(3 comments)
lgtm, can +2 it once the TableName#analyze() is refactored to include the
blacklist checks.
http://gerrit.cloudera.org:8080/#/c/14134/8/fe/src/main/java/org/apache/impala/analysis/TableName.java
File fe/src/main/java/org/apache/impala/analysis/TableName.java:
http://gerrit.cloudera.org:8080/#/c/14134/8/fe/src/main/java/org/apache/impala/analysis/TableName.java@86
PS8, Line 86: throw new AnalysisException("Invalid table/view name: " +
tbl_);
> I think they should always be called together. But BlacklistUtils.verifyTab
I think we can add the getFqTableName() logic into
BlackListUtils.verifyTableName().
My concern with separating these too is that if someone adds a new Statement in
the future that involves a tableName and they forget to to call
BlackListUtils.verify(), we might run into issues. Instead folding that logic
into TableName#analyze() makes life easier.
http://gerrit.cloudera.org:8080/#/c/14134/8/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/14134/8/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1519
PS8, Line 1519: addSummary(resp, "Can't drop blacklisted database: " +
dbName);
> This is tested in tests/custom_cluster/test_blacklisted_dbs_and_tables.py:
ack, sorry missed it.
http://gerrit.cloudera.org:8080/#/c/14134/8/fe/src/main/java/org/apache/impala/util/BlacklistUtils.java
File fe/src/main/java/org/apache/impala/util/BlacklistUtils.java:
http://gerrit.cloudera.org:8080/#/c/14134/8/fe/src/main/java/org/apache/impala/util/BlacklistUtils.java@32
PS8, Line 32:
> Sure. What about CatalogBlacklistUtils?
sgtm
--
To view, visit http://gerrit.cloudera.org:8080/14134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I02dbb07f8e08793b57b2a88d09b30fd32cff26dc
Gerrit-Change-Number: 14134
Gerrit-PatchSet: 9
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Comment-Date: Wed, 04 Sep 2019 17:47:51 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8572: Log query events before Unregister() call.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14143 ) Change subject: IMPALA-8572: Log query events before Unregister() call. .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/14143/4//COMMIT_MSG Commit Message: PS4: > I had a couple of high level questions. 1) I've spent some time looking at it, especially reading git blame and older jiras, but I didn't totally understand why a fetch was necessary. I didn't see an obvious downside either. So I ended up preserving the existing functionality. 2) Isn't that a deadlock? FetchInternal() blocks on Wait(), Wait() blocks on LogQueryEvents() which blocks on first fetch. Am I missing something? -- To view, visit http://gerrit.cloudera.org:8080/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Thu, 05 Sep 2019 00:18:05 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8797: Support database and table blacklist
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/14134 )
Change subject: IMPALA-8797: Support database and table blacklist
..
Patch Set 10:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14134/10/fe/src/main/java/org/apache/impala/util/CatalogBlacklistUtils.java
File fe/src/main/java/org/apache/impala/util/CatalogBlacklistUtils.java:
http://gerrit.cloudera.org:8080/#/c/14134/10/fe/src/main/java/org/apache/impala/util/CatalogBlacklistUtils.java@104
PS10, Line 104: if (BLACKLISTED_TABLES.contains(table)) {
Can we just do TableName temp = analyzer.getFqTableName(table);
if (BLACKLISTED_TABLES.contains(temp)) {
.
That saves all the callers from doing
analyzer.getFqTableName(tableName_).analyze();
--
To view, visit http://gerrit.cloudera.org:8080/14134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I02dbb07f8e08793b57b2a88d09b30fd32cff26dc
Gerrit-Change-Number: 14134
Gerrit-PatchSet: 10
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Comment-Date: Thu, 05 Sep 2019 00:46:07 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8797: Support database and table blacklist
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/14134 )
Change subject: IMPALA-8797: Support database and table blacklist
..
Patch Set 10: Code-Review+2
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14134/10/fe/src/main/java/org/apache/impala/analysis/TableName.java
File fe/src/main/java/org/apache/impala/analysis/TableName.java:
http://gerrit.cloudera.org:8080/#/c/14134/10/fe/src/main/java/org/apache/impala/analysis/TableName.java@81
PS10, Line 81: Preconditions.checkNotNull(db_);
Like I mentioned in the other comment,
Preconditions.checkState(isFullyQualified())?
http://gerrit.cloudera.org:8080/#/c/14134/10/fe/src/main/java/org/apache/impala/util/CatalogBlacklistUtils.java
File fe/src/main/java/org/apache/impala/util/CatalogBlacklistUtils.java:
http://gerrit.cloudera.org:8080/#/c/14134/10/fe/src/main/java/org/apache/impala/util/CatalogBlacklistUtils.java@104
PS10, Line 104: if (BLACKLISTED_TABLES.contains(table)) {
> But there are no "analyzer" object here... Even if we move this "verifyTabl
Ah, I see what you are saying. I think thats a deep refactor. Suggested a nit
in the Preconditions check.
--
To view, visit http://gerrit.cloudera.org:8080/14134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I02dbb07f8e08793b57b2a88d09b30fd32cff26dc
Gerrit-Change-Number: 14134
Gerrit-PatchSet: 10
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Comment-Date: Thu, 05 Sep 2019 02:29:24 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8572: Log query events before unregister.
Hello radford nguyen, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14143 to look at the new patch set (#5). Change subject: IMPALA-8572: Log query events before unregister. .. IMPALA-8572: Log query events before unregister. Currently, the query events (audits and lineages) are logged as a part of query unregistration. This delays the event logging in cases where the Unregister() is delayed by client for some reason (ex: Hue does not call Unregister until the browser tab is closed) or the client goes away without calling Unregister and the query timeout kicks in. This patch moves this event logging to an earlier stage in the query lifecycle. Moved the event logging related code into ClientRequestState for easier code refactoring. The conditions under which the events are logged are slightly modified by this patch. Without the patch, events are logged for unsuccessful queries if atleast a single fetch is perfomed. This patch relaxes this guarantee to log events for any query that reaches the FINISHED state (rows are available to fetch by the client) and does not wait for a fetch to be performed. This simplifies the coordinator state machine by avoid unnecessary synchronization. Added some test coverage for coordinator side code paths for writing lineages. fe specific lineage tests only verified the correctness of lineage created but did not test whether it was being flushed correctly to the disk. Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e --- M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/impala-server.cc M be/src/service/impala-server.h A testdata/workloads/functional-query/queries/QueryTest/lineage.test M tests/common/impala_test_suite.py M tests/common/test_result_verifier.py M tests/custom_cluster/test_lineage.py M tests/unittests/test_file_parser.py M tests/util/test_file_parser.py 10 files changed, 6,059 insertions(+), 239 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/14143/5 -- To view, visit http://gerrit.cloudera.org:8080/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 5 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: radford nguyen
[Impala-ASF-CR] IMPALA-8572: Log query events before unregister.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14143 ) Change subject: IMPALA-8572: Log query events before unregister. .. Patch Set 4: (6 comments) http://gerrit.cloudera.org:8080/#/c/14143/4//COMMIT_MSG Commit Message: PS4: > 1) the main reason to change the behaviour in this patch would be to simpli Done http://gerrit.cloudera.org:8080/#/c/14143/5/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: http://gerrit.cloudera.org:8080/#/c/14143/5/tests/common/impala_test_suite.py@335 PS5, Line 335: ; > flake8: E703 statement ends with a semicolon Done http://gerrit.cloudera.org:8080/#/c/14143/5/tests/common/impala_test_suite.py@646 PS5, Line 646: ) > flake8: E501 line too long (91 > 90 characters) Done http://gerrit.cloudera.org:8080/#/c/14143/5/tests/common/test_result_verifier.py File tests/common/test_result_verifier.py: http://gerrit.cloudera.org:8080/#/c/14143/5/tests/common/test_result_verifier.py@22 PS5, Line 22: import json > flake8: F401 'json' imported but unused Done http://gerrit.cloudera.org:8080/#/c/14143/5/tests/common/test_result_verifier.py@631 PS5, Line 631: def verify_lineage(expected, actual, lineage_skip_json_keys=DEFAULT_LINEAGE_SKIP_KEYS): > flake8: E302 expected 2 blank lines, found 1 Done http://gerrit.cloudera.org:8080/#/c/14143/5/tests/common/test_result_verifier.py@635 PS5, Line 635: p > flake8: E501 line too long (102 > 90 characters) Done -- To view, visit http://gerrit.cloudera.org:8080/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Thu, 05 Sep 2019 23:44:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8572: Log query events before unregister.
Hello radford nguyen, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14143 to look at the new patch set (#6). Change subject: IMPALA-8572: Log query events before unregister. .. IMPALA-8572: Log query events before unregister. Currently, the query events (audits and lineages) are logged as a part of query unregistration. This delays the event logging in cases where the Unregister() is delayed by client for some reason (ex: Hue does not call Unregister until the browser tab is closed) or the client goes away without calling Unregister and the query timeout kicks in. This patch moves this event logging to an earlier stage in the query lifecycle. Moved the event logging related code into ClientRequestState for easier code refactoring. The conditions under which the events are logged are slightly modified by this patch. Without the patch, events are logged for unsuccessful queries if atleast a single fetch is perfomed. This patch relaxes this guarantee to log events for any query that reaches the FINISHED state (rows are available to fetch by the client) and does not wait for a fetch to be performed. This simplifies the coordinator state machine by avoiding unnecessary synchronization. Added some test coverage for coordinator side code paths for writing lineages. fe specific lineage tests only verified the correctness of lineage created but did not test whether it was being flushed correctly to the disk. Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e --- M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/impala-server.cc M be/src/service/impala-server.h A testdata/workloads/functional-query/queries/QueryTest/lineage.test M tests/common/impala_test_suite.py M tests/common/test_result_verifier.py M tests/custom_cluster/test_lineage.py M tests/unittests/test_file_parser.py M tests/util/test_file_parser.py 10 files changed, 6,061 insertions(+), 239 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/14143/6 -- To view, visit http://gerrit.cloudera.org:8080/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 6 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: radford nguyen
[Impala-ASF-CR] IMPALA-8572: Log query events before unregister.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14143 ) Change subject: IMPALA-8572: Log query events before unregister. .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/14143/6/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: http://gerrit.cloudera.org:8080/#/c/14143/6/tests/common/impala_test_suite.py@646 PS6, Line 646: i > flake8: F821 undefined name 'i' aargh, vim troubles, extraneous character. -- To view, visit http://gerrit.cloudera.org:8080/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 6 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Fri, 06 Sep 2019 00:27:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8572: Log query events before unregister.
Hello radford nguyen, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14143 to look at the new patch set (#7). Change subject: IMPALA-8572: Log query events before unregister. .. IMPALA-8572: Log query events before unregister. Currently, the query events (audits and lineages) are logged as a part of query unregistration. This delays the event logging in cases where the Unregister() is delayed by client for some reason (ex: Hue does not call Unregister until the browser tab is closed) or the client goes away without calling Unregister and the query timeout kicks in. This patch moves this event logging to an earlier stage in the query lifecycle. Moved the event logging related code into ClientRequestState for easier code refactoring. The conditions under which the events are logged are slightly modified by this patch. Without the patch, events are logged for unsuccessful queries if atleast a single fetch is perfomed. This patch relaxes this guarantee to log events for any query that reaches the FINISHED state (rows are available to fetch by the client) and does not wait for a fetch to be performed. This simplifies the coordinator state machine by avoiding unnecessary synchronization. Added some test coverage for coordinator side code paths for writing lineages. fe specific lineage tests only verified the correctness of lineage created but did not test whether it was being flushed correctly to the disk. Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e --- M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/impala-server.cc M be/src/service/impala-server.h A testdata/workloads/functional-query/queries/QueryTest/lineage.test M tests/common/impala_test_suite.py M tests/common/test_result_verifier.py M tests/custom_cluster/test_lineage.py M tests/unittests/test_file_parser.py M tests/util/test_file_parser.py 10 files changed, 6,060 insertions(+), 239 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/14143/7 -- To view, visit http://gerrit.cloudera.org:8080/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 7 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: radford nguyen
[Impala-ASF-CR] IMPALA-8921: User short name for Ranger grant/revoke requests
Bharath Vissapragada has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14185 Change subject: IMPALA-8921: User short name for Ranger grant/revoke requests .. IMPALA-8921: User short name for Ranger grant/revoke requests For certain grant/revoke Ranger commmands, we ended up passing the full name which is a problem when kerberos is enabled. Ranger expects the short name during authorization. Testing: We do not have test coverage with kerberos enabled, so I inspected the code manually to make sure we are using getShortName() everywhere. Change-Id: I3dc1bf55d50dc2e5fa6e07f16644f0a2773f2d23 --- M common/thrift/CatalogService.thrift M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java 2 files changed, 6 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/85/14185/1 -- To view, visit http://gerrit.cloudera.org:8080/14185 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I3dc1bf55d50dc2e5fa6e07f16644f0a2773f2d23 Gerrit-Change-Number: 14185 Gerrit-PatchSet: 1 Gerrit-Owner: Bharath Vissapragada
