Fredy Wijaya has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/13095 )
Change subject: IMPALA-8444: Fix performance regression when building privilege name ...................................................................... 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 insensitive bug Change-Id: I942d9b55f07c8972f69e532567d9b7d80fceb6e5 --- M fe/src/main/java/org/apache/impala/authorization/AuthorizationPolicy.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationPolicy.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java M fe/src/main/java/org/apache/impala/catalog/Catalog.java M fe/src/main/java/org/apache/impala/catalog/CatalogObjectCache.java M fe/src/main/java/org/apache/impala/catalog/Principal.java M fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java M tests/authorization/test_grant_revoke.py 8 files changed, 141 insertions(+), 126 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/13095/9 -- To view, visit http://gerrit.cloudera.org:8080/13095 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I942d9b55f07c8972f69e532567d9b7d80fceb6e5 Gerrit-Change-Number: 13095 Gerrit-PatchSet: 9 Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
