Skip to content

HADOOP-18814. Direct class cast causes ClassCastException in TestRPC#testReaderExceptions - #8636

Open
anshuksi282-ksolves wants to merge 1 commit into
apache:trunkfrom
anshuksi282-ksolves:HADOOP-18814
Open

HADOOP-18814. Direct class cast causes ClassCastException in TestRPC#testReaderExceptions#8636
anshuksi282-ksolves wants to merge 1 commit into
apache:trunkfrom
anshuksi282-ksolves:HADOOP-18814

Conversation

@anshuksi282-ksolves

Copy link
Copy Markdown
Contributor

Description of PR

This PR fixes HADOOP-18814.

TestRPC#testReaderExceptions directly casts e.getCause() to RemoteException without first checking its type:

RemoteException re = (RemoteException)e.getCause();

If the root cause is not a RemoteException (e.g. when hadoop.security.authentication is set to kerberos, where the
cause can be a plain IOException such as an authentication failure), this direct cast throws a ClassCastException, hiding the actual underlying exception and making the test failure confusing to debug.

This PR adds a check for the cause's type before casting. If it is not a RemoteException, the original exception is rethrown so the real cause is surfaced instead of a misleading ClassCastException.

This continues the work started in #6076 by @teamconfx, which had already received a green CI run (checkstyle, unit tests, and test4tests all passed) but went stale due to inactivity.

How was this patch tested?

  • mvn -pl hadoop-common-project/hadoop-common test -Dtest=TestRPC#testReaderExceptions passes.

Note: per the JIRA's reproduction steps, the original ClassCastException only manifests when hadoop.security.authentication is set to kerberos. Setting up a local Kerberos environment was out of scope for this fix; the change here is a minimal, low-risk defensive check matching the fix originally proposed and CI-validated in #6076.

AI Tooling

Contains content generated by Claude (Anthropic). Used to help investigate the issue and draft the fix.

@anshuksi282-ksolves

Copy link
Copy Markdown
Contributor Author

Hi, requesting a review whenever you get a chance. This picks up HADOOP-18814 from #6076 by @teamconfx, which had already passed full CI (checkstyle, unit tests, test4tests) but went stale due to inactivity. The fix here is functionally the same, just adapted to the current JUnit 5 style used in trunk.

cc: @ayushtkn @dineshchitlangia @brahmareddybattula @pjfanning @slfan1989

@hadoop-yetus

Copy link
Copy Markdown

🎊 +1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 19m 4s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 codespell 0m 0s codespell was not available.
+0 🆗 detsecrets 0m 0s detect-secrets was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
+1 💚 test4tests 0m 0s The patch appears to include 1 new or modified test files.
_ trunk Compile Tests _
+1 💚 mvninstall 46m 38s trunk passed
+1 💚 compile 17m 39s trunk passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu
+1 💚 compile 18m 17s trunk passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu
+1 💚 checkstyle 1m 32s trunk passed
+1 💚 mvnsite 1m 58s trunk passed
+1 💚 javadoc 1m 26s trunk passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu
+1 💚 javadoc 1m 22s trunk passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu
+1 💚 spotbugs 3m 13s trunk passed
+1 💚 shadedclient 36m 54s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+1 💚 mvninstall 1m 15s the patch passed
+1 💚 compile 16m 49s the patch passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu
+1 💚 javac 16m 49s the patch passed
+1 💚 compile 18m 2s the patch passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu
+1 💚 javac 18m 2s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 checkstyle 1m 27s the patch passed
+1 💚 mvnsite 1m 56s the patch passed
+1 💚 javadoc 1m 22s the patch passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu
+1 💚 javadoc 1m 22s the patch passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu
+1 💚 spotbugs 3m 23s the patch passed
+1 💚 shadedclient 36m 54s patch has no errors when building and testing our client artifacts.
_ Other Tests _
+1 💚 unit 23m 35s hadoop-common in the patch passed.
+1 💚 asflicense 1m 13s The patch does not generate ASF License warnings.
256m 49s
Subsystem Report/Notes
Docker ClientAPI=1.55 ServerAPI=1.55 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8636/1/artifact/out/Dockerfile
GITHUB PR #8636
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname Linux 2143aefbcfea 5.15.0-181-generic #191-Ubuntu SMP Fri May 22 19:09:02 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision trunk / 969a095
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-8636/1/testReport/
Max. process+thread count 1284 (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-8636/1/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.

@ayushtkn ayushtkn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If this doesn't fail only, then what are we fixing. I don't think this is something we need

@anshuksi282-ksolves

Copy link
Copy Markdown
Contributor Author

If this doesn't fail only, then what are we fixing. I don't think this is something we need

Hi @ayushtkn, thanks for taking a look!

To clarify, the test does fail in specific scenarios—specifically when hadoop.security.authentication is set to kerberos (as reported in HADOOP-18814).

The issue is that when it fails in that environment, the direct cast (RemoteException)e.getCause() throws a ClassCastException. This completely swallows the actual underlying IOException, making it very confusing and difficult to debug what actually went wrong.

This PR doesn't change the core test logic; it just adds a minimal defensive check. If an exception occurs, it ensures the real root cause is surfaced instead of hiding it behind a misleading ClassCastException.

Since this was a reported pain point in JIRA (and previously validated in #6076 before going stale), I thought it would be a helpful quality-of-life improvement for developers debugging Kerberos setups. Let me know if that makes sense or if you'd still prefer to skip this!

@ayushtkn ayushtkn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the ticket itself is wrong, why would I run a test with some other config, when it isn't written for it. If the test passes doesn't logs anything irrelevant and behaving properly. IMO there is no need of code change here

@anshuksi282-ksolves

Copy link
Copy Markdown
Contributor Author

I think the ticket itself is wrong, why would I run a test with some other config, when it isn't written for it. If the test passes doesn't logs anything irrelevant and behaving properly. IMO there is no need of code change here

Hi @ayushtkn, fair point. The only reason for this PR is that sometimes developers run the entire test suite globally with Kerberos enabled. When this test fails during those runs, the ClassCastException hides the real error, which wastes debugging time.

However, if we shouldn't modify individual tests for custom global configs, I completely agree with avoiding unnecessary code changes. Just let me know, and I'm happy to close the PR!

@pan3793

pan3793 commented Jul 29, 2026

Copy link
Copy Markdown
Member

I'm fine with this defensive change, but it should not be classified as a Critical Bug as the JIRA ticket claimed, just a Minor Test improvement.

@anshuksi282-ksolves

Copy link
Copy Markdown
Contributor Author

I'm fine with this defensive change, but it should not be classified as a Critical Bug as the JIRA ticket claimed, just a Minor Test improvement.

Thanks @pan3793! I completely agree, it is definitely a minor test improvement rather than a critical bug.

Since I don't have the necessary permissions to update the JIRA ticket, could one of the maintainers please update its priority/type? Let me know if anything else is needed from my side to get this merged.

@pan3793

pan3793 commented Jul 29, 2026

Copy link
Copy Markdown
Member

@anshuksi282-ksolves, I updated the JIRA ticket, and I can help merge this unless other maintainers have objections. (will wait for 1 or 2 days)

@anshuksi282-ksolves

Copy link
Copy Markdown
Contributor Author

@anshuksi282-ksolves, I updated the JIRA ticket, and I can help merge this unless other maintainers have objections. (will wait for 1 or 2 days)

Thanks a lot for updating the JIRA and for your help with the merge, @pan3793! Sounds good, I'll wait.

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.

4 participants