You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "dParikesit (via GitHub)" <gi...@apache.org> on 2026/03/14 14:41:56 UTC
[PR] HDFS-17892 RBF: permit not released during invokeConcurrent if getOrderedNamenodes throws an exception [hadoop]
dParikesit opened a new pull request, #8345:
URL: https://github.com/apache/hadoop/pull/8345
JIRA: [HDFS-17892](https://issues.apache.org/jira/browse/HDFS-17892)
### Description of PR
After [HDFS-17265](https://issues.apache.org/jira/browse/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:
- [X] 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](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the `LICENSE`, `LICENSE-binary`, `NOTICE-binary` files?
### AI Tooling
If an AI tool was used:
- [ ] The PR includes the phrase "Contains content generated by <tool>"
where <tool> is the name of the AI tool used.
- [ ] My use of AI contributions follows the ASF legal policy
https://www.apache.org/legal/generative-tooling.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HDFS-17892 RBF: permit not released during invokeConcurrent if getOrderedNamenodes throws an exception [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #8345:
URL: https://github.com/apache/hadoop/pull/8345#issuecomment-4092332622
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 19m 36s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 43m 49s | | trunk passed |
| +1 :green_heart: | compile | 1m 10s | | trunk passed with JDK Ubuntu-21.0.10+7-Ubuntu-124.04 |
| +1 :green_heart: | compile | 1m 31s | | trunk passed with JDK Ubuntu-17.0.18+8-Ubuntu-124.04.1 |
| +1 :green_heart: | checkstyle | 1m 1s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 37s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 6s | | trunk passed with JDK Ubuntu-21.0.10+7-Ubuntu-124.04 |
| +1 :green_heart: | javadoc | 1m 5s | | trunk passed with JDK Ubuntu-17.0.18+8-Ubuntu-124.04.1 |
| +1 :green_heart: | spotbugs | 2m 28s | | trunk passed |
| +1 :green_heart: | shadedclient | 29m 24s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 1s | | the patch passed |
| +1 :green_heart: | compile | 0m 38s | | the patch passed with JDK Ubuntu-21.0.10+7-Ubuntu-124.04 |
| +1 :green_heart: | javac | 0m 38s | | the patch passed |
| +1 :green_heart: | compile | 1m 1s | | the patch passed with JDK Ubuntu-17.0.18+8-Ubuntu-124.04.1 |
| +1 :green_heart: | javac | 1m 1s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 27s | | the patch passed |
| +1 :green_heart: | mvnsite | 1m 6s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 34s | | the patch passed with JDK Ubuntu-21.0.10+7-Ubuntu-124.04 |
| +1 :green_heart: | javadoc | 0m 33s | | the patch passed with JDK Ubuntu-17.0.18+8-Ubuntu-124.04.1 |
| +1 :green_heart: | spotbugs | 2m 5s | | the patch passed |
| +1 :green_heart: | shadedclient | 28m 10s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 53m 51s | | hadoop-hdfs-rbf in the patch passed. |
| +1 :green_heart: | 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 | https://github.com/apache/hadoop/pull/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 / 912178b7e372ff2726e5d30818f1d4f5ed83ec28 |
| 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HDFS-17892 RBF: permit not released during invokeConcurrent if getOrderedNamenodes throws an exception [hadoop]
Posted by "dParikesit (via GitHub)" <gi...@apache.org>.
dParikesit commented on code in PR #8345:
URL: https://github.com/apache/hadoop/pull/8345#discussion_r2960672359
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java:
##########
@@ -1599,51 +1599,55 @@ protected static <T extends RemoteLocationContext, R> Map<T, R> postProcessResul
// transfer originCall & callerContext to worker threads of executor.
final Call originCall = Server.getCurCall().get();
final CallerContext originContext = CallerContext.getCurrent();
- for (final T location : locations) {
- String nsId = location.getNameserviceId();
- boolean isObserverRead = isObserverReadEligible(nsId, m);
- final List<? extends FederationNamenodeContext> namenodes =
- getOrderedNamenodes(nsId, isObserverRead);
- final Class<?> proto = method.getProtocol();
- final Object[] paramList = method.getParams(location);
- if (standby) {
- // Call the objectGetter to all NNs (including standby)
- for (final FederationNamenodeContext nn : namenodes) {
- String nnId = nn.getNamenodeId();
- final List<FederationNamenodeContext> nnList =
- Collections.singletonList(nn);
- T nnLocation = location;
- if (location instanceof RemoteLocation) {
- nnLocation = (T)new RemoteLocation(nsId, nnId, location.getDest());
+ try{
+ for (final T location : locations) {
+ String nsId = location.getNameserviceId();
+ boolean isObserverRead = isObserverReadEligible(nsId, m);
+ final List<? extends FederationNamenodeContext> namenodes =
+ getOrderedNamenodes(nsId, isObserverRead);
+ final Class<?> proto = method.getProtocol();
+ final Object[] paramList = method.getParams(location);
+ if (standby) {
+ // Call the objectGetter to all NNs (including standby)
+ for (final FederationNamenodeContext nn : namenodes) {
+ String nnId = nn.getNamenodeId();
+ final List<FederationNamenodeContext> nnList =
+ Collections.singletonList(nn);
+ T nnLocation = location;
+ if (location instanceof RemoteLocation) {
+ nnLocation = (T)new RemoteLocation(nsId, nnId, location.getDest());
+ }
+ orderedLocations.add(nnLocation);
+ callables.add(
+ () -> {
+ transferThreadLocalContext(originCall, originContext);
+ return invokeMethod(
+ ugi, nnList, isObserverRead, proto, m, paramList);
+ });
}
- orderedLocations.add(nnLocation);
+ } else {
+ // Call the objectGetter in order of nameservices in the NS list
+ orderedLocations.add(location);
callables.add(
() -> {
transferThreadLocalContext(originCall, originContext);
return invokeMethod(
- ugi, nnList, isObserverRead, proto, m, paramList);
+ ugi, namenodes, isObserverRead, proto, m, paramList);
});
}
- } else {
- // Call the objectGetter in order of nameservices in the NS list
- orderedLocations.add(location);
- callables.add(
- () -> {
- transferThreadLocalContext(originCall, originContext);
- return invokeMethod(
- ugi, namenodes, isObserverRead, proto, m, paramList);
- });
}
- }
- if (rpcMonitor != null) {
- rpcMonitor.proxyOp();
- }
- if (this.router.getRouterClientMetrics() != null) {
- this.router.getRouterClientMetrics().incInvokedConcurrent(m);
- }
+ if (rpcMonitor != null) {
+ rpcMonitor.proxyOp();
+ }
+ if (this.router.getRouterClientMetrics() != null) {
+ this.router.getRouterClientMetrics().incInvokedConcurrent(m);
+ }
- return getRemoteResults(method, timeOutMs, controller, orderedLocations, callables);
+ return getRemoteResults(method, timeOutMs, controller, orderedLocations, callables);
Review Comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HDFS-17892 RBF: permit not released during invokeConcurrent if getOrderedNamenodes throws an exception [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #8345:
URL: https://github.com/apache/hadoop/pull/8345#issuecomment-4060659465
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 0s | | Docker mode activated. |
| -1 :x: | patch | 0m 25s | | https://github.com/apache/hadoop/pull/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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HDFS-17892 RBF: permit not released during invokeConcurrent if getOrderedNamenodes throws an exception [hadoop]
Posted by "dParikesit (via GitHub)" <gi...@apache.org>.
dParikesit commented on PR #8345:
URL: https://github.com/apache/hadoop/pull/8345#issuecomment-4107609018
Thanks @KeeProMise for reviewing!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HDFS-17892 RBF: permit not released during invokeConcurrent if getOrderedNamenodes throws an exception [hadoop]
Posted by "KeeProMise (via GitHub)" <gi...@apache.org>.
KeeProMise merged PR #8345:
URL: https://github.com/apache/hadoop/pull/8345
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HDFS-17892 RBF: permit not released during invokeConcurrent if getOrderedNamenodes throws an exception [hadoop]
Posted by "KeeProMise (via GitHub)" <gi...@apache.org>.
KeeProMise commented on PR #8345:
URL: https://github.com/apache/hadoop/pull/8345#issuecomment-4107597485
Hi @dParikesit thanks for your contribution.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org