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 GitBox <gi...@apache.org> on 2022/12/30 08:14:18 UTC

[GitHub] [hadoop] hfutatzhanghb opened a new pull request, #5262: HDFS-16880. RBF: modify invokeSingleXXX interface in order to pass actual file src to namenode.

hfutatzhanghb opened a new pull request, #5262:
URL: https://github.com/apache/hadoop/pull/5262

   We found lots of INFO level log like below:
   
   >2022-12-30 15:31:04,169 INFO org.apache.hadoop.hdfs.StateChange: DIR* completeFile: / is closed by DFSClient_attempt_1671783180362_213003_m_000077_0_1102875551_1
   2022-12-30 15:31:04,186 INFO org.apache.hadoop.hdfs.StateChange: DIR* completeFile: / is closed by DFSClient_NONMAPREDUCE_1198313144_27480
   
   It lost the real path of completeFile. Actually this is caused by : 
   
   >org.apache.hadoop.hdfs.server.federation.router.RouterRpcClient#invokeSingle(java.lang.String, org.apache.hadoop.hdfs.server.federation.router.RemoteMethod)
   
   In this method, it instantiates a RemoteLocationContext object:
   
   >RemoteLocationContext loc = new RemoteLocation(nsId, "/", "/");
   
   and then execute: Object[] params = method.getParams(loc);
   
   The problem is right here, becasuse we always use new RemoteParam(), so, 
   
   context.getDest() always return "/"; That's why we saw lots of incorrect logs.
   
    
   
   After diving into invokeSingleXXX source code, I found the following RPCs classified as need actual src and not need actual src.
   
    
   
   **need src path RPC:**
   
   addBlock、abandonBlock、getAdditionalDatanode、complete
   
   **not need src path RPC:**
   
   updateBlockForPipeline、reportBadBlocks、getBlocks、updatePipeline、invokeAtAvailableNs(invoked by: getServerDefaults、getBlockKeys、getTransactionID、getMostRecentCheckpointTxId、versionRequest、getStoragePolicies)
   
    
   
   After changes, the src can be pass to NN correctly.


-- 
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


[GitHub] [hadoop] goiri commented on a diff in pull request #5262: HDFS-16880. RBF: modify invokeSingleXXX interface in order to pass actual file src to namenode.

Posted by GitBox <gi...@apache.org>.
goiri commented on code in PR #5262:
URL: https://github.com/apache/hadoop/pull/5262#discussion_r1061651607


