HDFS-17947. dfs.block.access.token.lifetime=0 can cause DataStreamer createBlockOutputStream failure. - #8638
Open
joseluisll wants to merge 3 commits into
Open
Conversation
…createBlockOutputStream failure. When block access tokens are enabled and dfs.block.access.token.lifetime is set to 0, BlockTokenSecretManager stamps every token with an expiry date equal to its creation instant (timer.now() + tokenLifetime). Because expiry is evaluated with a strict comparison (Time.now() > expiryDate), the token is already invalid by the time a DataNode verifies it, so DataXceiver answers ERROR_ACCESS_TOKEN and DataStreamer.createBlockOutputStream fails with InvalidBlockTokenException. Unlike InvalidEncryptionKeyException, the write path does not refetch the token, so every DataNode in the pipeline is excluded in turn and the write fails with "Unable to create new block". The same arithmetic is used by generateDataEncryptionKey(), so a non-positive lifetime also produces data transfer encryption keys that are born expired. The value additionally propagates to DataNodes and the Balancer through ExportedBlockKeys, so validating it at the NameNode covers the whole cluster. Reject a non-positive dfs.block.access.token.lifetime in BlockManager.createBlockTokenSecretManager, so the misconfiguration surfaces as a clear error instead of an unexplained write failure, and document the constraint in hdfs-default.xml. The check sits in the BlockManager constructor, so the NameNode will neither format nor start with an invalid value. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
🎊 +1 overall
This message was automatically generated. |
ayushtkn
reviewed
Jul 29, 2026
- Shorten the validation message to state only that the value is invalid and should be positive, matching the ensurePositiveInt idiom already in BlockManager. - Revert the hdfs-default.xml description change. - Drop the two new test classes and cover the validation with a single test in the existing TestBlockTokenWithDFS instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ayushtkn
reviewed
Jul 29, 2026
ayushtkn
left a comment
ayushtkn
left a comment
Member
There was a problem hiding this comment.
minor comment, rest changes LGTM
- Fold the message check into assertThrows. - Drop the test javadoc, matching the rest of the class. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
💔 -1 overall
This message was automatically generated. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of PR
dfs.block.access.token.lifetimeis read without validation. A non-positivevalue is accepted and produces block access tokens that are already expired at
the instant they are created, so every write fails with an error that names
neither the property nor the cause.
BlockManager.createBlockTokenSecretManagerconverts the value withlifetimeMin * 60 * 1000Land passes it toBlockTokenSecretManager, whichstamps every token with
and evaluates expiry with a strict comparison
With a lifetime of
0the expiry date equals the creation instant, so thetoken is invalid as soon as the clock advances by one millisecond — which it
always has by the time the token has travelled NameNode → client → DataNode.
DataXceiveranswersERROR_ACCESS_TOKENandDataStreamer.createBlockOutputStreamfails withInvalidBlockTokenException.Unlike
InvalidEncryptionKeyException, the write path has no token-refetchretry, so each DataNode is marked bad and excluded in turn until the pipeline
is exhausted.
Reproduced on a MiniDFSCluster with
dfs.block.access.token.enable=trueanddfs.block.access.token.lifetime=0, before the fix:Two further consequences of the same arithmetic are worth recording:
generateDataEncryptionKey()also stampstimer.now() + tokenLifetime(
BlockTokenSecretManager:541), so a non-positive lifetime additionallyproduces data-transfer encryption keys that are born expired.
ExportedBlockKeys: DataNodes(
DataNode:2132) and the Balancer (KeyManager:74) build their ownBlockTokenSecretManagerfrom the value the NameNode advertises. Validatingat the NameNode therefore covers the whole cluster, and no DataNode-side
check is needed.
The change
BlockManager.createBlockTokenSecretManagernow rejects a non-positivelifetime with
HadoopIllegalArgumentException, naming the property and theconstraint. The check runs only when
dfs.block.access.token.enableis true,so clusters that do not use block tokens are unaffected. Because it sits in the
BlockManagerconstructor it applies to every path that builds anFSNamesystem— the NameNode will neither format nor start with an invalidvalue, and
NameNode.mainreports it through the existingterminate(1, e)path.The bound is simply "greater than zero", not some larger floor. The property is
expressed in minutes, so the smallest representable positive value (
1, i.e.60,000 ms) already exceeds the mint-to-verify window by about four orders of
magnitude; there is no positive value in this config's units that reproduces
the failure. Choosing an arbitrary minimum would reject configurations that
work correctly today and would invent policy this project has never defined.
Short-but-positive lifetimes do have real costs — tolerance to NameNode/DataNode
clock skew, and read-path token refetch churn (cf. HDFS-16332) — but those are
tuning concerns rather than the defect reported here.
dfs.block.access.key.update.intervalwas examined as a possible sibling ofthis bug and deliberately left alone: with an interval of
0the retiring keystill receives an expiry of
now + keyUpdateInterval + tokenLifetime, so tokensremain verifiable. Key rotation becomes wasteful, not broken, and it is out of
scope here.
Compatibility
This is technically an incompatible change: a cluster configured with a
non-positive
dfs.block.access.token.lifetimestarts today and will now refuseto start. It cannot regress a working deployment, because such a cluster is
already unable to complete any write, but it is called out here explicitly
rather than left to be discovered in review.
Contains content generated by Anthropic Claude Code.
How was this patch tested?
New tests, both added by this patch:
TestBlockTokenZeroLifetime(6 tests, no cluster) pins the underlyingbehaviour at the secret-manager level in both legacy and protobuf token
formats: with a lifetime of
0the minted token's expiry date falls insidethe creation instant, and the DataNode-side (worker) manager then rejects it
with
InvalidToken ... is expired; a 600-minute control verifies cleanly.The expiry assertion waits for one clock tick past the recorded expiry rather
than sleeping, so it is deterministic rather than racing the clock.
TestBlockTokenZeroLifetimeWithDFS(5 tests, MiniDFSCluster) covers formatrejecting
0and-1, an already-formatted NameNode refusing to restartafter the value is changed to
0, a 600-minute control that writes andverifies a file, and — importantly — that the value is still ignored when
dfs.block.access.token.enableis false.Full runs on the branch (JDK 17,
mvn test -pl hadoop-hdfs-project/hadoop-hdfs):The pre-fix reproduction quoted above was produced by the same
TestBlockTokenZeroLifetimeWithDFSfixture with the validation reverted; thecommitted version of that test asserts the post-fix behaviour.
No existing test or configuration file in the tree sets
dfs.block.access.token.lifetime, so the new validation changes no existingtest's behaviour.
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles? (N/A)AI Tooling
This contribution was prepared with the assistance of Anthropic Claude Code:
the root-cause analysis, the validation change, the tests and this description
were drafted with its assistance, and were reviewed, executed and verified
against the source by the contributor.
where is the name of the AI tool used.
https://www.apache.org/legal/generative-tooling.html