[
https://issues.apache.org/jira/browse/HADOOP-20045?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18135467#comment-18135467
]
ASF GitHub Bot commented on HADOOP-20045:
-----------------------------------------
hadoop-yetus commented on PR #8888:
URL: https://github.com/apache/hadoop/pull/8888#issuecomment-6535461333
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 21s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files
found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available.
|
| +0 :ok: | markdownlint | 0m 0s | | markdownlint was not available.
|
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain
any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to
include 5 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 2m 9s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 28m 26s | | trunk passed |
| +1 :green_heart: | compile | 9m 30s | | trunk passed with JDK
Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu |
| +1 :green_heart: | compile | 9m 31s | | trunk passed with JDK
Ubuntu-17.0.20.1+1-1-24.04-Ubuntu |
| +1 :green_heart: | checkstyle | 3m 14s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 48s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 13s | | trunk passed with JDK
Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu |
| +1 :green_heart: | javadoc | 2m 7s | | trunk passed with JDK
Ubuntu-17.0.20.1+1-1-24.04-Ubuntu |
| +1 :green_heart: | spotbugs | 3m 55s | | trunk passed |
| +1 :green_heart: | shadedclient | 17m 33s | | branch has no errors
when building and testing our client artifacts. |
| -0 :warning: | patch | 17m 58s | | Used diff version of patch file.
Binary files and potentially other changes not applied. Please rebase and
squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 17s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 39s | | the patch passed |
| +1 :green_heart: | compile | 9m 3s | | the patch passed with JDK
Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu |
| +1 :green_heart: | javac | 9m 3s | | the patch passed |
| +1 :green_heart: | compile | 9m 46s | | the patch passed with JDK
Ubuntu-17.0.20.1+1-1-24.04-Ubuntu |
| +1 :green_heart: | javac | 9m 46s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks
issues. |
| +1 :green_heart: | checkstyle | 3m 9s | | the patch passed |
| +1 :green_heart: | mvnsite | 2m 49s | | the patch passed |
| +1 :green_heart: | pylint | 0m 2s | | No new issues. |
| +1 :green_heart: | javadoc | 2m 10s | | the patch passed with JDK
Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu |
| +1 :green_heart: | javadoc | 2m 15s | | the patch passed with JDK
Ubuntu-17.0.20.1+1-1-24.04-Ubuntu |
| +1 :green_heart: | spotbugs | 4m 24s | | the patch passed |
| +1 :green_heart: | shadedclient | 18m 0s | | patch has no errors
when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 36s | | hadoop-common in the patch
passed. |
| +1 :green_heart: | unit | 3m 4s | | hadoop-kms in the patch passed.
|
| +1 :green_heart: | unit | 43m 29s | | hadoop-hdfs-rbf in the patch
passed. |
| +1 :green_heart: | asflicense | 0m 49s | | The patch does not
generate ASF License warnings. |
| | | 206m 34s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.56 ServerAPI=1.56 base:
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8888/2/artifact/out/Dockerfile
|
| GITHUB PR | https://github.com/apache/hadoop/pull/8888 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets pylint
markdownlint |
| uname | Linux 643e016fd14a 5.15.0-190-generic #200-Ubuntu SMP Fri Aug 7
15:06:04 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / fde05571541c7d11350da81329ce6951b787e662 |
| Default Java | Ubuntu-17.0.20.1+1-1-24.04-Ubuntu |
| Multi-JDK versions |
/usr/lib/jvm/java-21-openjdk-amd64:Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu
/usr/lib/jvm/java-17-openjdk-amd64:Ubuntu-17.0.20.1+1-1-24.04-Ubuntu |
| Test Results |
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8888/2/testReport/ |
| Max. process+thread count | 3373 (vs. ulimit of 10000) |
| modules | C: hadoop-common-project/hadoop-common
hadoop-common-project/hadoop-kms hadoop-hdfs-project/hadoop-hdfs-rbf U: . |
| Console output |
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8888/2/console |
| versions | git=2.43.0 maven=3.9.15 spotbugs=4.9.7 pylint=3.0.3 |
| Powered by | Apache Yetus 0.14.1 https://yetus.apache.org |
This message was automatically generated.
> Avoid global write-lock contention during ZooKeeper delegation token creation
> and add a KMS HTTP benchmark
> ----------------------------------------------------------------------------------------------------------
>
> Key: HADOOP-20045
> URL: https://issues.apache.org/jira/browse/HADOOP-20045
> Project: Hadoop Common
> Issue Type: Improvement
> Components: common, kms
> Reporter: Wei-Chiu Chuang
> Assignee: Wei-Chiu Chuang
> Priority: Major
> Labels: pull-request-available
> Attachments: kms-http-latency.png, kms-http-throughput.png,
> zk-token-creation-lock-flow.png
>
>
> h3. Problem
> KMS requests authenticated with delegation tokens can stall behind a request
> issuing a new token.
> HADOOP-18851 replaced synchronization in AbstractDelegationTokenSecretManager
> with a read-write lock, allowing concurrent verification. However,
> createPassword() still holds the global write lock while allocating a
> sequence-number range and persisting the token in ZooKeeper. A slow ZooKeeper
> request therefore blocks unrelated requests that need the read lock to verify
> cached tokens.
> A thread dump from an affected KMS instance shows the lock holder waiting in:
> {noformat}
> AbstractDelegationTokenSecretManager.createPassword()
> -> ZKDelegationTokenSecretManager.storeToken()
> -> ZKDelegationTokenSecretManager.addOrUpdateToken()
> -> ZooKeeper.create()
> -> ClientCnxn.submitRequest()
> -> Object.wait()
> {noformat}
> KMS uses DelegationTokenManager.ZKSecretManager, which extends the common
> ZKDelegationTokenSecretManager. The contention fix therefore belongs in the
> common secret-manager implementation; the Router implementation is an
> additional consumer.
> h3. Proposed change
> Separate the ZooKeeper creation path into three stages:
> # Allocate a sequence number under a dedicated allocator lock, reserving a
> new range when necessary.
> # Initialize the identifier and compute its password under a short global
> write-lock section.
> # Persist the prepared token synchronously outside the global lock, returning
> success only after persistence completes.
> Preserve existing storage-hook locking for non-ZooKeeper implementations. An
> interrupted sequence-range refill must abort issuance rather than fall
> through and consume an unreserved number.
> The HDFS Router ZooKeeper subclass also needs an adjustment: creation must
> not insert a speculative cache entry before persistence or unconditionally
> insert one afterward. Existing watcher, cache-miss, and rebuild paths
> populate newly created tokens. This avoids a creator restoring stale local
> state after a concurrent cancellation.
> !zk-token-creation-lock-flow.png|width=1000!
> This change addresses creation-induced contention. Renewal, cancellation, key
> rotation, and existing distributed-cache ordering races remain separate work.
> It does not claim immediate cross-manager revocation or complete distributed
> consistency.
> h3. Correctness validation
> The change adds tests covering:
> * Verification of an existing cached token while creation is paused during
> allocation or persistence.
> * Unique sequence numbers during concurrent issuance across two managers.
> * Persistence failure without speculative cache publication.
> * Interrupted sequence-range refill.
> * Key rotation between token preparation and persistence.
> * Router creation and immediate verification with watchers disabled.
> The two blocking regression tests time out against the original code and pass
> with the fix. The selected common and Router validation comprised 43 passing
> tests. The HTTP benchmark's scheduler and latency-accounting tests and the
> existing KMS authentication-filter test also passed.
> h3. End-to-end KMS HTTP benchmark
> Add an opt-in benchmark in hadoop-kms and a comparison runner using:
> * One KMS server JVM, one standalone ZooKeeper JVM, and a separate
> load-generator JVM.
> * 32 concurrent HTTP requests; no concurrency sweep.
> * Sequence-number batch size 100, unchanged between versions.
> * No injected latency or sleep in the request/storage path.
> * Verify:create request mixes of 100:0, 99:1, 95:5, and 90:10.
> * Five seconds of warmup and fifteen seconds of measurement per case, with
> three repetitions.
> * Fresh server processes/data per case and alternating original/fixed pair
> order across repetitions.
> Verification traffic consists of metadata GET requests authenticated using
> existing delegation tokens. Creation traffic uses GETDELEGATIONTOKEN with
> simple primary authentication. Cookie reuse, redirects, and automatic retries
> are disabled to preserve the intended authentication paths and request mix.
> Before measuring, the harness checks real delegation-token authentication,
> rejection of an invalid token and an unauthenticated request, and the actual
> ZKSecretManager implementation. Diagnostic counters are disabled before load.
> Loaded secret-manager class fingerprints are checked against the selected
> original/fixed common jars in every case.
> Submitted mixes are enforced in shuffled blocks of 100. Block-tail
> submissions and drained completions are recorded separately; post-cutoff
> completions do not inflate measurement-window goodput. Token-node growth must
> equal the number of completed create operations. The harness records
> throughput, errors, per-operation latency, JVM CPU/GC snapshots, and
> ZooKeeper statistics, including real fsync activity.
> h3. Results
> Baseline: Apache trunk commit {{a2d306c65a84bdf11a6eb5251cfff1bfe110546e}}.
> Both versions use the same KMS/client benchmark and dependencies, differing
> in the selected common secret-manager implementation. JDK 17.0.15 was used.
> Median successful total HTTP requests per second across three repetitions:
> || Verify:create || Original requests/s || Fixed requests/s || Fixed/original
> ||
> | 100:0 | 30,811 | 30,150 | 0.98x |
> | 99:1 | 17,672 | 26,885 | 1.52x |
> | 95:5 | 5,349 | 23,949 | 4.48x |
> | 90:10 | 3,103 | 23,174 | 7.47x |
> !kms-http-throughput.png|width=1000!
> All 24 cases completed with zero HTTP or response-validation errors and
> passed loaded-artifact, submitted-ratio, and token-count checks. Whiskers in
> the charts show observed min-max ranges, not confidence intervals.
> Median per-run p99 latency in milliseconds:
> || Verify:create || Original verify p99 || Fixed verify p99 || Original
> create p99 || Fixed create p99 ||
> | 100:0 | 4.14 | 4.48 | N/A | N/A |
> | 99:1 | 5.31 | 4.18 | 7.08 | 18.97 |
> | 95:5 | 14.22 | 4.31 | 17.18 | 21.38 |
> | 90:10 | 21.81 | 3.39 | 22.47 | 19.35 |
> !kms-http-latency.png|width=1100!
> The 100:0 control showed variability, including one lower fixed-build
> observation; its median was 2.1 percent lower with the fix. Three short
> repetitions do not establish statistical significance for small differences.
> Creation p99 increased at 99:1 and 95:5 under the higher achieved load. These
> are closed-loop capacity comparisons, not matched-arrival-rate latency
> comparisons, and the results do not establish uniformly improved creation
> latency.
> The workload measures authenticated metadata requests with simple
> authentication for issuance and local standalone ZooKeeper. It does not
> establish production throughput for Kerberos, TLS, encryption operations, or
> a ZooKeeper quorum. No performance threshold is imposed in normal CI.
> h3. Reproduction
> The HTTP benchmark is opt-in; ordinary test runs execute only the lightweight
> scheduler and latency-accounting checks. Defaults match the matrix above.
> {noformat}
> mvn -pl hadoop-common-project/hadoop-kms \
> -Dtest=TestKMSHttpBenchmark -Dkms.benchmark=true \
> -Dmaven.test.failure.ignore=false test
> {noformat}
> For controlled comparison, build original and fixed hadoop-common jars with
> otherwise identical source/dependency versions, then run:
> {noformat}
> python3 dev-support/benchmarks/kms-http-benchmark.py \
> --baseline-common /path/to/original/hadoop-common.jar \
> --fixed-common /path/to/fixed/hadoop-common.jar \
> --output /path/to/fresh/results-directory
> {noformat}
> The runner saves incremental comparison JSON and per-case commands, logs,
> configuration, and metrics. Maven 3.9.15 or newer and JDK 17 are required for
> this trunk checkout. No native profile is required.
> Related: [HADOOP-18851|https://issues.apache.org/jira/browse/HADOOP-18851].
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]