##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcServer.java:
##########
@@ -687,7 +687,7 @@ <T> T invokeAtAvailableNs(RemoteMethod method, Class<T> clazz)
     // If default Ns is present return result from that namespace.
     if (!nsId.isEmpty()) {
       try {
-        return rpcClient.invokeSingle(nsId, method, clazz);
+        return rpcClient.invokeSingle(nsId, method, clazz, "");

Review Comment:
   I meant to make it part of RemoteMethod.



-- 
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


[GitHub] [hadoop] hfutatzhanghb commented on a diff in pull request #5262: HDFS-16880. RBF: modify invokeSingleXXX interface in order to pass actual file src to namenode.

Posted by GitBox <gi...@apache.org>.
hfutatzhanghb commented on code in PR #5262:
URL: https://github.com/apache/hadoop/pull/5262#discussion_r1060434510


##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcServer.java:
##########
@@ -687,7 +687,7 @@ <T> T invokeAtAvailableNs(RemoteMethod method, Class<T> clazz)
     // If default Ns is present return result from that namespace.
     if (!nsId.isEmpty()) {
       try {
-        return rpcClient.invokeSingle(nsId, method, clazz);
+        return rpcClient.invokeSingle(nsId, method, clazz, "");

Review Comment:
   > > Yes,the src is not the dst path, but we can use the src to lookup real dst path in mount table. Here, we can not execute this.subclusterResolver.getMountPoints(path);, because the statement is time-consuming.
   > 
   > From correctness point of view it isn't correct. If `/mount/path` points to Ns0 `/dir` and we log` /mount/path` in the namenode, it can lead to confusions if the namenode also has a path named `/mount/path`, If you operate on `/dir` via router `/mount/path` and if you directly operate on `/mount/path` on the namenode.
   > 
   > If it has serious performance penalties we need to make it configurable and then figure out what can be done when the config is disabled.
   
   Hi, @ayushtkn , can we prepend some specific prefix, such as "RBF#src_path:" to the dir?  In this way, we can distinguish RBF scene from direct namenode scene. What’s your opinion?  I am looking forward to your reply and do the next steps. thanks again~



-- 
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


[GitHub] [hadoop] hfutatzhanghb commented on pull request #5262: HDFS-16880. RBF: modify invokeSingleXXX interface in order to pass actual file src to namenode.

Posted by GitBox <gi...@apache.org>.
hfutatzhanghb commented on PR #5262:
URL: https://github.com/apache/hadoop/pull/5262#issuecomment-1368673356

   > > Yes,the src is not the dst path, but we can use the src to lookup real dst path in mount table. Here, we can not execute this.subclusterResolver.getMountPoints(path);, because the statement is time-consuming.
   > 
   > From correctness point of view it isn't correct. If `/mount/path` points to Ns0 `/dir` and we log` /mount/path` in the namenode, it can lead to confusions if the namenode also has a path named `/mount/path`, If you operate on `/dir` via router `/mount/path` and if you directly operate on `/mount/path` on the namenode.
   > 
   > If it has serious performance penalties we need to make it configurable and then figure out what can be done when the config is disabled.
   
   Thanks @ayushtkn again, i will continue to work on this problem.
   


-- 
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


[GitHub] [hadoop] hadoop-yetus commented on pull request #5262: HDFS-16880. RBF: modify invokeSingleXXX interface in order to pass actual file src to namenode.

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #5262:
URL: https://github.com/apache/hadoop/pull/5262#issuecomment-1367857104

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   3m 51s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  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 2 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  43m 27s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 51s |  |  trunk passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04  |
   | +1 :green_heart: |  compile  |   0m 40s |  |  trunk passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08  |
   | +1 :green_heart: |  checkstyle  |   0m 32s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 44s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   0m 49s |  |  trunk passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04  |
   | +1 :green_heart: |  javadoc  |   0m 55s |  |  trunk passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08  |
   | +1 :green_heart: |  spotbugs  |   1m 38s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  25m  0s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 36s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 38s |  |  the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04  |
   | +1 :green_heart: |  javac  |   0m 38s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 31s |  |  the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08  |
   | +1 :green_heart: |  javac  |   0m 31s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   0m 16s |  |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   0m 35s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   0m 33s |  |  the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04  |
   | +1 :green_heart: |  javadoc  |   0m 47s |  |  the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08  |
   | +1 :green_heart: |  spotbugs  |   1m 28s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  23m 40s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  41m 24s |  |  hadoop-hdfs-rbf in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 34s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 150m 41s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/3/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/5262 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
   | uname | Linux 36e81642d4ac 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / b4b3d29bff51985ab4b04a43b16d036edc5b7362 |
   | Default Java | Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/3/testReport/ |
   | Max. process+thread count | 2169 (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-5262/3/console |
   | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.14.0 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


[GitHub] [hadoop] hadoop-yetus commented on pull request #5262: HDFS-16880. RBF: modify invokeSingleXXX interface in order to pass actual file src to namenode.

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #5262:
URL: https://github.com/apache/hadoop/pull/5262#issuecomment-1367828698

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 54s |  |  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 2 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  41m 46s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 43s |  |  trunk passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04  |
   | +1 :green_heart: |  compile  |   0m 35s |  |  trunk passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08  |
   | +1 :green_heart: |  checkstyle  |   0m 31s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 42s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   0m 49s |  |  trunk passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04  |
   | +1 :green_heart: |  javadoc  |   0m 54s |  |  trunk passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08  |
   | +1 :green_heart: |  spotbugs  |   1m 29s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  23m 52s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | -1 :x: |  mvninstall  |   0m 24s | [/patch-mvninstall-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/1/artifact/out/patch-mvninstall-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) |  hadoop-hdfs-rbf in the patch failed.  |
   | -1 :x: |  compile  |   0m 25s | [/patch-compile-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/1/artifact/out/patch-compile-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt) |  hadoop-hdfs-rbf in the patch failed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.  |
   | -1 :x: |  javac  |   0m 25s | [/patch-compile-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/1/artifact/out/patch-compile-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt) |  hadoop-hdfs-rbf in the patch failed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.  |
   | -1 :x: |  compile  |   0m 22s | [/patch-compile-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkPrivateBuild-1.8.0_352-8u352-ga-1~20.04-b08.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/1/artifact/out/patch-compile-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkPrivateBuild-1.8.0_352-8u352-ga-1~20.04-b08.txt) |  hadoop-hdfs-rbf in the patch failed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08.  |
   | -1 :x: |  javac  |   0m 22s | [/patch-compile-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkPrivateBuild-1.8.0_352-8u352-ga-1~20.04-b08.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/1/artifact/out/patch-compile-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkPrivateBuild-1.8.0_352-8u352-ga-1~20.04-b08.txt) |  hadoop-hdfs-rbf in the patch failed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08.  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | -0 :warning: |  checkstyle  |   0m 17s | [/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/1/artifact/out/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) |  hadoop-hdfs-project/hadoop-hdfs-rbf: The patch generated 1 new + 5 unchanged - 0 fixed = 6 total (was 5)  |
   | -1 :x: |  mvnsite  |   0m 23s | [/patch-mvnsite-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/1/artifact/out/patch-mvnsite-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) |  hadoop-hdfs-rbf in the patch failed.  |
   | -1 :x: |  javadoc  |   0m 20s | [/patch-javadoc-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/1/artifact/out/patch-javadoc-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt) |  hadoop-hdfs-rbf in the patch failed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.  |
   | -1 :x: |  javadoc  |   0m 49s | [/results-javadoc-javadoc-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkPrivateBuild-1.8.0_352-8u352-ga-1~20.04-b08.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/1/artifact/out/results-javadoc-javadoc-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkPrivateBuild-1.8.0_352-8u352-ga-1~20.04-b08.txt) |  hadoop-hdfs-project_hadoop-hdfs-rbf-jdkPrivateBuild-1.8.0_352-8u352-ga-1~20.04-b08 with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)  |
   | -1 :x: |  spotbugs  |   0m 22s | [/patch-spotbugs-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/1/artifact/out/patch-spotbugs-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) |  hadoop-hdfs-rbf in the patch failed.  |
   | +1 :green_heart: |  shadedclient  |  25m 32s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | -1 :x: |  unit  |   0m 25s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/1/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) |  hadoop-hdfs-rbf in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   0m 34s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 101m 21s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/1/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/5262 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
   | uname | Linux 0992e87a686e 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 1d2154d5e939a0718698a6e6d7fae9ea129655b8 |
   | Default Java | Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/1/testReport/ |
   | Max. process+thread count | 622 (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-5262/1/console |
   | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.14.0 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


[GitHub] [hadoop] hfutatzhanghb commented on a diff in pull request #5262: HDFS-16880. RBF: modify invokeSingleXXX interface in order to pass actual file src to namenode.

Posted by GitBox <gi...@apache.org>.
hfutatzhanghb commented on code in PR #5262:
URL: https://github.com/apache/hadoop/pull/5262#discussion_r1060435056


##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcServer.java:
##########
@@ -687,7 +687,7 @@ <T> T invokeAtAvailableNs(RemoteMethod method, Class<T> clazz)
     // If default Ns is present return result from that namespace.
     if (!nsId.isEmpty()) {
       try {
-        return rpcClient.invokeSingle(nsId, method, clazz);
+        return rpcClient.invokeSingle(nsId, method, clazz, "");

Review Comment:
   > Can we add an actual test?
   
   OK~ @goiri , I will add actual test soon~ thanks a lot.



-- 
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


[GitHub] [hadoop] hfutatzhanghb commented on pull request #5262: HDFS-16880. RBF: modify invokeSingleXXX interface in order to pass actual file src to namenode.

Posted by GitBox <gi...@apache.org>.
hfutatzhanghb commented on PR #5262:
URL: https://github.com/apache/hadoop/pull/5262#issuecomment-1367786684

   hi, @tomscut @ayushtkn , could you please help me to review the code. thks!


-- 
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


[GitHub] [hadoop] hfutatzhanghb closed pull request #5262: HDFS-16880. RBF: modify invokeSingleXXX interface in order to pass actual file src to namenode.

Posted by "hfutatzhanghb (via GitHub)" <gi...@apache.org>.
hfutatzhanghb closed pull request #5262:  HDFS-16880. RBF: modify invokeSingleXXX interface in order to pass actual file src to namenode.
URL: https://github.com/apache/hadoop/pull/5262


-- 
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


[GitHub] [hadoop] ayushtkn commented on pull request #5262: HDFS-16880. RBF: modify invokeSingleXXX interface in order to pass actual file src to namenode.

Posted by GitBox <gi...@apache.org>.
ayushtkn commented on PR #5262:
URL: https://github.com/apache/hadoop/pull/5262#issuecomment-1368630324

   > Yes,the src is not the dst path, but we can use the src to lookup real dst path in mount table. Here, we can not execute this.subclusterResolver.getMountPoints(path);, because the statement is time-consuming.
   
   From correctness point of view it isn't correct. If ``/mount/path`` points to Ns0 ``/dir`` and we log`` /mount/path`` in the namenode, it can lead to confusions if the namenode also has a path named ``/mount/path``, If you operate on ``/dir`` via router ``/mount/path`` and if you directly operate on ``/mount/path`` on the namenode.
   
   If it has serious performance penalties we need to make it configurable and then figure out what can be done when the config is disabled.
   


-- 
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


[GitHub] [hadoop] hadoop-yetus commented on pull request #5262: HDFS-16880. RBF: modify invokeSingleXXX interface in order to pass actual file src to namenode.

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #5262:
URL: https://github.com/apache/hadoop/pull/5262#issuecomment-1367837057

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   1m  1s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  2s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  2s |  |  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 2 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  44m  4s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 48s |  |  trunk passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04  |
   | +1 :green_heart: |  compile  |   0m 36s |  |  trunk passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08  |
   | +1 :green_heart: |  checkstyle  |   0m 31s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 43s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   0m 51s |  |  trunk passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04  |
   | +1 :green_heart: |  javadoc  |   0m 57s |  |  trunk passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08  |
   | +1 :green_heart: |  spotbugs  |   1m 36s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  24m 54s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | -1 :x: |  mvninstall  |   0m 24s | [/patch-mvninstall-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/2/artifact/out/patch-mvninstall-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) |  hadoop-hdfs-rbf in the patch failed.  |
   | -1 :x: |  compile  |   0m 25s | [/patch-compile-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/2/artifact/out/patch-compile-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt) |  hadoop-hdfs-rbf in the patch failed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.  |
   | -1 :x: |  javac  |   0m 25s | [/patch-compile-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/2/artifact/out/patch-compile-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt) |  hadoop-hdfs-rbf in the patch failed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.  |
   | -1 :x: |  compile  |   0m 25s | [/patch-compile-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkPrivateBuild-1.8.0_352-8u352-ga-1~20.04-b08.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/2/artifact/out/patch-compile-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkPrivateBuild-1.8.0_352-8u352-ga-1~20.04-b08.txt) |  hadoop-hdfs-rbf in the patch failed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08.  |
   | -1 :x: |  javac  |   0m 25s | [/patch-compile-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkPrivateBuild-1.8.0_352-8u352-ga-1~20.04-b08.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/2/artifact/out/patch-compile-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkPrivateBuild-1.8.0_352-8u352-ga-1~20.04-b08.txt) |  hadoop-hdfs-rbf in the patch failed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08.  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   0m 19s |  |  the patch passed  |
   | -1 :x: |  mvnsite  |   0m 25s | [/patch-mvnsite-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/2/artifact/out/patch-mvnsite-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) |  hadoop-hdfs-rbf in the patch failed.  |
   | +1 :green_heart: |  javadoc  |   0m 32s |  |  the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04  |
   | +1 :green_heart: |  javadoc  |   0m 48s |  |  the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08  |
   | -1 :x: |  spotbugs  |   0m 23s | [/patch-spotbugs-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/2/artifact/out/patch-spotbugs-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) |  hadoop-hdfs-rbf in the patch failed.  |
   | +1 :green_heart: |  shadedclient  |  26m 31s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | -1 :x: |  unit  |   0m 25s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/2/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) |  hadoop-hdfs-rbf in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   0m 33s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 106m  2s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/2/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/5262 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
   | uname | Linux 9b53970e6b6c 4.15.0-200-generic #211-Ubuntu SMP Thu Nov 24 18:16:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / e566d73bdc42ad3f1248940295f7d150c7bdee0d |
   | Default Java | Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5262/2/testReport/ |
   | Max. process+thread count | 608 (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-5262/2/console |
   | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.14.0 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


[GitHub] [hadoop] ayushtkn commented on a diff in pull request #5262: HDFS-16880. RBF: modify invokeSingleXXX interface in order to pass actual file src to namenode.

Posted by GitBox <gi...@apache.org>.
ayushtkn commented on code in PR #5262:
URL: https://github.com/apache/hadoop/pull/5262#discussion_r1060442910


##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcServer.java:
##########
@@ -687,7 +687,7 @@ <T> T invokeAtAvailableNs(RemoteMethod method, Class<T> clazz)
     // If default Ns is present return result from that namespace.
     if (!nsId.isEmpty()) {
       try {
-        return rpcClient.invokeSingle(nsId, method, clazz);
+        return rpcClient.invokeSingle(nsId, method, clazz, "");

Review Comment:
   @goiri will that be ok to have a prefix then path wrt router rather than the actual path wrt namenode



-- 
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


[GitHub] [hadoop] hfutatzhanghb commented on pull request #5262: HDFS-16880. RBF: modify invokeSingleXXX interface in order to pass actual file src to namenode.

Posted by GitBox <gi...@apache.org>.
hfutatzhanghb commented on PR #5262:
URL: https://github.com/apache/hadoop/pull/5262#issuecomment-1368616110

   > Thanx @hfutatzhanghb for the fix. Had a quick look I was able to reproduce the issue. But I don't think the fix is correct. It is logging the path wrt the router not wrt the namespace. say, /mount -> ns0. /dir is a mount entry you create /mount/fiel1 on the namenode of ns0, with your fix it would be logging /mount/file1, which in actual should be /dir/file1
   > 
   > You need to extend a UT as well for the change.
   > 
   > Quick Suggestion for the fix, create a new invokeSingle method with the current signature, which calls your new invokeSingle with some default value, in order to avoid changes at places you don't have src
   
   Thanx @ayushtkn for your careful review. I was totally agree with your opinions。Yes,the src is not the dst path, but we can use the src to lookup real dst path in mount table. Here, we can not execute `this.subclusterResolver.getMountPoints(path);`, because the statement is time-consuming. It violates the purpose that we use previous block. I am looking forward to your reply and do the next steps. thanks again~


-- 
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


[GitHub] [hadoop] goiri commented on a diff in pull request #5262: HDFS-16880. RBF: modify invokeSingleXXX interface in order to pass actual file src to namenode.

Posted by GitBox <gi...@apache.org>.
goiri commented on code in PR #5262:
URL: https://github.com/apache/hadoop/pull/5262#discussion_r1059897696


##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java:
##########
@@ -865,18 +867,24 @@ public Object invokeSingleBlockPool(final String bpId, RemoteMethod method)
    *
    * @param nsId Target namespace for the method.
    * @param method The remote method and parameters to invoke.
+   * @param src file path, pass "" if not have.
    * @return The result of invoking the method.
    * @throws IOException If the invoke generated an error.
    */
-  public Object invokeSingle(final String nsId, RemoteMethod method)
+  public Object invokeSingle(final String nsId, RemoteMethod method, String src)
       throws IOException {
     UserGroupInformation ugi = RouterRpcServer.getRemoteUser();
     RouterRpcFairnessPolicyController controller = getRouterRpcFairnessPolicyController();
     acquirePermit(nsId, ugi, method, controller);
     try {
       boolean isObserverRead = isObserverReadEligible(nsId, method.getMethod());
       List<? extends FederationNamenodeContext> nns = getOrderedNamenodes(nsId, isObserverRead);
-      RemoteLocationContext loc = new RemoteLocation(nsId, "/", "/");
+      RemoteLocationContext loc;
+      if (org.apache.commons.lang3.StringUtils.isBlank(src)) {
+        loc = new RemoteLocation(nsId, "/", "/");
+      } else {
+        loc = new RemoteLocation(nsId, src, "/");

Review Comment:
   Couldn't we pass this in the Remote Method? 



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java:
##########
@@ -865,18 +867,24 @@ public Object invokeSingleBlockPool(final String bpId, RemoteMethod method)
    *
    * @param nsId Target namespace for the method.
    * @param method The remote method and parameters to invoke.
+   * @param src file path, pass "" if not have.
    * @return The result of invoking the method.
    * @throws IOException If the invoke generated an error.
    */
-  public Object invokeSingle(final String nsId, RemoteMethod method)
+  public Object invokeSingle(final String nsId, RemoteMethod method, String src)
       throws IOException {
     UserGroupInformation ugi = RouterRpcServer.getRemoteUser();
     RouterRpcFairnessPolicyController controller = getRouterRpcFairnessPolicyController();
     acquirePermit(nsId, ugi, method, controller);
     try {
       boolean isObserverRead = isObserverReadEligible(nsId, method.getMethod());
       List<? extends FederationNamenodeContext> nns = getOrderedNamenodes(nsId, isObserverRead);
-      RemoteLocationContext loc = new RemoteLocation(nsId, "/", "/");
+      RemoteLocationContext loc;
+      if (org.apache.commons.lang3.StringUtils.isBlank(src)) {

Review Comment:
   Why don't we use null as the default instead of blank? 



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcServer.java:
##########
@@ -687,7 +687,7 @@ <T> T invokeAtAvailableNs(RemoteMethod method, Class<T> clazz)
     // If default Ns is present return result from that namespace.
     if (!nsId.isEmpty()) {
       try {
-        return rpcClient.invokeSingle(nsId, method, clazz);
+        return rpcClient.invokeSingle(nsId, method, clazz, "");

Review Comment:
   Can we add an actual test? 



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java:
##########
@@ -865,18 +867,24 @@ public Object invokeSingleBlockPool(final String bpId, RemoteMethod method)
    *
    * @param nsId Target namespace for the method.
    * @param method The remote method and parameters to invoke.
+   * @param src file path, pass "" if not have.
    * @return The result of invoking the method.
    * @throws IOException If the invoke generated an error.
    */
-  public Object invokeSingle(final String nsId, RemoteMethod method)
+  public Object invokeSingle(final String nsId, RemoteMethod method, String src)

Review Comment:
   We should keep the old signature which sets it to "". 



-- 
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


[GitHub] [hadoop] hfutatzhanghb commented on pull request #5262: HDFS-16880. RBF: modify invokeSingleXXX interface in order to pass actual file src to namenode.

Posted by GitBox <gi...@apache.org>.
hfutatzhanghb commented on PR #5262:
URL: https://github.com/apache/hadoop/pull/5262#issuecomment-1368616370

   > 
   
   Thanks @ZanderXu for your comments. Yes, it was duplicated. I read the code of [HDFS-16865](https://issues.apache.org/jira/browse/HDFS-16865) and I think we can not execute this.subclusterResolver.getMountPoints(path);, because the statement is time-consuming. It violates the purpose that we use previous block. I am looking forward to your reply.


-- 
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


[GitHub] [hadoop] ZanderXu commented on pull request #5262: HDFS-16880. RBF: modify invokeSingleXXX interface in order to pass actual file src to namenode.

Posted by GitBox <gi...@apache.org>.
ZanderXu commented on PR #5262:
URL: https://github.com/apache/hadoop/pull/5262#issuecomment-1368197229

   Thanks @hfutatzhanghb , this PR may be redundancy, it seems to be the same as HDFS-16865.
   
   @ayushtkn @hfutatzhanghb Can we close this PR and fix this bug in HDFS-16865. And can you help me review the PR [5200](https://github.com/apache/hadoop/pull/5200)


-- 
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


[GitHub] [hadoop] hfutatzhanghb commented on a diff in pull request #5262: HDFS-16880. RBF: modify invokeSingleXXX interface in order to pass actual file src to namenode.

Posted by GitBox <gi...@apache.org>.
hfutatzhanghb commented on code in PR #5262:
URL: https://github.com/apache/hadoop/pull/5262#discussion_r1061099106


##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcServer.java:
##########
@@ -687,7 +687,7 @@ <T> T invokeAtAvailableNs(RemoteMethod method, Class<T> clazz)
     // If default Ns is present return result from that namespace.
     if (!nsId.isEmpty()) {
       try {
-        return rpcClient.invokeSingle(nsId, method, clazz);
+        return rpcClient.invokeSingle(nsId, method, clazz, "");

Review Comment:
   Hi, @ayushtkn @goiri . Could you please take a look at https://issues.apache.org/jira/browse/HDFS-16882
   is it feasible? thanks a lot.



-- 
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


[GitHub] [hadoop] goiri commented on a diff in pull request #5262: HDFS-16880. RBF: modify invokeSingleXXX interface in order to pass actual file src to namenode.

Posted by GitBox <gi...@apache.org>.
goiri commented on code in PR #5262:
URL: https://github.com/apache/hadoop/pull/5262#discussion_r1060525087


##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcServer.java:
##########
@@ -687,7 +687,7 @@ <T> T invokeAtAvailableNs(RemoteMethod method, Class<T> clazz)
     // If default Ns is present return result from that namespace.
     if (!nsId.isEmpty()) {
       try {
-        return rpcClient.invokeSingle(nsId, method, clazz);
+        return rpcClient.invokeSingle(nsId, method, clazz, "");

Review Comment:
   Can't we make it an actual field or attribute? 



-- 
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


[GitHub] [hadoop] hfutatzhanghb commented on a diff in pull request #5262: HDFS-16880. RBF: modify invokeSingleXXX interface in order to pass actual file src to namenode.

Posted by GitBox <gi...@apache.org>.
hfutatzhanghb commented on code in PR #5262:
URL: https://github.com/apache/hadoop/pull/5262#discussion_r1060536835


##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcServer.java:
##########
@@ -687,7 +687,7 @@ <T> T invokeAtAvailableNs(RemoteMethod method, Class<T> clazz)
     // If default Ns is present return result from that namespace.
     if (!nsId.isEmpty()) {
       try {
-        return rpcClient.invokeSingle(nsId, method, clazz);
+        return rpcClient.invokeSingle(nsId, method, clazz, "");

Review Comment:
   Hi, @goiri , here if we make it an actual field or attribute, It would invoke `getLocationsForPath` to get actual path.
   Then,  It violates the purpose that we use previous block to improve some RPC performance.



-- 
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


[GitHub] [hadoop] hfutatzhanghb commented on a diff in pull request #5262: HDFS-16880. RBF: modify invokeSingleXXX interface in order to pass actual file src to namenode.

Posted by GitBox <gi...@apache.org>.
hfutatzhanghb commented on code in PR #5262:
URL: https://github.com/apache/hadoop/pull/5262#discussion_r1060537469


##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcServer.java:
##########
@@ -687,7 +687,7 @@ <T> T invokeAtAvailableNs(RemoteMethod method, Class<T> clazz)
     // If default Ns is present return result from that namespace.
     if (!nsId.isEmpty()) {
       try {
-        return rpcClient.invokeSingle(nsId, method, clazz);
+        return rpcClient.invokeSingle(nsId, method, clazz, "");

Review Comment:
   Hi,@goiri ,  could you please also review the code of https://issues.apache.org/jira/browse/HDFS-16865



-- 
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