[Impala-ASF-CR] IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14076 ) Change subject: IMPALA-8869: Fix handling of HTTP keep-alive when returning 401 .. IMPALA-8869: Fix handling of HTTP keep-alive when returning 401 Recent work has added support for HTTP authentication both for the thrift hs2 interface and for the webserver. In both cases, we mishandle HTTP keep-alive semantics when returning a 401 because we close the connection but don't return a 'Connection: close' header, even though we're using HTTP/1.1 where keep-alive is assumed, which can cause clients to incorrectly believe that the connection has remained open. For the webserver, the fix is to enable keep-alive in squeasel so that the connection isn't closed after the 401 is returned. For the thrift hs2 interface, we throw an exception after the 401 which results in the connection being closed because otherwise it's tricky with the way thrift is structured to ensure that the unauthorized request isn't processed. So, the fix here is to return a 'Connection: close' header. Testing: - Ran existing HTTP auth tests. - Manually tested in a cluster with connections to Impala proxied through Apache Knox. Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa Reviewed-on: http://gerrit.cloudera.org:8080/14076 Reviewed-by: Impala Public Jenkins Reviewed-by: Thomas Tauber-Marshall Tested-by: Impala Public Jenkins --- M be/src/transport/THttpServer.cpp M be/src/util/webserver.cc 2 files changed, 5 insertions(+), 1 deletion(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified Thomas Tauber-Marshall: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/14076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa Gerrit-Change-Number: 14076 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14076 ) Change subject: IMPALA-8869: Fix handling of HTTP keep-alive when returning 401 .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa Gerrit-Change-Number: 14076 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 21 Aug 2019 04:09:21 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 5: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4825/ -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 5 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 21 Aug 2019 02:19:13 + Gerrit-HasComments: No
[Impala-ASF-CR] PREVIEW IMPALA-8863: Add support to run tests over HTTP/HS2
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14059 ) Change subject: PREVIEW IMPALA-8863: Add support to run tests over HTTP/HS2 .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4319/ : 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/14059 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7156558071781378fcb9c8941c0f4dd82eb0d018 Gerrit-Change-Number: 14059 Gerrit-PatchSet: 3 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 21 Aug 2019 01:38:09 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4318/ : 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/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 5 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 21 Aug 2019 01:23:43 + Gerrit-HasComments: No
[Impala-ASF-CR] PREVIEW IMPALA-8863: Add support to run tests over HTTP/HS2
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14059 ) Change subject: PREVIEW IMPALA-8863: Add support to run tests over HTTP/HS2 .. Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/14059/3/fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java File fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java: http://gerrit.cloudera.org:8080/#/c/14059/3/fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java@150 PS3, Line 150: RunShellCommand.Run(command, /* shouldSucceed */ false, "", "Not connected to Impala"); line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/14059/3/tests/util/run_impyla_http_query.py File tests/util/run_impyla_http_query.py: http://gerrit.cloudera.org:8080/#/c/14059/3/tests/util/run_impyla_http_query.py@30 PS3, Line 30: def run_query(query, args): flake8: E302 expected 2 blank lines, found 1 http://gerrit.cloudera.org:8080/#/c/14059/3/tests/util/run_impyla_http_query.py@33 PS3, Line 33: u flake8: F841 local variable 'url' is assigned to but never used http://gerrit.cloudera.org:8080/#/c/14059/3/tests/util/run_impyla_http_query.py@46 PS3, Line 46: def main(): flake8: E302 expected 2 blank lines, found 1 http://gerrit.cloudera.org:8080/#/c/14059/3/tests/util/run_impyla_http_query.py@57 PS3, Line 57: if __name__ == "__main__": flake8: E305 expected 2 blank lines after class or function definition, found 1 -- To view, visit http://gerrit.cloudera.org:8080/14059 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7156558071781378fcb9c8941c0f4dd82eb0d018 Gerrit-Change-Number: 14059 Gerrit-PatchSet: 3 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 21 Aug 2019 01:18:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] PREVIEW IMPALA-8863: Add support to run tests over HTTP/HS2
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14059 to look at the new patch set (#3). Change subject: PREVIEW IMPALA-8863: Add support to run tests over HTTP/HS2 .. PREVIEW IMPALA-8863: Add support to run tests over HTTP/HS2 This change adds support to run backend tests over HTTP using a new version of Impyla. Currently it only works with a private Impyla branch and can be used to further test that change. https://github.com/lekv/impyla/tree/http_transport Once Impyla releases a new version with the change, we can polish this change and submit it. Change-Id: I7156558071781378fcb9c8941c0f4dd82eb0d018 --- M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java A fe/src/test/java/org/apache/impala/customcluster/LdapImpylaHttpTest.java A fe/src/test/java/org/apache/impala/customcluster/RunShellCommand.java M tests/common/impala_connection.py M tests/common/impala_test_suite.py M tests/common/test_dimensions.py M tests/custom_cluster/test_client_ssl.py M tests/custom_cluster/test_shell_interactive.py M tests/custom_cluster/test_shell_interactive_reconnect.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py A tests/util/run_impyla_http_query.py 12 files changed, 309 insertions(+), 66 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/14059/3 -- To view, visit http://gerrit.cloudera.org:8080/14059 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7156558071781378fcb9c8941c0f4dd82eb0d018 Gerrit-Change-Number: 14059 Gerrit-PatchSet: 3 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8818: Replace deque with spillable queue in BufferedPRS
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14039 ) Change subject: IMPALA-8818: Replace deque with spillable queue in BufferedPRS .. Patch Set 8: (8 comments) I took a look at the resource management part of this http://gerrit.cloudera.org:8080/#/c/14039/8/be/src/runtime/spillable-row-batch-queue.cc File be/src/runtime/spillable-row-batch-queue.cc: http://gerrit.cloudera.org:8080/#/c/14039/8/be/src/runtime/spillable-row-batch-queue.cc@80 PS8, Line 80: RETURN_IF_ERROR(state_->StartSpilling(mem_tracker_)); Add something to the profile to indicate it spilled? http://gerrit.cloudera.org:8080/#/c/14039/8/be/src/runtime/spillable-row-batch-queue.cc@81 PS8, Line 81: // The pin should be stream at this point. : DCHECK(batch_queue_->is_pinned()); > It makes me wonder if the logic will be simpler if we just always use an u This wouldn't work with the current spilling infrastructure, which really assumes that you don't unpin things until you actually need to spill them. There's a few things: * Spilling can be disabled and we need to call StartSpilling() to check whether it's ok to spill things * Data is flushed to disk eagerly, even if there's plenty of memory * Errors in writing to disk (e.g. hitting a scratch limit, no configured scratch, etc) will result in a query failure You could add a mode where unpinned things don't get flushed until there is memory pressure (and errors from spilling being disabled are propagated in a different way), but it would be additional work and complexity in a different part of the code. http://gerrit.cloudera.org:8080/#/c/14039/8/be/src/service/query-options.cc File be/src/service/query-options.cc: http://gerrit.cloudera.org:8080/#/c/14039/8/be/src/service/query-options.cc@907 PS8, Line 907: int64_t max_pinned_mem = query_options->max_pinned_result_spooling_memory; I'd suggest just making this max_result_spooling_mem or max_result_spooling_memory. We haven't exposed the pinned/unpinned concept in user-facing options or documentation for the most part, and I think it would be confusing. For accounting purposes unpinned pages don't count against a query's memory limits. http://gerrit.cloudera.org:8080/#/c/14039/8/be/src/service/query-options.cc@911 PS8, Line 911: max_unpinned_result_spooling_memory We should use "spilled" instead of "unpinned" in user-facing terminology to be consistent with elsewhere. http://gerrit.cloudera.org:8080/#/c/14039/8/fe/src/main/java/org/apache/impala/planner/PlanRootSink.java File fe/src/main/java/org/apache/impala/planner/PlanRootSink.java: http://gerrit.cloudera.org:8080/#/c/14039/8/fe/src/main/java/org/apache/impala/planner/PlanRootSink.java@84 PS8, Line 84: long perInstanceInputCardinality = There will only ever be one instance of PLAN_ROOT_SINK, so we can remote the numInstances calculations. http://gerrit.cloudera.org:8080/#/c/14039/8/fe/src/main/java/org/apache/impala/planner/PlanRootSink.java@87 PS8, Line 87: (long) Math.ceil(perInstanceInputCardinality * inputNode.getAvgRowSize()); We should cap this at the max result spooling memory, otherwise it could be a huge overestimate. http://gerrit.cloudera.org:8080/#/c/14039/8/fe/src/main/java/org/apache/impala/planner/ResourceProfile.java File fe/src/main/java/org/apache/impala/planner/ResourceProfile.java: http://gerrit.cloudera.org:8080/#/c/14039/8/fe/src/main/java/org/apache/impala/planner/ResourceProfile.java@76 PS8, Line 76: memEstimateBytes_ = (maxMemReservationBytes > 0) ? This isn't generally correct for all plan nodes, the memory estimate can include non-reserved memory. You need to do this calculation in PlanRootSink since it depends on the node how much non-reserved memory there might be. http://gerrit.cloudera.org:8080/#/c/14039/8/fe/src/main/java/org/apache/impala/planner/ResourceProfileBuilder.java File fe/src/main/java/org/apache/impala/planner/ResourceProfileBuilder.java: http://gerrit.cloudera.org:8080/#/c/14039/8/fe/src/main/java/org/apache/impala/planner/ResourceProfileBuilder.java@78 PS8, Line 78: Preconditions.checkState(memEstimateBytes_ >= 0, "Mem estimate must be set"); Maybe check that maxMemReservationBytes_ >= minMemReservationBytes_ -- To view, visit http://gerrit.cloudera.org:8080/14039 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I10f9e72374cdf9501c0e5e2c5b39c13688ae65a9 Gerrit-Change-Number: 14039 Gerrit-PatchSet: 8 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 21 Aug 2019 00:47:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/14106/5/fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java File fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java: http://gerrit.cloudera.org:8080/#/c/14106/5/fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java@55 PS5, Line 55: Authorizable newColumnInTable(String dbName, String tableName, @Nullable String tblOwnerUser); line too long (96 > 90) -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 5 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 21 Aug 2019 00:45:10 + Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4825/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 5 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 21 Aug 2019 00:45:30 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Hello Austin Nobis, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14106
to look at the new patch set (#5).
Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz
..
[WIP] IMPALA-8228: Ownership support for Ranger authz
Without this patch, explicit privileges are needed even
for owners of databases/tables to perform actions on them.
Example: 'user' is the owner of database 'foo'. To create
a table 't' under 'foo', 'user' needs to be granted a CREATE
privilege on 'foo'
That is unintuitive from a user POV since users expect owners
to have ALL privileges on the objects they own. This patch extends
that support to Impala's ranger authorization plugin.
Ranger natively supports the concept of ownership by letting the
callers pass the ownership context to RangerAccessResourceImpl.
This patch plumbs the owner information for the authorizables
(currently only supported for Tables / Databases) which is then
evaulated during authorization.
For the ownership based authorization to work, ranger-admin side
policy on {OWNER} user needs to be defined.
(TODO) Working on tests.
Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/authorization/Authorizable.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableColumn.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableDb.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
M
fe/src/main/java/org/apache/impala/authorization/DefaultAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaResourceBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/FeDb.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
25 files changed, 237 insertions(+), 88 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/14106/5
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 5
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14076 ) Change subject: IMPALA-8869: Fix handling of HTTP keep-alive when returning 401 .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4317/ : 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/14076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa Gerrit-Change-Number: 14076 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 21 Aug 2019 00:44:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8803: Coordinator should release admitted memory per-backend
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14104 ) Change subject: IMPALA-8803: Coordinator should release admitted memory per-backend .. Patch Set 1: (7 comments) Did an initial pass over the review. Overall looks good. http://gerrit.cloudera.org:8080/#/c/14104/1/be/src/runtime/coordinator-backend-state.h File be/src/runtime/coordinator-backend-state.h: http://gerrit.cloudera.org:8080/#/c/14104/1/be/src/runtime/coordinator-backend-state.h@527 PS1, Line 527: const std::vector& backend_states_; What owns the object that is being referenced? The schedule? http://gerrit.cloudera.org:8080/#/c/14104/1/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: http://gerrit.cloudera.org:8080/#/c/14104/1/be/src/runtime/coordinator-backend-state.cc@810 PS1, Line 810: NumReleasedBackends I feel like the name is a little confusing in isolation, I'm not sure people will have an idea what it is. Maybe NumCompletedBackends or NumResourceReleasedBackends. http://gerrit.cloudera.org:8080/#/c/14104/1/be/src/scheduling/admission-controller.h File be/src/scheduling/admission-controller.h: http://gerrit.cloudera.org:8080/#/c/14104/1/be/src/scheduling/admission-controller.h@339 PS1, Line 339: void ReleaseQueryBackends( Needs a comment. We may also want to document the lifecycle of resource release in a section in the class comment above, since it's gotten a little more complex. http://gerrit.cloudera.org:8080/#/c/14104/1/be/src/scheduling/admission-controller.h@489 PS1, Line 489: void ReleaseQueryBackends( Needs a comment http://gerrit.cloudera.org:8080/#/c/14104/1/be/src/scheduling/admission-controller.h@816 PS1, Line 816: void UpdateHostStatsForQueryBackends( Needs a comment http://gerrit.cloudera.org:8080/#/c/14104/1/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/14104/1/be/src/scheduling/admission-controller.cc@375 PS1, Line 375: schedule.coord_backend_mem_to_admit() : This pattern appears in a few places - might be worth making a helper function in backend_exec_params. http://gerrit.cloudera.org:8080/#/c/14104/1/tests/query_test/test_result_spooling.py File tests/query_test/test_result_spooling.py: http://gerrit.cloudera.org:8080/#/c/14104/1/tests/query_test/test_result_spooling.py@82 PS1, Line 82: class TestDedicatedCoordinator(CustomClusterTestSuite): Let's move this into the custom_cluster directory. run-custom-cluster-tests.sh assumes that they are in certain subdirectories. -- To view, visit http://gerrit.cloudera.org:8080/14104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88bb11e0ede7574568020e0277dd8ac8d2586dc9 Gerrit-Change-Number: 14104 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 21 Aug 2019 00:25:36 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/14076 ) Change subject: IMPALA-8869: Fix handling of HTTP keep-alive when returning 401 .. Patch Set 3: Code-Review+2 gvo failed due to unrelated IMPALA-8880 -- To view, visit http://gerrit.cloudera.org:8080/14076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa Gerrit-Change-Number: 14076 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 21 Aug 2019 00:01:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14076 ) Change subject: IMPALA-8869: Fix handling of HTTP keep-alive when returning 401 .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4824/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa Gerrit-Change-Number: 14076 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 21 Aug 2019 00:01:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Hello Todd Lipcon, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14076 to look at the new patch set (#3). Change subject: IMPALA-8869: Fix handling of HTTP keep-alive when returning 401 .. IMPALA-8869: Fix handling of HTTP keep-alive when returning 401 Recent work has added support for HTTP authentication both for the thrift hs2 interface and for the webserver. In both cases, we mishandle HTTP keep-alive semantics when returning a 401 because we close the connection but don't return a 'Connection: close' header, even though we're using HTTP/1.1 where keep-alive is assumed, which can cause clients to incorrectly believe that the connection has remained open. For the webserver, the fix is to enable keep-alive in squeasel so that the connection isn't closed after the 401 is returned. For the thrift hs2 interface, we throw an exception after the 401 which results in the connection being closed because otherwise it's tricky with the way thrift is structured to ensure that the unauthorized request isn't processed. So, the fix here is to return a 'Connection: close' header. Testing: - Ran existing HTTP auth tests. - Manually tested in a cluster with connections to Impala proxied through Apache Knox. Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa --- M be/src/transport/THttpServer.cpp M be/src/util/webserver.cc 2 files changed, 5 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/14076/3 -- To view, visit http://gerrit.cloudera.org:8080/14076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa Gerrit-Change-Number: 14076 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8858: Add observability of idle executor groups
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14103 ) Change subject: IMPALA-8858: Add observability of idle executor groups .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4316/ : 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/14103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I58cde8699c33af8b87273437e9d8bf6371a34539 Gerrit-Change-Number: 14103 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-Comment-Date: Tue, 20 Aug 2019 23:28:49 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 4: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4823/ -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 20 Aug 2019 23:22:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8858: Add observability of idle executor groups
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/14103 ) Change subject: IMPALA-8858: Add observability of idle executor groups .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/14103/1/be/src/scheduling/cluster-membership-mgr.h File be/src/scheduling/cluster-membership-mgr.h: http://gerrit.cloudera.org:8080/#/c/14103/1/be/src/scheduling/cluster-membership-mgr.h@43 PS1, Line 43: /// This struct stores per-host statistics which are used during admission, by the cluster > I agree (sply about HostStats feeling out of place) but currently the metri I think there's a case for making it admission-controller.executor-groups.total-healthy-idle since "idle" is also an AC concept. -- To view, visit http://gerrit.cloudera.org:8080/14103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I58cde8699c33af8b87273437e9d8bf6371a34539 Gerrit-Change-Number: 14103 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Tue, 20 Aug 2019 23:00:53 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8818: Replace deque with spillable queue in BufferedPRS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14039 ) Change subject: IMPALA-8818: Replace deque with spillable queue in BufferedPRS .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4315/ : 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/14039 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I10f9e72374cdf9501c0e5e2c5b39c13688ae65a9 Gerrit-Change-Number: 14039 Gerrit-PatchSet: 8 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 20 Aug 2019 22:53:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8858: Add observability of idle executor groups
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/14103 ) Change subject: IMPALA-8858: Add observability of idle executor groups .. Patch Set 2: (1 comment) > This looks like the right approach. > Would there be any value by having a test in cluster-membership-mgr-test.cc > ? I thought about that, but since the metric update method relies on both admissionController and the clusterMembershipManager, I felt an e2e test would suit it better. The e2e test covers all cases so it should provide enough coverage. http://gerrit.cloudera.org:8080/#/c/14103/1/be/src/scheduling/cluster-membership-mgr.h File be/src/scheduling/cluster-membership-mgr.h: http://gerrit.cloudera.org:8080/#/c/14103/1/be/src/scheduling/cluster-membership-mgr.h@43 PS1, Line 43: /// This struct stores per-host statistics which are used during admission, by the cluster > I understand why this is here but I think semantically it belongs in the ad I agree (sply about HostStats feeling out of place) but currently the metrics in admission controller are all connected to resource pools (admission-controller.*.). This metric (cluster-membership.executor-groups.total-healthy-idle) which is tied to backends and executors groups fitted better inside the cluster membership manager. -- To view, visit http://gerrit.cloudera.org:8080/14103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I58cde8699c33af8b87273437e9d8bf6371a34539 Gerrit-Change-Number: 14103 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-Comment-Date: Tue, 20 Aug 2019 22:51:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8858: Add observability of idle executor groups
Hello Andrew Sherman, Lars Volker, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14103 to look at the new patch set (#2). Change subject: IMPALA-8858: Add observability of idle executor groups .. IMPALA-8858: Add observability of idle executor groups This patch adds a metric that displays a comma seperated list of executor group names that are in a healthy state and are idle according to the coordinator (no queries admitted locally by the coordinator are running on them). It gets updated when either the cluster membership changes or a query gets admitted or released by the admission controller. Testing: Added a custom cluster test. Change-Id: I58cde8699c33af8b87273437e9d8bf6371a34539 --- M be/src/scheduling/admission-controller.cc M be/src/scheduling/admission-controller.h M be/src/scheduling/cluster-membership-mgr.cc M be/src/scheduling/cluster-membership-mgr.h M common/thrift/metrics.json M tests/custom_cluster/test_executor_groups.py 6 files changed, 151 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/14103/2 -- To view, visit http://gerrit.cloudera.org:8080/14103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I58cde8699c33af8b87273437e9d8bf6371a34539 Gerrit-Change-Number: 14103 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4314/ : 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/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 20 Aug 2019 22:31:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8845: Cancel receiver's streams on exchange node's EOS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14101 ) Change subject: IMPALA-8845: Cancel receiver's streams on exchange node's EOS .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4313/ : 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/14101 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I10c805e9d63ed8af9f458bf71e8ef5ea9376b939 Gerrit-Change-Number: 14101 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Tue, 20 Aug 2019 22:31:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8818: Replace deque with spillable queue in BufferedPRS
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/14039 ) Change subject: IMPALA-8818: Replace deque with spillable queue in BufferedPRS .. Patch Set 8: Code-Review+1 (1 comment) Not super familiar with the resource profile part of the logic so Tim or Bikram may want to double check. http://gerrit.cloudera.org:8080/#/c/14039/8/be/src/runtime/spillable-row-batch-queue.cc File be/src/runtime/spillable-row-batch-queue.cc: http://gerrit.cloudera.org:8080/#/c/14039/8/be/src/runtime/spillable-row-batch-queue.cc@81 PS8, Line 81: // The pin should be stream at this point. : DCHECK(batch_queue_->is_pinned()); It makes me wonder if the logic will be simpler if we just always use an unpinned stream. For the most part, if the result row fits into a single buffer, this is the same as having a pinned stream. -- To view, visit http://gerrit.cloudera.org:8080/14039 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I10f9e72374cdf9501c0e5e2c5b39c13688ae65a9 Gerrit-Change-Number: 14039 Gerrit-PatchSet: 8 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 20 Aug 2019 22:16:54 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8818: Replace deque with spillable queue in BufferedPRS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14039 ) Change subject: IMPALA-8818: Replace deque with spillable queue in BufferedPRS .. Patch Set 8: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/4312/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/14039 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I10f9e72374cdf9501c0e5e2c5b39c13688ae65a9 Gerrit-Change-Number: 14039 Gerrit-PatchSet: 8 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 20 Aug 2019 22:09:28 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/14106/4/fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java File fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java: http://gerrit.cloudera.org:8080/#/c/14106/4/fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java@55 PS4, Line 55: Authorizable newColumnInTable(String dbName, String tableName, @Nullable String tblOwnerUser); line too long (96 > 90) -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 20 Aug 2019 21:52:38 + Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4823/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 20 Aug 2019 21:53:20 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/14106 )
Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz
..
Patch Set 4:
(13 comments)
Still working on tests. Meanwhile kicking off a test run to see what fails.
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2673
PS2, Line 2673: Preconditions.checkNotNull(privilege);
> we seem to have lost the "checkNotNull' here? was that intentional?
Done
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2818
PS3, Line 2818: } else {
> line too long (91 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2819
PS3, Line 2819: // Table does not exist and hence the owner information
cannot be deduced.
> line too long (92 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
File fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java:
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@138
PS2, Line 138:
> this could be null in the case of non-table-specific statements, which seem
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@189
PS2, Line 189: databa
> this should be 'database_' right?
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java:
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java@35
PS2, Line 35:
> mind adding @Nullable annotations here and below, if this is allowed to be
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java:
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java@30
PS2, Line 30: private final String tableName_;
> how about using @Nullable on this?
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java@37
PS2, Line 37: Preconditions.checkArgument(ownerUser == null ||
!ownerUser.isEmpty());
> would an empty owner string be valid? maybe we should be checking that it's
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java@55
PS2, Line 55: @Override
> this is @Override right?
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
File
fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java:
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java@71
PS2, Line 71:
> mind giving this a more explicit name like 'onTableWithUnknownOwner' or som
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java@74
PS2, Line 74: public PrivilegeRequestBuilder onTable(
> typo
Done
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
File
fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java:
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java@79
PS3, Line 79: }
> line too long (93 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/service/Frontend.java@786
PS3, Line 786: String tableOwner = table.getOwnerUser();
> line too long (96 > 90)
Done
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 4
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Bharath Vissap
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Hello Austin Nobis, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14106
to look at the new patch set (#4).
Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz
..
[WIP] IMPALA-8228: Ownership support for Ranger authz
Without this patch, explicit privileges are needed even
for owners of databases/tables to perform actions on them.
Example: 'user' is the owner of database 'foo'. To create
a table 't' under 'foo', 'user' needs to be granted a CREATE
privilege on 'foo'
That is unintuitive from a user POV since users expect owners
to have ALL privileges on the objects they own. This patch extends
that support to Impala's ranger authorization plugin.
Ranger natively supports the concept of ownership by letting the
callers pass the ownership context to RangerAccessResourceImpl.
This patch plumbs the owner information for the authorizables
(currently only supported for Tables / Databases) which is then
evaulated during authorization.
For the ownership based authorization to work, ranger-admin side
policy on {OWNER} user needs to be defined.
(TODO) Working on tests.
Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/authorization/Authorizable.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableColumn.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableDb.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
M
fe/src/main/java/org/apache/impala/authorization/DefaultAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaResourceBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/FeDb.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
25 files changed, 233 insertions(+), 85 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/14106/4
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 4
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8845: Cancel receiver's streams on exchange node's EOS
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/14101 ) Change subject: IMPALA-8845: Cancel receiver's streams on exchange node's EOS .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/14101/2/be/src/exec/exchange-node.cc File be/src/exec/exchange-node.cc: http://gerrit.cloudera.org:8080/#/c/14101/2/be/src/exec/exchange-node.cc@162 PS2, Line 162: stream_recvr_->CancelStream(); > does this mess up some of the logging done in KrpcDataStreamMgr::Cancel? it Reworded that log statement a bit to clarify the situation. We want to close the receiver without unregistering it so incoming senders won't hang. In fact, the same can already happen today if a fragment instance is first cancelled before being closed. http://gerrit.cloudera.org:8080/#/c/14101/2/tests/custom_cluster/test_exchange_eos.py File tests/custom_cluster/test_exchange_eos.py: http://gerrit.cloudera.org:8080/#/c/14101/2/tests/custom_cluster/test_exchange_eos.py@40 PS2, Line 40: @CustomClusterTestSuite.with_args(cluster_size=9, num_exclusive_coordinators=1) > is there a simpler cluster setup that re-produces this issue? e.g. with a s I tried with a smaller cluster size but couldn't quite reproduce it reliably. http://gerrit.cloudera.org:8080/#/c/14101/2/tests/custom_cluster/test_exchange_eos.py@57 PS2, Line 57: results = client.fetch(query, handle) > nit: validate the fetch was successful and close the query handle? Done -- To view, visit http://gerrit.cloudera.org:8080/14101 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I10c805e9d63ed8af9f458bf71e8ef5ea9376b939 Gerrit-Change-Number: 14101 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Tue, 20 Aug 2019 21:51:06 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8845: Cancel receiver's streams on exchange node's EOS
Hello Sahil Takiar, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14101 to look at the new patch set (#3). Change subject: IMPALA-8845: Cancel receiver's streams on exchange node's EOS .. IMPALA-8845: Cancel receiver's streams on exchange node's EOS When an exchange node reaches its row count limit, the current code will not notify the sender fragments about it. Consequently, sender fragments may keep sending row batches to the exchange node but they won't be dequeued anymore. The sender fragments may end up blocking in the RPC indefinitely until either the query is cancelled or closed. This change fixes the problem above by cancelling the underlying receiver's streams of an exchange node once it reaches the row count limit. This will unblock all senders whose TransmitData() RPCs haven't been replied to yet. Any future row batches sent to this receiver will also be immediately replied to with a response indicating that this receiver is already closed so the sender will stop sending any more row batches to it. Change-Id: I10c805e9d63ed8af9f458bf71e8ef5ea9376b939 --- M be/src/exec/exchange-node.cc M be/src/exec/exchange-node.h M be/src/runtime/krpc-data-stream-mgr.cc M be/src/runtime/krpc-data-stream-recvr.cc M be/src/runtime/krpc-data-stream-recvr.h A tests/custom_cluster/test_exchange_eos.py 6 files changed, 95 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/14101/3 -- To view, visit http://gerrit.cloudera.org:8080/14101 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I10c805e9d63ed8af9f458bf71e8ef5ea9376b939 Gerrit-Change-Number: 14101 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4311/ : 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/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 3 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 20 Aug 2019 21:32:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14076 ) Change subject: IMPALA-8869: Fix handling of HTTP keep-alive when returning 401 .. Patch Set 2: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4822/ -- To view, visit http://gerrit.cloudera.org:8080/14076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa Gerrit-Change-Number: 14076 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 20 Aug 2019 21:31:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8818: Replace deque with spillable queue in BufferedPRS
Sahil Takiar has posted comments on this change. (
http://gerrit.cloudera.org:8080/14039 )
Change subject: IMPALA-8818: Replace deque with spillable queue in BufferedPRS
..
Patch Set 7:
(16 comments)
http://gerrit.cloudera.org:8080/#/c/14039/7/be/src/exec/exec-node.h
File be/src/exec/exec-node.h:
http://gerrit.cloudera.org:8080/#/c/14039/7/be/src/exec/exec-node.h@196
PS7, Line 196: const std::string label() const;
> I think the const is unnecessary for a return by value (if you're returning
Done
http://gerrit.cloudera.org:8080/#/c/14039/7/be/src/runtime/buffered-tuple-stream-test.cc
File be/src/runtime/buffered-tuple-stream-test.cc:
http://gerrit.cloudera.org:8080/#/c/14039/7/be/src/runtime/buffered-tuple-stream-test.cc@323
PS7, Line 323: "-1"
> nit: use the name of the test instead. Same below.
Done
http://gerrit.cloudera.org:8080/#/c/14039/7/be/src/runtime/buffered-tuple-stream.h
File be/src/runtime/buffered-tuple-stream.h:
http://gerrit.cloudera.org:8080/#/c/14039/7/be/src/runtime/buffered-tuple-stream.h@370
PS7, Line 370: int64_t BytesUnpinned() const {
> I'd probably just call it bytes_unpinned() cause it's a plain accessor (the
Done
http://gerrit.cloudera.org:8080/#/c/14039/7/be/src/runtime/spillable-row-batch-queue.h
File be/src/runtime/spillable-row-batch-queue.h:
http://gerrit.cloudera.org:8080/#/c/14039/7/be/src/runtime/spillable-row-batch-queue.h@71
PS7, Line 71: /// successfully added, returns an error Status if the queue is
full, has already been
> Why not just make it invalid to append to the queue when it's full? We coul
Done
http://gerrit.cloudera.org:8080/#/c/14039/7/be/src/runtime/spillable-row-batch-queue.h@79
PS7, Line 79: /// Returns and removes the RowBatch at the head of the queue.
Returns Status::OK() if
> Same thing - if it's not valid to call GetBatch() on an empty queue, just m
Done
http://gerrit.cloudera.org:8080/#/c/14039/7/be/src/runtime/spillable-row-batch-queue.h@100
PS7, Line 100: BufferedTupleStream* batch_queue_;
> Shouldn't this use unique_ptr ? May be I am missing something but as the co
Yeah, thanks for catching that, it was leaking. Made it a unique_ptr.
http://gerrit.cloudera.org:8080/#/c/14039/7/be/src/runtime/spillable-row-batch-queue.cc
File be/src/runtime/spillable-row-batch-queue.cc:
http://gerrit.cloudera.org:8080/#/c/14039/7/be/src/runtime/spillable-row-batch-queue.cc@53
PS7, Line 53: batch_queue_ = new
> Use unique_ptr
Done
http://gerrit.cloudera.org:8080/#/c/14039/7/be/src/runtime/spillable-row-batch-queue.cc@67
PS7, Line 67: if (UNLIKELY(closed_)) return Status("SpillableRowBatchQueue is
already closed.");
> If it's a bug to call this when it's closed, let's just make it a DCHECK.
Done
http://gerrit.cloudera.org:8080/#/c/14039/7/be/src/runtime/spillable-row-batch-queue.cc@112
PS7, Line 112: if (UNLIKELY(closed_)) return false;
> I think it would be a bug to call this when it's closed, so we could docume
Done
http://gerrit.cloudera.org:8080/#/c/14039/7/be/src/runtime/spillable-row-batch-queue.cc@130
PS7, Line 130: if (batch_queue_ != nullptr)
> Missing braces for multi-line if
Done
http://gerrit.cloudera.org:8080/#/c/14039/7/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/14039/7/be/src/service/query-options.cc@907
PS7, Line 907: query_options->max_pinned_result_spooling_memory
> nits: code seems more legible if we factor out these two options into local
Done
http://gerrit.cloudera.org:8080/#/c/14039/7/be/src/service/query-options.cc@921
PS7, Line 921: if (query_options->max_pinned_result_spooling_memory == 0
: && query_options->max_unpinned_result_spooling_memory != 0)
> May be I mis-read it but isn't this the same check as the one on line 907 ?
Yeah, not sure what I was thinking here. Removed the extra checks and
re-factored the Status messages a bit. Tests all still pass.
http://gerrit.cloudera.org:8080/#/c/14039/7/be/src/service/query-options.cc@928
PS7, Line 928: if (query_options->max_unpinned_result_spooling_memory != 0
: && query_options->max_unpinned_result_spooling_memory
: < query_options->max_pinned_result_spooling_memory)
> Same question: isn't this the same as the one at line 912 ?
Done
http://gerrit.cloudera.org:8080/#/c/14039/7/common/thrift/ImpalaService.thrift
File common/thrift/ImpalaService.thrift:
http://gerrit.cloudera.org:8080/#/c/14039/7/common/thrift/ImpalaService.thrift@415
PS7, Line 415: // The maximum amount of pinned memory used when spooling
query results. If this value
> It's probably better to avoid renumbering things in thrift files. I think i
Done
http://gerrit.cloudera.org:8080/#/c/14039/7/common/thrift/ImpalaService.thrift@419
PS7, Line 419: MAX_PINNED_RESULT_SPOOLING_MEMORY = 86
> Also, please specify the behavior when setting this to 0. Same for any valu
Done
http://ge
[Impala-ASF-CR] IMPALA-8818: Replace deque with spillable queue in BufferedPRS
Hello Michael Ho, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14039 to look at the new patch set (#8). Change subject: IMPALA-8818: Replace deque with spillable queue in BufferedPRS .. IMPALA-8818: Replace deque with spillable queue in BufferedPRS Replaces DequeRowBatchQueue with SpillableRowBatchQueue in BufferedPlanRootSink. A few changes to BufferedPlanRootSink were necessary for it to work with the spillable queue, however, all the synchronization logic is the same. SpillableRowBatchQueue is a wrapper around a BufferedTupleStream and a ReservationManager. It takes in a TBackendResourceProfile that specifies the max / min memory reservation the BufferedTupleStream can use to buffer rows. The 'max_unpinned_bytes' parameter limits the max number of bytes that can be unpinned in the BufferedTupleStream. The limit is a 'soft' limit because calls to AddBatch may push the amount of unpinned memory over the limit. The queue is non-blocking and not thread safe. It provides AddBatch and GetBatch methods. Calls to AddBatch spill if the BufferedTupleStream does not have enough reservation to fit the entire RowBatch. Adds two new query options: 'MAX_PINNED_RESULT_SPOOLING_MEMORY' and 'MAX_UNPINNED_RESULT_SPOOLING_MEMORY', which bound the amount of pinned and unpinned memory that a query can use for spooling, respectively. MAX_PINNED_RESULT_SPOOLING_MEMORY must be <= MAX_UNPINNED_RESULT_SPOOLING_MEMORY in order to allow all the pinned data in the BufferedTupleStream to be unpinned. This is enforced in a new method in QueryOptions called 'ValidateQueryOptions'. Planner Changes: PlanRootSink.java now computes a full ResourceProfile if result spooling is enabled. The min mem reservation is bounded by the size of the read and write pages used by the BufferedTupleStream. The max mem reservation is bounded by 'MAX_PINNED_RESULT_SPOOLING_MEMORY'. The mem estimate is computed by estimating the size of the result set using stats. BufferedTupleStream Re-Factoring: For the most part, using a BufferedTupleStream outside an ExecNode works properly. However, some changes were necessary: * The message for the MAX_ROW_SIZE error is ExecNode specific. In order to fix this, this patch introduces the concept of an ExecNode 'label' which is a more generic version of an ExecNode 'id'. * The definition of TBackendResourceProfile lived in PlanNodes.thrift, it was moved to its own file so it can be used by DataSinks.thrift. * Modified BufferedTupleStream so it internally tracks how many bytes are unpinned (necessary for 'MAX_UNPINNED_RESULT_SPOOLING_MEMORY'). Metrics: * Added a few of the metrics mentioned in IMPALA-8825 to BufferedPlanRootSink. Specifically, added timers to track how much time is spent waiting in the BufferedPlanRootSink 'Send' and 'GetNext' methods. * The BufferedTupleStream in the SpillableRowBatchQueue exposes several BufferPool metrics such as number of reserved and unpinned bytes. Bug Fixes: * Fixed a bug in BufferedPlanRootSink where the MemPool used by the expression evaluators was not being cleared incrementally. * Fixed a bug where the inactive timer was not being properly updated in BufferedPlanRootSink. * Fixed a bug where RowBatch memory was not freed if BufferedPlanRootSink::GetNext terminated early because it could not handle requests where num_results < BATCH_SIZE. Testing: * Added new tests to test_result_spooling.py. * Updated errors thrown in spilling-large-rows.test. * Ran exhaustive tests. Change-Id: I10f9e72374cdf9501c0e5e2c5b39c13688ae65a9 --- M be/generated-sources/gen-cpp/CMakeLists.txt M be/src/exec/analytic-eval-node.cc M be/src/exec/blocking-plan-root-sink.cc M be/src/exec/blocking-plan-root-sink.h M be/src/exec/buffered-plan-root-sink.cc M be/src/exec/buffered-plan-root-sink.h M be/src/exec/data-sink.cc M be/src/exec/exec-node.cc M be/src/exec/exec-node.h M be/src/exec/grouping-aggregator-partition.cc M be/src/exec/grouping-aggregator.cc M be/src/exec/partial-sort-node.cc M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/partitioned-hash-join-builder.h M be/src/exec/partitioned-hash-join-node.cc M be/src/exec/plan-root-sink.h M be/src/exec/sort-node.cc M be/src/runtime/CMakeLists.txt M be/src/runtime/buffered-tuple-stream-test.cc M be/src/runtime/buffered-tuple-stream.cc M be/src/runtime/buffered-tuple-stream.h D be/src/runtime/deque-row-batch-queue.cc D be/src/runtime/deque-row-batch-queue.h M be/src/runtime/sorter.cc M be/src/runtime/sorter.h A be/src/runtime/spillable-row-batch-queue.cc A be/src/runtime/spillable-row-batch-queue.h M be/src/service/impala-beeswax-server.cc M be/src/service/impala-server.cc M be/src/service/query-options-test.cc M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/CMakeLists.txt M common/thrift/DataSinks.thrift M common/thrift/ImpalaInternalService.thrift M co
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 3: Oops I didn't mean to push this out for review, still haven't addressed the comments. -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 3 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 20 Aug 2019 20:52:22 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/14106 )
Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz
..
Patch Set 3:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2818
PS3, Line 2818: // Table does not exist and hence the owner information
cannot be deduced. Register
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2819
PS3, Line 2819: // a privilege request on the db and table name to mask
the TableNotFound exceptions
line too long (92 > 90)
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
File
fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java:
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java@79
PS3, Line 79: public PrivilegeRequestBuilder onTable(String dbName, String
tableName, String ownerUser) {
line too long (93 > 90)
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/service/Frontend.java@786
PS3, Line 786: "Table {} not yet loaded, ignoring it in table
listing.", dbName + "." + tblName);
line too long (96 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 3
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Tue, 20 Aug 2019 20:50:51 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Hello Austin Nobis, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14106
to look at the new patch set (#3).
Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz
..
[WIP] IMPALA-8228: Ownership support for Ranger authz
Without this patch, explicit privileges are needed even
for owners of databases/tables to perform actions on them.
Example: 'user' is the owner of database 'foo'. To create
a table 't' under 'foo', 'user' needs to be granted a CREATE
privilege on 'foo'
That is unintuitive from a user POV since users expect owners
to have ALL privileges on the objects they own. This patch extends
that support to Impala's ranger authorization plugin.
Ranger natively supports the concept of ownership by letting the
callers pass the ownership context to RangerAccessResourceImpl.
This patch plumbs the owner information for the authorizables
(currently only supported for Tables / Databases) which is then
evaulated during authorization.
For the ownership based authorization to work, ranger-admin side
policy on {OWNER} user needs to be defined.
(TODO) Working on tests.
Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/authorization/Authorizable.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableColumn.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableDb.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
M
fe/src/main/java/org/apache/impala/authorization/DefaultAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaResourceBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/FeDb.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
25 files changed, 250 insertions(+), 85 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/14106/3
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 3
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8842 part 2: (Hive3) Use 'engine' field in HMS stat API
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14108 ) Change subject: IMPALA-8842 part 2: (Hive3) Use 'engine' field in HMS stat API .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4310/ : 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/14108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I28e383156c48c7038e8bc235a3f3e5cc58a4fc01 Gerrit-Change-Number: 14108 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 20 Aug 2019 20:32:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8842 part 2: (Hive3) Use 'engine' field in HMS stat API
Attila Jeges has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14108 Change subject: IMPALA-8842 part 2: (Hive3) Use 'engine' field in HMS stat API .. IMPALA-8842 part 2: (Hive3) Use 'engine' field in HMS stat API HIVE-22046 added 'engine' column to TAB_COL_STATS and PART_COL_STATS HMS tables. The new column is used to differentiate among column stats computed by different engines. The related HMS API calls were changed accordingly. This change is Step 4 in a series of steps to coordinate the introduction of HMS API changes to Hive3 and Impala. For more information see IMPALA-8842 part 1. Step 4 replaces *V2 calls with *. The *V2 names were introduced temporarily and will be removed from the HMS API in the near future. TESTING: E2E tests were added to make sure that column statistics are differentiated by engine for partitioned and non-partitioned tables. The tests are executed for transactional and non-transactional tables. Change-Id: I28e383156c48c7038e8bc235a3f3e5cc58a4fc01 --- M bin/impala-config.sh M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M tests/metadata/test_hms_integration.py 3 files changed, 109 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/14108/1 -- To view, visit http://gerrit.cloudera.org:8080/14108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I28e383156c48c7038e8bc235a3f3e5cc58a4fc01 Gerrit-Change-Number: 14108 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Jeges
[Impala-ASF-CR] IMPALA-8845: Cancel receiver's streams on exchange node's EOS
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14101 ) Change subject: IMPALA-8845: Cancel receiver's streams on exchange node's EOS .. Patch Set 2: (3 comments) mostly minor comments, in general LGTM http://gerrit.cloudera.org:8080/#/c/14101/2/be/src/exec/exchange-node.cc File be/src/exec/exchange-node.cc: http://gerrit.cloudera.org:8080/#/c/14101/2/be/src/exec/exchange-node.cc@162 PS2, Line 162: stream_recvr_->CancelStream(); does this mess up some of the logging done in KrpcDataStreamMgr::Cancel? it prints out "cancelling all streams for..." but it doesn't really cancel anything because the streams have already been cancelled http://gerrit.cloudera.org:8080/#/c/14101/2/tests/custom_cluster/test_exchange_eos.py File tests/custom_cluster/test_exchange_eos.py: http://gerrit.cloudera.org:8080/#/c/14101/2/tests/custom_cluster/test_exchange_eos.py@40 PS2, Line 40: @CustomClusterTestSuite.with_args(cluster_size=9, num_exclusive_coordinators=1) is there a simpler cluster setup that re-produces this issue? e.g. with a smaller cluster_size http://gerrit.cloudera.org:8080/#/c/14101/2/tests/custom_cluster/test_exchange_eos.py@57 PS2, Line 57: client.fetch(query, handle) nit: validate the fetch was successful and close the query handle? -- To view, visit http://gerrit.cloudera.org:8080/14101 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I10c805e9d63ed8af9f458bf71e8ef5ea9376b939 Gerrit-Change-Number: 14101 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Tue, 20 Aug 2019 19:04:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8803: Coordinator should release admitted memory per-backend
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/14104 ) Change subject: IMPALA-8803: Coordinator should release admitted memory per-backend .. Patch Set 1: (7 comments) http://gerrit.cloudera.org:8080/#/c/14104/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14104/1//COMMIT_MSG@24 PS1, Line 24: * Resources are released at most every 1 seconds, this prevents short at most once http://gerrit.cloudera.org:8080/#/c/14104/1/be/src/runtime/coordinator-backend-state.h File be/src/runtime/coordinator-backend-state.h: http://gerrit.cloudera.org:8080/#/c/14104/1/be/src/runtime/coordinator-backend-state.h@403 PS1, Line 403: UNRELEASED Will "IN_USE" be slightly clear ? http://gerrit.cloudera.org:8080/#/c/14104/1/be/src/runtime/coordinator-backend-state.h@481 PS1, Line 481: RELEASE_BACKEND_STATES_DELAY Please add _MS suffix http://gerrit.cloudera.org:8080/#/c/14104/1/be/src/runtime/coordinator-backend-state.h@505 PS1, Line 505: num_not_unreleased_ nits: double-negative may not be very comprehensible. http://gerrit.cloudera.org:8080/#/c/14104/1/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: http://gerrit.cloudera.org:8080/#/c/14104/1/be/src/runtime/coordinator-backend-state.cc@823 PS1, Line 823: num_pending_++ Coding convention for Impala recommends pre-increment http://gerrit.cloudera.org:8080/#/c/14104/1/be/src/runtime/coordinator-backend-state.cc@846 PS1, Line 846: last_backend || coordinator_last_backend It seems a bit ad-hoc to detect these cases here in this function. I wonder if it makes sense to have a new state in coordinator called "ALL_ROWS_PRODUCED / EOS" which is accessible if result spooling is enabled. Once reaching this state, the thread will call SetNonErrorTerminalState() which end up calling HandleExecStateTransition(). In this way, we can handle these cases in ReleaseAdmissionControlResources(), which seems more natural as it marks the end of all non-coordinator fragments. http://gerrit.cloudera.org:8080/#/c/14104/1/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/14104/1/be/src/scheduling/admission-controller.cc@370 PS1, Line 370: DCHECK(false) << strings::Substitute( : "Error: Cannot find host address $0 for query $1.", PrintThrift(host_addr), : PrintId(schedule.query_id())); What happens in non-debug builds ? Should we log here instead and add a continue statement ? We can keep the DCHECK(false) though. -- To view, visit http://gerrit.cloudera.org:8080/14104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88bb11e0ede7574568020e0277dd8ac8d2586dc9 Gerrit-Change-Number: 14104 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 20 Aug 2019 18:47:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8691: Query option to disable data cache
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14015 ) Change subject: IMPALA-8691: Query option to disable data cache .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14015 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I39122ac38435cedf94b2b39145863764d0b5b6c8 Gerrit-Change-Number: 14015 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 20 Aug 2019 18:00:44 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/14106 )
Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz
..
Patch Set 2:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2673
PS2, Line 2673: FeTable table = getTable(fqTableName.getDb(),
fqTableName.getTbl());
we seem to have lost the "checkNotNull' here? was that intentional?
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
File fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java:
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@138
PS2, Line 138: tableName_
this could be null in the case of non-table-specific statements, which seems
like it would fail
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@189
PS2, Line 189: dbName
this should be 'database_' right?
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java:
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java@35
PS2, Line 35: String ownerUser
mind adding @Nullable annotations here and below, if this is allowed to be null
to indicate no known owner?
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java:
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java@30
PS2, Line 30: private final String ownerUser_;
how about using @Nullable on this?
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java@37
PS2, Line 37: ownerUser_ = ownerUser;
would an empty owner string be valid? maybe we should be checking that it's not
empty?
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java@55
PS2, Line 55: public String getOwnerUser() { return ownerUser_; }
this is @Override right?
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
File
fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java:
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java@71
PS2, Line 71: onTable
mind giving this a more explicit name like 'onTableWithUnknownOwner' or
something? think that's better than just overloading, so it's clear that when
you have an owner you should use a different call. Or, get rid of this overload
and explicitly pass the null owner at call sites
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java@74
PS2, Line 74: // TableNotFound Analsis exceptions and instead mask that as
an
typo
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 2
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Tue, 20 Aug 2019 17:25:20 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14076 ) Change subject: IMPALA-8869: Fix handling of HTTP keep-alive when returning 401 .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4822/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa Gerrit-Change-Number: 14076 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 20 Aug 2019 17:24:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14076 ) Change subject: IMPALA-8869: Fix handling of HTTP keep-alive when returning 401 .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa Gerrit-Change-Number: 14076 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 20 Aug 2019 17:24:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8691: Query option to disable data cache
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14015 ) Change subject: IMPALA-8691: Query option to disable data cache .. Patch Set 2: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/4309/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/14015 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I39122ac38435cedf94b2b39145863764d0b5b6c8 Gerrit-Change-Number: 14015 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 20 Aug 2019 17:03:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8858: Add observability of idle executor groups
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/14103 ) Change subject: IMPALA-8858: Add observability of idle executor groups .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/14103/1/be/src/scheduling/cluster-membership-mgr.h File be/src/scheduling/cluster-membership-mgr.h: http://gerrit.cloudera.org:8080/#/c/14103/1/be/src/scheduling/cluster-membership-mgr.h@43 PS1, Line 43: /// This struct stores per-host statistics which are used during admission, by the cluster I understand why this is here but I think semantically it belongs in the admission controller file. Similarly, the concept of an executor group being "idle" should move there. Have you tried adding it there and using a callback to trigger an update when the cluster membership changes? -- To view, visit http://gerrit.cloudera.org:8080/14103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I58cde8699c33af8b87273437e9d8bf6371a34539 Gerrit-Change-Number: 14103 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Tue, 20 Aug 2019 16:57:56 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8691: Query option to disable data cache
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/14015 ) Change subject: IMPALA-8691: Query option to disable data cache .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/14015/1/be/src/runtime/io/request-ranges.h File be/src/runtime/io/request-ranges.h: http://gerrit.cloudera.org:8080/#/c/14015/1/be/src/runtime/io/request-ranges.h@180 PS1, Line 180: BufferOpts(bool try_hdfs_cache, bool try_data_cache) > Yeah, it would minimise the chance of transposing or passing in the wrong a Done http://gerrit.cloudera.org:8080/#/c/14015/1/tests/custom_cluster/test_data_cache.py File tests/custom_cluster/test_data_cache.py: http://gerrit.cloudera.org:8080/#/c/14015/1/tests/custom_cluster/test_data_cache.py@79 PS1, Line 79: def test_data_cache_disablement(self, vector): > Can we move one or both of the other tests to exhaustive to make up for the Done -- To view, visit http://gerrit.cloudera.org:8080/14015 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I39122ac38435cedf94b2b39145863764d0b5b6c8 Gerrit-Change-Number: 14015 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 20 Aug 2019 16:24:08 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8691: Query option to disable data cache
Michael Ho has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/14015 ) Change subject: IMPALA-8691: Query option to disable data cache .. IMPALA-8691: Query option to disable data cache This change adds a query option to disable the data cache for a given session. By default, this option is set to false. When it's set to true, all queries will by-pass the data cache. This allows users to avoid polluting the cache for accesses to tables which they don't want to cache. A follow-up change will add a per-table query hint to allow caching disabled for a given table only. There is some small refactoring in the code to make it clearer the type of caching being referred to in the code. As the code stands now, we have both HDFS caching (for local reads) and the data cache (for remote reads). BufferOpts has been extended to allow users to explicitly state intention for using either/both of the caches. Change-Id: I39122ac38435cedf94b2b39145863764d0b5b6c8 --- M be/src/exec/base-sequence-scanner.cc M be/src/exec/hdfs-orc-scanner.cc M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/hdfs-scan-node-base.h M be/src/exec/hdfs-scanner.cc M be/src/exec/hdfs-text-scanner.cc M be/src/exec/parquet/hdfs-parquet-scanner.cc M be/src/exec/parquet/parquet-column-readers.cc M be/src/exec/parquet/parquet-page-index.cc M be/src/exec/scan-node.cc M be/src/exec/scan-node.h M be/src/exec/scanner-context.cc M be/src/runtime/io/disk-io-mgr-test.cc M be/src/runtime/io/hdfs-file-reader.cc M be/src/runtime/io/request-context.cc M be/src/runtime/io/request-ranges.h M be/src/runtime/io/scan-range.cc M be/src/runtime/tmp-file-mgr.cc M be/src/scheduling/scheduler-test-util.cc M be/src/scheduling/scheduler.cc M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift M tests/custom_cluster/test_data_cache.py 25 files changed, 171 insertions(+), 80 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/14015/2 -- To view, visit http://gerrit.cloudera.org:8080/14015 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I39122ac38435cedf94b2b39145863764d0b5b6c8 Gerrit-Change-Number: 14015 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7935: Disable /catalog object in local catalog mode.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12443 ) Change subject: IMPALA-7935: Disable /catalog_object in local catalog mode. .. IMPALA-7935: Disable /catalog_object in local catalog mode. getTCatalogObject() is not supported in local catalog mode since metadata is partially fetched on demand. Removed hyperlinks to the /catalog_object endpoints when local_catalog_mode is enabled. Testing: Added a test to test_local_catalog::TestObservability to verify /catalog_mode endpoint is disabled when in local catalog mode. Change-Id: Ia04797b32964c2edaa2e860dcf510d6f9cccd81c Reviewed-on: http://gerrit.cloudera.org:8080/12443 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/service/impala-http-handler.cc M tests/custom_cluster/test_local_catalog.py M tests/webserver/test_web_pages.py M www/catalog.tmpl 4 files changed, 78 insertions(+), 20 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/12443 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ia04797b32964c2edaa2e860dcf510d6f9cccd81c Gerrit-Change-Number: 12443 Gerrit-PatchSet: 13 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7935: Disable /catalog object in local catalog mode.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12443 ) Change subject: IMPALA-7935: Disable /catalog_object in local catalog mode. .. Patch Set 12: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12443 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia04797b32964c2edaa2e860dcf510d6f9cccd81c Gerrit-Change-Number: 12443 Gerrit-PatchSet: 12 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 20 Aug 2019 09:35:49 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4308/ : 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/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 2 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 20 Aug 2019 06:59:48 + Gerrit-HasComments: No
