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 2021/11/09 11:20:16 UTC

[GitHub] [hadoop] tomscut opened a new pull request #3635: HDFS-16310. RBF: Add client port to CallerContext for Router

tomscut opened a new pull request #3635:
URL: https://github.com/apache/hadoop/pull/3635


   JIRA: [HDFS-16310](https://issues.apache.org/jira/browse/HDFS-16310).
   
   We mentioned in [#3538](https://github.com/apache/hadoop/pull/3538#issuecomment-955108462) that adding the client port to the CallerContext of the Router.
   
   


-- 
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] tasanuma commented on pull request #3635: HDFS-16310. RBF: Add client port to CallerContext for Router

Posted by GitBox <gi...@apache.org>.
tasanuma commented on pull request #3635:
URL: https://github.com/apache/hadoop/pull/3635#issuecomment-972469127


   @tomscut Thank you for your explanation. It makes sense to me. Now I'm +1.


-- 
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 change in pull request #3635: HDFS-16310. RBF: Add client port to CallerContext for Router

Posted by GitBox <gi...@apache.org>.
goiri commented on a change in pull request #3635:
URL: https://github.com/apache/hadoop/pull/3635#discussion_r745844503



##########
File path: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/CallerContext.java
##########
@@ -228,6 +228,26 @@ public Builder append(String key, String value) {
       return this;
     }
 
+    /**
+     * Append new field which contains key and value to the context
+     * if the key("key:") is absent.
+     * @param key the key of field.
+     * @param value the value of field.
+     * @return the builder.
+     */
+    public Builder appendIfAbsent(String key, String value) {

Review comment:
       As this is pretty core; I would prefer to do the changes to commons in a HADOOP JIRA.




-- 
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] tomscut commented on a change in pull request #3635: HDFS-16310. RBF: Add client port to CallerContext for Router

Posted by GitBox <gi...@apache.org>.
tomscut commented on a change in pull request #3635:
URL: https://github.com/apache/hadoop/pull/3635#discussion_r750410323



##########
File path: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/CallerContext.java
##########
@@ -228,6 +228,26 @@ public Builder append(String key, String value) {
       return this;
     }
 
+    /**
+     * Append new field which contains key and value to the context
+     * if the key("key:") is absent.
+     * @param key the key of field.
+     * @param value the value of field.
+     * @return the builder.
+     */
+    public Builder appendIfAbsent(String key, String value) {

Review comment:
       > As this is pretty core; I would prefer to do the changes to commons in a HADOOP JIRA.
   
   Hi @goiri , I filed a HADOOP JIRA [HADOOP-18003](https://issues.apache.org/jira/browse/HADOOP-18003) and the relevant PR has been merged. I rebased this PR. Could you please help review the code. Thank you very much.




-- 
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 #3635: HDFS-16310. RBF: Add client port to CallerContext for Router

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 56s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  1s |  |  codespell 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 _ |
   | +0 :ok: |  mvndep  |  13m 15s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  24m 38s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  26m 42s |  |  trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  compile  |  19m 56s |  |  trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +1 :green_heart: |  checkstyle  |   3m 49s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   3m 48s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   2m 52s |  |  trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  javadoc  |   4m 12s |  |  trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +1 :green_heart: |  spotbugs  |   7m 19s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  23m 12s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 22s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   2m 40s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  22m 36s |  |  the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  javac  |  22m 36s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  20m  1s |  |  the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +1 :green_heart: |  javac  |  20m  1s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   3m 45s |  |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   3m 45s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   2m 49s |  |  the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  javadoc  |   4m  6s |  |  the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +1 :green_heart: |  spotbugs  |   7m 52s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  23m 49s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  17m 24s |  |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 328m 57s |  |  hadoop-hdfs in the patch passed.  |
   | -1 :x: |  unit  |  37m 17s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3635/2/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) |  hadoop-hdfs-rbf in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   1m  9s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 609m 55s |  |  |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | hadoop.hdfs.rbfbalance.TestRouterDistCpProcedure |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3635/2/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/3635 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
   | uname | Linux 6e591de47bbb 4.15.0-153-generic #160-Ubuntu SMP Thu Jul 29 06:54:29 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / f1f402f949d45da4a2afab76885ba374b69d0fd0 |
   | Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3635/2/testReport/ |
   | Max. process+thread count | 2375 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs hadoop-hdfs-project/hadoop-hdfs-rbf U: . |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3635/2/console |
   | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.14.0-SNAPSHOT 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] tasanuma commented on a change in pull request #3635: HDFS-16310. RBF: Add client port to CallerContext for Router

Posted by GitBox <gi...@apache.org>.
tasanuma commented on a change in pull request #3635:
URL: https://github.com/apache/hadoop/pull/3635#discussion_r751048259



##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java
##########
@@ -586,25 +587,20 @@ private Object invokeMethod(
 
   /**
    * For tracking which is the actual client address.
-   * It adds trace info "clientIp:ip" to caller context if it's absent.
+   * It adds trace info "clientIp:ip" and "clientPort:port"
+   * to caller context if they are absent.
    */
-  private void appendClientIpToCallerContextIfAbsent() {
-    String clientIpInfo = CLIENT_IP_STR + ":" + Server.getRemoteAddress();
-    final CallerContext ctx = CallerContext.getCurrent();
-    if (isClientIpInfoAbsent(clientIpInfo, ctx)) {
-      String origContext = ctx == null ? null : ctx.getContext();
-      byte[] origSignature = ctx == null ? null : ctx.getSignature();
-      CallerContext.setCurrent(
-          new CallerContext.Builder(origContext, contextFieldSeparator)
-              .append(clientIpInfo)
-              .setSignature(origSignature)
-              .build());
-    }
-  }
-
-  private boolean isClientIpInfoAbsent(String clientIpInfo, CallerContext ctx){
-    return ctx == null || ctx.getContext() == null
-        || !ctx.getContext().contains(clientIpInfo);
+  private void appendClientIpPortToCallerContextIfAbsent() {
+    CallerContext ctx = CallerContext.getCurrent();
+    String origContext = ctx == null ? null : ctx.getContext();
+    byte[] origSignature = ctx == null ? null : ctx.getSignature();
+    CallerContext.setCurrent(
+        new CallerContext.Builder(origContext, contextFieldSeparator)
+            .appendIfAbsent(CLIENT_IP_STR, Server.getRemoteAddress())
+            .appendIfAbsent(CLIENT_PORT_STR,

Review comment:
       Just a note, we allow the line length up to 100 after HADOOP-17813.




-- 
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 #3635: HDFS-16310. RBF: Add client port to CallerContext for Router

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


   :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  0s |  |  codespell 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 _ |
   | +0 :ok: |  mvndep  |  12m 38s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  24m 21s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |   5m 54s |  |  trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  compile  |   5m 22s |  |  trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +1 :green_heart: |  checkstyle  |   1m 13s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   2m  4s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   1m 40s |  |  trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  javadoc  |   2m 20s |  |  trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +1 :green_heart: |  spotbugs  |   4m 36s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  23m  6s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 23s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m 49s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   5m 52s |  |  the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  javac  |   5m 52s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   5m 31s |  |  the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +1 :green_heart: |  javac  |   5m 31s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   1m  6s |  |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   1m 57s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m 28s |  |  the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  javadoc  |   2m 12s |  |  the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +1 :green_heart: |  spotbugs  |   4m 47s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  23m 15s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 356m 42s |  |  hadoop-hdfs in the patch passed.  |
   | -1 :x: |  unit  |  33m  3s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3635/3/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) |  hadoop-hdfs-rbf in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 57s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 525m 11s |  |  |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | hadoop.hdfs.rbfbalance.TestRouterDistCpProcedure |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3635/3/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/3635 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
   | uname | Linux 2c5b168f9454 4.15.0-143-generic #147-Ubuntu SMP Wed Apr 14 16:10:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 0bad062784569c647a1103c6816c91668d9ff50e |
   | Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3635/3/testReport/ |
   | Max. process+thread count | 2227 (vs. ulimit of 5500) |
   | modules | C: hadoop-hdfs-project/hadoop-hdfs hadoop-hdfs-project/hadoop-hdfs-rbf U: hadoop-hdfs-project |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3635/3/console |
   | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.14.0-SNAPSHOT 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] tomscut commented on pull request #3635: HDFS-16310. RBF: Add client port to CallerContext for Router

Posted by GitBox <gi...@apache.org>.
tomscut commented on pull request #3635:
URL: https://github.com/apache/hadoop/pull/3635#issuecomment-972550909


   Thanks @tasanuma for your comment and the merge.


-- 
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] tomscut commented on a change in pull request #3635: HDFS-16310. RBF: Add client port to CallerContext for Router

Posted by GitBox <gi...@apache.org>.
tomscut commented on a change in pull request #3635:
URL: https://github.com/apache/hadoop/pull/3635#discussion_r746156233



##########
File path: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/CallerContext.java
##########
@@ -228,6 +228,26 @@ public Builder append(String key, String value) {
       return this;
     }
 
+    /**
+     * Append new field which contains key and value to the context
+     * if the key("key:") is absent.
+     * @param key the key of field.
+     * @param value the value of field.
+     * @return the builder.
+     */
+    public Builder appendIfAbsent(String key, String value) {

Review comment:
       > As this is pretty core; I would prefer to do the changes to commons in a HADOOP JIRA.
   
   Thanks @goiri for your review and comments. Do I need to close the issue and file a Hadoop Jira? Looking forward to your reply. 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] tomscut commented on a change in pull request #3635: HDFS-16310. RBF: Add client port to CallerContext for Router

Posted by GitBox <gi...@apache.org>.
tomscut commented on a change in pull request #3635:
URL: https://github.com/apache/hadoop/pull/3635#discussion_r751162479



##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java
##########
@@ -586,25 +587,20 @@ private Object invokeMethod(
 
   /**
    * For tracking which is the actual client address.
-   * It adds trace info "clientIp:ip" to caller context if it's absent.
+   * It adds trace info "clientIp:ip" and "clientPort:port"
+   * to caller context if they are absent.
    */
-  private void appendClientIpToCallerContextIfAbsent() {
-    String clientIpInfo = CLIENT_IP_STR + ":" + Server.getRemoteAddress();
-    final CallerContext ctx = CallerContext.getCurrent();
-    if (isClientIpInfoAbsent(clientIpInfo, ctx)) {
-      String origContext = ctx == null ? null : ctx.getContext();
-      byte[] origSignature = ctx == null ? null : ctx.getSignature();
-      CallerContext.setCurrent(
-          new CallerContext.Builder(origContext, contextFieldSeparator)
-              .append(clientIpInfo)
-              .setSignature(origSignature)
-              .build());
-    }
-  }
-
-  private boolean isClientIpInfoAbsent(String clientIpInfo, CallerContext ctx){
-    return ctx == null || ctx.getContext() == null
-        || !ctx.getContext().contains(clientIpInfo);
+  private void appendClientIpPortToCallerContextIfAbsent() {
+    CallerContext ctx = CallerContext.getCurrent();
+    String origContext = ctx == null ? null : ctx.getContext();
+    byte[] origSignature = ctx == null ? null : ctx.getSignature();
+    CallerContext.setCurrent(
+        new CallerContext.Builder(origContext, contextFieldSeparator)
+            .appendIfAbsent(CLIENT_IP_STR, Server.getRemoteAddress())
+            .appendIfAbsent(CLIENT_PORT_STR,

Review comment:
       > Just a note, we allow the line length up to 100 after [HADOOP-17813](https://issues.apache.org/jira/browse/HADOOP-17813).
   
   Thank you for reminding me. This is good.




-- 
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] tomscut commented on a change in pull request #3635: HDFS-16310. RBF: Add client port to CallerContext for Router

Posted by GitBox <gi...@apache.org>.
tomscut commented on a change in pull request #3635:
URL: https://github.com/apache/hadoop/pull/3635#discussion_r747086158



##########
File path: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/CallerContext.java
##########
@@ -228,6 +228,26 @@ public Builder append(String key, String value) {
       return this;
     }
 
+    /**
+     * Append new field which contains key and value to the context
+     * if the key("key:") is absent.
+     * @param key the key of field.
+     * @param value the value of field.
+     * @return the builder.
+     */
+    public Builder appendIfAbsent(String key, String value) {

Review comment:
       Thank you very much for your advice and I will create a new one.




-- 
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] tomscut commented on pull request #3635: HDFS-16310. RBF: Add client port to CallerContext for Router

Posted by GitBox <gi...@apache.org>.
tomscut commented on pull request #3635:
URL: https://github.com/apache/hadoop/pull/3635#issuecomment-971023970


   > There's a failure with TestRouterDistCpProcedure but I don't see how it can be related. Can you confirm?
   
   Thank @goiri for your review. This failed unit test is unrelated to the change. It failed because timeout and work fine locally.


-- 
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] tomscut commented on pull request #3635: HDFS-16310. RBF: Add client port to CallerContext for Router

Posted by GitBox <gi...@apache.org>.
tomscut commented on pull request #3635:
URL: https://github.com/apache/hadoop/pull/3635#issuecomment-971026573


   Hi @tasanuma @jojochuang @ferhui @ayushtkn , could you please take a look at this. Thank you very much.


-- 
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 pull request #3635: HDFS-16310. RBF: Add client port to CallerContext for Router

Posted by GitBox <gi...@apache.org>.
goiri commented on pull request #3635:
URL: https://github.com/apache/hadoop/pull/3635#issuecomment-970623166


   There's a failure with TestRouterDistCpProcedure but I don't see how it can be related.
   Can you confirm?


-- 
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] tasanuma commented on pull request #3635: HDFS-16310. RBF: Add client port to CallerContext for Router

Posted by GitBox <gi...@apache.org>.
tasanuma commented on pull request #3635:
URL: https://github.com/apache/hadoop/pull/3635#issuecomment-971386553


   @tomscut With the latest commit, it seems Router adds a client port even if `dfs.namenode.audit.log.with.remote.port=false`. We may want to add a client port only when `dfs.namenode.audit.log.with.remote.port=true`. What do you think of it?


-- 
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] tasanuma merged pull request #3635: HDFS-16310. RBF: Add client port to CallerContext for Router

Posted by GitBox <gi...@apache.org>.
tasanuma merged pull request #3635:
URL: https://github.com/apache/hadoop/pull/3635


   


-- 
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] tasanuma commented on pull request #3635: HDFS-16310. RBF: Add client port to CallerContext for Router

Posted by GitBox <gi...@apache.org>.
tasanuma commented on pull request #3635:
URL: https://github.com/apache/hadoop/pull/3635#issuecomment-972535516


   Merged it. Thanks for all your contributions to this work, @tomscut. Thanks for your review, @goiri.


-- 
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 #3635: HDFS-16310. RBF: Add client port to CallerContext for Router

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   1m  3s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  1s |  |  codespell 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 _ |
   | +0 :ok: |  mvndep  |  11m 19s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  24m 18s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  23m 31s |  |  trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  compile  |  19m 47s |  |  trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +1 :green_heart: |  checkstyle  |   3m 51s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   3m 49s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   2m 50s |  |  trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  javadoc  |   4m  8s |  |  trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +1 :green_heart: |  spotbugs  |   7m 10s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  23m 18s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 22s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   2m 40s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  22m 43s |  |  the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  javac  |  22m 43s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  19m 55s |  |  the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +1 :green_heart: |  javac  |  19m 55s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   3m 43s |  |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   3m 49s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   2m 54s |  |  the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04  |
   | +1 :green_heart: |  javadoc  |   4m 35s |  |  the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10  |
   | +1 :green_heart: |  spotbugs  |   9m  4s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  25m  0s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  17m 23s |  |  hadoop-common in the patch passed.  |
   | -1 :x: |  unit  | 331m 12s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3635/1/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt) |  hadoop-hdfs in the patch passed.  |
   | -1 :x: |  unit  |  37m 38s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3635/1/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) |  hadoop-hdfs-rbf in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   1m  0s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 609m 52s |  |  |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | hadoop.hdfs.tools.TestDFSAdminWithHA |
   |   | hadoop.hdfs.server.federation.router.TestRouterRpcMultiDestination |
   |   | hadoop.hdfs.rbfbalance.TestRouterDistCpProcedure |
   |   | hadoop.hdfs.server.federation.router.TestRouterRpc |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3635/1/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/3635 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
   | uname | Linux a54cef6a1f5f 4.15.0-153-generic #160-Ubuntu SMP Thu Jul 29 06:54:29 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / c95077422057f600eb89d98f43d2b73fea1fb0db |
   | Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3635/1/testReport/ |
   | Max. process+thread count | 2913 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs hadoop-hdfs-project/hadoop-hdfs-rbf U: . |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3635/1/console |
   | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.14.0-SNAPSHOT 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] goiri commented on a change in pull request #3635: HDFS-16310. RBF: Add client port to CallerContext for Router

Posted by GitBox <gi...@apache.org>.
goiri commented on a change in pull request #3635:
URL: https://github.com/apache/hadoop/pull/3635#discussion_r746873078



##########
File path: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/CallerContext.java
##########
@@ -228,6 +228,26 @@ public Builder append(String key, String value) {
       return this;
     }
 
+    /**
+     * Append new field which contains key and value to the context
+     * if the key("key:") is absent.
+     * @param key the key of field.
+     * @param value the value of field.
+     * @return the builder.
+     */
+    public Builder appendIfAbsent(String key, String value) {

Review comment:
       I would create a new one just for the commons side and once that's done, rebase this one to do just the RBF part.




-- 
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] tomscut commented on pull request #3635: HDFS-16310. RBF: Add client port to CallerContext for Router

Posted by GitBox <gi...@apache.org>.
tomscut commented on pull request #3635:
URL: https://github.com/apache/hadoop/pull/3635#issuecomment-971522587


   > @tomscut With the latest commit, it seems Router adds a client port even if `dfs.namenode.audit.log.with.remote.port=false`. We may want to add a client port only when `dfs.namenode.audit.log.with.remote.port=true`. What do you think of it?
   
   Thanks @tasanuma for your careful consideration and comments. This problem does exist at present. But `dfs.namenode.audit.log.with.remote.port` should only work within the Namenode. We can treat the Router as the client of Namenode. From the perspective of the Namenode, the behavior of the client is not controllable because the real client may also set the `clientPort`. What do you think of this?


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