[GitHub] [hadoop] omalley commented on a change in pull request #4054: HDFS-16495: RBF should prepend the client ip rather than append it.

[GitBox <gi...@apache.org>]

omalley commented on a change in pull request #4054:
URL: https://github.com/apache/hadoop/pull/4054#discussion_r825197599



##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java
##########
@@ -590,17 +590,26 @@ private Object invokeMethod(
    * It adds trace info "clientIp:ip" and "clientPort:port"
    * to caller context if they are absent.
    */
-  private void appendClientIpPortToCallerContextIfAbsent() {
+  private void addClientIpToCallerContext() {
     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,
+    CallerContext.Builder builder =
+        new CallerContext.Builder("", contextFieldSeparator)
+            .append(CLIENT_IP_STR, Server.getRemoteAddress())
+            .append(CLIENT_PORT_STR,
                 Integer.toString(Server.getRemotePort()))
-            .setSignature(origSignature)
-            .build());
+            .setSignature(origSignature);
+    // Append the original caller context
+    if (origContext != null) {
+      for (String part : origContext.split(contextFieldSeparator)) {
+        if (!part.startsWith(CLIENT_IP_STR) &&

Review comment:
       I considered including the KEY_VALUE_SEPARATOR in the startsWith pattern, but that requires changing the visibility of the constant in CallerContext.Builder. 
   
   Using appendIfAbsent forces this code to interpret the KEY_VALUE_SEPARATOR (and causes a scan that it is clear will match, since we added it). I can go ahead and do it though.




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