Repository navigation
Revert HADOOP-19668 and HADOOP-19670: Replace Thread with SubjectInheritingThread - #8568
Merged
Merged
Conversation
…read (apache#8062)" This reverts commit 9cfbbf5.
…o restore pre-JDK22 Subject behaviour (apache#8061)" This reverts commit e57306e.
github-actions
Bot
added
YARN
MapReduce
HDFS
DistCp
trunk
Common
RBF
TOOLS
AWS
ABFS
DYNAMOMETER
labels
|
💔 -1 overall
This message was automatically generated. |
Member
Author
1 of 4 tasks
steveloughran
left a comment
steveloughran
left a comment
Contributor
There was a problem hiding this comment.
with this reverted, does everything work properly on a jvm without the the subject stuff?
Member
Author
@steveloughran Do you mean SIMPLE mode? After reverting, this part basically stays in the same state as branch-3.4 - no issue with SIMPLE mode on JDK 25. |
Member
Author
|
@steveloughran Jenkins fails several times (each round takes over a day) with exit code 143, likely OOM. But GitHub Actions passes (full UT except for the excluded flaky test list). Could you take another look? |
|
💔 -1 overall
This message was automatically generated. |
steveloughran
approved these changes
Jul 6, 2026
steveloughran
left a comment
steveloughran
left a comment
Contributor
There was a problem hiding this comment.
Approved. Do add "SubjectInheritingThread" to the title of the commit so that on a git log it is obvious what the commit is.
pan3793
changed the title
pan3793
added a commit
that referenced
this pull request
Jul 6, 2026
…ritingThread (#8568) * Revert "HADOOP-19670. [JDK22] Replace Thread with SubjectPreservingThread (#8062)" This reverts commit 9cfbbf5. * Revert "HADOOP-19668. Add SubjectInheritingThread and update Daemon to restore pre-JDK22 Subject behaviour (#8061)" This reverts commit e57306e. Plus a few style tunes. Reviewed-by: Steve Loughran <stevel@cloudera.com> Signed-off-by: Cheng Pan <chengpan@apache.org>
Member
Author
|
@steveloughran, thanks for the review. I updated the PR title and committed it to trunk (3.6.0) and branch-3.5 (3.5.1) |
Merged
1 of 4 tasks
joseluisll
added a commit
to joseluisll/hadoop
that referenced
this pull request
Sep 25, 2026
…nService.PublicLocalizer.run() When pending.remove(completed) returns null, run() logged "Localized unknown resource" and returned, terminating the Public Localizer thread for the life of the NodeManager. The NM kept running, but all later public localization failed with RejectedExecutionException, so containers needing public resources hung until a restart -- the end state reported in YARN-9968. Skip the unattributable resource with continue instead. The branch is defensive rather than reachable today: addResource() wraps queue.submit(...) and pending.put(...) in one synchronized (pending) block so a future cannot complete and be dequeued before the map is updated, and pending is a synchronizedMap on the same monitor. It matters because YARN-11746 proposes shutting the NodeManager down when this thread exits, which would turn a single unknown resource into a node-wide outage. Hoisting the null check out of the try block also makes the assoc dereference in the ExecutionException handler provably safe, so SpotBugs no longer reports NP_NULL_ON_SOME_PATH_EXCEPTION. The exclude entry for it is therefore dropped rather than repaired; it had been inert since the revert of HADOOP-19668/19670 (apache#8568) restored run() without restoring the method name YARN-11912 changed to work(). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
joseluisll
added a commit
to joseluisll/hadoop
that referenced
this pull request
Sep 25, 2026
…ove() returns null When pending.remove(completed) returned null, run() logged "Localized unknown resource" and returned. That terminated the Public Localizer thread for the life of the NodeManager, and run()'s finally block shut the download pool down on the way out. Every later public-resource request was then rejected; YARN-1800's handler turns each RejectedExecutionException into a ResourceFailedLocalizationEvent, so containers fail rather than hang, but no public resource can be localized on that node again until the NM is restarted. That is the state reported in YARN-9968, which fixed a different branch of this method. Skip the unattributable resource with continue instead. The branch is defensive rather than reachable today: addResource() wraps queue.submit(...) and pending.put(...) in one synchronized (pending) block so a future cannot complete and be dequeued before the map is updated, and pending is a synchronizedMap on the same monitor. It matters because YARN-11746 proposes shutting the NodeManager down when this thread exits, which would turn a single unknown resource into a node-wide outage. Hoisting the null check out of the try block also makes the assoc dereference in the ExecutionException handler provably safe, so SpotBugs no longer reports NP_NULL_ON_SOME_PATH_EXCEPTION. The exclude entry for it is therefore dropped rather than repaired; it had been inert since the revert of HADOOP-19668/19670 (apache#8568) restored run() without restoring the method name YARN-11912 changed to work(). The new test submits a download straight to the completion queue so the Future is never recorded in pending, then asserts the localizer neither dies nor shuts its pool down. It fails on the unfixed code with "Public Localizer exited after taking an unknown resource". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
joseluisll
added a commit
to joseluisll/hadoop
that referenced
this pull request
Oct 6, 2026
…ove() returns null When pending.remove(completed) returned null, run() logged "Localized unknown resource" and returned. That terminated the Public Localizer thread for the life of the NodeManager, and run()'s finally block shut the download pool down on the way out. Every later public-resource request was then rejected; YARN-1800's handler turns each RejectedExecutionException into a ResourceFailedLocalizationEvent, so containers fail rather than hang, but no public resource can be localized on that node again until the NM is restarted. That is the state reported in YARN-9968, which fixed a different branch of this method. Skip the unattributable resource with continue instead. The branch is defensive rather than reachable today: addResource() wraps queue.submit(...) and pending.put(...) in one synchronized (pending) block so a future cannot complete and be dequeued before the map is updated, and pending is a synchronizedMap on the same monitor. It matters because YARN-11746 proposes shutting the NodeManager down when this thread exits, which would turn a single unknown resource into a node-wide outage. Hoisting the null check out of the try block also makes the assoc dereference in the ExecutionException handler provably safe, so SpotBugs no longer reports NP_NULL_ON_SOME_PATH_EXCEPTION. The exclude entry for it is therefore dropped rather than repaired; it had been inert since the revert of HADOOP-19668/19670 (apache#8568) restored run() without restoring the method name YARN-11912 changed to work(). The new test submits a download straight to the completion queue so the Future is never recorded in pending, then asserts the localizer neither dies nor shuts its pool down. It fails on the unfixed code with "Public Localizer exited after taking an unknown resource". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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
This PR reverts the
they are going to be replaced with
How was this patch tested?
Pass GHA.
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?AI Tooling
No AI usage.