[Impala-ASF-CR] IMPALA-8444: Fix performance regression when building privilege name

["Fredy Wijaya (Code Review)" <ge...@cloudera.org>]

Fredy Wijaya has uploaded a new patch set (#2). ( 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.PrivilegeNameBenchmark.fast":
  ≈ 10⁻³ ms/op

Result "org.apache.impala.PrivilegeNameBenchmark.slow":
  0.001 ±(99.9%) 0.001 ms/op [Average]
  (min, avg, max) = (0.001, 0.001, 0.001), stdev = 0.001
  CI (99.9%): [0.001, 0.001] (assumes normal distribution)

Benchmark                    Mode  Cnt   Score    Error  Units
PrivilegeNameBenchmark.fast  avgt   25  ≈ 10⁻³           ms/op
PrivilegeNameBenchmark.slow  avgt   25   0.001 ±  0.001  ms/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 the synchronized keyword in
SentryAuthorizationPolicy.listPrivileges() in order to not cause the
operation to run in serial in a highly concurrent workload. This has a
trade off of stale authorization metadata, which may be acceptable to
get a better performance.

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/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
6 files changed, 121 insertions(+), 78 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/13095/2
-- 
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: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>