[ 
https://issues.apache.org/jira/browse/IMPALA-8444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16827374#comment-16827374
 ] 

ASF subversion and git services commented on IMPALA-8444:
---------------------------------------------------------

Commit 349142635250f92412003a1e62829fc4b441dc75 in impala's branch 
refs/heads/master from Fredy Wijaya
[ https://gitbox.apache.org/repos/asf?p=impala.git;h=3491426 ]

IMPALA-8444: Fix performance regression when building privilege name

This patch fixes the performance regression when building privilege name
by rewriting PrincipalPrivilege.buildPrivilegeName() with a simple
string concatentation instead of using a list that gets converted into a
string.

Below is the result of running a benchmark using JMH comparing the old
and new implementations:

Result "org.apache.impala.BuildPrivilegeNameBenchmark.fast":
  0.344 ±(99.9%) 0.004 us/op [Average]
  (min, avg, max) = (0.336, 0.344, 0.355), stdev = 0.005
  CI (99.9%): [0.339, 0.348] (assumes normal distribution)

Result "org.apache.impala.BuildPrivilegeNameBenchmark.slow":
  0.831 ±(99.9%) 0.011 us/op [Average]
  (min, avg, max) = (0.807, 0.831, 0.856), stdev = 0.015
  CI (99.9%): [0.820, 0.842] (assumes normal distribution)

Benchmark                         Mode  Cnt  Score   Error Units
BuildPrivilegeNameBenchmark.fast  avgt   25  0.344 ± 0.004 us/op
BuildPrivilegeNameBenchmark.slow  avgt   25  0.831 ± 0.011 us/op

This patch also updates SentryAuthorizationPolicy.listPrivileges() to
reuse the privilege names that have already been built instead of
building them again. While fixing this, I found a bug where Principal
stores the PrincipalPrivilege in a case insensitive way. This is true
for all privilege scopes, except URI. This patch fixes the issue by
making privilege name to be case sensitive instead.

This patch removes incorrect synchronization in
SentryAuthorizationPolicy.listPrivileges() that can cause the operation
to run in serial in a highly concurrent workload.

Testing:
- Ran all FE tests
- Ran all E2E authorization tests
- Added E2E test for privilege name case sensitivity bug

Change-Id: I942d9b55f07c8972f69e532567d9b7d80fceb6e5
Reviewed-on: http://gerrit.cloudera.org:8080/13095
Reviewed-by: Impala Public Jenkins <[email protected]>
Tested-by: Impala Public Jenkins <[email protected]>


> Analysis perf regression after IMPALA-7616
> ------------------------------------------
>
>                 Key: IMPALA-8444
>                 URL: https://issues.apache.org/jira/browse/IMPALA-8444
>             Project: IMPALA
>          Issue Type: Bug
>          Components: Frontend
>    Affects Versions: Impala 3.2.0
>            Reporter: bharath v
>            Assignee: Fredy Wijaya
>            Priority: Critical
>             Fix For: Impala 3.3.0
>
>
> The patch for IMPALA-7616 caused a performance regression in analysis time 
> when run in an environment with ~1k roles and 10.5k. privileges. The 
> regression is evident when run as a role that has a large number of 
> privileges.
> Following is the stack to look for when jstacking the coordnator.
> {noformat}
> "Thread-21" #49 prio=5 os_prio=0 tid=0x000000000cd6e000 nid=0x6a3d runnable 
> [0x00007fa28e4a0000]
>    java.lang.Thread.State: RUNNABLE
>         at java.lang.String.toLowerCase(String.java:2670)
>         at 
> org.apache.impala.catalog.PrincipalPrivilege.buildPrivilegeName(PrincipalPrivilege.java:82)
>         at 
> org.apache.impala.catalog.PrincipalPrivilege.getName(PrincipalPrivilege.java:143)
>         at 
> org.apache.impala.catalog.AuthorizationPolicy.listPrivileges(AuthorizationPolicy.java:423)
>         - locked <0x00007fa376987100> (a 
> org.apache.impala.catalog.AuthorizationPolicy)
>         at 
> org.apache.impala.catalog.AuthorizationPolicy.listPrivileges(AuthorizationPolicy.java:443)
>         - locked <0x00007fa376987100> (a 
> org.apache.impala.catalog.AuthorizationPolicy)
>         at 
> org.apache.sentry.provider.cache.SimpleCacheProviderBackend.getPrivileges(SimpleCacheProviderBackend.java:75)
>         at 
> org.apache.sentry.policy.db.SimpleDBPolicyEngine.getPrivileges(SimpleDBPolicyEngine.java:98)
>         at 
> org.apache.sentry.provider.common.ResourceAuthorizationProvider.getPrivileges(ResourceAuthorizationProvider.java:147)
>         at 
> org.apache.sentry.provider.common.ResourceAuthorizationProvider.doHasAccess(ResourceAuthorizationProvider.java:120)
>         at 
> org.apache.sentry.provider.common.ResourceAuthorizationProvider.hasAccess(ResourceAuthorizationProvider.java:107)
>         at 
> org.apache.impala.authorization.AuthorizationChecker.hasAccess(AuthorizationChecker.java:215)
>         at 
> org.apache.impala.authorization.AuthorizationChecker.checkAccess(AuthorizationChecker.java:128)
>         at 
> org.apache.impala.analysis.AnalysisContext.authorizePrivilegeRequest(AnalysisContext.java:592)
>         at 
> org.apache.impala.analysis.AnalysisContext.authorize(AnalysisContext.java:564)
>         at 
> org.apache.impala.analysis.AnalysisContext.analyzeAndAuthorize(AnalysisContext.java:415)
>         at 
> org.apache.impala.service.Frontend.doCreateExecRequest(Frontend.java:1240)
>         at 
> org.apache.impala.service.Frontend.getTExecRequest(Frontend.java:1210)
>         at 
> org.apache.impala.service.Frontend.createExecRequest(Frontend.java:1182)
>         at 
> org.apache.impala.service.JniFrontend.createExecRequest(JniFrontend.java:158)
> {noformat}
> Issue worsens when running concurrent workloads, because the underlying 
> Sentry {{listPrivileges()}} is synchronized and that serializes all the query 
> analysis requests.
> {noformat}
>   public synchronized Set<String> listPrivileges(Set<String> groups,  <====
>       ActiveRoleSet roleSet) {
>     Set<String> privileges = Sets.newHashSet();
>     if (roleSet != ActiveRoleSet.ALL) {
>       throw new UnsupportedOperationException("Impala does not support role 
> subsets.");
>     }
> {noformat}
> Notes:
> - If the authorization metadata footprint is small, this issue can be ignored.
> - One workaround is to run the query using a role that has very small number 
> of privileges (revoke privileges that are not necessary to run a given query).
> - Another workaround is to disable Sentry authorization.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to