Fredy Wijaya has uploaded a new patch set (#8). ( 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 case insensitive way. This is true for all privilege scopes, except URI. This patch fixes the issue by storing URI privilege in a case sensitive manner. 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 URI privilege 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, 128 insertions(+), 126 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/13095/8 -- 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: 8 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>
