[Impala-ASF-CR] IMPALA-6479: Update DESCRIBE to respect column privileges
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9276 ) Change subject: IMPALA-6479: Update DESCRIBE to respect column privileges .. Patch Set 2: (8 comments) http://gerrit.cloudera.org:8080/#/c/9276/2/be/src/service/frontend.h File be/src/service/frontend.h: http://gerrit.cloudera.org:8080/#/c/9276/2/be/src/service/frontend.h@129 PS2, Line 129: /// be set to the user's current session. If this is an Impala internal request, Fix comment regarding "Impala internal calls". We should pass a const& TSessionState. We usually prefer const& for input params and pointers for output params. We're not religiously consistent, but I think in this case it's easy to change to const TSessionState& http://gerrit.cloudera.org:8080/#/c/9276/2/common/thrift/Frontend.thrift File common/thrift/Frontend.thrift: http://gerrit.cloudera.org:8080/#/c/9276/2/common/thrift/Frontend.thrift@171 PS2, Line 171: // enabled, only the functions this user has access to will be returned. If not Fix comment. We're not returning functions, and we don't have Impala-internal requests for describe. http://gerrit.cloudera.org:8080/#/c/9276/2/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java File fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java: http://gerrit.cloudera.org:8080/#/c/9276/2/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java@121 PS2, Line 121: if (path_.destColumn() != null) { We're missing a privilege request for the following case: describe functional.allcomplextypes.nested_struct_col.f1 * destColumn() is null * destType() is not complex I think this code can be condensed/simplified, see comment below. We need tests for all these code paths. http://gerrit.cloudera.org:8080/#/c/9276/2/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java@128 PS2, Line 128: analyzer.registerPrivReq(new PrivilegeRequestBuilder() I think this request should be unconditionally registered after L119, irrespective of destColumn() and destType() http://gerrit.cloudera.org:8080/#/c/9276/2/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/9276/2/fe/src/main/java/org/apache/impala/service/Frontend.java@805 PS2, Line 805: .onColumn(table.getDb().getName(), table.getName(),colName) space before 'colName' http://gerrit.cloudera.org:8080/#/c/9276/2/fe/src/main/java/org/apache/impala/service/Frontend.java@812 PS2, Line 812: filteredColumns = table.getColumnsInHiveOrder(); // User has table-level access. http://gerrit.cloudera.org:8080/#/c/9276/2/fe/src/main/java/org/apache/impala/service/Frontend.java@815 PS2, Line 815: filteredColumns = table.getColumnsInHiveOrder(); // Authorization is disabled. http://gerrit.cloudera.org:8080/#/c/9276/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java: http://gerrit.cloudera.org:8080/#/c/9276/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1787 PS1, Line 1787: assertEquals(expectedDbs, extractDbNames(dbs)); > Column-level privileges on views are not currently supported. I can remove I don't think we should check in dead code that will only confuse readers, reviewers and code coverage tools. Let's add the code and tests when we actually have support for that feature. -- To view, visit http://gerrit.cloudera.org:8080/9276 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic96ae184fccdc88ba970b5adcd501da1966accb9 Gerrit-Change-Number: 9276 Gerrit-PatchSet: 2 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Comment-Date: Tue, 13 Mar 2018 05:59:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege
Fredy Wijaya has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/9589 ) Change subject: IMPALA-6643: Add REFRESH fine-grained privilege .. IMPALA-6643: Add REFRESH fine-grained privilege With this patch, REFRESH privilege is now required to execute INVALIDATE METADATA or REFRESH statement. These are the new GRANT/REVOKE statements introduced. GRANT REFRESH on DATABASE db TO ROLE testrole; GRANT REFRESH on TABLE db.tbl TO ROLE testrole; REVOKE REFRESH on DATABASE db TO ROLE testrole; REVOKE REFRESH on TABLE db.tbl TO ROLE testrole; Testing: - Ran end-to-end tests Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4 --- M common/thrift/CatalogObjects.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/Privilege.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/resources/authz-policy.ini.template 8 files changed, 136 insertions(+), 36 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/89/9589/4 -- To view, visit http://gerrit.cloudera.org:8080/9589 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4 Gerrit-Change-Number: 9589 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya
[Impala-ASF-CR] Removing (broken) retries from split-hbase.sh.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/9588 ) Change subject: Removing (broken) retries from split-hbase.sh. .. Patch Set 1: https://jenkins.impala.io/job/gerrit-verify-dryrun-external/92/ passed with this change. -- To view, visit http://gerrit.cloudera.org:8080/9588 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I715891c9e744f21002330c3ae3ebc14095d94ffd Gerrit-Change-Number: 9588 Gerrit-PatchSet: 1 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Tue, 13 Mar 2018 04:57:41 + Gerrit-HasComments: No
[Impala-ASF-CR] Removing (broken) retries from split-hbase.sh.
Philip Zeyliger has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9588 Change subject: Removing (broken) retries from split-hbase.sh. .. Removing (broken) retries from split-hbase.sh. The retries in split-hbase.sh don't work in the common case, because $MINIKDC_PRINC_HIVE is not set in non-kerberized (common) environments. The regular data load scripts (create-load-data.sh) have code to manage that, but split-hbase.sh blindly forges ahead, leading to errors like: /home/impdev/Impala/testdata/bin/split-hbase.sh: line 49: MINIKDC_PRINC_HIVE: unbound variable Error in /home/impdev/Impala/testdata/bin/create-load-data.sh at line 48: LOAD_DATA_ARGS="" Since this hasn't been working, I opted to remove it entirely, as a failure on the line where HBase splitting actually failed would be significantly more useful than the error here. A search of mailing lists suggested that I was at least the second person to have run into this. (In my case, I did break HBase splitting, but it took me a second to identify the error, since the log was spammed with unrelated information relating to the cluster restart.) Testing: core tests. Change-Id: I715891c9e744f21002330c3ae3ebc14095d94ffd --- M testdata/bin/split-hbase.sh 1 file changed, 3 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/9588/1 -- To view, visit http://gerrit.cloudera.org:8080/9588 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I715891c9e744f21002330c3ae3ebc14095d94ffd Gerrit-Change-Number: 9588 Gerrit-PatchSet: 1 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege
Adam Holley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9589 ) Change subject: IMPALA-6643: Add REFRESH fine-grained privilege .. Patch Set 2: (8 comments) http://gerrit.cloudera.org:8080/#/c/9589/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/9589/2/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@116 PS2, Line 116: analyzer.registerPrivReq(new PrivilegeRequest(Privilege.ALL)); Shouldn't the refresh privilege be applied to this case as well? i.e. onServer() Might require an e2e test instead of AuthorizationTest. http://gerrit.cloudera.org:8080/#/c/9589/2/fe/src/main/java/org/apache/impala/authorization/Privilege.java File fe/src/main/java/org/apache/impala/authorization/Privilege.java: http://gerrit.cloudera.org:8080/#/c/9589/2/fe/src/main/java/org/apache/impala/authorization/Privilege.java@50 PS2, Line 50: SentryAction Is this the best name? Since Sentry does not care what the string is, and it's more specific to SQL. Refresh allows the invalidate metadata action. This list is really privileges. Maybe SQLPrivilege? Also, should this be it's own file? I know there's cases for either way in the code. http://gerrit.cloudera.org:8080/#/c/9589/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java: http://gerrit.cloudera.org:8080/#/c/9589/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@236 PS2, Line 236: Update comment at top of file to reflect new setup. http://gerrit.cloudera.org:8080/#/c/9589/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@249 PS2, Line 249: roleName = "refresh_functional_text_lzo"; : sentryService.createRole(USER, roleName, true); : sentryService.grantRoleToGroup(USER, roleName, USER.getName()); Does this chunk need to be duplicated? http://gerrit.cloudera.org:8080/#/c/9589/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@261 PS2, Line 261: roleName = "refresh_functional_text_lzo"; : sentryService.createRole(USER, roleName, true); : sentryService.grantRoleToGroup(USER, roleName, USER.getName()); duplicate? http://gerrit.cloudera.org:8080/#/c/9589/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@273 PS2, Line 273: roleName = "refresh_functional_text_lzo"; : sentryService.createRole(USER, roleName, true); : sentryService.grantRoleToGroup(USER, roleName, USER.getName()); duplicate? http://gerrit.cloudera.org:8080/#/c/9589/2/fe/src/test/java/org/apache/impala/analysis/ParserTest.java File fe/src/test/java/org/apache/impala/analysis/ParserTest.java: http://gerrit.cloudera.org:8080/#/c/9589/2/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@3579 PS2, Line 3579: // REFRESH privilege object. I think we need refresh on SERVER also. http://gerrit.cloudera.org:8080/#/c/9589/2/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@3580 PS2, Line 3580: ParsesOk(String.format("%s REFRESH ON DATABASE foo %s myRole", formatStr)); There's no current method to "REFRESH" or "INVALIDATE METADATA" at the database level, only server and table. -- To view, visit http://gerrit.cloudera.org:8080/9589 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4 Gerrit-Change-Number: 9589 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Comment-Date: Tue, 13 Mar 2018 04:31:42 + Gerrit-HasComments: Yes
[Impala-ASF-CR] :IMPALA-3916: [DOCS] Reserved keywords updated for Impala 3.0
Hello Greg Rahn, Tianyi Wang, John Russell, Alex Behm, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9540 to look at the new patch set (#3). Change subject: :IMPALA-3916: [DOCS] Reserved keywords updated for Impala 3.0 .. :IMPALA-3916: [DOCS] Reserved keywords updated for Impala 3.0 Change-Id: I09dbaf5e2d22f07a8bca8a601c04e94f3a4b36a0 Cherry-picks: not for 2.x --- M docs/topics/impala_reserved_words.xml 1 file changed, 3,431 insertions(+), 235 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/9540/3 -- To view, visit http://gerrit.cloudera.org:8080/9540 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I09dbaf5e2d22f07a8bca8a601c04e94f3a4b36a0 Gerrit-Change-Number: 9540 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: John Russell Gerrit-Reviewer: Tianyi Wang
[Impala-ASF-CR] IMPALA-6635: Add DECIMAL type to Kudu predicates
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9578 ) Change subject: IMPALA-6635: Add DECIMAL type to Kudu predicates .. Patch Set 2: This failure was a Kudu side issue tracked and fixed in KUDU-2338. When the next Kudu 1.7.0-cdh5.15.0-SNAPSHOT jar is pulled in it should be resolved. I will kick off a test tomorrow. -- To view, visit http://gerrit.cloudera.org:8080/9578 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2569a9e1d58f1c58884d58633d46348364888ed7 Gerrit-Change-Number: 9578 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 13 Mar 2018 03:48:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4168: [DOCS] Adds Oracle-style hint placement for INSERT/UPSERT
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/9030 ) Change subject: IMPALA-4168: [DOCS] Adds Oracle-style hint placement for INSERT/UPSERT .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/9030 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I43e0a782087c2e67f2e012424fb9261be445efc9 Gerrit-Change-Number: 9030 Gerrit-PatchSet: 2 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: John Russell Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Tue, 13 Mar 2018 03:48:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5191, IMPALA-6415: [DOCS] Document breaking change of alias and ordinal substitution
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/9211 ) Change subject: IMPALA-5191, IMPALA-6415: [DOCS] Document breaking change of alias and ordinal substitution .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/9211 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I558230d07212da62d2cd12e07a52ceba03e980a8 Gerrit-Change-Number: 9211 Gerrit-PatchSet: 3 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: John Russell Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 13 Mar 2018 03:43:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6638: Reduce file handle cache lock contention
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9576 ) Change subject: IMPALA-6638: Reduce file handle cache lock contention .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/9576 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c695b21ca556e9c73c703c0c891e64939271c8d Gerrit-Change-Number: 9576 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Tue, 13 Mar 2018 02:40:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6638: Reduce file handle cache lock contention
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9576 ) Change subject: IMPALA-6638: Reduce file handle cache lock contention .. IMPALA-6638: Reduce file handle cache lock contention FileHandleCache::OpenFileHandle() currently holds the lock while opening a file handle. This lengthens the duration holding the lock considerably, causing contention when a lot of file handles are being opened (i.e. when the cache is cold). This changes FileHandleCache::OpenFileHandle() drops the lock while opening the file handle, then reacquires it to add the file handle to the cache. When running a simple select on a table with 46801 Parquet files, this fixes a small performance overhead for the cold cache case compared to having the cache off. All results are averaged over 5 runs: File handle cache off: 7.19s File handle cache cold (no patch): 7.92s File handle cache cold (with patch): 7.20s When running a select that accesses and aggregates 4 columns from the same Parquet table (thus requiring more scan ranges for the same file), the fix reduces contention, particularly for the case with 8 IO threads per disk: 1 IO thread per disk: Without patch: 9.59s With patch: 8.11s 8 IO threads per disk: Without patch: 14.2s With patch: 8.17s The patch has no impact on the performance when the cache is hot. Change-Id: I4c695b21ca556e9c73c703c0c891e64939271c8d Reviewed-on: http://gerrit.cloudera.org:8080/9576 Reviewed-by: Joe McDonnell Tested-by: Impala Public Jenkins --- M be/src/runtime/io/handle-cache.inline.h 1 file changed, 37 insertions(+), 24 deletions(-) Approvals: Joe McDonnell: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/9576 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I4c695b21ca556e9c73c703c0c891e64939271c8d Gerrit-Change-Number: 9576 Gerrit-PatchSet: 5 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell
[Impala-ASF-CR] IMPALA-6635: Add DECIMAL type to Kudu predicates
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9578 ) Change subject: IMPALA-6635: Add DECIMAL type to Kudu predicates .. Patch Set 2: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2088/ -- To view, visit http://gerrit.cloudera.org:8080/9578 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2569a9e1d58f1c58884d58633d46348364888ed7 Gerrit-Change-Number: 9578 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 13 Mar 2018 02:27:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6610: Add inspection and warning for LDAP password in Impala shell when authentication fails
Donghui Xu has posted comments on this change. ( http://gerrit.cloudera.org:8080/9506 ) Change subject: IMPALA-6610: Add inspection and warning for LDAP password in Impala shell when authentication fails .. Patch Set 9: (3 comments) http://gerrit.cloudera.org:8080/#/c/9506/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9506/8//COMMIT_MSG@7 PS8, Line 7: Add inspection and warning for LDAP password in Impala shell wh > Update Done http://gerrit.cloudera.org:8080/#/c/9506/8//COMMIT_MSG@9 PS8, Line 9: The value of LDAP password in Impala shell contains extra line break causes > nit: Remove trailing spaces and wrap (here and below) Done http://gerrit.cloudera.org:8080/#/c/9506/8/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/9506/8/shell/impala_shell.py@821 PS8, Line 821: if options.ldap_password_cmd is not None and \ > Add checks for ldap_password_cmd & ldap_password not being None. Else shell Done -- To view, visit http://gerrit.cloudera.org:8080/9506 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie570166aea62af223905b7f0124e9efb15a88ac7 Gerrit-Change-Number: 9506 Gerrit-PatchSet: 9 Gerrit-Owner: Donghui Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Donghui Xu Gerrit-Reviewer: Fredy Wijaya Gerrit-Comment-Date: Tue, 13 Mar 2018 02:08:46 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6610: Add inspection and warning for LDAP password in Impala shell when authentication fails
Donghui Xu has posted comments on this change. ( http://gerrit.cloudera.org:8080/9506 ) Change subject: IMPALA-6610: Add inspection and warning for LDAP password in Impala shell when authentication fails .. Patch Set 9: Thank you very much. It seems that there is a rule that requires no more than 80 characters per line, so that a sentence with a length exceeding the limit will have newline. -- To view, visit http://gerrit.cloudera.org:8080/9506 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie570166aea62af223905b7f0124e9efb15a88ac7 Gerrit-Change-Number: 9506 Gerrit-PatchSet: 9 Gerrit-Owner: Donghui Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Donghui Xu Gerrit-Reviewer: Fredy Wijaya Gerrit-Comment-Date: Tue, 13 Mar 2018 02:07:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6610: Add inspection and warning for LDAP password in Impala shell when authentication fails
Hello Bharath Vissapragada, Fredy Wijaya, Alex Behm, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9506 to look at the new patch set (#9). Change subject: IMPALA-6610: Add inspection and warning for LDAP password in Impala shell when authentication fails .. IMPALA-6610: Add inspection and warning for LDAP password in Impala shell when authentication fails The value of LDAP password in Impala shell contains extra line break causes authentication failure, but the user can't detect the cause of the failure. I fixed the issue by adding inspection to the password for common pitfalls and issuing a warning in the shell when authentication fails. Change-Id: Ie570166aea62af223905b7f0124e9efb15a88ac7 --- M shell/impala_shell.py 1 file changed, 6 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/9506/9 -- To view, visit http://gerrit.cloudera.org:8080/9506 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie570166aea62af223905b7f0124e9efb15a88ac7 Gerrit-Change-Number: 9506 Gerrit-PatchSet: 9 Gerrit-Owner: Donghui Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Donghui Xu Gerrit-Reviewer: Fredy Wijaya
[Impala-ASF-CR] IMPALA-6576: Add metrics for data stream service memory usage
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9562 ) Change subject: IMPALA-6576: Add metrics for data stream service memory usage .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/9562 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5033b8dfda0b23d4230535ba13c3e050a35d01a3 Gerrit-Change-Number: 9562 Gerrit-PatchSet: 5 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 13 Mar 2018 02:01:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6576: Add metrics for data stream service memory usage
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9562 ) Change subject: IMPALA-6576: Add metrics for data stream service memory usage .. IMPALA-6576: Add metrics for data stream service memory usage This change adds metrics for the data stream service memory usage. Both current and peak usage are exposed. It adds a test to test_krpc_metrics.py to make sure that the expected metrics are present and that the peak usage shows a non-zero value after running a query. Change-Id: I5033b8dfda0b23d4230535ba13c3e050a35d01a3 Reviewed-on: http://gerrit.cloudera.org:8080/9562 Reviewed-by: Tim Armstrong Tested-by: Impala Public Jenkins --- M be/src/runtime/exec-env.cc M be/src/service/data-stream-service.cc M be/src/service/data-stream-service.h M be/src/util/memory-metrics.cc M be/src/util/memory-metrics.h M common/thrift/metrics.json M tests/custom_cluster/test_krpc_metrics.py 7 files changed, 104 insertions(+), 17 deletions(-) Approvals: Tim Armstrong: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/9562 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I5033b8dfda0b23d4230535ba13c3e050a35d01a3 Gerrit-Change-Number: 9562 Gerrit-PatchSet: 6 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-5980: Upgrade to LLVM 5.0.1
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/9584 ) Change subject: IMPALA-5980: Upgrade to LLVM 5.0.1 .. Patch Set 1: > Do any of those benchmarks verify compile time performance? The query time in the attached benchmarks include the codegen compile time as well. I also tired pulling out the codegen compile time individually from the query profiles and doing a similar analysis. The delta in those varies alot along with st.dev being high too. Since the scale of those timings is in milliseconds and codegen time reported is the wall clock time, it might be difficult to seperate the noise. The variation in codegen time would be compensated by the variation in query execution time. I believe that just looking at their cumulative effect should be good enough for now. I also did some initial testing to see if codegen compile time improves for one of the queries (large num of OR-predicates) that has been a problem in the past. That time improved from ~20sec to ~14sec. -- To view, visit http://gerrit.cloudera.org:8080/9584 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib0a15cb53feab89e7b35a56b67b3b30eb3e62c6b Gerrit-Change-Number: 9584 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Comment-Date: Tue, 13 Mar 2018 01:30:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3833: Fix invalid data handling in Sequence and RCFile scanners
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8936 ) Change subject: IMPALA-3833: Fix invalid data handling in Sequence and RCFile scanners .. Patch Set 10: I need to take another pass over this just to convince myself I didn't miss any bugs first time over. -- To view, visit http://gerrit.cloudera.org:8080/8936 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9cfc38af3f30c65ada9734eb471dbfa6ecdd74a Gerrit-Change-Number: 8936 Gerrit-PatchSet: 10 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Tue, 13 Mar 2018 01:02:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3833: Fix invalid data handling in Sequence and RCFile scanners
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8936 ) Change subject: IMPALA-3833: Fix invalid data handling in Sequence and RCFile scanners .. Patch Set 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/8936/10//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8936/10//COMMIT_MSG@9 PS10, Line 9: Enabled the fuzz test for Sequence and RCFiles and added new checks to return Commit message is stale. -- To view, visit http://gerrit.cloudera.org:8080/8936 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9cfc38af3f30c65ada9734eb471dbfa6ecdd74a Gerrit-Change-Number: 8936 Gerrit-PatchSet: 10 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Tue, 13 Mar 2018 01:02:12 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5315: Cast to timestamp fails for YYYY-M-D format
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/7009 ) Change subject: IMPALA-5315: Cast to timestamp fails for -M-D format .. Patch Set 17: Code-Review+1 Did anyone else want to take a look? I'm happy with the code and I think the test coverage is good. -- To view, visit http://gerrit.cloudera.org:8080/7009 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f Gerrit-Change-Number: 7009 Gerrit-PatchSet: 17 Gerrit-Owner: Vincent Tran Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vincent Tran Gerrit-Comment-Date: Tue, 13 Mar 2018 00:48:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6341, IMPALA-5917: Reduce mem-limit for start-impala-cluster.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9395 ) Change subject: IMPALA-6341, IMPALA-5917: Reduce mem-limit for start-impala-cluster. .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/9395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iabca7a95560bd27c2de2b0a147ee9a3c45199db7 Gerrit-Change-Number: 9395 Gerrit-PatchSet: 2 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 13 Mar 2018 00:39:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6341, IMPALA-5917: Reduce mem-limit for start-impala-cluster.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9395 ) Change subject: IMPALA-6341, IMPALA-5917: Reduce mem-limit for start-impala-cluster. .. IMPALA-6341, IMPALA-5917: Reduce mem-limit for start-impala-cluster. We've observed empirically that giving Impala 80% of system memory doesn't leave enough room for the minicluster and ASAN overhead, leading to the OOM killer striking during test runs (sometimes). This commit reduces the threshold to 70%. This commit also reduces the memory usage of semi-joins-exhaustive.test by roughly halving the number of records it deals with. This was necessary for tests to pass on a machine with 32GB of RAM. Testing: I've run the ASAN build (more) happily with this change. I've run exhaustive tests on a 32GB machine. Change-Id: Iabca7a95560bd27c2de2b0a147ee9a3c45199db7 Reviewed-on: http://gerrit.cloudera.org:8080/9395 Reviewed-by: Tim Armstrong Tested-by: Impala Public Jenkins --- M bin/start-impala-cluster.py M testdata/workloads/functional-query/queries/QueryTest/semi-joins-exhaustive.test 2 files changed, 13 insertions(+), 3 deletions(-) Approvals: Tim Armstrong: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/9395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Iabca7a95560bd27c2de2b0a147ee9a3c45199db7 Gerrit-Change-Number: 9395 Gerrit-PatchSet: 3 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-5315: Cast to timestamp fails for YYYY-M-D format
Hello Lars Volker, Tim Armstrong, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7009 to look at the new patch set (#17). Change subject: IMPALA-5315: Cast to timestamp fails for -M-D format .. IMPALA-5315: Cast to timestamp fails for -M-D format This change allows casting of a string in 'lazy' date/time format to timestamp. The supported lazy date formats are: -[M]M-[d]d -[M]M-[d]d [H]H:[m]m:[s]s[.S] [H]H:[m]m:[s]s[.S] We will incur a SCAN performance penalty (approximately 1/2 TotalReadThroughput) when the string is in one of these lazy date/time format. Testing: Benchmarked the performance consequence by executing this SQL on a private build over 3.8 billion rows: select min(cast (time_string as timestamp)) from private.impala_5315 Added tests for valid and invalid date/time format strings in expr-test.cc to be inline with existing tests for CAST() function. Added end-to-end tests into exprs.test and select-lazy-timestamp.test to exercise the new function within the context of a query. Added tests to exercise the leading and trailing white space trimming behaviour in default and lazy date/time string format (IMPALA-6630). Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f --- M be/src/exprs/expr-test.cc M be/src/runtime/timestamp-parse-util.cc M be/src/runtime/timestamp-parse-util.h A testdata/data/lazy_timestamp.csv M testdata/workloads/functional-query/queries/QueryTest/exprs.test A testdata/workloads/functional-query/queries/QueryTest/select-lazy-timestamp.test M tests/query_test/test_scanners.py 7 files changed, 398 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/7009/17 -- To view, visit http://gerrit.cloudera.org:8080/7009 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f Gerrit-Change-Number: 7009 Gerrit-PatchSet: 17 Gerrit-Owner: Vincent Tran Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vincent Tran
[Impala-ASF-CR] IMPALA-5717: Support for reading ORC data files
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9134 ) Change subject: IMPALA-5717: Support for reading ORC data files .. Patch Set 5: (3 comments) Thanks for the new patchset. I need to do a deep dive into the new changes but will respond to your comments first. http://gerrit.cloudera.org:8080/#/c/9134/3/be/src/exec/hdfs-orc-scanner.cc File be/src/exec/hdfs-orc-scanner.cc: http://gerrit.cloudera.org:8080/#/c/9134/3/be/src/exec/hdfs-orc-scanner.cc@517 PS3, Line 517: > Though ORC-262 has no progress, I think we can still prefech data and let t Thanks for filing it. I did spent a little time reading the ORC code and it does seem like we could achieve this with some modifications to the ORC library - they have two layers of InputStream abstraction, the top-level which does the decompression and a lower level that does I/O.) http://gerrit.cloudera.org:8080/#/c/9134/5/testdata/bin/run-hive-server.sh File testdata/bin/run-hive-server.sh: http://gerrit.cloudera.org:8080/#/c/9134/5/testdata/bin/run-hive-server.sh@75 PS5, Line 75: tabls typo: tables http://gerrit.cloudera.org:8080/#/c/9134/3/testdata/workloads/functional-query/functional-query_exhaustive.csv File testdata/workloads/functional-query/functional-query_exhaustive.csv: http://gerrit.cloudera.org:8080/#/c/9134/3/testdata/workloads/functional-query/functional-query_exhaustive.csv@25 PS3, Line 25: file_format: orc, dataset: functional, compression_codec: none, compression_type: none > Yeah, the default ORC codec is zlib (deflate in Impala). We should the name of compression_codec to be deflate for our ORC tables so that it's accurate (we did the wrong thing with Parquet here). -- To view, visit http://gerrit.cloudera.org:8080/9134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia7b6ae4ce3b9ee8125b21993702faa87537790a4 Gerrit-Change-Number: 9134 Gerrit-PatchSet: 5 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 13 Mar 2018 00:28:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5315: Cast to timestamp fails for YYYY-M-D format
Vincent Tran has posted comments on this change. ( http://gerrit.cloudera.org:8080/7009 ) Change subject: IMPALA-5315: Cast to timestamp fails for -M-D format .. Patch Set 16: (4 comments) Changed the name of the e2e test to be more appropriate for its purpose. http://gerrit.cloudera.org:8080/#/c/7009/16/be/src/runtime/timestamp-parse-util.cc File be/src/runtime/timestamp-parse-util.cc: http://gerrit.cloudera.org:8080/#/c/7009/16/be/src/runtime/timestamp-parse-util.cc@28 PS16, Line 28: #include > Standard C++ headers go before the 3rd-party library headers. I.e. line 19 Done http://gerrit.cloudera.org:8080/#/c/7009/16/testdata/workloads/functional-query/queries/QueryTest/cast-lazy-datetime-string.test File testdata/workloads/functional-query/queries/QueryTest/cast-lazy-datetime-string.test: http://gerrit.cloudera.org:8080/#/c/7009/16/testdata/workloads/functional-query/queries/QueryTest/cast-lazy-datetime-string.test@3 PS16, Line 3: select cast(dt_str as timestamp) from lazy_dt_str > should not need a cast here, we want to test the text scanner parsing the d Done http://gerrit.cloudera.org:8080/#/c/7009/16/testdata/workloads/functional-query/queries/QueryTest/cast-lazy-datetime-string.test@4 PS16, Line 4: RESULTS > Let's make this RESULTS: VERIFY_IS_EQUAL_SORTED Done http://gerrit.cloudera.org:8080/#/c/7009/16/tests/query_test/test_scanners.py File tests/query_test/test_scanners.py: http://gerrit.cloudera.org:8080/#/c/7009/16/tests/query_test/test_scanners.py@874 PS16, Line 874: STRING > This should be a timestamp column, so we can make sure that the text scanne Done -- To view, visit http://gerrit.cloudera.org:8080/7009 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f Gerrit-Change-Number: 7009 Gerrit-PatchSet: 16 Gerrit-Owner: Vincent Tran Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vincent Tran Gerrit-Comment-Date: Tue, 13 Mar 2018 00:23:33 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6589: remove invalid DCHECK in parquet reader
Hello Pranay Singh, Lars Volker, Csaba Ringhofer, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9556 to look at the new patch set (#3). Change subject: IMPALA-6589: remove invalid DCHECK in parquet reader .. IMPALA-6589: remove invalid DCHECK in parquet reader The DCHECK was only valid if the Parquet file metadata is internally consistent, with the number of values reported by the metadata matching the number of encoded levels. The DCHECK was intended to directly detect misuse of the RleBatchDecoder interface, which would lead to incorrect results. However, our other test coverage for reading Parquet files is sufficient to test the correctness of level decoding. Testing: Added a minimal corrupt test file that reproduces the issue. Change-Id: Idd6e09f8c8cca8991be5b5b379f6420adaa97daa --- M be/src/exec/parquet-column-readers.cc M be/src/exec/parquet-column-readers.h M testdata/data/README A testdata/data/num_values_def_levels_mismatch.parquet A testdata/workloads/functional-query/queries/QueryTest/parquet-num-values-def-levels-mismatch.test M tests/query_test/test_scanners.py 6 files changed, 50 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/56/9556/3 -- To view, visit http://gerrit.cloudera.org:8080/9556 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idd6e09f8c8cca8991be5b5b379f6420adaa97daa Gerrit-Change-Number: 9556 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6589: remove invalid DCHECK in parquet reader
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9556 ) Change subject: IMPALA-6589: remove invalid DCHECK in parquet reader .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/9556/2/be/src/exec/parquet-column-readers.cc File be/src/exec/parquet-column-readers.cc: http://gerrit.cloudera.org:8080/#/c/9556/2/be/src/exec/parquet-column-readers.cc@1207 PS2, Line 1207: rep_level_ = rep_levels_.ReadLevel(); > Can you also add a similar check for the repetition level? Done http://gerrit.cloudera.org:8080/#/c/9556/2/testdata/workloads/functional-query/queries/QueryTest/parquet-num-values-def-levels-mismatch.test File testdata/workloads/functional-query/queries/QueryTest/parquet-num-values-def-levels-mismatch.test: http://gerrit.cloudera.org:8080/#/c/9556/2/testdata/workloads/functional-query/queries/QueryTest/parquet-num-values-def-levels-mismatch.test@7 PS2, Line 7: > nit: unnecessary line Done http://gerrit.cloudera.org:8080/#/c/9556/2/tests/query_test/test_scanners.py File tests/query_test/test_scanners.py: http://gerrit.cloudera.org:8080/#/c/9556/2/tests/query_test/test_scanners.py@433 PS2, Line 433: > nit: unnecessary line Done -- To view, visit http://gerrit.cloudera.org:8080/9556 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idd6e09f8c8cca8991be5b5b379f6420adaa97daa Gerrit-Change-Number: 9556 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 13 Mar 2018 00:00:37 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5315: Cast to timestamp fails for YYYY-M-D format
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/7009 ) Change subject: IMPALA-5315: Cast to timestamp fails for -M-D format .. Patch Set 16: (1 comment) http://gerrit.cloudera.org:8080/#/c/7009/16/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/7009/16/be/src/exprs/expr-test.cc@6176 PS16, Line 6176: TestStringValue("monthname(cast('2011-02-18 09:10:11.00' as timestamp))", "February"); > nit: long lines. It might be easier to run the patch through clang-format t NM, these were pulled in with a rebase -- To view, visit http://gerrit.cloudera.org:8080/7009 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f Gerrit-Change-Number: 7009 Gerrit-PatchSet: 16 Gerrit-Owner: Vincent Tran Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vincent Tran Gerrit-Comment-Date: Mon, 12 Mar 2018 23:57:15 + Gerrit-HasComments: Yes
[Impala-ASF-CR](2.x) IMPALA-6405: Error when string to decimal cast overflows
Lars Volker has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9530 ) Change subject: IMPALA-6405: Error when string to decimal cast overflows .. IMPALA-6405: Error when string to decimal cast overflows Before this patch, when there was an error when converting a string to a decimal, a NULL was returned. In this patch, we change this behavior so that an error is returned if decimal_v2 is enabled. The reasoning is that we want stricter behavior in decimal_v2. Testing: - Added some EE tests. - Ran an exhaustive build, which passed. Change-Id: Icffccac1c1c2361447ae4b0de9b6c2ec7de071db Reviewed-on: http://gerrit.cloudera.org:8080/9339 Reviewed-by: Dan Hecht Tested-by: Impala Public Jenkins Reviewed-on: http://gerrit.cloudera.org:8080/9530 Reviewed-by: Taras Bobrovytsky --- M be/src/exprs/decimal-operators-ir.cc M fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test 3 files changed, 55 insertions(+), 7 deletions(-) Approvals: Taras Bobrovytsky: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/9530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: merged Gerrit-Change-Id: Icffccac1c1c2361447ae4b0de9b6c2ec7de071db Gerrit-Change-Number: 9530 Gerrit-PatchSet: 3 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Taras Bobrovytsky
[Impala-ASF-CR](2.x) IMPALA-6405: Error when string to decimal cast overflows
Hello Dan Hecht, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9530 to look at the new patch set (#2). Change subject: IMPALA-6405: Error when string to decimal cast overflows .. IMPALA-6405: Error when string to decimal cast overflows Before this patch, when there was an error when converting a string to a decimal, a NULL was returned. In this patch, we change this behavior so that an error is returned if decimal_v2 is enabled. The reasoning is that we want stricter behavior in decimal_v2. Testing: - Added some EE tests. - Ran an exhaustive build, which passed. Change-Id: Icffccac1c1c2361447ae4b0de9b6c2ec7de071db Reviewed-on: http://gerrit.cloudera.org:8080/9339 Reviewed-by: Dan Hecht Tested-by: Impala Public Jenkins --- M be/src/exprs/decimal-operators-ir.cc M fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test 3 files changed, 55 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/30/9530/2 -- To view, visit http://gerrit.cloudera.org:8080/9530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icffccac1c1c2361447ae4b0de9b6c2ec7de071db Gerrit-Change-Number: 9530 Gerrit-PatchSet: 2 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Taras Bobrovytsky
[Impala-ASF-CR] IMPALA-6638: Reduce file handle cache lock contention
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9576 ) Change subject: IMPALA-6638: Reduce file handle cache lock contention .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2089/ -- To view, visit http://gerrit.cloudera.org:8080/9576 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c695b21ca556e9c73c703c0c891e64939271c8d Gerrit-Change-Number: 9576 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Mon, 12 Mar 2018 23:02:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5717: Support for reading ORC data files
Hello Tim Armstrong, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9134 to look at the new patch set (#5). Change subject: IMPALA-5717: Support for reading ORC data files .. IMPALA-5717: Support for reading ORC data files This patch integrates the orc library into Impala and implements HdfsOrcScanner as a middle layer between them. The HdfsOrcScanner supplies input needed from the orc-reader, tracks memory consumption of the reader and transfers the reader's output (orc::ColumnVectorBatch) into impala::RowBatch. The ORC version we used is release-1.4.3. Currently, we only support reading premitive types. Writing into ORC table has not been supported neither. Tests - Most of the end-to-end tests can run on ORC format. - Add tpcds, tpch tests for ORC. - Add some ORC specific tests. - Have passed all the tests. Change-Id: Ia7b6ae4ce3b9ee8125b21993702faa87537790a4 --- M CMakeLists.txt M be/CMakeLists.txt M be/src/exec/CMakeLists.txt A be/src/exec/hdfs-orc-scanner.cc A be/src/exec/hdfs-orc-scanner.h M be/src/exec/hdfs-parquet-scanner.cc M be/src/exec/hdfs-parquet-scanner.h M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/hdfs-scan-node-mt.cc M be/src/exec/hdfs-scanner.cc M be/src/exec/hdfs-scanner.h M bin/bootstrap_toolchain.py M bin/impala-config.sh A cmake_modules/FindOrc.cmake M common/thrift/CatalogObjects.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java M fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/jflex/sql-scanner.flex M testdata/LineItemMultiBlock/README.dox A testdata/LineItemMultiBlock/lineitem_orc_multiblock_one_stripe.orc A testdata/LineItemMultiBlock/lineitem_sixblocks.orc A testdata/LineItemMultiBlock/lineitem_threeblocks.orc M testdata/bin/generate-schema-statements.py M testdata/bin/load_nested.py M testdata/bin/run-hive-server.sh M testdata/datasets/functional/schema_constraints.csv M testdata/workloads/functional-query/functional-query_core.csv M testdata/workloads/functional-query/functional-query_dimensions.csv M testdata/workloads/functional-query/functional-query_exhaustive.csv M testdata/workloads/functional-query/functional-query_pairwise.csv A testdata/workloads/functional-query/queries/QueryTest/nested-types-orc-basic.test M testdata/workloads/tpcds/tpcds_core.csv M testdata/workloads/tpcds/tpcds_dimensions.csv M testdata/workloads/tpcds/tpcds_exhaustive.csv M testdata/workloads/tpcds/tpcds_pairwise.csv M testdata/workloads/tpch/tpch_core.csv M testdata/workloads/tpch/tpch_dimensions.csv M testdata/workloads/tpch/tpch_exhaustive.csv M testdata/workloads/tpch/tpch_pairwise.csv M tests/common/test_dimensions.py M tests/comparison/cli_options.py M tests/query_test/test_decimal_queries.py M tests/query_test/test_nested_types.py M tests/query_test/test_scanners.py M tests/query_test/test_tpch_queries.py 49 files changed, 1,425 insertions(+), 156 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/9134/5 -- To view, visit http://gerrit.cloudera.org:8080/9134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia7b6ae4ce3b9ee8125b21993702faa87537790a4 Gerrit-Change-Number: 9134 Gerrit-PatchSet: 5 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6638: Reduce file handle cache lock contention
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/9576 ) Change subject: IMPALA-6638: Reduce file handle cache lock contention .. Patch Set 4: Code-Review+2 Rebase, carry +2 -- To view, visit http://gerrit.cloudera.org:8080/9576 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c695b21ca556e9c73c703c0c891e64939271c8d Gerrit-Change-Number: 9576 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Mon, 12 Mar 2018 23:02:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6389: Make '\0' delimited text files work
Hello Tim Armstrong, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9525 to look at the new patch set (#2). Change subject: IMPALA-6389: Make '\0' delimited text files work .. IMPALA-6389: Make '\0' delimited text files work Initially I didn't want to fully implement this, as the metadata for these tables can't even be fully stored in Postgres; however after digging into some older documentation, it appears that the ASCII NUL character actually has been used as a field separator in various vendors CSV implementation. Therefore, this patch attempts to make things as non-broken as possible and allows \0 as a field or tuple delimiter. Collection column delimiters are not allowed to be \0, as they genuinly may not exist and we don't want to force special escaping on an arbitrary character. Note that the field delimiter must be distinct from the tuple delimiter when they both exist; if it is not, the effect will be that there is no field delimiter (this is actually possible with single column tables). Testing: Created a zero delimited table as described in the JIRA, using MySQL backed Hive metastore; ran select * from tab_separated on the table, updated the unit test. Change-Id: I4b6f38cbe3f1036f60efd31a31d82d0cd8f3d2a8 --- M be/src/exec/delimited-text-parser-test.cc M be/src/exec/delimited-text-parser.cc M be/src/exec/delimited-text-parser.h M be/src/exec/delimited-text-parser.inline.h M be/src/exec/hdfs-sequence-scanner.cc M be/src/exec/hdfs-sequence-scanner.h M be/src/exec/hdfs-text-scanner.cc M be/src/exec/hdfs-text-scanner.h 8 files changed, 147 insertions(+), 78 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/25/9525/2 -- To view, visit http://gerrit.cloudera.org:8080/9525 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4b6f38cbe3f1036f60efd31a31d82d0cd8f3d2a8 Gerrit-Change-Number: 9525 Gerrit-PatchSet: 2 Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zach Amsden
[Impala-ASF-CR] IMPALA-6551: Change Kudu TPCDS and TPCH columns to DECIMAL
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/9484 ) Change subject: IMPALA-6551: Change Kudu TPCDS and TPCH columns to DECIMAL .. Patch Set 4: Code-Review+2 > Fixed and ran the FE tests locally. I also kicked off an exhaustive > pre-review test to make sure I didn't miss anything here: > https://jenkins.impala.io/job/pre-review-test/149/ The failure in that run is a known flaky test (IMPALA-6338) -- To view, visit http://gerrit.cloudera.org:8080/9484 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2f7e4464dc6705cadd610a82c459390a9c0dfe4f Gerrit-Change-Number: 9484 Gerrit-PatchSet: 4 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 12 Mar 2018 22:47:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6635: Add DECIMAL type to Kudu predicates
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9578 ) Change subject: IMPALA-6635: Add DECIMAL type to Kudu predicates .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2088/ -- To view, visit http://gerrit.cloudera.org:8080/9578 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2569a9e1d58f1c58884d58633d46348364888ed7 Gerrit-Change-Number: 9578 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 12 Mar 2018 22:40:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6635: Add DECIMAL type to Kudu predicates
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9578 ) Change subject: IMPALA-6635: Add DECIMAL type to Kudu predicates .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9578 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2569a9e1d58f1c58884d58633d46348364888ed7 Gerrit-Change-Number: 9578 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 12 Mar 2018 22:39:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5717: Support for reading ORC data files
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/9134 ) Change subject: IMPALA-5717: Support for reading ORC data files .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/9134/4/be/src/exec/hdfs-scanner.h File be/src/exec/hdfs-scanner.h: http://gerrit.cloudera.org:8080/#/c/9134/4/be/src/exec/hdfs-scanner.h@379 PS4, Line 379: virtual > why make this virtual? This subroutine interface is pretty ill-defined, so Oh, I forgot this! At first, I found that the signature of HdfsParquetScanner::CommitRows differ from this (just has reverse argument list). I think it may be a mistake so I was going to fix that. But finally abandon it to keep this patch smaller. I'll remove the HdfsOrcScanner::CommitRows as well :) -- To view, visit http://gerrit.cloudera.org:8080/9134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia7b6ae4ce3b9ee8125b21993702faa87537790a4 Gerrit-Change-Number: 9134 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 12 Mar 2018 22:38:56 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6609: Fix ownership of class members in KrpcDataStreamRecvr
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9527 ) Change subject: IMPALA-6609: Fix ownership of class members in KrpcDataStreamRecvr .. IMPALA-6609: Fix ownership of class members in KrpcDataStreamRecvr A KrpcDataStreamRecvr is co-owned by the singleton KrpcDataStreamMgr and an exchange node. It's possible that some threads (e.g. RPC service threads) may still retain reference to the KrpcDataStreamRecvr after its owning exchange node has been closed. This is problematic as some members in the receiver (e.g. sender/receiver profiles) are actually owned by the exchange node so accessing them after the exchange node is closed and possibly deleted may lead to user-after-free. This patch changes the ownership of some members in KrpcDataStreamRecvr to the receiver. In particular, the profiles are now owned by the receiver and various stat counters and timers are in turn owned by these profiles. This prevents the use-after-free problem described above. This patch also moves the access to deferred_rpc_tracker_ in TakeOverEarlySenders() to be under the sender queue's 'lock_' to prevent another instance of IMPALA-6554. Testing done: core debug build. Change-Id: I3378496e2201b16c662b9daa634333480705c61a Reviewed-on: http://gerrit.cloudera.org:8080/9527 Reviewed-by: Dan Hecht Reviewed-by: Sailesh Mukil Tested-by: Impala Public Jenkins --- M be/src/runtime/fragment-instance-state.cc M be/src/runtime/fragment-instance-state.h M be/src/runtime/krpc-data-stream-recvr.cc M be/src/runtime/krpc-data-stream-recvr.h M be/src/runtime/query-state.cc 5 files changed, 104 insertions(+), 30 deletions(-) Approvals: Dan Hecht: Looks good to me, approved Sailesh Mukil: Looks good to me, but someone else must approve Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/9527 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I3378496e2201b16c662b9daa634333480705c61a Gerrit-Change-Number: 9527 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6609: Fix ownership of class members in KrpcDataStreamRecvr
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9527 ) Change subject: IMPALA-6609: Fix ownership of class members in KrpcDataStreamRecvr .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/9527 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3378496e2201b16c662b9daa634333480705c61a Gerrit-Change-Number: 9527 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 12 Mar 2018 22:31:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6576: Add metrics for data stream service memory usage
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9562 ) Change subject: IMPALA-6576: Add metrics for data stream service memory usage .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2087/ -- To view, visit http://gerrit.cloudera.org:8080/9562 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5033b8dfda0b23d4230535ba13c3e050a35d01a3 Gerrit-Change-Number: 9562 Gerrit-PatchSet: 5 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 12 Mar 2018 22:27:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6576: Add metrics for data stream service memory usage
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9562 ) Change subject: IMPALA-6576: Add metrics for data stream service memory usage .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9562 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5033b8dfda0b23d4230535ba13c3e050a35d01a3 Gerrit-Change-Number: 9562 Gerrit-PatchSet: 5 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 12 Mar 2018 22:24:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6576: Add metrics for data stream service memory usage
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/9562 ) Change subject: IMPALA-6576: Add metrics for data stream service memory usage .. Patch Set 5: I wasn't happy with the metric naming, I think "current" captures the idea better than "total". -- To view, visit http://gerrit.cloudera.org:8080/9562 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5033b8dfda0b23d4230535ba13c3e050a35d01a3 Gerrit-Change-Number: 9562 Gerrit-PatchSet: 5 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 12 Mar 2018 22:23:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6498: test query profile thrift timestamps causes following tests to fail.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/9590 ) Change subject: IMPALA-6498: test_query_profile_thrift_timestamps causes following tests to fail. .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/9590/3/tests/query_test/test_observability.py File tests/query_test/test_observability.py: http://gerrit.cloudera.org:8080/#/c/9590/3/tests/query_test/test_observability.py@198 PS3, Line 198: Moving this test to its own suite (IMPALA-6498). This test case forces Unregistration : # of the query, so that we force computation of query end time, which shows up as : # a non-empty 'End Time' in the profile. We use self.client.close() since the : # Beeswax client does not have a cencellation interface. self.client cannot be used : # after close(), so if new test cases are added to this suite, the test cases must be : # executed before this one. Always better to keep comments brief, if possible. I think all of this can be simplified to something like: "This test needs to call self.client.close() to force computation of query end time, so it has to be in its own suite (IMPALA-6498)." -- To view, visit http://gerrit.cloudera.org:8080/9590 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I287a05f3c90b1a71a5b7ee0f5c06a8840ebac4c9 Gerrit-Change-Number: 9590 Gerrit-PatchSet: 3 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Mon, 12 Mar 2018 22:22:44 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6576: Add metrics for data stream service memory usage
Hello Michael Ho, Sailesh Mukil, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9562 to look at the new patch set (#5). Change subject: IMPALA-6576: Add metrics for data stream service memory usage .. IMPALA-6576: Add metrics for data stream service memory usage This change adds metrics for the data stream service memory usage. Both current and peak usage are exposed. It adds a test to test_krpc_metrics.py to make sure that the expected metrics are present and that the peak usage shows a non-zero value after running a query. Change-Id: I5033b8dfda0b23d4230535ba13c3e050a35d01a3 --- M be/src/runtime/exec-env.cc M be/src/service/data-stream-service.cc M be/src/service/data-stream-service.h M be/src/util/memory-metrics.cc M be/src/util/memory-metrics.h M common/thrift/metrics.json M tests/custom_cluster/test_krpc_metrics.py 7 files changed, 104 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/9562/5 -- To view, visit http://gerrit.cloudera.org:8080/9562 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5033b8dfda0b23d4230535ba13c3e050a35d01a3 Gerrit-Change-Number: 9562 Gerrit-PatchSet: 5 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6498: test query profile thrift timestamps causes following tests to fail.
Zoram Thanga has posted comments on this change. ( http://gerrit.cloudera.org:8080/9590 ) Change subject: IMPALA-6498: test_query_profile_thrift_timestamps causes following tests to fail. .. Patch Set 3: (1 comment) Please have a look at ps #3 http://gerrit.cloudera.org:8080/#/c/9590/2/tests/query_test/test_thrift_profile.py File tests/query_test/test_thrift_profile.py: http://gerrit.cloudera.org:8080/#/c/9590/2/tests/query_test/test_thrift_profile.py@24 PS2, Line 24: > It should be sufficient just to put it into its own class still within the Done -- To view, visit http://gerrit.cloudera.org:8080/9590 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I287a05f3c90b1a71a5b7ee0f5c06a8840ebac4c9 Gerrit-Change-Number: 9590 Gerrit-PatchSet: 3 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Mon, 12 Mar 2018 22:17:53 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6498: test query profile thrift timestamps causes following tests to fail.
Zoram Thanga has posted comments on this change. ( http://gerrit.cloudera.org:8080/9590 ) Change subject: IMPALA-6498: test_query_profile_thrift_timestamps causes following tests to fail. .. Patch Set 3: > (1 comment) Thanks for the suggestion. I've moved it back to the old file, but a separate class. -- To view, visit http://gerrit.cloudera.org:8080/9590 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I287a05f3c90b1a71a5b7ee0f5c06a8840ebac4c9 Gerrit-Change-Number: 9590 Gerrit-PatchSet: 3 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Mon, 12 Mar 2018 22:17:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6498: test query profile thrift timestamps causes following tests to fail.
Hello Michael Ho, Thomas Tauber-Marshall, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9590 to look at the new patch set (#3). Change subject: IMPALA-6498: test_query_profile_thrift_timestamps causes following tests to fail. .. IMPALA-6498: test_query_profile_thrift_timestamps causes following tests to fail. test_query_profile_thrift_timestamps uses ImapaTestSuite.client.close() to force cancellation/unregistration of the query, so that 'EndTime' of the query shows up in the profile. Since other test cases also need a valid ImpalaTestSuite.client, we move the test case in question to its own test suite. Have also reduced the query to 'select sleep(5)', as the earlier 'select sleep(1)' is just really excessively long. Change-Id: I287a05f3c90b1a71a5b7ee0f5c06a8840ebac4c9 --- M tests/query_test/test_observability.py 1 file changed, 60 insertions(+), 49 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/9590/3 -- To view, visit http://gerrit.cloudera.org:8080/9590 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I287a05f3c90b1a71a5b7ee0f5c06a8840ebac4c9 Gerrit-Change-Number: 9590 Gerrit-PatchSet: 3 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-6638: Reduce file handle cache lock contention
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/9576 ) Change subject: IMPALA-6638: Reduce file handle cache lock contention .. Patch Set 3: Code-Review+2 Thanks, the new comments make it clearer why we're doing what we're doing. -- To view, visit http://gerrit.cloudera.org:8080/9576 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c695b21ca556e9c73c703c0c891e64939271c8d Gerrit-Change-Number: 9576 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Mon, 12 Mar 2018 22:10:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6498: test query profile thrift timestamps causes following tests to fail.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/9590 ) Change subject: IMPALA-6498: test_query_profile_thrift_timestamps causes following tests to fail. .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/9590/2/tests/query_test/test_thrift_profile.py File tests/query_test/test_thrift_profile.py: http://gerrit.cloudera.org:8080/#/c/9590/2/tests/query_test/test_thrift_profile.py@24 PS2, Line 24: class TestThriftProfile(ImpalaTestSuite): It should be sufficient just to put it into its own class still within the original file. -- To view, visit http://gerrit.cloudera.org:8080/9590 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I287a05f3c90b1a71a5b7ee0f5c06a8840ebac4c9 Gerrit-Change-Number: 9590 Gerrit-PatchSet: 2 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 12 Mar 2018 22:00:35 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6498: test query profile thrift timestamps causes following tests to fail.
Hello Michael Ho, Thomas Tauber-Marshall, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9590 to look at the new patch set (#2). Change subject: IMPALA-6498: test_query_profile_thrift_timestamps causes following tests to fail. .. IMPALA-6498: test_query_profile_thrift_timestamps causes following tests to fail. test_query_profile_thrift_timestamps uses ImapaTestSuite.client.close() to force cancellation/unregistration of the query, so that 'EndTime' of the query shows up in the profile. Since other test cases also need a valid ImpalaTestSuite.client, we move the test case in question to its own test suite. Have also reduced the query to 'select sleep(5)', as the earlier 'select sleep(1)' is just really excessively long. Change-Id: I287a05f3c90b1a71a5b7ee0f5c06a8840ebac4c9 --- M tests/query_test/test_observability.py A tests/query_test/test_thrift_profile.py 2 files changed, 82 insertions(+), 49 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/9590/2 -- To view, visit http://gerrit.cloudera.org:8080/9590 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I287a05f3c90b1a71a5b7ee0f5c06a8840ebac4c9 Gerrit-Change-Number: 9590 Gerrit-PatchSet: 2 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-6498: test query profile thrift timestamps causes following tests to fail.
Zoram Thanga has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9590 Change subject: IMPALA-6498: test_query_profile_thrift_timestamps causes following tests to fail. .. IMPALA-6498: test_query_profile_thrift_timestamps causes following tests to fail. test_query_profile_thrift_timestamps uses ImapaTestSuite.client.close() to force cancellation/unregistration of the query, so that 'EndTime' of the query shows up in the profile. Since other test cases also need a valid ImpalaTestSuite.client, we move the test case in question to its own test suite. Have also reduced the query to 'select sleep(5)', as the earlier 'select sleep(1)' is just really excessively long. Change-Id: I287a05f3c90b1a71a5b7ee0f5c06a8840ebac4c9 --- A tests/query_test/test_thrift_profile.py 1 file changed, 82 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/9590/1 -- To view, visit http://gerrit.cloudera.org:8080/9590 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I287a05f3c90b1a71a5b7ee0f5c06a8840ebac4c9 Gerrit-Change-Number: 9590 Gerrit-PatchSet: 1 Gerrit-Owner: Zoram Thanga
[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9589 Change subject: IMPALA-6643: Add REFRESH fine-grained privilege .. IMPALA-6643: Add REFRESH fine-grained privilege With this patch, REFRESH privilege is now required to execute INVALIDATE METADATA or REFRESH statement. Testing: - Ran end-to-end tests Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4 --- M common/thrift/CatalogObjects.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/Privilege.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/resources/authz-policy.ini.template 8 files changed, 137 insertions(+), 36 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/89/9589/2 -- To view, visit http://gerrit.cloudera.org:8080/9589 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4 Gerrit-Change-Number: 9589 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya
[Impala-ASF-CR] IMPALA-3040: add logging to test caching ddl
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9579 ) Change subject: IMPALA-3040: add logging to test_caching_ddl .. IMPALA-3040: add logging to test_caching_ddl We don't currently have enough information to reconstruct why the test failed, so lets add logging with timestamps to understand which timeout we're actually hitting. Change-Id: Iabc30445440e0fb358856da407d833f5ae975213 Reviewed-on: http://gerrit.cloudera.org:8080/9579 Reviewed-by: Thomas Tauber-Marshall Tested-by: Impala Public Jenkins --- M tests/query_test/test_hdfs_caching.py 1 file changed, 5 insertions(+), 1 deletion(-) Approvals: Thomas Tauber-Marshall: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/9579 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Iabc30445440e0fb358856da407d833f5ae975213 Gerrit-Change-Number: 9579 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-3040: add logging to test caching ddl
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9579 ) Change subject: IMPALA-3040: add logging to test_caching_ddl .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/9579 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iabc30445440e0fb358856da407d833f5ae975213 Gerrit-Change-Number: 9579 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 12 Mar 2018 21:53:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6638: Reduce file handle cache lock contention
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/9576 ) Change subject: IMPALA-6638: Reduce file handle cache lock contention .. Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/9576/2/be/src/runtime/io/handle-cache.inline.h File be/src/runtime/io/handle-cache.inline.h: http://gerrit.cloudera.org:8080/#/c/9576/2/be/src/runtime/io/handle-cache.inline.h@128 PS2, Line 128: This only happens if there is no entry for this file or all the : // existing file handles are in use. > That was true while holding the lock above but is no longer necessarily tru Changed this to insert in the front and updated this comment about why. Also added a comment when iterating above to explain why iterating in order is important. http://gerrit.cloudera.org:8080/#/c/9576/2/be/src/runtime/io/handle-cache.inline.h@130 PS2, Line 130: matter whether we insert this entry before or after or in between the : // existing entries for this file > why is the previous sentence needed in order for this to be okay? This was poorly worded. What I meant is that not much time has passed since we saw that there wasn't any entry available. Any ordering decision is mostly unimportant under that circumstance. I thought about it some more, and we are very slightly better off explicitly specifying the ordering. It is mostly an edge case, but inserting at the front solves it. http://gerrit.cloudera.org:8080/#/c/9576/2/be/src/runtime/io/handle-cache.inline.h@134 PS2, Line 134: FileHandleEntry entry(new_fh, p.lru_list); : typename MapType::iterator new_it = p.cache.emplace(*fname, std::move(entry)); > I would remove the constructor and move the arguments to emplace: emplace(* I appreciate the comment, but I try to avoid style changes in adjacent code. http://gerrit.cloudera.org:8080/#/c/9576/2/be/src/runtime/io/handle-cache.inline.h@137 PS2, Line 137: ++p.size; : if (p.size > p.capacity) EvictHandles(p); > I would prefer to move this before the creation of the new entry, to make i I appreciate the comment, but I try to avoid style changes in adjacent code. http://gerrit.cloudera.org:8080/#/c/9576/2/be/src/runtime/io/handle-cache.inline.h@139 PS2, Line 139: DCHECK(!new_elem->in_use); > This DCHECK could be probably removed. This could go either way. We are relying on the constructor setting in_use to false. I figure a DCHECK doesn't hurt. -- To view, visit http://gerrit.cloudera.org:8080/9576 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c695b21ca556e9c73c703c0c891e64939271c8d Gerrit-Change-Number: 9576 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Mon, 12 Mar 2018 21:51:19 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6638: Reduce file handle cache lock contention
Hello Csaba Ringhofer, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9576 to look at the new patch set (#3). Change subject: IMPALA-6638: Reduce file handle cache lock contention .. IMPALA-6638: Reduce file handle cache lock contention FileHandleCache::OpenFileHandle() currently holds the lock while opening a file handle. This lengthens the duration holding the lock considerably, causing contention when a lot of file handles are being opened (i.e. when the cache is cold). This changes FileHandleCache::OpenFileHandle() drops the lock while opening the file handle, then reacquires it to add the file handle to the cache. When running a simple select on a table with 46801 Parquet files, this fixes a small performance overhead for the cold cache case compared to having the cache off. All results are averaged over 5 runs: File handle cache off: 7.19s File handle cache cold (no patch): 7.92s File handle cache cold (with patch): 7.20s When running a select that accesses and aggregates 4 columns from the same Parquet table (thus requiring more scan ranges for the same file), the fix reduces contention, particularly for the case with 8 IO threads per disk: 1 IO thread per disk: Without patch: 9.59s With patch: 8.11s 8 IO threads per disk: Without patch: 14.2s With patch: 8.17s The patch has no impact on the performance when the cache is hot. Change-Id: I4c695b21ca556e9c73c703c0c891e64939271c8d --- M be/src/runtime/io/handle-cache.inline.h 1 file changed, 37 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/9576/3 -- To view, visit http://gerrit.cloudera.org:8080/9576 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4c695b21ca556e9c73c703c0c891e64939271c8d Gerrit-Change-Number: 9576 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Joe McDonnell
[Impala-ASF-CR] IMPALA-3866: Improve error reporting for scratch write errors
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9420 ) Change subject: IMPALA-3866: Improve error reporting for scratch write errors .. Patch Set 7: (6 comments) http://gerrit.cloudera.org:8080/#/c/9420/8/be/src/runtime/io/disk-io-mgr.h File be/src/runtime/io/disk-io-mgr.h: http://gerrit.cloudera.org:8080/#/c/9420/8/be/src/runtime/io/disk-io-mgr.h@373 PS8, Line 373: /// queued buffers (plus the buffer that is returned to the client) gives good Let's make it clear that it's a test-only function. http://gerrit.cloudera.org:8080/#/c/9420/8/be/src/runtime/io/disk-io-mgr.h@425 PS8, Line 425: /// provide a MemTracker. We try to avoid shared_ptr as much as possible because shared ownership is generally harder to reason about. I think in this case unique_ptr is sufficient if you move() it into set_local_file_system http://gerrit.cloudera.org:8080/#/c/9420/8/be/src/runtime/io/disk-io-mgr.cc File be/src/runtime/io/disk-io-mgr.cc: http://gerrit.cloudera.org:8080/#/c/9420/8/be/src/runtime/io/disk-io-mgr.cc@1101 PS8, Line 1101: DCHECK(write_range != nullptr); I missed on my first pass that we aren't fclose()ing the file when we hit an error now. It would be ok to add it before each call to HandleWriteFinished(), but I would probably use the "goto end" pattern since we need to execute the exact same two lines of code on all exit paths, i.e.: ret_status = ...; if (!ret_status.ok()) goto end: ... end: ret_status = local_file_system_->Fclose(file_handle, write_range); HandleWriteFinished(writer_context, write_range, ret_status); http://gerrit.cloudera.org:8080/#/c/9420/7/be/src/runtime/io/errno-to-error-status-converter.cc File be/src/runtime/io/errno-to-error-status-converter.cc: http://gerrit.cloudera.org:8080/#/c/9420/7/be/src/runtime/io/errno-to-error-status-converter.cc@23 PS7, Line 23: using std::unordered_map; > I wasn't aware of this common/names.h, thx! Yeah that's a bit of an unfortunate case. We started off with boost::unordered_map. In most cases where a std:: equivalent appeared we've automatically switched, but there was some concern here that the std:: versions of the maps would use somewhat more memory and be slower because the standard requires using doubly-linked lists for buckets: http://bannalia.blogspot.co.uk/2013/10/implementation-of-c-unordered.html We could probably benchmark std::unordered_map and switch over if it looked acceptable but we haven't put the time in yet. I'd suggest using std::unordered_map here since the map is small and its not perf-critical. http://gerrit.cloudera.org:8080/#/c/9420/8/be/src/runtime/io/local-file-system-with-fault-injection.cc File be/src/runtime/io/local-file-system-with-fault-injection.cc: http://gerrit.cloudera.org:8080/#/c/9420/8/be/src/runtime/io/local-file-system-with-fault-injection.cc@31 PS8, Line 31: Long lines. It might be useful to run clang-format on the patches to detect some of these minor issues: https://wiki.cloudera.com/pages/viewpage.action?pageId=24646528 The downside is that clang-format is sometimes overly aggressive at reformatting surrounding lines, which adds unnecessary code churn. http://gerrit.cloudera.org:8080/#/c/9420/8/be/src/runtime/io/local-file-system.cc File be/src/runtime/io/local-file-system.cc: http://gerrit.cloudera.org:8080/#/c/9420/8/be/src/runtime/io/local-file-system.cc@57 PS8, Line 57: I'm not sure what this TODO means -- To view, visit http://gerrit.cloudera.org:8080/9420 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5aa7b424209b1a5ef8dc7d04c5ba58788e91aad7 Gerrit-Change-Number: 9420 Gerrit-PatchSet: 7 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 12 Mar 2018 21:45:01 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5315: Cast to timestamp fails for YYYY-M-D format
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/7009 ) Change subject: IMPALA-5315: Cast to timestamp fails for -M-D format .. Patch Set 16: I started a dry run of the merge just in case there were any test failures or clang-tidy warnings: https://jenkins.impala.io/job/gerrit-verify-dryrun/2086/ -- To view, visit http://gerrit.cloudera.org:8080/7009 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f Gerrit-Change-Number: 7009 Gerrit-PatchSet: 16 Gerrit-Owner: Vincent Tran Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vincent Tran Gerrit-Comment-Date: Mon, 12 Mar 2018 21:16:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5315: Cast to timestamp fails for YYYY-M-D format
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/7009 ) Change subject: IMPALA-5315: Cast to timestamp fails for -M-D format .. Patch Set 16: (5 comments) I think this should be the last round of comments. http://gerrit.cloudera.org:8080/#/c/7009/16/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/7009/16/be/src/exprs/expr-test.cc@6176 PS16, Line 6176: TestStringValue("monthname(cast('2011-02-18 09:10:11.00' as timestamp))", "February"); nit: long lines. It might be easier to run the patch through clang-format to fix some of these whitespace and include ordering issues: https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=65868536 http://gerrit.cloudera.org:8080/#/c/7009/16/be/src/runtime/timestamp-parse-util.cc File be/src/runtime/timestamp-parse-util.cc: http://gerrit.cloudera.org:8080/#/c/7009/16/be/src/runtime/timestamp-parse-util.cc@28 PS16, Line 28: #include Standard C++ headers go before the 3rd-party library headers. I.e. line 19 above. http://gerrit.cloudera.org:8080/#/c/7009/16/testdata/workloads/functional-query/queries/QueryTest/cast-lazy-datetime-string.test File testdata/workloads/functional-query/queries/QueryTest/cast-lazy-datetime-string.test: http://gerrit.cloudera.org:8080/#/c/7009/16/testdata/workloads/functional-query/queries/QueryTest/cast-lazy-datetime-string.test@3 PS16, Line 3: select cast(dt_str as timestamp) from lazy_dt_str should not need a cast here, we want to test the text scanner parsing the datetime column directly http://gerrit.cloudera.org:8080/#/c/7009/16/testdata/workloads/functional-query/queries/QueryTest/cast-lazy-datetime-string.test@4 PS16, Line 4: RESULTS Let's make this RESULTS: VERIFY_IS_EQUAL_SORTED We don't guarantee the order of returned rows without an order by clause (although in practice it will be in the file order for this query plan). http://gerrit.cloudera.org:8080/#/c/7009/16/tests/query_test/test_scanners.py File tests/query_test/test_scanners.py: http://gerrit.cloudera.org:8080/#/c/7009/16/tests/query_test/test_scanners.py@874 PS16, Line 874: STRING This should be a timestamp column, so we can make sure that the text scanner does the conversion correctly. -- To view, visit http://gerrit.cloudera.org:8080/7009 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f Gerrit-Change-Number: 7009 Gerrit-PatchSet: 16 Gerrit-Owner: Vincent Tran Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vincent Tran Gerrit-Comment-Date: Mon, 12 Mar 2018 21:15:51 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6341, IMPALA-5917: Reduce mem-limit for start-impala-cluster.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9395 ) Change subject: IMPALA-6341, IMPALA-5917: Reduce mem-limit for start-impala-cluster. .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2084/ -- To view, visit http://gerrit.cloudera.org:8080/9395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iabca7a95560bd27c2de2b0a147ee9a3c45199db7 Gerrit-Change-Number: 9395 Gerrit-PatchSet: 2 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 12 Mar 2018 20:55:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6341, IMPALA-5917: Reduce mem-limit for start-impala-cluster.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/9395 ) Change subject: IMPALA-6341, IMPALA-5917: Reduce mem-limit for start-impala-cluster. .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/9395/2/testdata/workloads/functional-query/queries/QueryTest/semi-joins-exhaustive.test File testdata/workloads/functional-query/queries/QueryTest/semi-joins-exhaustive.test: http://gerrit.cloudera.org:8080/#/c/9395/2/testdata/workloads/functional-query/queries/QueryTest/semi-joins-exhaustive.test@9 PS2, Line 9: # of the relevant fragment is 3.6GB when tested.) > It might be worth reducing it further, since we could potentially still hav I've not done this at the moment. -- To view, visit http://gerrit.cloudera.org:8080/9395 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iabca7a95560bd27c2de2b0a147ee9a3c45199db7 Gerrit-Change-Number: 9395 Gerrit-PatchSet: 2 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 12 Mar 2018 20:53:46 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6621: Improve set lookup performance for in-predicate evaluation
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9570 ) Change subject: IMPALA-6621: Improve set lookup performance for in-predicate evaluation .. Patch Set 1: (1 comment) Out of curiousity I tried out this alternative hash map: https://github.com/greg7mdp/sparsepp It was actually slower for decimal (700ms vs 500ms). I also concluded that google's dense_hash_set was somewhat tricky to make work, since it requires providing a sentinel value to represent empty entries. http://gerrit.cloudera.org:8080/#/c/9570/1/be/src/exprs/in-predicate.h File be/src/exprs/in-predicate.h: http://gerrit.cloudera.org:8080/#/c/9570/1/be/src/exprs/in-predicate.h@359 PS1, Line 359: state->val_set.insert(GetVal(state->type, *arg)); We should change this function to use the bulk insert API to avoid N^2 behaviour with flat_set: http://www.boost.org/doc/libs/1_56_0/doc/html/boost/container/flat_set.html#idp30015536-bb -- To view, visit http://gerrit.cloudera.org:8080/9570 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifd1627d779d10a16468cc3c2d0bc26a497e048df Gerrit-Change-Number: 9570 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 12 Mar 2018 20:40:02 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6576: Add metrics for data stream service memory usage
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9562 ) Change subject: IMPALA-6576: Add metrics for data stream service memory usage .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2083/ -- To view, visit http://gerrit.cloudera.org:8080/9562 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5033b8dfda0b23d4230535ba13c3e050a35d01a3 Gerrit-Change-Number: 9562 Gerrit-PatchSet: 4 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 12 Mar 2018 20:25:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6635: Add DECIMAL type to Kudu predicates
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/9578 ) Change subject: IMPALA-6635: Add DECIMAL type to Kudu predicates .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/9578 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2569a9e1d58f1c58884d58633d46348364888ed7 Gerrit-Change-Number: 9578 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 12 Mar 2018 20:15:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6576: Add metrics for data stream service memory usage
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9562 ) Change subject: IMPALA-6576: Add metrics for data stream service memory usage .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9562 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5033b8dfda0b23d4230535ba13c3e050a35d01a3 Gerrit-Change-Number: 9562 Gerrit-PatchSet: 4 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 12 Mar 2018 20:03:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5717: Support for reading ORC data files
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/9134 ) Change subject: IMPALA-5717: Support for reading ORC data files .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/9134/4/be/src/exec/hdfs-scanner.h File be/src/exec/hdfs-scanner.h: http://gerrit.cloudera.org:8080/#/c/9134/4/be/src/exec/hdfs-scanner.h@379 PS4, Line 379: virtual why make this virtual? This subroutine interface is pretty ill-defined, so I think we should avoid making it overridable. I don't think it's needed to be virtual anyway, since only the subclasses call it, so you could always just not use it from the orc scanner. But, it looks like the orc scanner version is the same implementation anyway... -- To view, visit http://gerrit.cloudera.org:8080/9134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia7b6ae4ce3b9ee8125b21993702faa87537790a4 Gerrit-Change-Number: 9134 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 12 Mar 2018 19:58:41 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5980: Upgrade to LLVM 5.0.1
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/9584 ) Change subject: IMPALA-5980: Upgrade to LLVM 5.0.1 .. Patch Set 1: Do any of those benchmarks verify compile time performance? -- To view, visit http://gerrit.cloudera.org:8080/9584 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib0a15cb53feab89e7b35a56b67b3b30eb3e62c6b Gerrit-Change-Number: 9584 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Comment-Date: Mon, 12 Mar 2018 19:45:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6576: Add metrics for data stream service memory usage
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/9562 ) Change subject: IMPALA-6576: Add metrics for data stream service memory usage .. Patch Set 4: Code-Review+1 Thanks for the reviews. PS4 addresses Tim's comment. Carrying +1 from Michael and Sailesh. -- To view, visit http://gerrit.cloudera.org:8080/9562 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5033b8dfda0b23d4230535ba13c3e050a35d01a3 Gerrit-Change-Number: 9562 Gerrit-PatchSet: 4 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 12 Mar 2018 19:39:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6576: Add metrics for data stream service memory usage
Hello Michael Ho, Sailesh Mukil, Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9562 to look at the new patch set (#4). Change subject: IMPALA-6576: Add metrics for data stream service memory usage .. IMPALA-6576: Add metrics for data stream service memory usage This change adds metrics for the data stream service memory usage. Both current and peak usage are exposed. It adds a test to test_krpc_metrics.py to make sure that the expected metrics are present and that the peak usage shows a non-zero value after running a query. Change-Id: I5033b8dfda0b23d4230535ba13c3e050a35d01a3 --- M be/src/runtime/exec-env.cc M be/src/service/data-stream-service.cc M be/src/service/data-stream-service.h M be/src/util/memory-metrics.cc M be/src/util/memory-metrics.h M common/thrift/metrics.json M tests/custom_cluster/test_krpc_metrics.py 7 files changed, 104 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/9562/4 -- To view, visit http://gerrit.cloudera.org:8080/9562 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5033b8dfda0b23d4230535ba13c3e050a35d01a3 Gerrit-Change-Number: 9562 Gerrit-PatchSet: 4 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6638: Reduce file handle cache lock contention
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/9576 ) Change subject: IMPALA-6638: Reduce file handle cache lock contention .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/9576/2/be/src/runtime/io/handle-cache.inline.h File be/src/runtime/io/handle-cache.inline.h: http://gerrit.cloudera.org:8080/#/c/9576/2/be/src/runtime/io/handle-cache.inline.h@128 PS2, Line 128: This only happens if there is no entry for this file or all the : // existing file handles are in use. That was true while holding the lock above but is no longer necessarily true since we no longer hold the lock -- there might now be an unused entry for this file. So how can we rely on that? http://gerrit.cloudera.org:8080/#/c/9576/2/be/src/runtime/io/handle-cache.inline.h@130 PS2, Line 130: matter whether we insert this entry before or after or in between the : // existing entries for this file why is the previous sentence needed in order for this to be okay? -- To view, visit http://gerrit.cloudera.org:8080/9576 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c695b21ca556e9c73c703c0c891e64939271c8d Gerrit-Change-Number: 9576 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Mon, 12 Mar 2018 19:28:00 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6479: Update DESCRIBE to respect column privileges
Adam Holley has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/9276 ) Change subject: IMPALA-6479: Update DESCRIBE to respect column privileges .. IMPALA-6479: Update DESCRIBE to respect column privileges Modified the Frontend to filter columns from the DESCRIBE statement. Additionally, if a user has select on at least one column, they can run DESCRIBE and see most metadata. If they do not have full table access, they will not see location or view query metadata. Testing: Added tests to validate users that have one or more column access can run describe and that the output is filtered accordingly. Change-Id: Ic96ae184fccdc88ba970b5adcd501da1966accb9 --- M be/src/service/client-request-state.cc M be/src/service/frontend.cc M be/src/service/frontend.h M common/thrift/Frontend.thrift 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/catalog/Column.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/test/java/org/apache/impala/analysis/AuditingTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java 13 files changed, 333 insertions(+), 67 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/9276/2 -- To view, visit http://gerrit.cloudera.org:8080/9276 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic96ae184fccdc88ba970b5adcd501da1966accb9 Gerrit-Change-Number: 9276 Gerrit-PatchSet: 2 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm
[Impala-ASF-CR] IMPALA-6479: Update DESCRIBE to respect column privileges
Adam Holley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9276 ) Change subject: IMPALA-6479: Update DESCRIBE to respect column privileges .. Patch Set 1: (22 comments) Changes applied. http://gerrit.cloudera.org:8080/#/c/9276/1/be/src/service/client-request-state.cc File be/src/service/client-request-state.cc: http://gerrit.cloudera.org:8080/#/c/9276/1/be/src/service/client-request-state.cc@381 PS1, Line 381: params->__isset.table_name ? &(params->table_name) : NULL; > NULL -> nullptr Removed lines from other refactoring. http://gerrit.cloudera.org:8080/#/c/9276/1/be/src/service/frontend.h File be/src/service/frontend.h: http://gerrit.cloudera.org:8080/#/c/9276/1/be/src/service/frontend.h@133 PS1, Line 133: Status DescribeTable(const TDescribeOutputStyle::type output_style, > Why do we need to change the signature so dramatically? Should it not be su Refactored. http://gerrit.cloudera.org:8080/#/c/9276/1/common/thrift/Frontend.thrift File common/thrift/Frontend.thrift: http://gerrit.cloudera.org:8080/#/c/9276/1/common/thrift/Frontend.thrift@173 PS1, Line 173: 4: optional ImpalaInternalService.TSessionState session > Yes, you are right. We need to keep this field. Done http://gerrit.cloudera.org:8080/#/c/9276/1/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java File fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java: http://gerrit.cloudera.org:8080/#/c/9276/1/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java@122 PS1, Line 122: resultStruct_ = Path.getTypeAsStruct(path_.destType()); > Maybe I'm missing something, but it seems like the following scenario is no Done http://gerrit.cloudera.org:8080/#/c/9276/1/fe/src/main/java/org/apache/impala/catalog/Table.java File fe/src/main/java/org/apache/impala/catalog/Table.java: http://gerrit.cloudera.org:8080/#/c/9276/1/fe/src/main/java/org/apache/impala/catalog/Table.java@489 PS1, Line 489: public StructType getHiveColumnsAsStruct(List columns) { > Seems weird to have this as a member, since it's totally non-specific to th Done http://gerrit.cloudera.org:8080/#/c/9276/1/fe/src/main/java/org/apache/impala/catalog/Table.java@490 PS1, Line 490: ArrayList fields = Lists.newArrayListWithCapacity(colsByPos_.size()); > columns.size() Done http://gerrit.cloudera.org:8080/#/c/9276/1/fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java File fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java: http://gerrit.cloudera.org:8080/#/c/9276/1/fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java@199 PS1, Line 199: List nonClustered = new ArrayList(table.getNonClusteringColumns()); > Will this code work if 'nonClustered' or 'clustered' is empty? Done http://gerrit.cloudera.org:8080/#/c/9276/1/fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java@200 PS1, Line 200: nonClustered.retainAll(filteredColumns); > Concise but slow. I suggest this instead Done http://gerrit.cloudera.org:8080/#/c/9276/1/fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java@259 PS1, Line 259:* Builds a TDescribeResult for a table. > update comment Done http://gerrit.cloudera.org:8080/#/c/9276/1/fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java@261 PS1, Line 261: public static TDescribeResult buildDescribeMinimalResult(List columns) { > buildKuduDescribeMinimalResult() Done http://gerrit.cloudera.org:8080/#/c/9276/1/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/9276/1/fe/src/main/java/org/apache/impala/service/Frontend.java@796 PS1, Line 796: for (Column col: table.getColumnsInHiveOrder()) { > Can we do a table-level check first? Checking all columns all the time is p Done http://gerrit.cloudera.org:8080/#/c/9276/1/fe/src/main/java/org/apache/impala/service/Frontend.java@806 PS1, Line 806: filteredColumns = table.getColumns(); > Shouldn't this be getColumnsInHiveOrder()? Done http://gerrit.cloudera.org:8080/#/c/9276/1/fe/src/main/java/org/apache/impala/service/JniFrontend.java File fe/src/main/java/org/apache/impala/service/JniFrontend.java: http://gerrit.cloudera.org:8080/#/c/9276/1/fe/src/main/java/org/apache/impala/service/JniFrontend.java@434 PS1, Line 434: // If the session was not set it indicates this is an internal Impala call. > Where is this called internally? I only see this function being called Clie Done http://gerrit.cloudera.org:8080/#/c/9276/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java: http://gerrit.cloudera.org:8080/#/c/9276/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@116 PS1, Line 116: private static final List FUNCTIONAL_
[Impala-ASF-CR] IMPALA-6635: Add DECIMAL type to Kudu predicates
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9578 ) Change subject: IMPALA-6635: Add DECIMAL type to Kudu predicates .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/9578/1/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test File testdata/workloads/functional-planner/queries/PlannerTest/kudu.test: http://gerrit.cloudera.org:8080/#/c/9578/1/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test@553 PS1, Line 553: # Decimal InList predicate. > Good point. You could still make it work by having that test join alltypes Done -- To view, visit http://gerrit.cloudera.org:8080/9578 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2569a9e1d58f1c58884d58633d46348364888ed7 Gerrit-Change-Number: 9578 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 12 Mar 2018 19:19:58 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6635: Add DECIMAL type to Kudu predicates
Hello Thomas Tauber-Marshall, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9578 to look at the new patch set (#2). Change subject: IMPALA-6635: Add DECIMAL type to Kudu predicates .. IMPALA-6635: Add DECIMAL type to Kudu predicates This patch enables pushing scan predicates on DECIMAL columns down to Kudu. Testing: - Added Planner decimal predicate test to kudu.test - Added Planner decimal in-list test to kudu-selectivity.test Change-Id: I2569a9e1d58f1c58884d58633d46348364888ed7 --- M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test 3 files changed, 35 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/78/9578/2 -- To view, visit http://gerrit.cloudera.org:8080/9578 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2569a9e1d58f1c58884d58633d46348364888ed7 Gerrit-Change-Number: 9578 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-6635: Add DECIMAL type to Kudu predicates
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/9578 ) Change subject: IMPALA-6635: Add DECIMAL type to Kudu predicates .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/9578/1/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test File testdata/workloads/functional-planner/queries/PlannerTest/kudu.test: http://gerrit.cloudera.org:8080/#/c/9578/1/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test@553 PS1, Line 553: # Decimal InList predicate. > I can't add the column there because the functional alltypes table doesn't Good point. You could still make it work by having that test join alltypes and decimal_tbl, but that's messy. If you just move this to be next to that test (and also add comments on both test cases to make it clear what's going on) that should be fine -- To view, visit http://gerrit.cloudera.org:8080/9578 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2569a9e1d58f1c58884d58633d46348364888ed7 Gerrit-Change-Number: 9578 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 12 Mar 2018 19:18:37 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6635: Add DECIMAL type to Kudu predicates
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9578 ) Change subject: IMPALA-6635: Add DECIMAL type to Kudu predicates .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/9578/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9578/1//COMMIT_MSG@8 PS1, Line 8: > Could you add a little more color here, eg. "This patch enables pushing sca Done http://gerrit.cloudera.org:8080/#/c/9578/1/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test File testdata/workloads/functional-planner/queries/PlannerTest/kudu.test: http://gerrit.cloudera.org:8080/#/c/9578/1/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test@553 PS1, Line 553: # Decimal InList predicate. > There's an existing test in 'PlannerTest/kudu-selectivity.test' (around lin I can't add the column there because the functional alltypes table doesn't actually contain all types and adding the decimal type would be extremely invasive given how widely used that table is in tests. I could move this test to that location if you prefer, but it would still be stand alone. -- To view, visit http://gerrit.cloudera.org:8080/9578 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2569a9e1d58f1c58884d58633d46348364888ed7 Gerrit-Change-Number: 9578 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 12 Mar 2018 19:13:37 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5980: Upgrade to LLVM 5.0.1
Bikramjeet Vig has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9584 Change subject: IMPALA-5980: Upgrade to LLVM 5.0.1 .. IMPALA-5980: Upgrade to LLVM 5.0.1 Highlighting a few changes in LLVM: - Minor changes to some function signatures - Minor changes to error handling - Split Bitcode/ReaderWriter.h - https://reviews.llvm.org/D26502 - Introduced an optional new GVN optimization pass. Needed to fix a few new clang-tidy warnings. Testing: Ran core and ASAN tests successfully. Performance: Ran single node TPC-H and targeted perf with scale factor 60. Both improved on average. Identified regression in "primitive_filter_in_predicate" which will be addressed by IMPALA-6621. +---+---+-++++ | Workload | File Format | Avg (s) | Delta(Avg) | GeoMean(s) | Delta(GeoMean) | +---+---+-++++ | TARGETED-PERF(60) | parquet / none / none | 22.29 | -0.12% | 3.90 | +3.16% | | TPCH(60) | parquet / none / none | 15.97 | -3.64% | 10.14 | -4.92% | +---+---+-++++ +---++---++-++++-+---+ | Workload | Query | File Format | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%) | Base StdDev(%) | Num Clients | Iters | +---++---++-++++-+---+ | TARGETED-PERF(60) | PERF_LIMIT-Q1 | parquet / none / none | 0.01 | 0.00| R +156.43% | * 25.80% * | * 17.14% * | 1 | 5 | | TARGETED-PERF(60) | primitive_filter_in_predicate | parquet / none / none | 3.39 | 1.92| R +76.33% | 3.23%| 4.37%| 1 | 5 | | TARGETED-PERF(60) | primitive_filter_string_non_selective | parquet / none / none | 1.25 | 1.11| +12.46% | 3.41%| 5.36%| 1 | 5 | | TARGETED-PERF(60) | primitive_filter_decimal_selective | parquet / none / none | 1.40 | 1.25| +12.25% | 3.57%| 3.44%| 1 | 5 | | TARGETED-PERF(60) | primitive_filter_string_like | parquet / none / none | 16.87 | 15.65 | +7.78% | 5.05%| 0.37%| 1 | 5 | | TARGETED-PERF(60) | primitive_min_max_runtime_filter | parquet / none / none | 1.79 | 1.71| +4.77% | 0.71%| 1.73%| 1 | 5 | | TARGETED-PERF(60) | primitive_broadcast_join_2 | parquet / none / none | 0.60 | 0.58| +3.64% | 3.19%| 3.81%| 1 | 5 | | TARGETED-PERF(60) | primitive_filter_string_selective | parquet / none / none | 0.95 | 0.93| +2.91% | 5.23%| 5.85%| 1 | 5 | | TARGETED-PERF(60) | primitive_broadcast_join_3 | parquet / none / none | 4.33 | 4.21| +2.83% | 5.46%| 3.25%| 1 | 5 | | TARGETED-PERF(60) | primitive_groupby_bigint_lowndv| parquet / none / none | 4.59 | 4.47| +2.82% | 3.73%| 1.14%| 1 | 5 | | TARGETED-PERF(60) | primitive_conjunct_ordering_3 | parquet / none / none | 0.20 | 0.19| +2.65% | 4.76%| 2.24%| 1 | 5 | | TARGETED-PERF(60) | PERF_AGG-Q1| parquet / none / none | 2.49 | 2.43| +2.31% | 1.06%| 1.93%| 1 | 5 | | TARGETED-PERF(60) | PERF_AGG-Q6| parquet / none / none | 2.04 | 2.00| +2.09% | 3.51%| 2.80%| 1 | 5 | | TPCH(60) | TPCH-Q3| parquet / none / none | 12.37 | 12.17 | +1.62% | 0.80%| 2.45%| 1 | 5 | | TARGETED-PERF(60) | PERF_STRING-Q5 | parquet / none / none | 4.52 | 4.45| +1.54% | 1.23%| 1.08%| 1 | 5 | | TPCH(60) | TPCH-Q6| parquet / none / none | 2.95 | 2.91| +1.33% | 1.92%|
[Impala-ASF-CR] IMPALA-6576: Add metrics for data stream service memory usage
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9562 ) Change subject: IMPALA-6576: Add metrics for data stream service memory usage .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/9562/3/be/src/service/data-stream-service.cc File be/src/service/data-stream-service.cc: http://gerrit.cloudera.org:8080/#/c/9562/3/be/src/service/data-stream-service.cc@58 PS3, Line 58: InitMetrics I think InitMetrics() is misleading - the name is too similar to other functions that do some kind of global initialization. Maybe something like MemTrackerMetric::CreateMetrics() so it sounds more like a constructor/factory method. -- To view, visit http://gerrit.cloudera.org:8080/9562 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5033b8dfda0b23d4230535ba13c3e050a35d01a3 Gerrit-Change-Number: 9562 Gerrit-PatchSet: 3 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 12 Mar 2018 19:07:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5191, IMPALA-6415: [DOCS] Document breaking change of alias and ordinal substitution
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9211 ) Change subject: IMPALA-5191, IMPALA-6415: [DOCS] Document breaking change of alias and ordinal substitution .. Patch Set 1: (1 comment) thanks! http://gerrit.cloudera.org:8080/#/c/9211/1/docs/topics/impala_aliases.xml File docs/topics/impala_aliases.xml: http://gerrit.cloudera.org:8080/#/c/9211/1/docs/topics/impala_aliases.xml@76 PS1, Line 76: , you can read more about it at > Replace with "." Conref will render the text here. Done -- To view, visit http://gerrit.cloudera.org:8080/9211 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I558230d07212da62d2cd12e07a52ceba03e980a8 Gerrit-Change-Number: 9211 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: John Russell Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 12 Mar 2018 19:05:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5191, IMPALA-6415: [DOCS] Document breaking change of alias and ordinal substitution
Hello Alex Rodoni, John Russell, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9211 to look at the new patch set (#2). Change subject: IMPALA-5191, IMPALA-6415: [DOCS] Document breaking change of alias and ordinal substitution .. IMPALA-5191, IMPALA-6415: [DOCS] Document breaking change of alias and ordinal substitution The alias and ordinal substitution logic has been changed by IMPALA-5191. This commit updates the documentation regarding to the new behavior. Change-Id: I558230d07212da62d2cd12e07a52ceba03e980a8 Cherry-picks: not for 2.x. --- M docs/shared/impala_common.xml M docs/topics/impala_aliases.xml 2 files changed, 77 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/9211/2 -- To view, visit http://gerrit.cloudera.org:8080/9211 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I558230d07212da62d2cd12e07a52ceba03e980a8 Gerrit-Change-Number: 9211 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: John Russell Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-6635: Add DECIMAL type to Kudu predicates
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/9578 ) Change subject: IMPALA-6635: Add DECIMAL type to Kudu predicates .. Patch Set 1: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/9578/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9578/1//COMMIT_MSG@8 PS1, Line 8: Could you add a little more color here, eg. "This patch enables pushing scan predicates on DECIMAL columns down to Kudu. Testing: ..." http://gerrit.cloudera.org:8080/#/c/9578/1/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test File testdata/workloads/functional-planner/queries/PlannerTest/kudu.test: http://gerrit.cloudera.org:8080/#/c/9578/1/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test@553 PS1, Line 553: # Decimal InList predicate. There's an existing test in 'PlannerTest/kudu-selectivity.test' (around line 124) that covers in-list predicates for all of the other types. Can you add this there? (and could you also add a brief comment for that test case that notes that its testing push down for various types of in-list predicates?) -- To view, visit http://gerrit.cloudera.org:8080/9578 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2569a9e1d58f1c58884d58633d46348364888ed7 Gerrit-Change-Number: 9578 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 12 Mar 2018 19:00:11 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6576: Add metrics for data stream service memory usage
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/9562 ) Change subject: IMPALA-6576: Add metrics for data stream service memory usage .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/9562/3/be/src/service/data-stream-service.cc File be/src/service/data-stream-service.cc: http://gerrit.cloudera.org:8080/#/c/9562/3/be/src/service/data-stream-service.cc@58 PS3, Line 58: DataStreamService > If we're going to do a CM change anyway, then I suggest staying consistent Lars pointed out to me that non KRPC RPC metrics are also camel cased. So, since that's the case, lets leave this as it is. It's not important enough to have to change everything at this point. -- To view, visit http://gerrit.cloudera.org:8080/9562 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5033b8dfda0b23d4230535ba13c3e050a35d01a3 Gerrit-Change-Number: 9562 Gerrit-PatchSet: 3 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 12 Mar 2018 18:58:03 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6609: Fix ownership of class members in KrpcDataStreamRecvr
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9527 ) Change subject: IMPALA-6609: Fix ownership of class members in KrpcDataStreamRecvr .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2082/ -- To view, visit http://gerrit.cloudera.org:8080/9527 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3378496e2201b16c662b9daa634333480705c61a Gerrit-Change-Number: 9527 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 12 Mar 2018 18:56:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6638: Reduce file handle cache lock contention
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/9576 ) Change subject: IMPALA-6638: Reduce file handle cache lock contention .. Patch Set 2: Code-Review+1 (3 comments) Thanks for the explanation! I have left some optional comments, but the code is ok for me as it is. http://gerrit.cloudera.org:8080/#/c/9576/2/be/src/runtime/io/handle-cache.inline.h File be/src/runtime/io/handle-cache.inline.h: http://gerrit.cloudera.org:8080/#/c/9576/2/be/src/runtime/io/handle-cache.inline.h@134 PS2, Line 134: FileHandleEntry entry(new_fh, p.lru_list); : typename MapType::iterator new_it = p.cache.emplace(*fname, std::move(entry)); I would remove the constructor and move the arguments to emplace: emplace(*fname, new_fh, p.lru_list) or emplace(*fname, FileHandleEntry(new_fh, p.lru_list)) http://gerrit.cloudera.org:8080/#/c/9576/2/be/src/runtime/io/handle-cache.inline.h@137 PS2, Line 137: ++p.size; : if (p.size > p.capacity) EvictHandles(p); I would prefer to move this before the creation of the new entry, to make it clear that EvictHandles() can not have any effect on it. This would also make it a little little bit faster, because evicted elements would be removed from a smaller map. http://gerrit.cloudera.org:8080/#/c/9576/2/be/src/runtime/io/handle-cache.inline.h@139 PS2, Line 139: DCHECK(!new_elem->in_use); This DCHECK could be probably removed. -- To view, visit http://gerrit.cloudera.org:8080/9576 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c695b21ca556e9c73c703c0c891e64939271c8d Gerrit-Change-Number: 9576 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Mon, 12 Mar 2018 18:56:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6609: Fix ownership of class members in KrpcDataStreamRecvr
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/9527 ) Change subject: IMPALA-6609: Fix ownership of class members in KrpcDataStreamRecvr .. Patch Set 5: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/9527 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3378496e2201b16c662b9daa634333480705c61a Gerrit-Change-Number: 9527 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 12 Mar 2018 18:52:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6576: Add metrics for data stream service memory usage
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/9562 ) Change subject: IMPALA-6576: Add metrics for data stream service memory usage .. Patch Set 3: Code-Review+1 (1 comment) Feel free to upgrade to a +2 if no one else is reviewing it aside from Michael and I. http://gerrit.cloudera.org:8080/#/c/9562/3/be/src/service/data-stream-service.cc File be/src/service/data-stream-service.cc: http://gerrit.cloudera.org:8080/#/c/9562/3/be/src/service/data-stream-service.cc@58 PS3, Line 58: DataStreamService > Yes, we seem to be a bit inconsistent in the naming convention for the RPC If we're going to do a CM change anyway, then I suggest staying consistent with the other metrics and change both this and the one from IMPALA-6269. If the CM change is to be deferred for later, then sticking with this is fine. But the preference is the former. -- To view, visit http://gerrit.cloudera.org:8080/9562 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5033b8dfda0b23d4230535ba13c3e050a35d01a3 Gerrit-Change-Number: 9562 Gerrit-PatchSet: 3 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 12 Mar 2018 18:48:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6610: Impala shell fetches the value of ldap password cmd incorrectly
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/9506 ) Change subject: IMPALA-6610: Impala shell fetches the value of ldap_password_cmd incorrectly .. Patch Set 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/9506/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9506/8//COMMIT_MSG@7 PS8, Line 7: Impala shell fetches the value of ldap_password_cmd incorrectly Update http://gerrit.cloudera.org:8080/#/c/9506/8//COMMIT_MSG@9 PS8, Line 9: The value of LDAP password in Impala shell contains extra line break causes nit: Remove trailing spaces and wrap (here and below) http://gerrit.cloudera.org:8080/#/c/9506/8/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/9506/8/shell/impala_shell.py@821 PS8, Line 821: if options.ldap_password_cmd.strip().startswith('echo') and \ Add checks for ldap_password_cmd & ldap_password not being None. Else shell can crash with an error if it hits this Exception and --ldap_password_cmd is not used. -- To view, visit http://gerrit.cloudera.org:8080/9506 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie570166aea62af223905b7f0124e9efb15a88ac7 Gerrit-Change-Number: 9506 Gerrit-PatchSet: 8 Gerrit-Owner: Donghui Xu Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Donghui Xu Gerrit-Reviewer: Fredy Wijaya Gerrit-Comment-Date: Mon, 12 Mar 2018 18:34:44 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3040: add logging to test caching ddl
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9579 ) Change subject: IMPALA-3040: add logging to test_caching_ddl .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2081/ -- To view, visit http://gerrit.cloudera.org:8080/9579 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iabc30445440e0fb358856da407d833f5ae975213 Gerrit-Change-Number: 9579 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 12 Mar 2018 18:15:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3040: add logging to test caching ddl
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/9579 ) Change subject: IMPALA-3040: add logging to test_caching_ddl .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9579 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iabc30445440e0fb358856da407d833f5ae975213 Gerrit-Change-Number: 9579 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 12 Mar 2018 18:13:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6576: Add metrics for data stream service memory usage
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/9562 ) Change subject: IMPALA-6576: Add metrics for data stream service memory usage .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/9562/3/be/src/service/data-stream-service.cc File be/src/service/data-stream-service.cc: http://gerrit.cloudera.org:8080/#/c/9562/3/be/src/service/data-stream-service.cc@58 PS3, Line 58: DataStreamService > I noticed that, too, but the one we added in IMPALA-6269 is camel case. htt Yes, we seem to be a bit inconsistent in the naming convention for the RPC related metrics and other metrics. My suggestion would be to stay consistent with other RPC related metrics for now. -- To view, visit http://gerrit.cloudera.org:8080/9562 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5033b8dfda0b23d4230535ba13c3e050a35d01a3 Gerrit-Change-Number: 9562 Gerrit-PatchSet: 3 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 12 Mar 2018 18:09:43 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3040: add logging to test caching ddl
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9579 Change subject: IMPALA-3040: add logging to test_caching_ddl .. IMPALA-3040: add logging to test_caching_ddl We don't currently have enough information to reconstruct why the test failed, so lets add logging with timestamps to understand which timeout we're actually hitting. Change-Id: Iabc30445440e0fb358856da407d833f5ae975213 --- M tests/query_test/test_hdfs_caching.py 1 file changed, 5 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/79/9579/1 -- To view, visit http://gerrit.cloudera.org:8080/9579 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iabc30445440e0fb358856da407d833f5ae975213 Gerrit-Change-Number: 9579 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] IMPALA-6638: Reduce file handle cache lock contention
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/9576 ) Change subject: IMPALA-6638: Reduce file handle cache lock contention .. Patch Set 1: (1 comment) > (1 comment) > > This change can increase the number of parallel calls to the > namenode by FileHandleCache, which was limited by the number of > partitions before. I do not know if this can cause problems or not, > but I would prefer to mention it as a possible side effect, or > limit the number of parallel hdfsOpenFile calls somehow. Good point. This code executes in the disk IO threads, and there are a limited number of disk IO threads (1/spinning disk, 8/SSD, etc). This can be larger than the number of partitions for the cache, but it is limited. The reason that I consider it ok to increase the parallelism is that we already use this level of parallelism when the file handle cache is off. When the cache is off, we use DiskIoMgr::GetExclusiveHdfsFileHandle(), which doesn't need to get any lock. This is fixing up the file handle cache so that it can be as fast as when the cache is off. http://gerrit.cloudera.org:8080/#/c/9576/1/be/src/runtime/io/handle-cache.inline.h File be/src/runtime/io/handle-cache.inline.h: http://gerrit.cloudera.org:8080/#/c/9576/1/be/src/runtime/io/handle-cache.inline.h@129 PS1, Line 129: pair range = : p.cache.equal_range(*fname); : FileHandleEntry entry(new_fh, p.lru_list); : typename MapType::iterator new_it = p.cache.emplace_hint(range.second, : *fname, std::move(entry)); > Does the hint logic make a difference anymore? I think that it would be bet Good point, that is simpler/better. Done. -- To view, visit http://gerrit.cloudera.org:8080/9576 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c695b21ca556e9c73c703c0c891e64939271c8d Gerrit-Change-Number: 9576 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Mon, 12 Mar 2018 17:24:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6638: Reduce file handle cache lock contention
Hello Csaba Ringhofer, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9576 to look at the new patch set (#2). Change subject: IMPALA-6638: Reduce file handle cache lock contention .. IMPALA-6638: Reduce file handle cache lock contention FileHandleCache::OpenFileHandle() currently holds the lock while opening a file handle. This lengthens the duration holding the lock considerably, causing contention when a lot of file handles are being opened (i.e. when the cache is cold). This changes FileHandleCache::OpenFileHandle() drops the lock while opening the file handle, then reacquires it to add the file handle to the cache. When running a simple select on a table with 46801 Parquet files, this fixes a small performance overhead for the cold cache case compared to having the cache off. All results are averaged over 5 runs: File handle cache off: 7.19s File handle cache cold (no patch): 7.92s File handle cache cold (with patch): 7.20s When running a select that accesses and aggregates 4 columns from the same Parquet table (thus requiring more scan ranges for the same file), the fix reduces contention, particularly for the case with 8 IO threads per disk: 1 IO thread per disk: Without patch: 9.59s With patch: 8.11s 8 IO threads per disk: Without patch: 14.2s With patch: 8.17s The patch has no impact on the performance when the cache is hot. Change-Id: I4c695b21ca556e9c73c703c0c891e64939271c8d --- M be/src/runtime/io/handle-cache.inline.h 1 file changed, 30 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/9576/2 -- To view, visit http://gerrit.cloudera.org:8080/9576 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4c695b21ca556e9c73c703c0c891e64939271c8d Gerrit-Change-Number: 9576 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Joe McDonnell
[Impala-ASF-CR] IMPALA-6551: Change Kudu TPCDS and TPCH columns to DECIMAL
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9484 ) Change subject: IMPALA-6551: Change Kudu TPCDS and TPCH columns to DECIMAL .. Patch Set 4: Added a dependency on a patch for IMPALA-6635 which contains all functional changes so this patch is still test/tpc changes only. -- To view, visit http://gerrit.cloudera.org:8080/9484 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2f7e4464dc6705cadd610a82c459390a9c0dfe4f Gerrit-Change-Number: 9484 Gerrit-PatchSet: 4 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 12 Mar 2018 17:00:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6609: Fix ownership of class members in KrpcDataStreamRecvr
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/9527 ) Change subject: IMPALA-6609: Fix ownership of class members in KrpcDataStreamRecvr .. Patch Set 5: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/9527/4/be/src/runtime/krpc-data-stream-recvr.h File be/src/runtime/krpc-data-stream-recvr.h: http://gerrit.cloudera.org:8080/#/c/9527/4/be/src/runtime/krpc-data-stream-recvr.h@73 PS4, Line 73: /// the recvr instance from the tracking structure of its KrpcDataStreamMgr in all cases. > Done The new comment doesn't really explicitly talk about the synchronization rules, but I guess they can be inferred. -- To view, visit http://gerrit.cloudera.org:8080/9527 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3378496e2201b16c662b9daa634333480705c61a Gerrit-Change-Number: 9527 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 12 Mar 2018 16:59:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6551: Change Kudu TPCDS and TPCH columns to DECIMAL
Hello Thomas Tauber-Marshall, Dimitris Tsirogiannis, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9484 to look at the new patch set (#4). Change subject: IMPALA-6551: Change Kudu TPCDS and TPCH columns to DECIMAL .. IMPALA-6551: Change Kudu TPCDS and TPCH columns to DECIMAL Before Kudu supported DECIMAL columns the TPCDS and TPCH columns were djusted to use DOUBLE in place of DECIMAL. This patch undoes that change now that Kudu supports DECIMAL. Testing: - Updated concurrent_select.py - Updated test_tpch_queries.py - Excersized by the Kudu planner tests Change-Id: I2f7e4464dc6705cadd610a82c459390a9c0dfe4f --- M testdata/datasets/tpcds/tpcds_kudu_template.sql M testdata/datasets/tpch/tpch_kudu_template.sql M testdata/datasets/tpch/tpch_schema_template.sql D testdata/workloads/tpcds/queries/tpcds-kudu-q19.test D testdata/workloads/tpcds/queries/tpcds-kudu-q27.test D testdata/workloads/tpcds/queries/tpcds-kudu-q3.test D testdata/workloads/tpcds/queries/tpcds-kudu-q34.test D testdata/workloads/tpcds/queries/tpcds-kudu-q42.test D testdata/workloads/tpcds/queries/tpcds-kudu-q43.test D testdata/workloads/tpcds/queries/tpcds-kudu-q46.test D testdata/workloads/tpcds/queries/tpcds-kudu-q47.test D testdata/workloads/tpcds/queries/tpcds-kudu-q52.test D testdata/workloads/tpcds/queries/tpcds-kudu-q53.test D testdata/workloads/tpcds/queries/tpcds-kudu-q55.test D testdata/workloads/tpcds/queries/tpcds-kudu-q59.test D testdata/workloads/tpcds/queries/tpcds-kudu-q6.test D testdata/workloads/tpcds/queries/tpcds-kudu-q61.test D testdata/workloads/tpcds/queries/tpcds-kudu-q63.test D testdata/workloads/tpcds/queries/tpcds-kudu-q65.test D testdata/workloads/tpcds/queries/tpcds-kudu-q68.test D testdata/workloads/tpcds/queries/tpcds-kudu-q7.test D testdata/workloads/tpcds/queries/tpcds-kudu-q73.test D testdata/workloads/tpcds/queries/tpcds-kudu-q79.test D testdata/workloads/tpcds/queries/tpcds-kudu-q8.test D testdata/workloads/tpcds/queries/tpcds-kudu-q88.test D testdata/workloads/tpcds/queries/tpcds-kudu-q89.test D testdata/workloads/tpcds/queries/tpcds-kudu-q96.test D testdata/workloads/tpcds/queries/tpcds-kudu-q98.test D testdata/workloads/tpch/queries/tpch-kudu-q1.test D testdata/workloads/tpch/queries/tpch-kudu-q10.test D testdata/workloads/tpch/queries/tpch-kudu-q11.test D testdata/workloads/tpch/queries/tpch-kudu-q12.test D testdata/workloads/tpch/queries/tpch-kudu-q13.test D testdata/workloads/tpch/queries/tpch-kudu-q14.test D testdata/workloads/tpch/queries/tpch-kudu-q15.test D testdata/workloads/tpch/queries/tpch-kudu-q16.test D testdata/workloads/tpch/queries/tpch-kudu-q17.test D testdata/workloads/tpch/queries/tpch-kudu-q18.test D testdata/workloads/tpch/queries/tpch-kudu-q19.test D testdata/workloads/tpch/queries/tpch-kudu-q2.test D testdata/workloads/tpch/queries/tpch-kudu-q20.test D testdata/workloads/tpch/queries/tpch-kudu-q21.test D testdata/workloads/tpch/queries/tpch-kudu-q22.test D testdata/workloads/tpch/queries/tpch-kudu-q3.test D testdata/workloads/tpch/queries/tpch-kudu-q4.test D testdata/workloads/tpch/queries/tpch-kudu-q5.test D testdata/workloads/tpch/queries/tpch-kudu-q6.test D testdata/workloads/tpch/queries/tpch-kudu-q7.test D testdata/workloads/tpch/queries/tpch-kudu-q8.test D testdata/workloads/tpch/queries/tpch-kudu-q9.test M tests/query_test/test_tpch_queries.py M tests/stress/concurrent_select.py 52 files changed, 108 insertions(+), 22,157 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/84/9484/4 -- To view, visit http://gerrit.cloudera.org:8080/9484 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2f7e4464dc6705cadd610a82c459390a9c0dfe4f Gerrit-Change-Number: 9484 Gerrit-PatchSet: 4 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6635: Add DECIMAL type to Kudu predicates
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9578 ) Change subject: IMPALA-6635: Add DECIMAL type to Kudu predicates .. Patch Set 1: Started https://jenkins.impala.io/job/pre-review-test/151/ -- To view, visit http://gerrit.cloudera.org:8080/9578 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2569a9e1d58f1c58884d58633d46348364888ed7 Gerrit-Change-Number: 9578 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 12 Mar 2018 16:51:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6635: Add DECIMAL type to Kudu predicates
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9578 Change subject: IMPALA-6635: Add DECIMAL type to Kudu predicates .. IMPALA-6635: Add DECIMAL type to Kudu predicates Change-Id: I2569a9e1d58f1c58884d58633d46348364888ed7 --- M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test 2 files changed, 36 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/78/9578/1 -- To view, visit http://gerrit.cloudera.org:8080/9578 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I2569a9e1d58f1c58884d58633d46348364888ed7 Gerrit-Change-Number: 9578 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke