This is an automated email from the ASF dual-hosted git repository. omalley pushed a commit to branch branch-3.3 in repository https://gitbox.apache.org/repos/asf/hadoop.git
commit c52b97d08491a869ecefd39bfd08c8f57eaeab84 Author: Owen O'Malley <[email protected]> AuthorDate: Fri Mar 4 16:17:46 2022 -0800 HDFS-16495: RBF should prepend the client ip rather than append it. Fixes #4054 Signed-off-by: Owen O'Malley <[email protected]> --- .../java/org/apache/hadoop/ipc/CallerContext.java | 2 +- .../server/federation/router/RouterRpcClient.java | 31 +++++++++++++++------- .../server/federation/router/TestRouterRpc.java | 13 ++++----- .../router/TestRouterRpcMultiDestination.java | 5 ++-- 4 files changed, 32 insertions(+), 19 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/CallerContext.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/CallerContext.java index ed8dcf2..c8b4135 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/CallerContext.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/CallerContext.java @@ -116,7 +116,7 @@ public final class CallerContext { /** The caller context builder. */ public static final class Builder { - private static final String KEY_VALUE_SEPARATOR = ":"; + public static final String KEY_VALUE_SEPARATOR = ":"; /** * The illegal separators include '\t', '\n', '='. * User should not set illegal separator. diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java index 351926b..16cd7a5 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java @@ -417,7 +417,7 @@ public class RouterRpcClient { + router.getRouterId()); } - appendClientIpPortToCallerContextIfAbsent(); + addClientIpToCallerContext(); Object ret = null; if (rpcMonitor != null) { @@ -536,19 +536,32 @@ public class RouterRpcClient { /** * For tracking which is the actual client address. * It adds trace info "clientIp:ip" and "clientPort:port" - * to caller context if they are absent. + * in the caller context, removing the old values if they were + * already present. */ - 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)) { + String[] keyValue = + part.split(CallerContext.Builder.KEY_VALUE_SEPARATOR, 2); + if (keyValue.length == 2) { + builder.appendIfAbsent(keyValue[0], keyValue[1]); + } else if (keyValue.length == 1) { + builder.append(keyValue[0]); + } + } + } + CallerContext.setCurrent(builder.build()); } /** diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRpc.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRpc.java index f2b86dc..94b3a5e 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRpc.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRpc.java @@ -1863,9 +1863,10 @@ public class TestRouterRpc { FsPermission permission = new FsPermission("755"); routerProtocol.mkdirs(dirPath, permission, false); - // The audit log should contains "callerContext=clientContext,clientIp:" - assertTrue(auditlog.getOutput() - .contains("callerContext=clientContext,clientIp:")); + // The audit log should contains "callerContext=clientIp:...,clientContext" + final String logOutput = auditlog.getOutput(); + assertTrue(logOutput.contains("callerContext=clientIp:")); + assertTrue(logOutput.contains(",clientContext")); assertTrue(verifyFileExists(routerFS, dirPath)); } @@ -1910,9 +1911,9 @@ public class TestRouterRpc { // Create a directory via the router. routerProtocol.getFileInfo(dirPath); - // The audit log should contains the original clientIp and clientPort + // The audit log should not contain the original clientIp and clientPort // set by client. - assertTrue(auditLog.getOutput().contains("clientIp:1.1.1.1")); - assertTrue(auditLog.getOutput().contains("clientPort:1234")); + assertFalse(auditLog.getOutput().contains("clientIp:1.1.1.1")); + assertFalse(auditLog.getOutput().contains("clientPort:1234")); } } diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRpcMultiDestination.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRpcMultiDestination.java index e50464c..370a125 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRpcMultiDestination.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRpcMultiDestination.java @@ -464,9 +464,8 @@ public class TestRouterRpcMultiDestination extends TestRouterRpc { for (String line : auditLog.getOutput().split("\n")) { if (line.contains(auditFlag)) { // assert origin caller context exist in audit log - assertTrue(line.contains("callerContext=clientContext")); - String callerContext = line.substring( - line.indexOf("callerContext=clientContext")); + String callerContext = line.substring(line.indexOf("callerContext=")); + assertTrue(callerContext.contains("clientContext")); // assert client ip info exist in caller context assertTrue(callerContext.contains(clientIpInfo)); // assert client ip info appears only once in caller context --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
