Skip to content

HDFS-17892 RBF: permit not released during invokeConcurrent if getOrderedNamenodes throws an exception#8345

Open
dParikesit wants to merge 3 commits intoapache:trunkfrom
dParikesit:4-new-acquirePermit
Open

HDFS-17892 RBF: permit not released during invokeConcurrent if getOrderedNamenodes throws an exception#8345
dParikesit wants to merge 3 commits intoapache:trunkfrom
dParikesit:4-new-acquirePermit

Conversation

@dParikesit
Copy link
Contributor

JIRA: HDFS-17892

Description of PR

After HDFS-17265, we either move acquirePermit after getOrderedNamenodes or wrap getOrderedNamenodes in a try-finally block so that the permit can be released if getOrderedNamenodes throws an exception.

However, the behavior of the latest trunk (a178eb7) has regressed, where we don't handle the permit release anymore.

The test failed to detect this problem because it only use 1 locations. In invokeConcurrent, the execution flows into invokeSingle where the exception is correctly handled using this branch

else if (locations.size() == 1 && timeOutMs <= 0){      
    // Shortcut, just one call       
    return invokeSingle(locations.iterator().next(), method);     
}

In this patch, I've fixed the invokeConcurrent and the test to reflect this behavior.

How was this patch tested?

mvn -pl hadoop-hdfs-project/hadoop-hdfs-rbf -am -Dtest=TestRouterHandlersFairness#testReleasedWhenExceptionOccurs test

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?

AI Tooling

If an AI tool was used:

@hadoop-yetus
Copy link

💔 -1 overall

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

This message was automatically generated.

Copy link
Member

@KeeProMise KeeProMise left a comment

Choose a reason for hiding this comment

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

Hi @dParikesit leave comments.

}

return getRemoteResults(method, timeOutMs, controller, orderedLocations, callables);
return getRemoteResults(method, timeOutMs, controller, orderedLocations, callables);
Copy link
Member

Choose a reason for hiding this comment

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

In the getRemoteResults method, the permit will be released.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback! I have moved getRemoteResults outside the try-block and change finally to catch. Please let me know if you have any other comments.

@hadoop-yetus
Copy link

🎊 +1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 19m 36s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 codespell 0m 1s codespell was not available.
+0 🆗 detsecrets 0m 1s 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 43m 49s trunk passed
+1 💚 compile 1m 10s trunk passed with JDK Ubuntu-21.0.10+7-Ubuntu-124.04
+1 💚 compile 1m 31s trunk passed with JDK Ubuntu-17.0.18+8-Ubuntu-124.04.1
+1 💚 checkstyle 1m 1s trunk passed
+1 💚 mvnsite 1m 37s trunk passed
+1 💚 javadoc 1m 6s trunk passed with JDK Ubuntu-21.0.10+7-Ubuntu-124.04
+1 💚 javadoc 1m 5s trunk passed with JDK Ubuntu-17.0.18+8-Ubuntu-124.04.1
+1 💚 spotbugs 2m 28s trunk passed
+1 💚 shadedclient 29m 24s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+1 💚 mvninstall 1m 1s the patch passed
+1 💚 compile 0m 38s the patch passed with JDK Ubuntu-21.0.10+7-Ubuntu-124.04
+1 💚 javac 0m 38s the patch passed
+1 💚 compile 1m 1s the patch passed with JDK Ubuntu-17.0.18+8-Ubuntu-124.04.1
+1 💚 javac 1m 1s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 checkstyle 0m 27s the patch passed
+1 💚 mvnsite 1m 6s the patch passed
+1 💚 javadoc 0m 34s the patch passed with JDK Ubuntu-21.0.10+7-Ubuntu-124.04
+1 💚 javadoc 0m 33s the patch passed with JDK Ubuntu-17.0.18+8-Ubuntu-124.04.1
+1 💚 spotbugs 2m 5s the patch passed
+1 💚 shadedclient 28m 10s patch has no errors when building and testing our client artifacts.
_ Other Tests _
+1 💚 unit 53m 51s hadoop-hdfs-rbf in the patch passed.
+1 💚 asflicense 0m 36s The patch does not generate ASF License warnings.
196m 29s
Subsystem Report/Notes
Docker ClientAPI=1.54 ServerAPI=1.54 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8345/2/artifact/out/Dockerfile
GITHUB PR #8345
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname Linux e2684c0bbe43 5.15.0-173-generic #183-Ubuntu SMP Fri Mar 6 13:29:34 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision trunk / 912178b
Default Java Ubuntu-17.0.18+8-Ubuntu-124.04.1
Multi-JDK versions /usr/lib/jvm/java-21-openjdk-amd64:Ubuntu-21.0.10+7-Ubuntu-124.04 /usr/lib/jvm/java-17-openjdk-amd64:Ubuntu-17.0.18+8-Ubuntu-124.04.1
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8345/2/testReport/
Max. process+thread count 3150 (vs. ulimit of 5500)
modules C: hadoop-hdfs-project/hadoop-hdfs-rbf U: hadoop-hdfs-project/hadoop-hdfs-rbf
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8345/2/console
versions git=2.43.0 maven=3.9.11 spotbugs=4.9.7
Powered by Apache Yetus 0.14.1 https://yetus.apache.org

This message was automatically generated.

Copy link
Member

@KeeProMise KeeProMise left a comment

Choose a reason for hiding this comment

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

LGTM! @dParikesit Let's wait for two business days to see if anyone else leaves a comment.

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.

3 participants