[Impala-ASF-CR] Update kudu/security from 9ebcb77aa911aae76c48e717af24e643cb81908d
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13767 ) Change subject: Update kudu/security from 9ebcb77aa911aae76c48e717af24e643cb81908d .. Update kudu/security from 9ebcb77aa911aae76c48e717af24e643cb81908d This updates the kudu security code to the latest version, which includes support for GSSAPI calls, necessary for SPNEGO. This is a straight rsync of the kudu/util source code except for some minor CMakeLists changes that were carried over from the old version. Change-Id: Ie7c91193fd49f8ca1234b23cf61fc90c1fdbe2e0 Reviewed-on: http://gerrit.cloudera.org:8080/13767 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/kudu/security/CMakeLists.txt A be/src/kudu/security/gssapi.cc A be/src/kudu/security/gssapi.h M be/src/kudu/security/init.cc M be/src/kudu/security/openssl_util.cc M be/src/kudu/security/test/mini_kdc.cc M be/src/kudu/security/test/mini_kdc.h M be/src/kudu/security/tls_socket-test.cc M be/src/kudu/security/tls_socket.cc M be/src/kudu/security/token-test.cc M be/src/kudu/security/token.proto M be/src/kudu/security/token_signer.cc M be/src/kudu/security/token_signer.h M be/src/kudu/security/token_verifier.cc M be/src/kudu/util/test_util.cc M be/src/kudu/util/test_util.h 16 files changed, 848 insertions(+), 172 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/13767 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ie7c91193fd49f8ca1234b23cf61fc90c1fdbe2e0 Gerrit-Change-Number: 13767 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] Update kudu/security from 9ebcb77aa911aae76c48e717af24e643cb81908d
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13767 ) Change subject: Update kudu/security from 9ebcb77aa911aae76c48e717af24e643cb81908d .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/13767 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie7c91193fd49f8ca1234b23cf61fc90c1fdbe2e0 Gerrit-Change-Number: 13767 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Sat, 29 Jun 2019 05:08:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8718: Fixed AnalysisException in inline view with outer join complex type column
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13765 ) Change subject: IMPALA-8718: Fixed AnalysisException in inline view with outer join complex type column .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3789/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13765 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ida65503ca4b1342b8fe0049753bc664da227dca9 Gerrit-Change-Number: 13765 Gerrit-PatchSet: 3 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Sat, 29 Jun 2019 00:56:58 + Gerrit-HasComments: No
[Impala-ASF-CR] Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13768 ) Change subject: Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95 .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3788/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13768 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17f90561e0ea6b0917fff51b055225060a4fa549 Gerrit-Change-Number: 13768 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Sat, 29 Jun 2019 00:22:16 + Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-8484: Run queries on disjoint executor groups
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13550 ) Change subject: WIP IMPALA-8484: Run queries on disjoint executor groups .. Patch Set 8: (15 comments) First round of feedback. really like the overall direction that it's going. I think generally my main feedback if I did a full pass would be: * The usually code readability stuff * More documentation in code comments and the commit message of the expected behaviour * Clarifications of the various edge cases when things are in not-fully-healthy states. http://gerrit.cloudera.org:8080/#/c/13550/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13550/8//COMMIT_MSG@19 PS8, Line 19: - When using executor groups, only a single coordinator and a single AC What happens if you have multiple coordinators? http://gerrit.cloudera.org:8080/#/c/13550/8/be/src/scheduling/admission-controller.h File be/src/scheduling/admission-controller.h: http://gerrit.cloudera.org:8080/#/c/13550/8/be/src/scheduling/admission-controller.h@370 PS8, Line 370: /// The per host number of queries admitted only for the queries admitted locally. It seems weird to me to have all these maps with one value each instead of one map with a struct in. Not the biggest deal since you're following the existing pattern. http://gerrit.cloudera.org:8080/#/c/13550/8/be/src/scheduling/admission-controller.h@589 PS8, Line 589: /// Map from executor group to schedule There are a few members here that are set in various states. Might be worth documenting it in terms of a state machine and what members are set in which state. http://gerrit.cloudera.org:8080/#/c/13550/8/be/src/scheduling/admission-controller.h@696 PS8, Line 696: /// number of slots per executors does not change with the group or cluster size and Per offline discussion, I lean towards the alternate option of # slots being something that each executor determines (based on # cores or configuration). I don't think it it critical though - you can achieve the same behaviour either way so long as you don't want to have heterogeneous executors. http://gerrit.cloudera.org:8080/#/c/13550/8/be/src/scheduling/admission-controller.h@707 PS8, Line 707: void UpdateHostNumAdmitted(const QuerySchedule& schedule, int64_t num); I feel like these two functions should really be combined - there's no reason to ever call one but not the other, right? http://gerrit.cloudera.org:8080/#/c/13550/8/be/src/scheduling/admission-controller.h@710 PS8, Line 710: /// Rejection happens in several stages This really helps clarify, thanks! http://gerrit.cloudera.org:8080/#/c/13550/8/be/src/scheduling/admission-controller.h@725 PS8, Line 725: /// - Thread reservation limit (thread_reservation_limit, When we added the largest_min_mem_reservation and thread_reservation_limit checks, we didn't think about what might happen if the schedule changed. Maybe if we'd thought about that we would have done it in a way that was schedule-agnostic, e.g. assume that all fragments will run on a host, which overestimates the resource requirements somewhat. Anyway, I guess there are plenty of other checks in this bucket, so we wouldn't be simplifying things. http://gerrit.cloudera.org:8080/#/c/13550/8/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/13550/8/be/src/scheduling/admission-controller.cc@549 PS8, Line 549: // All other executor groups are limited by the number of running queries per It's a little counter-intuitive that the slots mechanism doesn't apply to the default executor group. This is just to preserve the previous behaviour, right? http://gerrit.cloudera.org:8080/#/c/13550/8/be/src/scheduling/admission-controller.cc@888 PS8, Line 888: schedule_result->reset(queue_node.admitted_schedule.release()); This is OK, but probably is cleaner just to convert the output to unique_ptr. http://gerrit.cloudera.org:8080/#/c/13550/8/be/src/scheduling/admission-controller.cc@1088 PS8, Line 1088: if (executor_group->NumExecutors() == 0) continue; Maybe we should factor this out into a helper like executor_group->IsHealthy(), just as a placeholder for a more sophisticated check. I think in future we'll probably want to avoid scheduling on executor groups that don't have enough members or similar. I'm wondering if we could use a somewhat simple heuristic like "only schedule on executor groups that have 75% of the members of the largest group". That would avoid two kinds of pathological behaviour - never running queries when there are available groups, because no groups are considered health, and running queries on unhealthy groups when healthy groups are available (and maybe just busy). http://gerrit.cloudera.org:8080/#/c/13550/8/be/src/scheduling/admission-controller.cc@1154 PS8, Line 1154: void
[Impala-ASF-CR] IMPALA-8718: Fixed AnalysisException in inline view with outer join complex type column
Hello Bharath Vissapragada, Sahil Takiar, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13765 to look at the new patch set (#3). Change subject: IMPALA-8718: Fixed AnalysisException in inline view with outer join complex type column .. IMPALA-8718: Fixed AnalysisException in inline view with outer join complex type column For Inline view with left outer join, in order to make right side exprs nullable, it makes a wrapper for the expr that return non-null when all its contained slotref is null. The wrap is TupleIsNullPredicate. Complex type does not need the wrapper for its value cannot both be null and non-null. But during its checking whether the wrapper needed, because of code limitation, AnalysisException is thrown. This fix overcomes the limitation by avoiding to use unsupported IsNullPredicate for complex type. It should be a safe change for: 1. Other data types such as int return false after a call to backend to check isNotNull(nullvalue). 2.Left outer join with complex type works fine for the queries without inline view. Join code can handle complex type correctly without the wrapper. Tests: Added ee tests to test left outer joins. Manual tests. Change-Id: Ida65503ca4b1342b8fe0049753bc664da227dca9 --- M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java A testdata/workloads/functional-query/queries/QueryTest/complex_joins.test A tests/query_test/test_complex.py 3 files changed, 84 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/13765/3 -- To view, visit http://gerrit.cloudera.org:8080/13765 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ida65503ca4b1342b8fe0049753bc664da227dca9 Gerrit-Change-Number: 13765 Gerrit-PatchSet: 3 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Yongzhi Chen
[Impala-ASF-CR] IMPALA-6159 / KUDU-2192: Enable TCP keepalive for all outbound connections
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13764 ) Change subject: IMPALA-6159 / KUDU-2192: Enable TCP keepalive for all outbound connections .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13764 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa1d66d83aea1cc82d07fc6217be5fc1306695bc Gerrit-Change-Number: 13764 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 28 Jun 2019 23:42:36 + Gerrit-HasComments: No
[Impala-ASF-CR] Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/13768 )
Change subject: Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13768/1/be/src/util/webserver.cc
File be/src/util/webserver.cc:
http://gerrit.cloudera.org:8080/#/c/13768/1/be/src/util/webserver.cc@393
PS1, Line 393: sq_callback_result_t
Webserver::BeginRequestCallbackStatic(struct sq_connection* connection) {
line too long (94 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/13768
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I17f90561e0ea6b0917fff51b055225060a4fa549
Gerrit-Change-Number: 13768
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Michael Ho
Gerrit-Comment-Date: Fri, 28 Jun 2019 23:42:25 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95
Hello Michael Ho, Lars Volker, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/13768 to review the following change. Change subject: Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95 .. Update squeasel to 7973705170f4744d1806e32695f7ea1e8308ee95 This updates to the latest build of squeasel, which fixes a few small issues and also improves support for keepalive. This patch doesn't itself enable keepalive, but switches to the new APIs. Change-Id: I17f90561e0ea6b0917fff51b055225060a4fa549 --- M be/src/thirdparty/squeasel/squeasel.c M be/src/thirdparty/squeasel/squeasel.h M be/src/util/webserver.cc M be/src/util/webserver.h 4 files changed, 76 insertions(+), 31 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/13768/1 -- To view, visit http://gerrit.cloudera.org:8080/13768 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I17f90561e0ea6b0917fff51b055225060a4fa549 Gerrit-Change-Number: 13768 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] [WIP] IMPALA-7486: Prototype for admitting less mem on coord
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13740 ) Change subject: [WIP] IMPALA-7486: Prototype for admitting less mem on coord .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3787/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2b94e7293b91dec8a18491079c34923eadd94b21 Gerrit-Change-Number: 13740 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 28 Jun 2019 23:34:42 + Gerrit-HasComments: No
[Impala-ASF-CR] Update kudu/security from 9ebcb77aa911aae76c48e717af24e643cb81908d
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13767 ) Change subject: Update kudu/security from 9ebcb77aa911aae76c48e717af24e643cb81908d .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13767 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie7c91193fd49f8ca1234b23cf61fc90c1fdbe2e0 Gerrit-Change-Number: 13767 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Fri, 28 Jun 2019 23:29:05 + Gerrit-HasComments: No
[Impala-ASF-CR] Update kudu/security from 9ebcb77aa911aae76c48e717af24e643cb81908d
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13767 ) Change subject: Update kudu/security from 9ebcb77aa911aae76c48e717af24e643cb81908d .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4570/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/13767 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie7c91193fd49f8ca1234b23cf61fc90c1fdbe2e0 Gerrit-Change-Number: 13767 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Fri, 28 Jun 2019 23:29:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8427: [DOCS] Document the new startup flag IMPALA-7800 introduced
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13762 ) Change subject: IMPALA-8427: [DOCS] Document the new startup flag IMPALA-7800 introduced .. Patch Set 2: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/361/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/13762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a4c1975721c32a78a003d91babc5d2bb90f3949 Gerrit-Change-Number: 13762 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Fri, 28 Jun 2019 23:22:50 + Gerrit-HasComments: No
[Impala-ASF-CR] Update kudu/security from 9ebcb77aa911aae76c48e717af24e643cb81908d
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13767 ) Change subject: Update kudu/security from 9ebcb77aa911aae76c48e717af24e643cb81908d .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3786/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13767 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie7c91193fd49f8ca1234b23cf61fc90c1fdbe2e0 Gerrit-Change-Number: 13767 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Fri, 28 Jun 2019 23:09:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8427: [DOCS] Document the new startup flag IMPALA-7800 introduced
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13762 ) Change subject: IMPALA-8427: [DOCS] Document the new startup flag IMPALA-7800 introduced .. Patch Set 2: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/361/ Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/13762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a4c1975721c32a78a003d91babc5d2bb90f3949 Gerrit-Change-Number: 13762 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Fri, 28 Jun 2019 23:02:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8427: [DOCS] Document the new startup flag IMPALA-7800 introduced
Hello Michael Ho, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13762 to look at the new patch set (#2). Change subject: IMPALA-8427: [DOCS] Document the new startup flag IMPALA-7800 introduced .. IMPALA-8427: [DOCS] Document the new startup flag IMPALA-7800 introduced - Added a new doc impala_client.xml as the overview of Impala impala client access. Change-Id: I1a4c1975721c32a78a003d91babc5d2bb90f3949 --- M docs/impala.ditamap A docs/topics/impala_client.xml M docs/topics/impala_config_options.xml 3 files changed, 153 insertions(+), 55 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/13762/2 -- To view, visit http://gerrit.cloudera.org:8080/13762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1a4c1975721c32a78a003d91babc5d2bb90f3949 Gerrit-Change-Number: 13762 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-8427: [DOCS] Document the new startup flag IMPALA-7800 introduced
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/13762 ) Change subject: IMPALA-8427: [DOCS] Document the new startup flag IMPALA-7800 introduced .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/13762/1/docs/topics/impala_client.xml File docs/topics/impala_client.xml: http://gerrit.cloudera.org:8080/#/c/13762/1/docs/topics/impala_client.xml@115 PS1, Line 115: >--accepted_client_cnxn_timeout > 0, > May want to add that this is in seconds. Done -- To view, visit http://gerrit.cloudera.org:8080/13762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a4c1975721c32a78a003d91babc5d2bb90f3949 Gerrit-Change-Number: 13762 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Fri, 28 Jun 2019 23:02:16 + Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP] IMPALA-7486: Prototype for admitting less mem on coord
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13740 ) Change subject: [WIP] IMPALA-7486: Prototype for admitting less mem on coord .. Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/13740/2/be/src/scheduling/query-schedule.cc File be/src/scheduling/query-schedule.cc: http://gerrit.cloudera.org:8080/#/c/13740/2/be/src/scheduling/query-schedule.cc@55 PS2, Line 55: const TQueryOptions& query_options, RuntimeProfile* summary_profile, bool is_dedicated_coord) line too long (97 > 90) http://gerrit.cloudera.org:8080/#/c/13740/2/be/src/scheduling/query-schedule.cc@266 PS2, Line 266: coord_backend_mem_to_admit_ = is_dedicated_coord() ? GetDedicatedCoordMemoryEstimate() : line too long (92 > 90) http://gerrit.cloudera.org:8080/#/c/13740/2/fe/src/main/java/org/apache/impala/planner/Planner.java File fe/src/main/java/org/apache/impala/planner/Planner.java: http://gerrit.cloudera.org:8080/#/c/13740/2/fe/src/main/java/org/apache/impala/planner/Planner.java@418 PS2, Line 418: // fragment instances, so keep aside all the untracked memory that might be required. line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/13740/2/tests/custom_cluster/test_admission_controller.py File tests/custom_cluster/test_admission_controller.py: http://gerrit.cloudera.org:8080/#/c/13740/2/tests/custom_cluster/test_admission_controller.py@563 PS2, Line 563: flake8: E203 whitespace before ':' http://gerrit.cloudera.org:8080/#/c/13740/2/tests/custom_cluster/test_admission_controller.py@578 PS2, Line 578: flake8: E203 whitespace before ':' -- To view, visit http://gerrit.cloudera.org:8080/13740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2b94e7293b91dec8a18491079c34923eadd94b21 Gerrit-Change-Number: 13740 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 28 Jun 2019 22:55:05 + Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP] IMPALA-7486: Prototype for admitting less mem on coord
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/13740 ) Change subject: [WIP] IMPALA-7486: Prototype for admitting less mem on coord .. Patch Set 2: Thanks everyone for reviewing. PS2 just contains my updates since PS1, will address comments in the next patchset -- To view, visit http://gerrit.cloudera.org:8080/13740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2b94e7293b91dec8a18491079c34923eadd94b21 Gerrit-Change-Number: 13740 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 28 Jun 2019 22:55:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6159 / KUDU-2192: Enable TCP keepalive for all outbound connections
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13764 ) Change subject: IMPALA-6159 / KUDU-2192: Enable TCP keepalive for all outbound connections .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/13764 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa1d66d83aea1cc82d07fc6217be5fc1306695bc Gerrit-Change-Number: 13764 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 28 Jun 2019 22:54:31 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-7486: Prototype for admitting less mem on coord
Hello Andrew Sherman, Lars Volker, Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13740 to look at the new patch set (#2). Change subject: [WIP] IMPALA-7486: Prototype for admitting less mem on coord .. [WIP] IMPALA-7486: Prototype for admitting less mem on coord Complete: - Calculate the mem needed for the coord fragment + accounting for the runtime filter agg mem required + a safety buffer of 100MB - changes to get the mem_to_admit/limit for coord, accesible through the QuerySchedule - add/remove/modify checks in both reject and admit methods to seperate the coord and executor checks - Update memory accounting in admission controller - use coord's mem limit for the coord queryState - added BE unit tests for most cases - Updated the Admission debug page to show the coord mem values TODO: - add e2e tests: - Simple case (done) - query with num_node = 1 - Any additional test cases not covered by BE unit tests or need an equivalent e2e test - e2e tests are not written with dedicated coord, so all should work fine Change-Id: I2b94e7293b91dec8a18491079c34923eadd94b21 --- M be/src/runtime/coordinator.cc M be/src/scheduling/admission-controller-test.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/admission-controller.h M be/src/scheduling/query-schedule.cc M be/src/scheduling/query-schedule.h M be/src/scheduling/scheduler.cc M be/src/service/client-request-state.cc M be/src/service/impala-http-handler.cc M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/planner/PlanFragment.java M fe/src/main/java/org/apache/impala/planner/Planner.java M tests/custom_cluster/test_admission_controller.py M www/admission_controller.tmpl 14 files changed, 516 insertions(+), 93 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/13740/2 -- To view, visit http://gerrit.cloudera.org:8080/13740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2b94e7293b91dec8a18491079c34923eadd94b21 Gerrit-Change-Number: 13740 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8718: Fixed AnalysisException in inline view with outer join complex type column
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13765 ) Change subject: IMPALA-8718: Fixed AnalysisException in inline view with outer join complex type column .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3785/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13765 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ida65503ca4b1342b8fe0049753bc664da227dca9 Gerrit-Change-Number: 13765 Gerrit-PatchSet: 2 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Fri, 28 Jun 2019 22:44:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 ) Change subject: IMPALA-8716: Log a group of privileges into a single audit event. .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java: http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@408 PS2, Line 408: RangerAccessResult result = plugin_.isAccessAllowed(request, auditHandler); Instead of modifying entries in a tmpAuditHandler, can we make it simple by calling auditHandler.processEvents() ourselves (with an additional context of whether it is an implied privilege or not? Something like, RangerAccessResult result = plugin_.isAccessAllowed(request, null); authzCtx.getAuditHandler().processResultNew(result, privilege.isAny, originalPriv...) ? Does that not work for some reason? -- To view, visit http://gerrit.cloudera.org:8080/13744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca Gerrit-Change-Number: 13744 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 28 Jun 2019 22:41:51 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8427: [DOCS] Document the new startup flag IMPALA-7800 introduced
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/13762 ) Change subject: IMPALA-8427: [DOCS] Document the new startup flag IMPALA-7800 introduced .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/13762/1/docs/topics/impala_client.xml File docs/topics/impala_client.xml: http://gerrit.cloudera.org:8080/#/c/13762/1/docs/topics/impala_client.xml@115 PS1, Line 115: >--accepted_client_cnxn_timeout > 0, May want to add that this is in seconds. -- To view, visit http://gerrit.cloudera.org:8080/13762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a4c1975721c32a78a003d91babc5d2bb90f3949 Gerrit-Change-Number: 13762 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Fri, 28 Jun 2019 22:39:15 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Update kudu/security from 9ebcb77aa911aae76c48e717af24e643cb81908d
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/13767 ) Change subject: Update kudu/security from 9ebcb77aa911aae76c48e717af24e643cb81908d .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13767 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie7c91193fd49f8ca1234b23cf61fc90c1fdbe2e0 Gerrit-Change-Number: 13767 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Fri, 28 Jun 2019 22:37:41 + Gerrit-HasComments: No
[Impala-ASF-CR] Update kudu/security from 9ebcb77aa911aae76c48e717af24e643cb81908d
Hello Michael Ho, Lars Volker, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/13767 to review the following change. Change subject: Update kudu/security from 9ebcb77aa911aae76c48e717af24e643cb81908d .. Update kudu/security from 9ebcb77aa911aae76c48e717af24e643cb81908d This updates the kudu security code to the latest version, which includes support for GSSAPI calls, necessary for SPNEGO. This is a straight rsync of the kudu/util source code except for some minor CMakeLists changes that were carried over from the old version. Change-Id: Ie7c91193fd49f8ca1234b23cf61fc90c1fdbe2e0 --- M be/src/kudu/security/CMakeLists.txt A be/src/kudu/security/gssapi.cc A be/src/kudu/security/gssapi.h M be/src/kudu/security/init.cc M be/src/kudu/security/openssl_util.cc M be/src/kudu/security/test/mini_kdc.cc M be/src/kudu/security/test/mini_kdc.h M be/src/kudu/security/tls_socket-test.cc M be/src/kudu/security/tls_socket.cc M be/src/kudu/security/token-test.cc M be/src/kudu/security/token.proto M be/src/kudu/security/token_signer.cc M be/src/kudu/security/token_signer.h M be/src/kudu/security/token_verifier.cc M be/src/kudu/util/test_util.cc M be/src/kudu/util/test_util.h 16 files changed, 848 insertions(+), 172 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/13767/1 -- To view, visit http://gerrit.cloudera.org:8080/13767 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ie7c91193fd49f8ca1234b23cf61fc90c1fdbe2e0 Gerrit-Change-Number: 13767 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-8681: Only show ValidWriteIdLists for Acid tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13736 ) Change subject: IMPALA-8681: Only show ValidWriteIdLists for Acid tables .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3784/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13736 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifcc31c7ddcfc471b0e5308f7e4aaadfa8189a905 Gerrit-Change-Number: 13736 Gerrit-PatchSet: 3 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 28 Jun 2019 22:18:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8718: Fixed AnalysisException in inline view with outer join complex type column
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13765 ) Change subject: IMPALA-8718: Fixed AnalysisException in inline view with outer join complex type column .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/13765/2/tests/query_test/test_complex.py File tests/query_test/test_complex.py: http://gerrit.cloudera.org:8080/#/c/13765/2/tests/query_test/test_complex.py@22 PS2, Line 22: class TestComplex(ImpalaTestSuite): flake8: E302 expected 2 blank lines, found 1 -- To view, visit http://gerrit.cloudera.org:8080/13765 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ida65503ca4b1342b8fe0049753bc664da227dca9 Gerrit-Change-Number: 13765 Gerrit-PatchSet: 2 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Fri, 28 Jun 2019 22:06:41 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8718: Fixed AnalysisException in inline view with outer join complex type column
Hello Bharath Vissapragada, Sahil Takiar, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13765 to look at the new patch set (#2). Change subject: IMPALA-8718: Fixed AnalysisException in inline view with outer join complex type column .. IMPALA-8718: Fixed AnalysisException in inline view with outer join complex type column For Inline view with left outer join, in order to make right side exprs nullable, it makes a wrapper for the expr that return non-null when all its contained slotref is null. The wrap is TupleIsNullPredicate. Complex type does not need the wrapper for its value cannot both be null and non-null. But during its checking whether the wrapper needed, because of code limitation, AnalysisException is thrown. This fix overcomes the limitation by avoiding to use unsupported IsNullPredicate for complex type. It should be a safe change for: 1. Other data types such as int return false after a call to backend to check isNotNull(nullvalue). 2.Left outer join with complex type works fine for the queries without inline view. Join code can handle complex type correctly without the wrapper. Tests: Added ee tests to test left outer joins. Manual tests. Change-Id: Ida65503ca4b1342b8fe0049753bc664da227dca9 --- M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java A testdata/workloads/functional-query/queries/QueryTest/complex_joins.test A tests/query_test/test_complex.py 3 files changed, 83 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/13765/2 -- To view, visit http://gerrit.cloudera.org:8080/13765 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ida65503ca4b1342b8fe0049753bc664da227dca9 Gerrit-Change-Number: 13765 Gerrit-PatchSet: 2 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Yongzhi Chen
[Impala-ASF-CR] IMPALA-8718: Fixed AnalysisException in inline view with outer join complex type column
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13765 ) Change subject: IMPALA-8718: Fixed AnalysisException in inline view with outer join complex type column .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3783/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13765 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ida65503ca4b1342b8fe0049753bc664da227dca9 Gerrit-Change-Number: 13765 Gerrit-PatchSet: 1 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Fri, 28 Jun 2019 22:02:36 + Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-8484: Run queries on disjoint executor groups
Lars Volker has posted comments on this change. (
http://gerrit.cloudera.org:8080/13550 )
Change subject: WIP IMPALA-8484: Run queries on disjoint executor groups
..
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13550/7/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:
http://gerrit.cloudera.org:8080/#/c/13550/7/be/src/scheduling/admission-controller.cc@1336
PS7, Line 1336: for (auto& it: queue_node->per_group_schedules) {
> I agree that the advantage of filling one group before starting another is
That's a good point. I think it's ok to start with either option, and filling
one before moving to the next one seems more straightforward to me. I added a
comment in the code and filed IMPALA-8731 to track implementing the alternative
approach.
--
To view, visit http://gerrit.cloudera.org:8080/13550
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a1d0900f2a82bd2fc0a906cc094e442cffa189b
Gerrit-Change-Number: 13550
Gerrit-PatchSet: 7
Gerrit-Owner: Lars Volker
Gerrit-Reviewer: Andrew Sherman
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Fri, 28 Jun 2019 22:00:53 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6159 / KUDU-2192: Enable TCP keepalive for all outbound connections
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13764 ) Change subject: IMPALA-6159 / KUDU-2192: Enable TCP keepalive for all outbound connections .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3782/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13764 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa1d66d83aea1cc82d07fc6217be5fc1306695bc Gerrit-Change-Number: 13764 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 28 Jun 2019 21:54:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8681: Only show ValidWriteIdLists for Acid tables
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/13736 ) Change subject: IMPALA-8681: Only show ValidWriteIdLists for Acid tables .. Patch Set 3: (1 comment) patch 3 addresses the comment issue. http://gerrit.cloudera.org:8080/#/c/13736/2/testdata/workloads/functional-query/queries/QueryTest/acid-profile.test File testdata/workloads/functional-query/queries/QueryTest/acid-profile.test: http://gerrit.cloudera.org:8080/#/c/13736/2/testdata/workloads/functional-query/queries/QueryTest/acid-profile.test@16 PS2, Line 16: # Verify that ValidWriteIdLists is in the profile > I will change Done -- To view, visit http://gerrit.cloudera.org:8080/13736 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifcc31c7ddcfc471b0e5308f7e4aaadfa8189a905 Gerrit-Change-Number: 13736 Gerrit-PatchSet: 3 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 28 Jun 2019 21:54:05 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8681: Only show ValidWriteIdLists for Acid tables
Hello Zoltan Borok-Nagy, Todd Lipcon, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13736 to look at the new patch set (#3). Change subject: IMPALA-8681: Only show ValidWriteIdLists for Acid tables .. IMPALA-8681: Only show ValidWriteIdLists for Acid tables Lists ValidWriteIds for transactional tables in profile. If a query does not trigger any transactional table loading, the query profile will not have the "Loaded ValidWriteIdLists" timeline. Tests: Manual tests. Fixed StmtMetadataLoaderTest. Added acid_profile test Sample output: Query Compilation: 3s525ms - Metadata load started: 37.369ms (37.369ms) - Metadata load finished. loaded-tables=1/1 ... - Loaded ValidWriteIdLists for transactional tables: functional.insert_only_transactional_table:0:9223372036854775807:: : 3s312ms (551.463us) - Analysis finished: 3s370ms (58.110ms) ... Change-Id: Ifcc31c7ddcfc471b0e5308f7e4aaadfa8189a905 --- M fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java A testdata/workloads/functional-query/queries/QueryTest/acid-profile.test M tests/common/skip.py M tests/query_test/test_acid.py 5 files changed, 42 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/13736/3 -- To view, visit http://gerrit.cloudera.org:8080/13736 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifcc31c7ddcfc471b0e5308f7e4aaadfa8189a905 Gerrit-Change-Number: 13736 Gerrit-PatchSet: 3 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8718: Fixed AnalysisException in inline view with outer join complex type column
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13765 ) Change subject: IMPALA-8718: Fixed AnalysisException in inline view with outer join complex type column .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/13765/1/tests/query_test/test_complex.py File tests/query_test/test_complex.py: http://gerrit.cloudera.org:8080/#/c/13765/1/tests/query_test/test_complex.py@19 PS1, Line 19: import pytest flake8: F401 'pytest' imported but unused -- To view, visit http://gerrit.cloudera.org:8080/13765 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ida65503ca4b1342b8fe0049753bc664da227dca9 Gerrit-Change-Number: 13765 Gerrit-PatchSet: 1 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 28 Jun 2019 21:25:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] WIP IMPALA-8484: Run queries on disjoint executor groups
Andrew Sherman has posted comments on this change. (
http://gerrit.cloudera.org:8080/13550 )
Change subject: WIP IMPALA-8484: Run queries on disjoint executor groups
..
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13550/7/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:
http://gerrit.cloudera.org:8080/#/c/13550/7/be/src/scheduling/admission-controller.cc@1336
PS7, Line 1336: for (auto& it: queue_node->per_group_schedules) {
> Yes, per_group_schedules is a map, so the order is deterministic and this i
I agree that the advantage of filling one group before starting another is that
is does make it easier to choose an executor group to remove when scaling down.
On the other hand does this result in situations that look weird to the user?
For example, suppose we predict we need more capacity, scale up and add a new
executor group. Now we have extra capacity but we don't use it until we have
to, does that look weird? Another reason to pack executor groups might be to
make better use of the remote read cache. Overall I think this is probably OK
overall. I do think the policy should be documented more explicitly in the code.
--
To view, visit http://gerrit.cloudera.org:8080/13550
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a1d0900f2a82bd2fc0a906cc094e442cffa189b
Gerrit-Change-Number: 13550
Gerrit-PatchSet: 7
Gerrit-Owner: Lars Volker
Gerrit-Reviewer: Andrew Sherman
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Fri, 28 Jun 2019 21:24:50 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8718: Fixed AnalysisException in inline view with outer join complex type column
Yongzhi Chen has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13765 Change subject: IMPALA-8718: Fixed AnalysisException in inline view with outer join complex type column .. IMPALA-8718: Fixed AnalysisException in inline view with outer join complex type column For Inline view with left outer join, in order to make right side exprs nullable, it makes a wrapper for the expr that return non-null when all its contained slotref is null. The wrap is TupleIsNullPredicate. Complex type does not need the wrapper for its value cannot both be null and non-null. But during its checking whether the wrapper needed, because of code limitation, AnalysisException is thrown. This fix overcomes the limitation by avoiding to use unsupported IsNullPredicate for complex type. It should be a safe change for: 1. Other data types such as int return false after a call to backend to check isNotNull(nullvalue). 2.Left outer join with complex type works fine for the queries without inline view. Join code can handle complex type correctly without the wrapper. Tests: Added ee tests to test left outer joins. Manual tests. Change-Id: Ida65503ca4b1342b8fe0049753bc664da227dca9 --- M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java A testdata/workloads/functional-query/queries/QueryTest/complex_joins.test A tests/query_test/test_complex.py 3 files changed, 85 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/13765/1 -- To view, visit http://gerrit.cloudera.org:8080/13765 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ida65503ca4b1342b8fe0049753bc664da227dca9 Gerrit-Change-Number: 13765 Gerrit-PatchSet: 1 Gerrit-Owner: Yongzhi Chen
[Impala-ASF-CR] IMPALA-6159 / KUDU-2192: Enable TCP keepalive for all outbound connections
Michael Ho has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/13764 ) Change subject: IMPALA-6159 / KUDU-2192: Enable TCP keepalive for all outbound connections .. Removed reviewer Kudu Jenkins. -- To view, visit http://gerrit.cloudera.org:8080/13764 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: Iaa1d66d83aea1cc82d07fc6217be5fc1306695bc Gerrit-Change-Number: 13764 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6159 / KUDU-2192: Enable TCP keepalive for all outbound connections
Hello Alexey Serbin, Kudu Jenkins, Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/13764 to review the following change. Change subject: IMPALA-6159 / KUDU-2192: Enable TCP keepalive for all outbound connections .. IMPALA-6159 / KUDU-2192: Enable TCP keepalive for all outbound connections This change enables TCP keepalive for all outbound connections. This aims to handle cases in which the remote peer may have dropped off the network without sending a TCP RST. For instance, a remote host could have hit a kernel panic and got power cycled. In which case, the existing TCP connection to that host may be stale. In an idle cluster, this stale connection may not be detected until the next use of it, in which case it will result in a RPC failure due to TCP RST sent from the restarted peer. By enabling TCP keepalive, we ensure that stale TCP connections in an idle cluster will be detected and closed within a time bound so a new connection will be created on the next use. This change introduces 3 different flags: --tcp_keepalive_probe_period_s: the duration in seconds a TCP connection has to be idle before keepalive probes started to be sent. --tcp_keepalive_retry_period_s: the duration in seconds between successive keepalive probes if previous probes didn't get an ACK from remote peer. --tcp_keepalive_retry_count: the maximum number of TCP keepalive probes sent without an ACK before declaring the remote peer as dead. Testing: - Used TCP dump to verify that keepalive probes are being sent periodically. - Verified that blocking all incoming traffic to a server's port via an iptable rule caused the TCP connection to be closed and the keepalive probes to stop eventually. Change-Id: Iaa1d66d83aea1cc82d07fc6217be5fc1306695bc Reviewed-on: http://gerrit.cloudera.org:8080/13702 Reviewed-by: Alexey Serbin Reviewed-by: Todd Lipcon Tested-by: Kudu Jenkins --- M be/src/kudu/rpc/connection.cc M be/src/kudu/rpc/connection.h M be/src/kudu/rpc/reactor.cc M be/src/kudu/rpc/reactor.h M be/src/kudu/rpc/rpc-test.cc M be/src/kudu/util/net/socket.cc M be/src/kudu/util/net/socket.h 7 files changed, 112 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/13764/1 -- To view, visit http://gerrit.cloudera.org:8080/13764 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iaa1d66d83aea1cc82d07fc6217be5fc1306695bc Gerrit-Change-Number: 13764 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8427: [DOCS] Document the new startup flag IMPALA-7800 introduced
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13762 ) Change subject: IMPALA-8427: [DOCS] Document the new startup flag IMPALA-7800 introduced .. Patch Set 1: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/360/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/13762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a4c1975721c32a78a003d91babc5d2bb90f3949 Gerrit-Change-Number: 13762 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Fri, 28 Jun 2019 20:47:18 + Gerrit-HasComments: No
[Impala-ASF-CR] [IMPALA-8587] Show inherited privileges with Ranger show grant
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/13673 )
Change subject: [IMPALA-8587] Show inherited privileges with Ranger show grant
..
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13673/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
File
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java:
http://gerrit.cloudera.org:8080/#/c/13673/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@367
PS3, Line 367: private static class RangerResourceResult {
: List server = new ArrayList<>();
: List uri = new ArrayList<>();
: List database = new ArrayList<>();
: List udf = new ArrayList<>();
: List table = new ArrayList<>();
: List column = new ArrayList<>();
make these private
http://gerrit.cloudera.org:8080/#/c/13673/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@392
PS3, Line 392: public RangerResourceResult addUdfResult(RangerResultRow
result) {
: udf.add(result);
: return this;
: }
:
: public RangerResourceResult addUriResult(RangerResultRow
result) {
: uri.add(result);
: return this;
: }
These two methods are unused, that means uri and udf will always be empty.
http://gerrit.cloudera.org:8080/#/c/13673/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@407
PS3, Line 407: public List getResultRows() {
: List results = new ArrayList<>();
:
: results.addAll(filterIfAll(server));
: results.addAll(filterIfAll(database));
: results.addAll(filterIfAll(table));
: results.addAll(filterIfAll(column));
: results.addAll(filterIfAll(udf));
: results.addAll(filterIfAll(uri));
:
: return results;
: }
I don't quite follow the logic why we have to filter ALL. A comment will be
helpful.
--
To view, visit http://gerrit.cloudera.org:8080/13673
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c4c9327acb12abc12d130ef5c1ace6a08ed193c
Gerrit-Change-Number: 13673
Gerrit-PatchSet: 3
Gerrit-Owner: Austin Nobis
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Fri, 28 Jun 2019 20:38:14 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8427: [DOCS] Document the new startup flag IMPALA-7800 introduced
Alex Rodoni has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13762 Change subject: IMPALA-8427: [DOCS] Document the new startup flag IMPALA-7800 introduced .. IMPALA-8427: [DOCS] Document the new startup flag IMPALA-7800 introduced - Added a new doc impala_client.xml as the overview of Impala impala client access. Change-Id: I1a4c1975721c32a78a003d91babc5d2bb90f3949 --- M docs/impala.ditamap A docs/topics/impala_client.xml M docs/topics/impala_config_options.xml 3 files changed, 149 insertions(+), 55 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/13762/1 -- To view, visit http://gerrit.cloudera.org:8080/13762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I1a4c1975721c32a78a003d91babc5d2bb90f3949 Gerrit-Change-Number: 13762 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni
[Impala-ASF-CR] IMPALA-8427: [DOCS] Document the new startup flag IMPALA-7800 introduced
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13762 ) Change subject: IMPALA-8427: [DOCS] Document the new startup flag IMPALA-7800 introduced .. Patch Set 1: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/360/ Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/13762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a4c1975721c32a78a003d91babc5d2bb90f3949 Gerrit-Change-Number: 13762 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 28 Jun 2019 20:26:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8681: Only show ValidWriteIdLists for Acid tables
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/13736 ) Change subject: IMPALA-8681: Only show ValidWriteIdLists for Acid tables .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/13736/2/testdata/workloads/functional-query/queries/QueryTest/acid-profile.test File testdata/workloads/functional-query/queries/QueryTest/acid-profile.test: http://gerrit.cloudera.org:8080/#/c/13736/2/testdata/workloads/functional-query/queries/QueryTest/acid-profile.test@16 PS2, Line 16: # Verify that ValidWriteIdLists in the profile > nit: missing "is" I will change http://gerrit.cloudera.org:8080/#/c/13736/2/tests/query_test/test_acid.py File tests/query_test/test_acid.py: http://gerrit.cloudera.org:8080/#/c/13736/2/tests/query_test/test_acid.py@55 PS2, Line 55: @SkipIfCatalogV2.hms_event_polling_enabled() > Can you explain why this is needed? Other acid tests also use the "create i When local catalogV2 combines with hms_enent_polling enabled, it seems catalog loading the table by itself, the query statement cannot trigger loading the table(not sure it is by design or a bug). Because of the WriteIdlist profile is part of table loading, it can not be shown. This skip to avoid flaky tests. -- To view, visit http://gerrit.cloudera.org:8080/13736 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifcc31c7ddcfc471b0e5308f7e4aaadfa8189a905 Gerrit-Change-Number: 13736 Gerrit-PatchSet: 2 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 28 Jun 2019 19:59:40 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8717: impala-shell support for HS2 HTTP endpoint
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/13746 )
Change subject: IMPALA-8717: impala-shell support for HS2 HTTP endpoint
..
Patch Set 4:
(6 comments)
This is really cool. I didn't fully grok all the Thrift transport stuff yet,
but thought I'd flush out comments at this point since I had some higher level
feedback about the command line
http://gerrit.cloudera.org:8080/#/c/13746/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13746/4//COMMIT_MSG@17
PS4, Line 17: impala-shell --protocol='hs2' --http (No auth)
I kinda feel like we should be able to enable this with a single flag. That
would be more convenient for users. There's now two options with two settings
each, with one combination - beeswax/http that doesn't make sense.
I think either we could make it a protocol - i.e. "hs2-http", or the --http
flag could make the default protocol hs2.
http://gerrit.cloudera.org:8080/#/c/13746/4/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/13746/4/be/src/service/impala-server.cc@2080
PS4, Line 2080: << " to server " << connection_context.server_name
<< " closed.";
I think it might still be useful to log the number of associated sessions even
if they're not closed.
http://gerrit.cloudera.org:8080/#/c/13746/4/shell/impala_client.py
File shell/impala_client.py:
http://gerrit.cloudera.org:8080/#/c/13746/4/shell/impala_client.py@367
PS4, Line 367: host_and_port = "%s:%s" % (self.impalad_host,
self.impalad_port)
nit: prefer format(), e.g.
"{0}:{1}".format(self.impalad_host, self.impalad_port)
http://gerrit.cloudera.org:8080/#/c/13746/4/shell/impala_client.py@377
PS4, Line 377: transport = THttpClient("https://%s; % host_and_port,
ssl_context=ssl_ctx)
Same comment about format()
http://gerrit.cloudera.org:8080/#/c/13746/4/shell/impala_client.py@379
PS4, Line 379: transport = THttpClient("http://%s; % host_and_port)
Same comment about format()
http://gerrit.cloudera.org:8080/#/c/13746/4/shell/impala_client.py@383
PS4, Line 383: auth = base64.encodestring('%s:%s' % (self.user,
self.ldap_password)).strip('\n')
Same comment about format()
--
To view, visit http://gerrit.cloudera.org:8080/13746
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8323950857dfe1c1dfd5377fde79f87bc2ce9534
Gerrit-Change-Number: 13746
Gerrit-PatchSet: 4
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Fri, 28 Jun 2019 16:58:50 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8673: Add query option to force plan hints for insert queries
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/13753 )
Change subject: IMPALA-8673: Add query option to force plan hints for insert
queries
..
Patch Set 1:
(7 comments)
I did a pass over this. I think it's pretty close but I might not be able to
finish the review (going on holiday for a couple of weeks). I think Lars or
Bharath could finish this.
http://gerrit.cloudera.org:8080/#/c/13753/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13753/1//COMMIT_MSG@28
PS1, Line 28:
Can you mention which table formats this applies to - HDFS/Kudu/HBase?
http://gerrit.cloudera.org:8080/#/c/13753/1/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
File fe/src/main/java/org/apache/impala/analysis/InsertStmt.java:
http://gerrit.cloudera.org:8080/#/c/13753/1/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@841
PS1, Line 841: ((table_ instanceof FeHBaseTable) ||
I think the HBase bit deserves some explanation. The idea is that the hints
don't make sense for HBase inserts, so we don't want to add nonsensical hints,
right?
http://gerrit.cloudera.org:8080/#/c/13753/1/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@844
PS1, Line 844: Setup
Set up
http://gerrit.cloudera.org:8080/#/c/13753/1/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@850
PS1, Line 850: toks
nit: could skip the intermediate value and inline the RHS on the below line.
http://gerrit.cloudera.org:8080/#/c/13753/1/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@852
PS1, Line 852: hint.trim();
Is there a test to make sure that whitespace is ignored?
http://gerrit.cloudera.org:8080/#/c/13753/1/fe/src/test/java/org/apache/impala/planner/PlannerTest.java
File fe/src/test/java/org/apache/impala/planner/PlannerTest.java:
http://gerrit.cloudera.org:8080/#/c/13753/1/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@190
PS1, Line 190: @Test
The tests are pretty comprehensive, but can you add a sanity test for HBase and
Kudu to prove that the hints do not and do take effect, respectively. I don't
think testing the full matrix below is necessary.
http://gerrit.cloudera.org:8080/#/c/13753/1/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@193
PS1, Line 193: options.setDefault_hints_insert_statement("clustered");
You added the .trim() to make this whitespace-insensitive. Maybe add some
variations in whitespace around the hints here so we have testing to catch
regressions in that.
Or we could also be stricter about the whitespace.
--
To view, visit http://gerrit.cloudera.org:8080/13753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1c3f213402b8e4d1940f96738ad21edf800fa43a
Gerrit-Change-Number: 13753
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Rawat
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Fri, 28 Jun 2019 16:48:29 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8681: Only show ValidWriteIdLists for Acid tables
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/13736 ) Change subject: IMPALA-8681: Only show ValidWriteIdLists for Acid tables .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/13736/2/testdata/workloads/functional-query/queries/QueryTest/acid-profile.test File testdata/workloads/functional-query/queries/QueryTest/acid-profile.test: http://gerrit.cloudera.org:8080/#/c/13736/2/testdata/workloads/functional-query/queries/QueryTest/acid-profile.test@16 PS2, Line 16: # Verify that ValidWriteIdLists in the profile nit: missing "is" http://gerrit.cloudera.org:8080/#/c/13736/2/tests/query_test/test_acid.py File tests/query_test/test_acid.py: http://gerrit.cloudera.org:8080/#/c/13736/2/tests/query_test/test_acid.py@55 PS2, Line 55: @SkipIfCatalogV2.hms_event_polling_enabled() Can you explain why this is needed? Other acid tests also use the "create in Hive, then invalidate and select in Impala" pattern. -- To view, visit http://gerrit.cloudera.org:8080/13736 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifcc31c7ddcfc471b0e5308f7e4aaadfa8189a905 Gerrit-Change-Number: 13736 Gerrit-PatchSet: 2 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 28 Jun 2019 16:11:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/13722 ) Change subject: IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1 .. Patch Set 2: (49 comments) I've reviewed the first portion of the change, will continue looking at it on Monday. http://gerrit.cloudera.org:8080/#/c/13722/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13722/2//COMMIT_MSG@49 PS2, Line 49: In a string type to timestamp conversion the timezone offset tokens : are parsed, expected to match with the input but they don't adjust : the result as the input is already expected to be in UTC format. Is this behavior consistent with how other SQL systems work? http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/benchmarks/parse-timestamp-benchmark.cc File be/src/benchmarks/parse-timestamp-benchmark.cc: http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/benchmarks/parse-timestamp-benchmark.cc@47 PS2, Line 47: // Benchmark for parsing timestamps. : // Machine Info: Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz : // ParseDate:Function Rate (iters/ms) Comparison : // -- : //BoostStringDate 1.277 1X : // BoostDate 1.229 0.962X : // Impala 16.83 13.17X : // : // ParseTimestamp: Function Rate (iters/ms) Comparison : // -- : // BoostTime 0.9074 1X : // Impala 15.01 16.54X : // : // ParseTimestampWithFormat:Function Rate (iters/ms) Comparison : // -- : // BoostDateTime 0.4488 1X : //ImpalaTimeStamp 37.41 83.35X : // ImpalaTZTimeStamp 37.39 83.3X Maybe it wold make sense to add the new parsing functions to these benchmarks. What do you think? http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/common/init.cc File be/src/common/init.cc: http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/common/init.cc@312 PS2, Line 312: InitParseCtx I think this should be renamed to InitSimpleDateParseCtx() to make it clear that this is for the simple date format parsing. http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/exprs/cast-expr.h File be/src/exprs/cast-expr.h: http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/exprs/cast-expr.h@29 PS2, Line 29: CastExpr If I understand this correctly, this class is used only for the new cast operator with format. Maybe it should be called CastFormatExpr or something similar. http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/exprs/cast-functions-ir.cc File be/src/exprs/cast-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/exprs/cast-functions-ir.cc@172 PS2, Line 172: DateTimeFormatContext* const DateTimeFormatContext* http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/exprs/cast-functions-ir.cc@180 PS2, Line 180: tv.Format(*format_ctx, buf_len, buf); Check the return value, like you do in L199-200. http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/exprs/cast-functions-ir.cc@191 PS2, Line 191: DateTimeFormatContext This should be const too. http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/exprs/cast-functions-ir.cc@308 PS2, Line 308: dt_ctx Rename to 'format_ctx' for consistency. http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/exprs/cast-functions-ir.cc@308 PS2, Line 308: DateTimeFormatContext This should be const too. http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/exprs/cast-functions-ir.cc@312 PS2, Line 312: char* const char* http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/exprs/cast-functions-ir.cc@315 PS2, Line 315: char* const char* http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/exprs/cast-functions-ir.cc@342 PS2, Line 342: dt_ctx Rename to 'format_ctx' http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/exprs/cast-functions-ir.cc@342 PS2, Line 342: DateTimeFormatContext* Should be const. http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/exprs/cast-functions-ir.cc@346 PS2, Line 346: char* const char* http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/exprs/cast-functions-ir.cc@348 PS2, Line 348: char* const char* http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc:
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Csaba Ringhofer has posted comments on this change. (
http://gerrit.cloudera.org:8080/13558 )
Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
..
Patch Set 3:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/13558/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13558/3//COMMIT_MSG@9
PS3, Line 9:
nit: extra space
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@60
PS3, Line 60: import org.apache.impala.common.Id;
I didn't find where we use this.
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@64
PS3, Line 64: import org.apache.impala.service.BackendConfig;
This is only mentioned in comment, but is not used.
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@438
PS3, Line 438:* Impala supports external table read/write
:* insert-only Acid table read
:* virtual view read
:* materialized view read
Can you make a bullet list from this?
e.g:
* Impala supports:
* - external table read/write
* - insert-only Acid table read
* - virtual view read
* - materialized view read
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@445
PS3, Line 445: version
Unused variable.
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@451
PS3, Line 451: // BackendConfig.INSTANCE.getImpalaBuildVersion() is NULL or
: // BackendConfig.INSTANCE is NULL when the method is called.
: // Use MAJOR_VERSION
I do not understand the intention here.
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@454
PS3, Line 454: String impalaId = String.format("Impala%s@%s",
: MAJOR_VERSION, hostName);
nit: one line
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@466
PS3, Line 466: // number -1. It makes clients unable to
know it a bucketed table.
nit: missing "is"
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@300
PS3, Line 300: Check if support the table type or not
nit: "Check if the table type is supported" would be better.
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@311
PS3, Line 311: // the operations not supported, we will generate error
messages
nit: missing "are"
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@319
PS3, Line 319: ensureTableNotFullAcid(table);
Does this make sense in the MetastoreShim.getMajorVersion() == 2 case?
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
File fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java:
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java@365
PS3, Line 365: analyzer.checkTableCapability(table_,
Analyzer.OperationType.WRITE);
I also mentioned in AnalyzerTest.java that prohibiting COMPUTE STATS is strange
for me. Impala can generally do COMPUTE STATS for tables it cannot write, e.g.
Avro tables.
I think that Transactional tables were excluded because Hive uses the stats for
them to answer some queries, and we are afraid that differences in stats by
Impala can lead to incorrect results in Hive.
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java:
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java@897
PS3, Line 897: AnalysisError("compute stats functional.bucketed_table",
errorMsgBucketed);
It is strange for me that DROP STATS is allowed, while COMPUTE STATS is not for
bucketed tables. Is this intentional?
http://gerrit.cloudera.org:8080/#/c/13558/3/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:
http://gerrit.cloudera.org:8080/#/c/13558/3/testdata/datasets/functional/functional_schema_template.sql@2162
PS3, Line 2162: STORED AS ORC
Does this
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3781/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 5 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 28 Jun 2019 15:11:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 5: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 5 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 28 Jun 2019 14:41:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 5: Removed trailing whitespace. -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 5 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 28 Jun 2019 14:31:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. IMPALA-8710: Increase allowed bit width to 64 for bit packing Increasing the allowed bit width for bit packing and bit unpacking to 64 bits. This will be needed to support the Parquet delta encoding. Added new methods to BitWriter and BatchedBitReader handling Uleb and ZigZag integers for 64 bits, also needed by delta encoding. The performance of bit unpacking is either the same or better with the new implementation, depending on bit width. Testing: - Modified bit packing and unpacking tests to test bit widths up to 64 bits. - Tests covering the additions in BitWriter and BatchedBitReader. Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 --- M be/src/benchmarks/bit-packing-benchmark.cc M be/src/util/CMakeLists.txt M be/src/util/bit-packing-test.cc M be/src/util/bit-packing.h M be/src/util/bit-packing.inline.h M be/src/util/bit-stream-utils-test.cc M be/src/util/bit-stream-utils.h M be/src/util/bit-stream-utils.inline.h M be/src/util/rle-encoding.h M be/src/util/rle-test.cc 10 files changed, 413 insertions(+), 204 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/13737/5 -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 5 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3780/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 28 Jun 2019 14:10:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 4: Updated the results in bit-packing-benchmark.cc -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 28 Jun 2019 13:30:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. IMPALA-8710: Increase allowed bit width to 64 for bit packing Increasing the allowed bit width for bit packing and bit unpacking to 64 bits. This will be needed to support the Parquet delta encoding. Added new methods to BitWriter and BatchedBitReader handling Uleb and ZigZag integers for 64 bits, also needed by delta encoding. The performance of bit unpacking is either the same or better with the new implementation, depending on bit width. Testing: - Modified bit packing and unpacking tests to test bit widths up to 64 bits. - Tests covering the additions in BitWriter and BatchedBitReader. Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 --- M be/src/benchmarks/bit-packing-benchmark.cc M be/src/util/CMakeLists.txt M be/src/util/bit-packing-test.cc M be/src/util/bit-packing.h M be/src/util/bit-packing.inline.h M be/src/util/bit-stream-utils-test.cc M be/src/util/bit-stream-utils.h M be/src/util/bit-stream-utils.inline.h M be/src/util/rle-encoding.h M be/src/util/rle-test.cc 10 files changed, 445 insertions(+), 236 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/13737/4 -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3779/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 28 Jun 2019 12:52:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has posted comments on this change. (
http://gerrit.cloudera.org:8080/13737 )
Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing
..
Patch Set 3:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/13737/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13737/2//COMMIT_MSG@14
PS2, Line 14:
> Performance effects could be mentioned +
Done
http://gerrit.cloudera.org:8080/#/c/13737/2//COMMIT_MSG@16
PS2, Line 16:
> nit: wrap at 72
Done
http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-packing.inline.h
File be/src/util/bit-packing.inline.h:
http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-packing.inline.h@177
PS2, Line 177: const uint32_t* const in = re
> I would move this to the very beginning to make it clear that the rest of t
Done
http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-packing.inline.h@181
PS2, Line 181: p
> nit: extra space
Done
http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-packing.inline.h@184
PS2, Line 184: == 1
> Can you move this to a constexpr to make it clear that this happens at comp
Done
http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils-test.cc
File be/src/util/bit-stream-utils-test.cc:
http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils-test.cc@294
PS2, Line 294: TestZigZagEncode(1629267541664, {0xc0, 0xfa, 0xcd,
0xfe, 0xea, 0x5e});
> nit:extra line
Done
http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils-test.cc@315
PS2, Line 315: TestZigZagDecode(-9223372036854775808U, // Most
negative int64_t.
> nit:extra line
Done
http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils-test.cc@320
PS2, Line 320: TEST(VLQInt, TestMaxVlqByteLen) {
> nit:extra line
Done
http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils.inline.h
File be/src/util/bit-stream-utils.inline.h:
http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils.inline.h@44
PS2, Line 44: un
> Can you expand the abbreviation? It took me a few seconds to understand it.
Done
--
To view, visit http://gerrit.cloudera.org:8080/13737
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8
Gerrit-Change-Number: 13737
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Fri, 28 Jun 2019 12:11:36 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. IMPALA-8710: Increase allowed bit width to 64 for bit packing Increasing the allowed bit width for bit packing and bit unpacking to 64 bits. This will be needed to support the Parquet delta encoding. Added new methods to BitWriter and BatchedBitReader handling Uleb and ZigZag integers for 64 bits, also needed by delta encoding. The performance of bit unpacking is either the same or better with the new implementation, depending on bit width. Testing: - Modified bit packing and unpacking tests to test bit widths up to 64 bits. - Tests covering the additions in BitWriter and BatchedBitReader. Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 --- M be/src/util/CMakeLists.txt M be/src/util/bit-packing-test.cc M be/src/util/bit-packing.h M be/src/util/bit-packing.inline.h M be/src/util/bit-stream-utils-test.cc M be/src/util/bit-stream-utils.h M be/src/util/bit-stream-utils.inline.h M be/src/util/rle-encoding.h M be/src/util/rle-test.cc 9 files changed, 314 insertions(+), 105 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/13737/3 -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 2: Code-Review+1 (10 comments) I found only nits. http://gerrit.cloudera.org:8080/#/c/13737/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13737/2//COMMIT_MSG@14 PS2, Line 14: Performance effects could be mentioned + https://gerrit.cloudera.org/#/c/12621/ as it contains the benchmarks that were used to verify that there is no performance regression. http://gerrit.cloudera.org:8080/#/c/13737/2//COMMIT_MSG@16 PS2, Line 16: 64 bits. nit: wrap at 72 http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-packing.inline.h File be/src/util/bit-packing.inline.h: http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-packing.inline.h@177 PS2, Line 177: if (BIT_WIDTH == 0) return 0; I would move this to the very beginning to make it clear that the rest of the logic does not have to handle this case. http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-packing.inline.h@181 PS2, Line 181: nit: extra space http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-packing.inline.h@184 PS2, Line 184: BitUtil::IsPowerOf2(BIT_WIDTH) Can you move this to a constexpr to make it clear that this happens at compile time? http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils-test.cc File be/src/util/bit-stream-utils-test.cc: http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils-test.cc@294 PS2, Line 294: nit:extra line http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils-test.cc@315 PS2, Line 315: nit:extra line http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils-test.cc@320 PS2, Line 320: nit:extra line http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils.inline.h File be/src/util/bit-stream-utils.inline.h: http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils.inline.h@44 PS2, Line 44: UB Can you expand the abbreviation? It took me a few seconds to understand it. http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils.inline.h@196 PS2, Line 196: UB Same as line 44. -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 28 Jun 2019 11:59:55 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8689: test hive impala interop failing with "Timeout >7200s"
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13755 ) Change subject: IMPALA-8689: test_hive_impala_interop failing with "Timeout >7200s" .. IMPALA-8689: test_hive_impala_interop failing with "Timeout >7200s" The newly added Hive<->Impala interop test fails due to unexpected wrong results when reading TimeStamp column value written by Hive. The short term measure is to remove TimeStamp column from the interop tests. The original issue will be fixed by IMPALA-8721. Testing: Ran the testcase N number of times on both upstream and downstream code base. Change-Id: I148c79a31f9aada1b75614390434462d1e483f28 Reviewed-on: http://gerrit.cloudera.org:8080/13755 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M tests/custom_cluster/test_hive_parquet_codec_interop.py 1 file changed, 4 insertions(+), 2 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/13755 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I148c79a31f9aada1b75614390434462d1e483f28 Gerrit-Change-Number: 13755 Gerrit-PatchSet: 4 Gerrit-Owner: Abhishek Rawat Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8689: test hive impala interop failing with "Timeout >7200s"
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13755 ) Change subject: IMPALA-8689: test_hive_impala_interop failing with "Timeout >7200s" .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/13755 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I148c79a31f9aada1b75614390434462d1e483f28 Gerrit-Change-Number: 13755 Gerrit-PatchSet: 3 Gerrit-Owner: Abhishek Rawat Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 28 Jun 2019 11:27:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3778/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 28 Jun 2019 11:16:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. IMPALA-8710: Increase allowed bit width to 64 for bit packing Increasing the allowed bit width for bit packing and bit unpacking to 64 bits. This will be needed to support the Parquet delta encoding. Added new methods to BitWriter and BatchedBitReader handling Uleb and ZigZag integers for 64 bits, also needed by delta encoding. Testing: - Modified bit packing and unpacking tests to test bit widths up to 64 bits. - Tests covering the additions in BitWriter and BatchedBitReader. Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 --- M be/src/util/CMakeLists.txt M be/src/util/bit-packing-test.cc M be/src/util/bit-packing.h M be/src/util/bit-packing.inline.h M be/src/util/bit-stream-utils-test.cc M be/src/util/bit-stream-utils.h M be/src/util/bit-stream-utils.inline.h M be/src/util/rle-encoding.h M be/src/util/rle-test.cc 9 files changed, 314 insertions(+), 105 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/13737/2 -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins
