You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "pan3793 (via GitHub)" <gi...@apache.org> on 2026/08/17 14:03:40 UTC
[PR] HADOOP-19966. ZKDelegationTokenSecretManager may fail to load tokens and keys from ZooKeeper on startup [hadoop]
pan3793 opened a new pull request, #8688:
URL: https://github.com/apache/hadoop/pull/8688
### Description of PR
ZKDelegationTokenSecretManager reads the CuratorCache immediately after
starting it, but CuratorCache populates asynchronously, so the cache can
still be empty and existing tokens/keys are silently not loaded into memory
on startup. It also created the root container znodes data-less, which
Curator 5.2.0+ defaults to the local address, so the container znode data
could not be parsed.
This change:
- Waits for CuratorCache initialization (forInitialized) before loading each cache.
- Skips the root container znode (/ZKDTSMMasterKeyRoot, /ZKDTSMTokensRoot)
when processing cache events and when streaming the cache, since it is a
container rather than a key/token.
- Creates the root container znodes with explicit empty data.
- Restores the thread-local curator and closes the client in the tests that
use setCurator, and waits for the good token to load in
testNodesLoadedAfterRestart.
Contains content generated by deepseek-v4-pro
### How was this patch tested?
mvn -pl hadoop-common-project/hadoop-common -am \
-Dtest=TestZKDelegationTokenSecretManager test
### For code changes:
- [x] Does the title of this PR start with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
- [ ] Object storage: Have the integration tests been executed and the endpoint
declared according to the connector-specific documentation? *Note: Automated CI
testing doesn't cover all cases so manual testing with cloud storage is still
required.*
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the `LICENSE`, `LICENSE-binary`, `NOTICE-binary` files?
### AI Tooling
If an AI tool was used:
- [x] The PR includes the phrase "Contains content generated by <tool>"
where <tool> is the name of the AI tool used.
- [x] My use of AI contributions follows the ASF legal policy
https://www.apache.org/legal/generative-tooling.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19966. ZKDelegationTokenSecretManager may fail to load tokens and keys from ZooKeeper on startup [hadoop]
Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on PR #8688:
URL: https://github.com/apache/hadoop/pull/8688#issuecomment-5389580338
> @slfan1989, mind taking another look? still see such testing failure occasionly
>
> https://github.com/apache/hadoop/actions/runs/32465896350/job/96725649556
@pan3793 Thank you very much for your contribution! I will take a closer look at this PR today.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19966. ZKDelegationTokenSecretManager may fail to load tokens and keys from ZooKeeper on startup [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #8688:
URL: https://github.com/apache/hadoop/pull/8688#issuecomment-5316942793
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 0s | | Docker mode activated. |
| -1 :x: | patch | 0m 20s | | https://github.com/apache/hadoop/pull/8688 does not apply to trunk. Rebase required? Wrong Branch? See https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute for help. |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8688/1/console |
| versions | git=2.34.1 |
| Powered by | Apache Yetus 0.14.1 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19966. ZKDelegationTokenSecretManager startup cache load races with CuratorCache initialization [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #8688:
URL: https://github.com/apache/hadoop/pull/8688#issuecomment-5679046702
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 23s | | 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. |
| +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 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 27m 49s | | trunk passed |
| +1 :green_heart: | compile | 8m 39s | | trunk passed with JDK Ubuntu-21.0.12+8-1-24.04-Ubuntu |
| +1 :green_heart: | compile | 8m 49s | | trunk passed with JDK Ubuntu-17.0.20+8-1-24.04-Ubuntu |
| +1 :green_heart: | checkstyle | 0m 53s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 7s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 53s | | trunk passed with JDK Ubuntu-21.0.12+8-1-24.04-Ubuntu |
| +1 :green_heart: | javadoc | 0m 54s | | trunk passed with JDK Ubuntu-17.0.20+8-1-24.04-Ubuntu |
| +1 :green_heart: | spotbugs | 1m 50s | | trunk passed |
| +1 :green_heart: | shadedclient | 17m 1s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 17m 18s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 44s | | the patch passed |
| +1 :green_heart: | compile | 8m 5s | | the patch passed with JDK Ubuntu-21.0.12+8-1-24.04-Ubuntu |
| +1 :green_heart: | javac | 8m 5s | | the patch passed |
| +1 :green_heart: | compile | 8m 55s | | the patch passed with JDK Ubuntu-17.0.20+8-1-24.04-Ubuntu |
| +1 :green_heart: | javac | 8m 55s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 52s | | the patch passed |
| +1 :green_heart: | mvnsite | 1m 9s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 52s | | the patch passed with JDK Ubuntu-21.0.12+8-1-24.04-Ubuntu |
| +1 :green_heart: | javadoc | 0m 54s | | the patch passed with JDK Ubuntu-17.0.20+8-1-24.04-Ubuntu |
| +1 :green_heart: | spotbugs | 1m 54s | | the patch passed |
| +1 :green_heart: | shadedclient | 17m 8s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 19s | | hadoop-common in the patch passed. |
| +1 :green_heart: | asflicense | 0m 43s | | The patch does not generate ASF License warnings. |
| | | 128m 42s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.56 ServerAPI=1.56 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8688/4/artifact/out/Dockerfile |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux b1ac3639415a 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 / 7104bb9734727c416e5d97bda8685aaae11f9a60 |
| Default Java | Ubuntu-17.0.20+8-1-24.04-Ubuntu |
| Multi-JDK versions | /usr/lib/jvm/java-21-openjdk-amd64:Ubuntu-21.0.12+8-1-24.04-Ubuntu /usr/lib/jvm/java-17-openjdk-amd64:Ubuntu-17.0.20+8-1-24.04-Ubuntu |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8688/4/testReport/ |
| Max. process+thread count | 1271 (vs. ulimit of 10000) |
| modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8688/4/console |
| versions | git=2.43.0 maven=3.9.15 spotbugs=4.9.7 |
| Powered by | Apache Yetus 0.14.1 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19966. ZKDelegationTokenSecretManager startup cache load races with CuratorCache initialization [hadoop]
Posted by "Hexiaoqiao (via GitHub)" <gi...@apache.org>.
Hexiaoqiao commented on PR #8688:
URL: https://github.com/apache/hadoop/pull/8688#issuecomment-5727049803
> @Hexiaoqiao Token verify/renew/cancel are not affected: `getTokenInfo` and `getDelegationKey` fall back to ZK on a cache miss. The in-memory load exists for the expired-token remover, which only scans `currentTokens` ([HADOOP-13487](https://issues.apache.org/jira/browse/HADOOP-13487)). That relied on PathChildrenCache `BUILD_INITIAL_CACHE` being synchronous; [HADOOP-17835](https://issues.apache.org/jira/browse/HADOOP-17835) (3.4.0) switched to `CuratorCache.start()`, which is asynchronous. The listener still fills the map a few seconds later, so the lasting effects are: token owner stats (Router `getTopTokenRealOwners`) are computed from a partial map and never corrected for pre-restart tokens, the root container znode is parsed on every startup and logs an ERROR, and `testNodesLoadedAfterRestart` is flaky in CI. Router with the token watcher disabled uses its own synchronous `rebuildTokenCache` and is unaffected. Retitled the PR and rewrote the description accordingly.
Got it. LGTM. Give my +1 for this PR. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19966. ZKDelegationTokenSecretManager may fail to load tokens and keys from ZooKeeper on startup [hadoop]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #8688:
URL: https://github.com/apache/hadoop/pull/8688#discussion_r3804574463
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/token/delegation/TestZKDelegationTokenSecretManager.java:
##########
@@ -551,22 +558,27 @@ public void testCreatingParentContainersIfNeeded() throws Exception {
.build();
curatorFramework.start();
ZKDelegationTokenSecretManager.setCurator(curatorFramework);
- DelegationTokenManager tm1 = new DelegationTokenManager(conf, new Text("foo"));
+ try {
+ DelegationTokenManager tm1 = new DelegationTokenManager(conf, new Text("foo"));
- // When the init method is called,
- // the ZKDelegationTokenSecretManager#startThread method will be called,
- // and the creatingParentContainersIfNeeded will be called to create the nameSpace.
- tm1.init();
+ // When the init method is called,
+ // the ZKDelegationTokenSecretManager#startThread method will be called,
+ // and the creatingParentContainersIfNeeded will be called to create the nameSpace.
+ tm1.init();
Review Comment:
Fixed: `destroy()` and executor shutdown/await moved into `finally` in both tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19966. ZKDelegationTokenSecretManager may fail to load tokens and keys from ZooKeeper on startup [hadoop]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #8688:
URL: https://github.com/apache/hadoop/pull/8688#discussion_r3797807178
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/token/delegation/TestZKDelegationTokenSecretManager.java:
##########
@@ -551,22 +558,27 @@ public void testCreatingParentContainersIfNeeded() throws Exception {
.build();
curatorFramework.start();
ZKDelegationTokenSecretManager.setCurator(curatorFramework);
- DelegationTokenManager tm1 = new DelegationTokenManager(conf, new Text("foo"));
+ try {
+ DelegationTokenManager tm1 = new DelegationTokenManager(conf, new Text("foo"));
- // When the init method is called,
- // the ZKDelegationTokenSecretManager#startThread method will be called,
- // and the creatingParentContainersIfNeeded will be called to create the nameSpace.
- tm1.init();
+ // When the init method is called,
+ // the ZKDelegationTokenSecretManager#startThread method will be called,
+ // and the creatingParentContainersIfNeeded will be called to create the nameSpace.
+ tm1.init();
Review Comment:
`tm1.destroy()` is only called on the success path; if `tm1.init()` or the ZK assertion throws, the secret manager threads may be left running and can make subsequent tests flaky. Put `tm1.destroy()` in a `finally` so cleanup always happens.
This issue also appears on line 631 of the same file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19966. ZKDelegationTokenSecretManager startup cache load races with CuratorCache initialization [hadoop]
Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on PR #8688:
URL: https://github.com/apache/hadoop/pull/8688#issuecomment-5813608885
> Hi @pan3793 and @slfan1989 , I think it is ready to push this improvement forward, would you mind take another review if available?
@Hexiaoqiao Thanks for the updates and for addressing the review comments! I'll take another look at the latest changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19966. ZKDelegationTokenSecretManager may fail to load tokens and keys from ZooKeeper on startup [hadoop]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #8688:
URL: https://github.com/apache/hadoop/pull/8688#issuecomment-5369367244
@slfan1989, mind taking another look? still see such testing failure occasionly
https://github.com/apache/hadoop/actions/runs/32465896350/job/96725649556
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19966. ZKDelegationTokenSecretManager startup cache load races with CuratorCache initialization [hadoop]
Posted by "joseluisll (via GitHub)" <gi...@apache.org>.
joseluisll commented on code in PR #8688:
URL: https://github.com/apache/hadoop/pull/8688#discussion_r4288047632
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/ZKDelegationTokenSecretManager.java:
##########
@@ -386,6 +433,24 @@ private void loadFromZKCache(final boolean isTokenCache) {
LOG.info("Loaded {} cache.", cacheName);
}
+ private void awaitCacheInitialized(CountDownLatch initialized,
+ String cacheName) throws IOException {
+ try {
+ if (cacheInitTimeoutMs > 0) {
+ if (!initialized.await(cacheInitTimeoutMs, TimeUnit.MILLISECONDS)) {
+ throw new IOException("Timed out after " + cacheInitTimeoutMs
+ + " ms waiting for " + cacheName + " cache initialization");
+ }
+ } else {
+ initialized.await();
Review Comment:
**Ask:** log a WARN periodically while waiting here, for example by calling `await(timeout)` in a loop and logging on each pass.
**Why:** if ZooKeeper becomes unreachable, startup now hangs here with nothing in the logs.
**Optional:** add a code comment noting that in Curator 5.2.0 a failed read during the first load still counts as done, so the latch can release with the cache only partly loaded.
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/ZKDelegationTokenSecretManager.java:
##########
@@ -301,9 +334,11 @@ public void startThreads() throws IOException {
}
})
.forDeletes(childData -> processKeyRemoved(childData.getPath()))
+ .forInitialized(keyCacheInitialized::countDown)
Review Comment:
**Ask (nit, optional):** add `.afterInitialized()` to both listener builders.
**Why:** without it, these listeners also process every existing node during the first load, and `loadFromZKCache` then decodes them all again. The latch still works with `.afterInitialized()`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19966. ZKDelegationTokenSecretManager startup cache load races with CuratorCache initialization [hadoop]
Posted by "Hexiaoqiao (via GitHub)" <gi...@apache.org>.
Hexiaoqiao commented on PR #8688:
URL: https://github.com/apache/hadoop/pull/8688#issuecomment-5812087189
Hi @pan3793 and @slfan1989 , I think it is ready to push this improvement forward, would you mind take another review if available?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19966. ZKDelegationTokenSecretManager may fail to load tokens and keys from ZooKeeper on startup [hadoop]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #8688:
URL: https://github.com/apache/hadoop/pull/8688#discussion_r3912672644
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/ZKDelegationTokenSecretManager.java:
##########
@@ -386,6 +409,20 @@ private void loadFromZKCache(final boolean isTokenCache) {
LOG.info("Loaded {} cache.", cacheName);
}
+ private void awaitCacheInitialized(CountDownLatch initialized,
+ String cacheName) throws IOException {
+ try {
+ if (!initialized.await(CACHE_INITIALIZED_TIMEOUT_SECONDS, TimeUnit.SECONDS)) {
Review Comment:
Made configurable via `zk-dt-secret-manager.zkCacheInitTimeout` (ms) in f35a83a. Default 0 waits indefinitely; the tests set 10s.
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/token/delegation/TestZKDelegationTokenSecretManager.java:
##########
@@ -522,6 +522,13 @@ public Boolean get() {
// The good token should be loaded on startup, and removed after expiry.
id = smNew.decodeTokenIdentifier(token);
+ final AbstractDelegationTokenIdentifier idGood = id;
+ GenericTestUtils.waitFor(new Supplier<Boolean>() {
Review Comment:
Restored the immediate assertion after `tm.init()` in f35a83a. The trunk failure linked above fails on exactly this assertion without the production change.
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/ZKDelegationTokenSecretManager.java:
##########
@@ -301,9 +310,11 @@ public void startThreads() throws IOException {
}
})
.forDeletes(childData -> processKeyRemoved(childData.getPath()))
+ .forInitialized(keyCacheInitialized::countDown)
.build();
keyCache.listenable().addListener(keyCacheListener);
keyCache.start();
+ awaitCacheInitialized(keyCacheInitialized, "key");
Review Comment:
`startThreads()` now calls `stopThreads()` before rethrowing on any failure, with an `isRunning()` guard so the cleanup never stops a running instance, in f35a83a.
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/ZKDelegationTokenSecretManager.java:
##########
@@ -333,9 +349,11 @@ public void startThreads() throws IOException {
throw new UncheckedIOException(e);
}
})
+ .forInitialized(tokenCacheInitialized::countDown)
.build();
tokenCache.listenable().addListener(tokenCacheListener);
tokenCache.start();
+ awaitCacheInitialized(tokenCacheInitialized, "token");
Review Comment:
Same as above, fixed in f35a83a.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19966. ZKDelegationTokenSecretManager may fail to load tokens and keys from ZooKeeper on startup [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #8688:
URL: https://github.com/apache/hadoop/pull/8688#issuecomment-5330759282
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 6m 35s | | 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. |
| +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 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 26m 25s | | trunk passed |
| +1 :green_heart: | compile | 8m 47s | | trunk passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | compile | 8m 51s | | trunk passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | checkstyle | 0m 57s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 20s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 45s | | trunk passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | javadoc | 0m 50s | | trunk passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | spotbugs | 1m 50s | | trunk passed |
| +1 :green_heart: | shadedclient | 19m 56s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 41s | | the patch passed |
| +1 :green_heart: | compile | 8m 57s | | the patch passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | javac | 8m 57s | | the patch passed |
| +1 :green_heart: | compile | 9m 4s | | the patch passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | javac | 9m 4s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 46s | | the patch passed |
| +1 :green_heart: | mvnsite | 1m 11s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 51s | | the patch passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | javadoc | 0m 54s | | the patch passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | spotbugs | 1m 52s | | the patch passed |
| +1 :green_heart: | shadedclient | 17m 12s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 16s | | hadoop-common in the patch passed. |
| +1 :green_heart: | asflicense | 0m 47s | | The patch does not generate ASF License warnings. |
| | | 138m 2s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.55 ServerAPI=1.55 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8688/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/8688 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 409f60a6e195 5.15.0-179-generic #189-Ubuntu SMP Tue May 5 18:20:56 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / d2bddcc76011e1fca5b1f39235bd5808ce8618fc |
| Default Java | Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| Multi-JDK versions | /usr/lib/jvm/java-21-openjdk-amd64:Ubuntu-21.0.11+10-1-24.04.2-Ubuntu /usr/lib/jvm/java-17-openjdk-amd64:Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8688/2/testReport/ |
| Max. process+thread count | 3158 (vs. ulimit of 10000) |
| modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8688/2/console |
| versions | git=2.43.0 maven=3.9.15 spotbugs=4.9.7 |
| Powered by | Apache Yetus 0.14.1 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19966. ZKDelegationTokenSecretManager startup cache load races with CuratorCache initialization [hadoop]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #8688:
URL: https://github.com/apache/hadoop/pull/8688#issuecomment-5677391891
@Hexiaoqiao Token verify/renew/cancel are not affected: `getTokenInfo` and `getDelegationKey` fall back to ZK on a cache miss. The in-memory load exists for the expired-token remover, which only scans `currentTokens` (HADOOP-13487). That relied on PathChildrenCache `BUILD_INITIAL_CACHE` being synchronous; HADOOP-17835 (3.4.0) switched to `CuratorCache.start()`, which is asynchronous. The listener still fills the map a few seconds later, so the lasting effects are: token owner stats (Router `getTopTokenRealOwners`) are computed from a partial map and never corrected for pre-restart tokens, the root container znode is parsed on every startup and logs an ERROR, and `testNodesLoadedAfterRestart` is flaky in CI. Router with the token watcher disabled uses its own synchronous `rebuildTokenCache` and is unaffected. Retitled the PR and rewrote the description accordingly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19966. ZKDelegationTokenSecretManager startup cache load races with CuratorCache initialization [hadoop]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #8688:
URL: https://github.com/apache/hadoop/pull/8688#discussion_r4013740949
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/ZKDelegationTokenSecretManager.java:
##########
@@ -386,6 +409,20 @@ private void loadFromZKCache(final boolean isTokenCache) {
LOG.info("Loaded {} cache.", cacheName);
}
+ private void awaitCacheInitialized(CountDownLatch initialized,
+ String cacheName) throws IOException {
+ try {
+ if (!initialized.await(CACHE_INITIALIZED_TIMEOUT_SECONDS, TimeUnit.SECONDS)) {
Review Comment:
Dropped the config key in ccf5b7f. The wait is unbounded in production; the tests set a 10s bound via a test-only setter.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19966. ZKDelegationTokenSecretManager startup cache load races with CuratorCache initialization [hadoop]
Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on code in PR #8688:
URL: https://github.com/apache/hadoop/pull/8688#discussion_r4093273562
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/ZKDelegationTokenSecretManager.java:
##########
@@ -386,6 +433,24 @@ private void loadFromZKCache(final boolean isTokenCache) {
LOG.info("Loaded {} cache.", cacheName);
}
+ private void awaitCacheInitialized(CountDownLatch initialized,
+ String cacheName) throws IOException {
+ try {
+ if (cacheInitTimeoutMs > 0) {
+ if (!initialized.await(cacheInitTimeoutMs, TimeUnit.MILLISECONDS)) {
+ throw new IOException("Timed out after " + cacheInitTimeoutMs
+ + " ms waiting for " + cacheName + " cache initialization");
+ }
+ } else {
Review Comment:
Could we retain a configurable initialization timeout for production?
If ZooKeeper becomes unavailable after the counters and root paths have been created but before the cache's persistent watch is registered, `initialized()` may never be delivered.
ZooKeeper `connection/session` timeouts do not bound this latch wait, so `startThreads()` can remain blocked indefinitely without reaching the new failure cleanup.
With the timeout exposed only through a test-only setter, operators cannot bound this wait.
A configurable timeout would accommodate large caches while allowing deployments to fail startup and release resources when initialization cannot complete.
Could we also add a regression test for initialization failure that verifies startup exits and closes the partially started resources?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19966. ZKDelegationTokenSecretManager may fail to load tokens and keys from ZooKeeper on startup [hadoop]
Posted by "Hexiaoqiao (via GitHub)" <gi...@apache.org>.
Hexiaoqiao commented on PR #8688:
URL: https://github.com/apache/hadoop/pull/8688#issuecomment-5581711589
Thanks @pan3793 and @slfan1989 for your works here. IIRC, `ZKDelegationTokenSecretManager` is generally used by DFSRouter, and it will retrieve from zookeeper if no entry in local cache when use its one implement `ZKDelegationTokenSecretManagerImpl`. So my question is 'Is this still an issue here'. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19966. ZKDelegationTokenSecretManager may fail to load tokens and keys from ZooKeeper on startup [hadoop]
Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on code in PR #8688:
URL: https://github.com/apache/hadoop/pull/8688#discussion_r3889022307
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/ZKDelegationTokenSecretManager.java:
##########
@@ -301,9 +310,11 @@ public void startThreads() throws IOException {
}
})
.forDeletes(childData -> processKeyRemoved(childData.getPath()))
+ .forInitialized(keyCacheInitialized::countDown)
.build();
keyCache.listenable().addListener(keyCacheListener);
keyCache.start();
+ awaitCacheInitialized(keyCacheInitialized, "key");
Review Comment:
When cache initialization times out or is interrupted, `startThreads()` throws after the cache and shared counters have already been started. The caller does not automatically invoke `stopThreads()` after a failed initialization, so these resources may remain active. Could the partially started resources be closed before propagating the failure?
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/ZKDelegationTokenSecretManager.java:
##########
@@ -386,6 +409,20 @@ private void loadFromZKCache(final boolean isTokenCache) {
LOG.info("Loaded {} cache.", cacheName);
}
+ private void awaitCacheInitialized(CountDownLatch initialized,
+ String cacheName) throws IOException {
+ try {
+ if (!initialized.await(CACHE_INITIALIZED_TIMEOUT_SECONDS, TimeUnit.SECONDS)) {
Review Comment:
`initialized()` is emitted only after all initial nodes have been loaded into the cache, so the required time depends on the size of the token/key trees and ZooKeeper latency. A fixed 10-second timeout can make an otherwise healthy service fail to start when the cache is large or the configured ZooKeeper timeout is longer.
Could this timeout be configurable, or derived from an existing ZooKeeper timeout setting, with an appropriate default?
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/token/delegation/TestZKDelegationTokenSecretManager.java:
##########
@@ -522,6 +522,13 @@ public Boolean get() {
// The good token should be loaded on startup, and removed after expiry.
id = smNew.decodeTokenIdentifier(token);
+ final AbstractDelegationTokenIdentifier idGood = id;
+ GenericTestUtils.waitFor(new Supplier<Boolean>() {
Review Comment:
The production change is intended to guarantee that the cache is fully loaded before `tm.init()` returns. Adding an eventual 5-second wait here weakens that regression guarantee: the old asynchronous implementation could also pass once its listener catches up.
Could we keep the immediate assertion after `tm.init()` ? Ideally, the test should fail without the `awaitCacheInitialized()` production change and pass with it.
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/ZKDelegationTokenSecretManager.java:
##########
@@ -333,9 +349,11 @@ public void startThreads() throws IOException {
throw new UncheckedIOException(e);
}
})
+ .forInitialized(tokenCacheInitialized::countDown)
.build();
tokenCache.listenable().addListener(tokenCacheListener);
tokenCache.start();
+ awaitCacheInitialized(tokenCacheInitialized, "token");
Review Comment:
Same Line#317
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19966. ZKDelegationTokenSecretManager may fail to load tokens and keys from ZooKeeper on startup [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #8688:
URL: https://github.com/apache/hadoop/pull/8688#issuecomment-5508912636
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 23s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets 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 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 27m 49s | | trunk passed |
| +1 :green_heart: | compile | 8m 32s | | trunk passed with JDK Ubuntu-21.0.12+8-1-24.04-Ubuntu |
| +1 :green_heart: | compile | 8m 50s | | trunk passed with JDK Ubuntu-17.0.20+8-1-24.04-Ubuntu |
| +1 :green_heart: | checkstyle | 0m 55s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 11s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 54s | | trunk passed with JDK Ubuntu-21.0.12+8-1-24.04-Ubuntu |
| +1 :green_heart: | javadoc | 0m 53s | | trunk passed with JDK Ubuntu-17.0.20+8-1-24.04-Ubuntu |
| +1 :green_heart: | spotbugs | 1m 45s | | trunk passed |
| +1 :green_heart: | shadedclient | 16m 57s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 43s | | the patch passed |
| +1 :green_heart: | compile | 8m 9s | | the patch passed with JDK Ubuntu-21.0.12+8-1-24.04-Ubuntu |
| +1 :green_heart: | javac | 8m 9s | | the patch passed |
| +1 :green_heart: | compile | 8m 44s | | the patch passed with JDK Ubuntu-17.0.20+8-1-24.04-Ubuntu |
| +1 :green_heart: | javac | 8m 44s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 0m 50s | [/results-checkstyle-hadoop-common-project_hadoop-common.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8688/3/artifact/out/results-checkstyle-hadoop-common-project_hadoop-common.txt) | hadoop-common-project/hadoop-common: The patch generated 1 new + 22 unchanged - 0 fixed = 23 total (was 22) |
| +1 :green_heart: | mvnsite | 1m 13s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 51s | | the patch passed with JDK Ubuntu-21.0.12+8-1-24.04-Ubuntu |
| +1 :green_heart: | javadoc | 0m 53s | | the patch passed with JDK Ubuntu-17.0.20+8-1-24.04-Ubuntu |
| +1 :green_heart: | spotbugs | 1m 52s | | the patch passed |
| +1 :green_heart: | shadedclient | 16m 40s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 21s | | hadoop-common in the patch passed. |
| +1 :green_heart: | asflicense | 0m 47s | | The patch does not generate ASF License warnings. |
| | | 128m 17s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.55 ServerAPI=1.55 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8688/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/8688 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 4cd208ebe904 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 / f35a83af9c70f5af3d89b141d07974fee822b7fe |
| Default Java | Ubuntu-17.0.20+8-1-24.04-Ubuntu |
| Multi-JDK versions | /usr/lib/jvm/java-21-openjdk-amd64:Ubuntu-21.0.12+8-1-24.04-Ubuntu /usr/lib/jvm/java-17-openjdk-amd64:Ubuntu-17.0.20+8-1-24.04-Ubuntu |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8688/3/testReport/ |
| Max. process+thread count | 3159 (vs. ulimit of 10000) |
| modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8688/3/console |
| versions | git=2.43.0 maven=3.9.15 spotbugs=4.9.7 |
| Powered by | Apache Yetus 0.14.1 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org