Skip to content

Conversation

@stoty
Copy link
Contributor

@stoty stoty commented Nov 7, 2025

Description of PR

The second part of HADOOP-19574.

Replace Thread wit SubjectPreservingThread

How was this patch tested?

Tested with HADOOP-19668 which it depends on.
Ran the full test suite with JDK25, and confirmed that it does not cause regressions
(i.e. only tests which also fail with Java 17 fail with Java 25)

For code changes:

  • Does the title or this PR starts 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?
  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • If applicable, have you updated the LICENSE, LICENSE-binary, NOTICE-binary files?

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 0s Docker mode activated.
-1 ❌ patch 0m 20s #8062 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-8062/1/console
versions git=2.34.1
Powered by Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

@stoty stoty mentioned this pull request Nov 7, 2025
4 tasks
@stoty stoty changed the title HADOOP-19670 Replace Thread with SubjectPreservingThread to restore pre JDK22 Subject behaviour in Threads HADOOP-19670. Replace Thread with SubjectPreservingThread to restore pre JDK22 Subject behaviour in Threads Nov 7, 2025
@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 0s Docker mode activated.
-1 ❌ patch 0m 23s #8062 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-8062/2/console
versions git=2.34.1
Powered by Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

@stoty
Copy link
Contributor Author

stoty commented Nov 12, 2025

Rebased on trunk @steveloughran .

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 0s Docker mode activated.
-1 ❌ patch 0m 22s #8062 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-8062/3/console
versions git=2.34.1
Powered by Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 0s Docker mode activated.
-1 ❌ patch 0m 21s #8062 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-8062/4/console
versions git=2.34.1
Powered by Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

Copy link
Contributor

@steveloughran steveloughran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 pending some minor changes.

I have looked at all the production code and a few of the tests; all looks good.

one nit about javadocs; some javadoc versions fail if there isn't a "." at the end of the first sentence

and in a couple of places I saw the new import removed a blank line. In these situations, when the line below isn't a org.apache import, keep the line in.

It's a big pr, but each file change is as minimal as you can get, and I don't see any way to do it which is any less complex and which doesn't actually involve bigger code code changes. And I don't see what is to be gained by splitting into different modules unless that's easier to get through yetus

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 0s Docker mode activated.
-1 ❌ patch 0m 22s #8062 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-8062/5/console
versions git=2.34.1
Powered by Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

@stoty
Copy link
Contributor Author

stoty commented Nov 13, 2025

Thanks, I've pushed the change to finish the sentence and restore the blank lines.
I've also changed the import order to make more sense in a few file.

Re Yetus, there is a fixed version that can deal with large patches, which was included in some previous iterations, but I did not include that here to make the PR directly applicable.

Copy link
Contributor

@steveloughran steveloughran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@steveloughran
Copy link
Contributor

what about a separate PR for that yetus update?

Anyway, this is is all building locally is it? what modules have you tested?

@stoty
Copy link
Contributor Author

stoty commented Nov 18, 2025

Opened #8090 which includes the fixed yetus, @steveloughran .

Yes, it builds locally, but I have not run the tests locally on this one (I built it with -DskipTests)

@stoty
Copy link
Contributor Author

stoty commented Nov 18, 2025

I have ran the full test suite a few weeks back on an earlier PR, and confirmed that it does not cause regressions.

Copy link
Contributor

@steveloughran steveloughran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@steveloughran steveloughran merged commit 9cfbbf5 into apache:trunk Nov 21, 2025
1 of 3 checks passed
@pan3793
Copy link
Member

pan3793 commented Nov 21, 2025

Big thanks to @stoty @steveloughran, and I think this is the last patch of the UGI compatibility story, so it's time to start backporting to branch-3.4?

@stoty
Copy link
Contributor Author

stoty commented Nov 21, 2025

Yes, this was the last big one.
I still have an outstandig minor PR: #7961
IMO the next priority would be setting up the CI for JDK21 and 25 to avoid future regressions.

@h-vetinari
Copy link

so it's time to start backporting to branch-3.4?

is it realistic to backport all the pieces necessary for newer Java support to the 3.4 maintenance branch?

@stoty
Copy link
Contributor Author

stoty commented Nov 22, 2025

I don't see any issues, apart from the amount of work required.

trunk still compiles and runs the test suite with Java 8 , so the Java 8 requirement in 3.4 is not a problem.

Some of the fixes are already backported as they were also needed for JDK 17 (or even for JDK8), but it is still a huge job.
Also many fixes will not not backport cleanly due to the JUnit 4->5 migration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants