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

Wei-Chiu Chuang commented on HADOOP-20046:
------------------------------------------

h3. Implementation and benchmark results

Draft PR: [apache/hadoop#8889|https://github.com/apache/hadoop/pull/8889], 
follow-up commit {{bdbcd97c790}}. This is stacked on HADOOP-20045 / PR #8888, 
which is still open. The trunk diff includes that parent; the PR description 
makes the dependency explicit.

The fix uses 256 local token lock stripes, ZooKeeper version checks for 
renewal/deletion, and a separate master-key rotation coordinator. It persists 
replacement keys and retirement bounds before publication, preserves 
non-ZooKeeper storage-hook locking, and removes Router's speculative renewal 
publication. Read-lock cache misses and immediate cross-manager revocation 
remain outside scope.

h4. HTTP renewal and cancellation checks

One KMS JVM, one standalone ZooKeeper JVM, 32 clients, sequence batch size 100, 
zero injected delay. Baseline already includes HADOOP-20045. Single run per 
variant at 95% verification / 5% selected mutation; 5 s warmup and 15 s 
measurement. Cancellation tokens were issued before warmup, with no replacement 
issuance during load.

|| Mutation || Parent requests/s || Fixed requests/s || Ratio || Parent verify 
p99 ms || Fixed verify p99 ms ||
| Renewal | 3,748 | 22,258 | 5.94x | 18.60 | 5.86 |
| Cancellation | 4,261 | 28,056 | 6.58x | 18.05 | 1.71 |

h4. Verification during maintenance (three repetitions per build)

24 accepted cases: four scenarios x two builds x three repetitions, alternating 
pair order. HTTP traffic is 100% verification while a background thread invokes 
the actual KMS manager's maintenance method. These are fixed stress batches, 
not production cadence. Key scenarios use 15-second windows; expired-token 
cleanup uses 60 seconds, following 5-second warmup.

Rates below count successful HTTP completions during the actual maintenance 
interval. The attached report also gives whole-window rates, complete latency 
statistics, observed ranges, validation, and rejected attempts.

|| Maintenance batch || Parent active verify/s || Fixed active verify/s || 
Parent duration s || Fixed duration s ||
| 500 new-key installations | 13,739 | 29,025 | 3.850 | 6.054 |
| 500 rotations (retiring + replacement key) | 11,471 | 26,208 | 6.062 | 6.144 |
| Delete 1,000 expired keys | 8 | 26,650 | 3.858 | 3.230 |
| Delete 5,000 expired tokens | 29,782 | 31,589 | 23.542 | 18.226 |

!HADOOP-20046-maintenance-comparison.png|width=900!

!HADOOP-20046-expired-key-stall.png|width=900!

Expired-key deletion gives the clearest stall removal: parent verification 
stopped for roughly 3.9 seconds; overlap p99 fell from 3891.30 to 10.98 ms. 
Tail latency did not uniformly improve. Median per-run maximum verification 
latency increased from 34.79 to 126.25 ms during installation and from 35.12 to 
179.47 ms during rotation; rotation p99 increased from 7.15 to 9.64 ms.

New-key installation in the fixed build also durably updates the retiring key. 
Expired-token persistence was already outside the parent's global write lock; 
that scenario measures changed cleanup coordination and conditional deletion. 
These closed-loop capacity experiments do not establish latency at matched 
external arrival rates or production quorum/TLS/Kerberos throughput. Three 
repetitions do not establish significance for small differences; cross-JVM 
interval alignment is approximately 1 ms.

h4. Validation

Fresh JDK 17 / Maven 3.9.15 validation with 
{{-Dmaven.test.failure.ignore=false}}: common 51 passing Java tests plus one 
opt-in skip; Router 16 passing; KMS four passing plus one opt-in skip. Total: 
71 passing, two skipped opt-in benchmarks, no Java failures/errors. 
Deterministic tests cover all five blocking paths and renewal/cancellation 
ordering, monotonic renewal, interrupted/failed persistence, and conditional 
cache publication. All five blocking cases previously reproduced verification 
timeouts against the parent.

All 24 accepted maintenance cases passed loaded-class fingerprints, 
authentication preflight, zero HTTP/validation errors, exact key/token node 
changes, and completion inside the measurement window. The initial too-short 
cleanup window and a later host timing interruption were rejected and rerun, as 
recorded in the report. The normal tests include interval accounting for 
requests stalled past maintenance completion.

Attached: design, full maintenance report, exported JSON, and both charts. 
Reproduction runners are included in the PR under {{dev-support/benchmarks/}}. 
The older design attachment's recorded verification section predates the latest 
maintenance runs; the counts and maintenance results in this comment supersede 
it.

Contains content generated by OpenAI Codex.

> Avoid holding the delegation-token secret manager API write lock during 
> ZooKeeper renewal, cancellation, and master-key operations
> ----------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HADOOP-20046
>                 URL: https://issues.apache.org/jira/browse/HADOOP-20046
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: common, kms
>            Reporter: Wei-Chiu Chuang
>            Priority: Major
>              Labels: pull-request-available
>         Attachments: HADOOP-20046-expired-key-stall.png, 
> HADOOP-20046-maintenance-benchmark.md, 
> HADOOP-20046-maintenance-comparison.png, 
> HADOOP-20046-maintenance-results.json, HADOOP-20046-write-locks.md
>
>
> h3. Problem
> HADOOP-18851 introduced a read-write lock in 
> AbstractDelegationTokenSecretManager to allow concurrent token verification. 
> HADOOP-20045 addresses token creation holding that lock during ZooKeeper 
> operations.
> Other paths still hold {{apiLock.writeLock()}} while performing synchronous 
> ZooKeeper operations:
> || Write-lock holder || ZooKeeper operations under the lock ||
> | renewToken() | Read signing key/token information on cache misses; check 
> existence and update/create token |
> | cancelToken() | Read token information, check existence, and delete token |
> | updateCurrentKey() | Check existence and persist the new master key |
> | rollMasterKey() | Check existence and persist the retiring key’s updated 
> expiry |
> | removeExpiredKeys() | Check existence and delete expired master keys, 
> potentially multiple keys per lock hold |
> ZooKeeper latency or retries in these paths can block unrelated requests that 
> need the API read lock to verify cached tokens. This affects KMS and other 
> consumers of the shared ZKDelegationTokenSecretManager, including HDFS Router.
> h3. Proposed improvement
> Move ZooKeeper operations outside the global API write-lock sections. Retain 
> short critical sections for inspecting or publishing local state, with 
> explicit coordination to preserve operation ordering.
> The design must ensure:
> * Renewal cannot restore a token after cancellation or overwrite a newer 
> renewal.
> * Cache publication and storage-failure handling remain consistent with the 
> operation’s outcome.
> * A new master key is persisted before it becomes available for signing 
> tokens.
> * Retiring-key updates and expired-key deletion remain correctly ordered with 
> rotation and issuance.
> * Existing storage-hook behavior for non-ZooKeeper implementations is 
> preserved.
> Per-token coordination, a separate key-rotation coordinator, and versioned 
> ZooKeeper operations are potential mechanisms to evaluate. Simply releasing 
> the global lock around existing storage calls is insufficient.
> h3. Validation
> Add deterministic tests that pause each relevant ZooKeeper operation and 
> confirm that verification of an unrelated cached token continues. Add 
> concurrency and failure tests covering renewal versus cancellation, 
> concurrent renewals, key rotation versus issuance, interrupted operations, 
> and persistence failures.
> Extend the KMS HTTP benchmark from HADOOP-20045 to measure renewal and 
> cancellation contention. Exercise key rotation and expired-key cleanup 
> separately. Compare throughput and verification tail latency against the 
> original implementation, using real ZooKeeper operations without injected 
> delay for performance measurements.
> h3. Scope
> This issue specifically tracks ZooKeeper operations executed while holding 
> {{apiLock.writeLock()}}. Read-lock cache misses, Router cache rebuilding, and 
> CPU-only cleanup scans are outside its scope.
> Implementation and real-ZooKeeper benchmarks are now available. The attached 
> design describes token lock stripes, versioned renewal/deletion, durable 
> master-key publication, and compatibility with non-ZooKeeper stores. 
> Measurements include renewal/cancellation HTTP workloads and 24 validated 
> background-maintenance cases (four scenarios, two builds, three repetitions). 
> See the implementation/benchmark comment and attached report, charts, and raw 
> results for methods, validation, and limitations.
> h3. Related issues
> * [HADOOP-18851|https://issues.apache.org/jira/browse/HADOOP-18851]
> * [HADOOP-20045|https://issues.apache.org/jira/browse/HADOOP-20045]



--
This message was sent by Atlassian Jira
(v10.3.26#10030026)

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

Reply via email to