You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tamas Mate (Code Review)" <ge...@cloudera.org> on 2019/05/16 16:37:45 UTC
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Tamas Mate has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13353
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
IMPALA-8443: Record time spent in authorization in the runtime profile
The analysis and authorization is handled together as authorization
depends on the results of the analysis. I have moved the events inside
the analyzeAndAuthorize method and split them, this will help locating
which part of the analysis was slow.
Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
---
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
2 files changed, 2 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/13353/2
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Austin Nobis (Code Review)" <ge...@cloudera.org>.
Austin Nobis has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13353/2/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java:
http://gerrit.cloudera.org:8080/#/c/13353/2/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@426
PS2, Line 426: (Ranger|Sentry)
If we expose access to the AuthorizationConfig stored in the AuthorizationChecker, it would be possible to determine which of Ranger/Sentry/NOOP authorization is being used.
Would this be useful for profiling?
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 May 2019 14:32:48 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13353/2/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java:
http://gerrit.cloudera.org:8080/#/c/13353/2/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@426
PS2, Line 426: (Ranger|Sentry)
> Thanks guys for looking into it.
We can put the timeline in the AuthorizationContext and it will be available in the postAuthorize(). It looks something like this.
@Override
public void postAuthorize(AuthorizationContext authzCtx) {
long durationMs = authzCtx.getTimeline().markEvent("Authorization finished");
LOG.debug("Authorization check took {} ms", durationMs);
}
Yeah, I think it's best to wait till this CR is merged. Hopefully I can get a +2 soon :)
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 22 May 2019 15:33:59 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/3653/ : 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/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 8
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 07:05:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 10: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4500/
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 10
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Jun 2019 23:59:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 9: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4490/
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 9
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 21:51:44 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 3:
Hi Fredy, thanks for keeping an eye on this.
Yes, I am working on it, I just got interrupted in the past couple of weeks.
I was moving the timeline into the AuthorizationContext here:
https://github.com/apache/impala/blob/ab908d54c22861967f693428ec7d9f6d7008607f/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java#L424
However, in some cases a new AuthorizationContext is created during AuthorizationChecker 'hasAccess' call.
https://github.com/apache/impala/blob/ab908d54c22861967f693428ec7d9f6d7008607f/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java#L66
I am working on to untangle this part at the moment, will get back to you with a new patch by EOB Monday.
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Jun 2019 09:51:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13353/2/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java:
http://gerrit.cloudera.org:8080/#/c/13353/2/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@426
PS2, Line 426: (Ranger|Sentry)
> My CR is merged: https://gerrit.cloudera.org/c/13309/ You can rebase your C
I too think it helps to clarify the authz provider in use. Infact that is what I meant in the jira. Something like
Authorization finished (Ranger)
Authorization finished (Sentry)
Sorry, should've made it clear.
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Jun 2019 16:36:07 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 10: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 10
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Jun 2019 17:08:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 11:
Yes, I have seen that, just in case I have started a dry run :).
Thanks for re-running the merge.
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 11
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Jun 2019 17:15:50 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 11: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 11
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Jun 2019 22:42:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 7:
(2 comments)
You need to rebase again to resolve the merge conflict.
http://gerrit.cloudera.org:8080/#/c/13353/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java:
http://gerrit.cloudera.org:8080/#/c/13353/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@232
PS7, Line 232:
nit: overly indented, use 4 spaces for continued indentation.
http://gerrit.cloudera.org:8080/#/c/13353/7/fe/src/main/java/org/apache/impala/util/EventSequence.java
File fe/src/main/java/org/apache/impala/util/EventSequence.java:
http://gerrit.cloudera.org:8080/#/c/13353/7/fe/src/main/java/org/apache/impala/util/EventSequence.java@44
PS7, Line 44: duration
nit: duration in nano seconds.
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 7
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 17 Jun 2019 19:59:31 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 8: Code-Review+2
Thanks for the patch!
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 8
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 16:30:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 11: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 11
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Jun 2019 17:14:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13353/2/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java:
http://gerrit.cloudera.org:8080/#/c/13353/2/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@426
PS2, Line 426: (Ranger|Sentry)
> If we expose access to the AuthorizationConfig stored in the AuthorizationC
Adding the current provider sounds good to me, it would be one less thing to check during troubleshooting and would look better.
Another solution I was thinking could be to move the analyzeAndAuthorize to Frontend and get the AuthorizationConfig from the AuthorizationFactory. This would clean the AnalysisContext from the authorization part, however the Frontend would grow slightly bigger again.
What do you think?
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 May 2019 16:47:59 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 10:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4500/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 10
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Jun 2019 18:28:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 9: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 9
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 16:30:17 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 2:
(2 comments)
Thanks for the patience! Can you also add a simple test for this?
http://gerrit.cloudera.org:8080/#/c/13353/2/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java:
http://gerrit.cloudera.org:8080/#/c/13353/2/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@416
PS2, Line 416: timeline_.markEvent("Analysis finished");
This will end up with a duplicate entry in: https://github.com/apache/impala/blob/9ee4a5e1940afa47227a92e0f6fba6d4c9909f63/fe/src/main/java/org/apache/impala/service/Frontend.java#L1239.
We can update the one in Frontend to mean analysis + authorization or just remove it.
http://gerrit.cloudera.org:8080/#/c/13353/2/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@426
PS2, Line 426: (Ranger|Sentry)
> We can put the timeline in the AuthorizationContext and it will be availabl
My CR is merged: https://gerrit.cloudera.org/c/13309/ You can rebase your CR.
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Fri, 24 May 2019 15:49:30 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
IMPALA-8443: Record time spent in authorization in the runtime profile
The analysis and authorization is handled together as authorization
depends on the results of the analysis. The timeline EventSequence is
moved into the AuthorizationContext and the markEvent is called during
the postAuthorize method.
In some cases the EventSequence is not available when the
AuthorizationContext is created therefore it is wrapped in Optional.
Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
---
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationContext.java
M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java
M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/EventSequence.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M tests/observability/test_log_fragments.py
M tests/query_test/test_observability.py
13 files changed, 85 insertions(+), 26 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/13353/7
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 7
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 9:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4490/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 9
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 16:30:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 10:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4508/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 10
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Jun 2019 11:42:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
IMPALA-8443: Record time spent in authorization in the runtime profile
The analysis and authorization is handled together as authorization
depends on the results of the analysis. The timeline EventSequence is
moved into the AuthorizationContext and the markEvent is called during
the postAuthorize method.
In some cases the EventSequence is not available when the
AuthorizationContext is created therefore it is wrapped in Optional.
Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
---
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationContext.java
M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java
M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/EventSequence.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M tests/observability/test_log_fragments.py
M tests/query_test/test_observability.py
13 files changed, 81 insertions(+), 26 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/13353/8
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 8
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 10:
> Patch Set 10: Verified-1
>
> Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4500/
This looks like some flaky Maven issue. I'm re-running the merge.
23:59:12 ] [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.3:compile (default-compile) on project query-event-hook-api: Execution default-compile of goal org.apache.maven.plugins:maven-compiler-plugin:3.3:compile failed: Plugin org.apache.maven.plugins:maven-compiler-plugin:3.3 or one of its dependencies could not be resolved: Failed to collect dependencies at org.apache.maven.plugins:maven-compiler-plugin:jar:3.3 -> org.codehaus.plexus:plexus-container-default:jar:1.5.5 -> org.apache.xbean:xbean-reflect:jar:3.4: Failed to read artifact descriptor for org.apache.xbean:xbean-reflect:jar:3.4: Could not transfer artifact org.apache.xbean:xbean-reflect:pom:3.4 from/to central (https://repo.maven.apache.org/maven2): /home/ubuntu/.m2/repository/org/apache/xbean/xbean-reflect/3.4/xbean-reflect-3.4.pom.part (No such file or directory) -> [Help 1]
23:59:12 ] [ERROR]
23:59:12 ] [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
23:59:12 ] [ERROR] Re-run Maven using the -X switch to enable full debug logging.
23:59:12 ] [ERROR]
23:59:12 ] [ERROR] For more information about the errors and possible solutions, please read the following articles:
23:59:12 ] [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/PluginResolutionException
23:59:12 ] mvn -B install -DskipTests exited with code 0
23:59:12 ] query-event-hook-api/CMakeFiles/query-event-hook-api.dir/build.make:57: recipe for target 'query-event-hook-api/CMakeFiles/query-event-hook-api' failed
23:59:12 ] make[3]: *** [query-event-hook-api/CMakeFiles/query-event-hook-api] Error 1
23:59:12 ] CMakeFiles/Makefile2:14640: recipe for target 'query-event-hook-api/CMakeFiles/query-event-hook-api.dir/all' failed
23:59:12 ] make[2]: *** [query-event-hook-api/CMakeFiles/query-event-hook-api.dir/all] Error 2
23:59:12 ] make[2]: *** Waiting for unfinished jobs....
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 10
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Jun 2019 17:13:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 10: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 10
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Jun 2019 18:28:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/3643/ : 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/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 7
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 17 Jun 2019 19:59:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/3249/ : 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/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 May 2019 17:24:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 3:
> Patch Set 3: Patch Set 2 was rebased
Hi Tamas, I saw that you just rebased the patch. Are you going to make a change to make sure the authorization time logged here: https://github.com/apache/impala/blob/ab908d54c22861967f693428ec7d9f6d7008607f/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java#L89 is the same as the one in timeline.markEvent("Authorization finished")?
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Jun 2019 01:23:36 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13353/2/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java:
http://gerrit.cloudera.org:8080/#/c/13353/2/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@426
PS2, Line 426: (Ranger|Sentry)
> Yeah in my CR: https://gerrit.cloudera.org/c/13309/6/fe/src/main/java/org/a
Thanks guys for looking into it.
Austin, a new class would be ideal, but I could not find anything else to refactor with the analyzeAndAuthorize and it seems it would have too many boilerplate parts alone. The best option as you mentioned exposing the AuthorizationConfig.
Fredy, I assume we should pass the timeline to the AuthorizationChecker and it could be called next to the logging. The EventSequence should be modified that when it marks an event it returns the timestamp as well. So, I should wait for https://gerrit.cloudera.org/#/c/13309/ to be submitted for now.
Is my understanding correct?
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 22 May 2019 09:17:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
IMPALA-8443: Record time spent in authorization in the runtime profile
The analysis and authorization is handled together as authorization
depends on the results of the analysis. The timeline EventSequence is
moved into the AuthorizationContext and the markEvent is called during
the postAuthorize method.
In some cases the EventSequence is not available when the
AuthorizationContext is created therefore it is wrapped in Optional.
Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Reviewed-on: http://gerrit.cloudera.org:8080/13353
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationContext.java
M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java
M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/EventSequence.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M tests/observability/test_log_fragments.py
M tests/query_test/test_observability.py
13 files changed, 84 insertions(+), 27 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 12
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Austin Nobis (Code Review)" <ge...@cloudera.org>.
Austin Nobis has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13353/2/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java:
http://gerrit.cloudera.org:8080/#/c/13353/2/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@426
PS2, Line 426: (Ranger|Sentry)
> Adding the current provider sounds good to me, it would be one less thing t
I think going with that approach will cause a lot of conflicts with Fredy's open CR https://gerrit.cloudera.org/c/13309/.
Would the approach be to move all of the Authorization related code from AnalysisContext into Frontend? I'm not opposed to moving the code out of the AnalysisContext but maybe we should create a new class as opposed to putting it all in the Frontend.
Thoughts? Would also be helpful if Fredy commented because he may have other plans for this code.
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 May 2019 18:29:04 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 11:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4511/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 11
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Jun 2019 17:14:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 3:
> Patch Set 3:
>
> Hi Fredy, thanks for keeping an eye on this.
>
> Yes, I am working on it, I just got interrupted in the past couple of weeks.
>
> I was moving the timeline into the AuthorizationContext here:
> https://github.com/apache/impala/blob/ab908d54c22861967f693428ec7d9f6d7008607f/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java#L424
>
> However, in some cases a new AuthorizationContext is created during AuthorizationChecker 'hasAccess' call.
> https://github.com/apache/impala/blob/ab908d54c22861967f693428ec7d9f6d7008607f/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java#L66
>
> I am working on to untangle this part at the moment, will get back to you with a new patch by EOB Monday.
No worries :)
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Jun 2019 16:30:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13353/2/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java:
http://gerrit.cloudera.org:8080/#/c/13353/2/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@426
PS2, Line 426: (Ranger|Sentry)
> I think going with that approach will cause a lot of conflicts with Fredy's
Yeah in my CR: https://gerrit.cloudera.org/c/13309/6/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java#98, I also log the authorization check time. We should try to consolidate this by using timeline.markEvent() and updating timeline.markEvent() to return the time so that we the times in logging vs profile are the same.
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 May 2019 18:31:46 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
IMPALA-8443: Record time spent in authorization in the runtime profile
The analysis and authorization is handled together as authorization
depends on the results of the analysis. The timeline EventSequence is
moved into the AuthorizationContext and the markEvent is called during
the postAuthorize method.
In some cases the EventSequence is not available when the
AuthorizationContext is created therefore it is wrapped in Optional.
Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
---
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationContext.java
M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java
M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/EventSequence.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M tests/observability/test_log_fragments.py
M tests/query_test/test_observability.py
13 files changed, 84 insertions(+), 27 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/13353/10
--
To view, visit http://gerrit.cloudera.org:8080/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 10
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-8443: Record time spent in authorization in the runtime profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13353 )
Change subject: IMPALA-8443: Record time spent in authorization in the runtime profile
......................................................................
Patch Set 10:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/3677/ : 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/13353
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb85e57fcc75d41f3eb2911e6d375e0da6f82ae
Gerrit-Change-Number: 13353
Gerrit-PatchSet: 10
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Jun 2019 12:22:19 +0000
Gerrit-HasComments: No