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/03/08 22:04:34 UTC
[GitHub] [hadoop] omalley opened a new pull request #4054: HDFS-16495: RBF should prepend the client ip rather than append it.
omalley opened a new pull request #4054:
URL: https://github.com/apache/hadoop/pull/4054
### Description of PR
Makes RBF prepends the client ip & port to the caller context and removes previous values. This avoids a couple problems:
* User can't fake their network address to the NN.
* It is less likely to have false positives (accidental conflicts), since the critical information that is under system control comes first.
### How was this patch tested?
The relevant unit tests were fixed and run.
--
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 #4054: HDFS-16495: RBF should prepend the client ip rather than append it.
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #4054:
URL: https://github.com/apache/hadoop/pull/4054#issuecomment-1064721585
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 49s | | 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 _ |
| +1 :green_heart: | mvninstall | 36m 20s | | trunk passed |
| +1 :green_heart: | compile | 0m 41s | | trunk passed with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 0m 36s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 0m 24s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 40s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 41s | | trunk passed with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 0m 56s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 1m 18s | | trunk passed |
| +1 :green_heart: | shadedclient | 23m 51s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 32s | | the patch passed |
| +1 :green_heart: | compile | 0m 36s | | the patch passed with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javac | 0m 36s | | the patch passed |
| +1 :green_heart: | compile | 0m 30s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 0m 30s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 15s | | the patch passed |
| +1 :green_heart: | mvnsite | 0m 33s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 31s | | the patch passed with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 0m 47s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 1m 21s | | the patch passed |
| +1 :green_heart: | shadedclient | 23m 49s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 31m 43s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4054/2/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) | hadoop-hdfs-rbf in the patch passed. |
| +1 :green_heart: | asflicense | 0m 31s | | The patch does not generate ASF License warnings. |
| | | 128m 32s | | |
| 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-4054/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4054 |
| JIRA Issue | HDFS-16495 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
| uname | Linux 96bedbe2a1f0 4.15.0-163-generic #171-Ubuntu SMP Fri Nov 5 11:55:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 31edfde41e21ef125439d1006b1894882b67c471 |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4054/2/testReport/ |
| Max. process+thread count | 2292 (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-4054/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] omalley commented on a change in pull request #4054: HDFS-16495: RBF should prepend the client ip rather than append it.
Posted by 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
[GitHub] [hadoop] hchaverri commented on a change in pull request #4054: HDFS-16495: RBF should prepend the client ip rather than append it.
Posted by GitBox <gi...@apache.org>.
hchaverri commented on a change in pull request #4054:
URL: https://github.com/apache/hadoop/pull/4054#discussion_r824957652
##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRpc.java
##########
@@ -1951,8 +1951,9 @@ public void testMkdirsWithCallerContext() throws IOException {
routerProtocol.mkdirs(dirPath, permission, false);
// The audit log should contains "callerContext=clientContext,clientIp:"
Review comment:
We should fix this comment to show the expected order of the fields
--
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 #4054: HDFS-16495: RBF should prepend the client ip rather than append it.
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #4054:
URL: https://github.com/apache/hadoop/pull/4054#issuecomment-1062430216
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 50s | | 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 2 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 36m 18s | | trunk passed |
| +1 :green_heart: | compile | 0m 40s | | trunk passed with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 0m 35s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 0m 23s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 41s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 41s | | trunk passed with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 0m 53s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 1m 19s | | trunk passed |
| +1 :green_heart: | shadedclient | 24m 8s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 32s | | the patch passed |
| +1 :green_heart: | compile | 0m 36s | | the patch passed with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javac | 0m 36s | | the patch passed |
| +1 :green_heart: | compile | 0m 30s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 0m 30s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 0m 15s | [/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4054/1/artifact/out/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) | hadoop-hdfs-project/hadoop-hdfs-rbf: The patch generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0) |
| +1 :green_heart: | mvnsite | 0m 32s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 31s | | the patch passed with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 0m 50s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| -1 :x: | spotbugs | 1m 26s | [/new-spotbugs-hadoop-hdfs-project_hadoop-hdfs-rbf.html](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4054/1/artifact/out/new-spotbugs-hadoop-hdfs-project_hadoop-hdfs-rbf.html) | hadoop-hdfs-project/hadoop-hdfs-rbf generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | shadedclient | 23m 27s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 50m 4s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4054/1/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) | hadoop-hdfs-rbf in the patch passed. |
| +1 :green_heart: | asflicense | 0m 43s | | The patch does not generate ASF License warnings. |
| | | 147m 4s | | |
| Reason | Tests |
|-------:|:------|
| SpotBugs | module:hadoop-hdfs-project/hadoop-hdfs-rbf |
| | Possible null pointer dereference of null in org.apache.hadoop.hdfs.server.federation.router.RouterRpcClient.addClientIpToCallerContext() Dereferenced at RouterRpcClient.java:null in org.apache.hadoop.hdfs.server.federation.router.RouterRpcClient.addClientIpToCallerContext() Dereferenced at RouterRpcClient.java:[line 604] |
| Failed junit tests | hadoop.fs.contract.router.TestRouterHDFSContractCreate |
| | hadoop.fs.contract.router.web.TestRouterWebHDFSContractSeek |
| | hadoop.hdfs.server.federation.metrics.TestNameserviceRPCMetrics |
| | hadoop.fs.contract.router.TestRouterHDFSContractSetTimes |
| | hadoop.hdfs.server.federation.router.TestRouterFederationRenamePermission |
| | hadoop.hdfs.server.federation.router.TestRouterMountTable |
| | hadoop.hdfs.server.federation.router.TestRouterFederationRenameInKerberosEnv |
| | hadoop.hdfs.server.federation.router.TestRouterClientRejectOverload |
| | hadoop.fs.contract.router.TestRouterHDFSContractConcatSecure |
| | hadoop.hdfs.server.federation.router.TestRouterWebHdfsMethods |
| | hadoop.hdfs.server.federation.router.TestRouterMultiRack |
| | hadoop.hdfs.server.federation.router.TestRouterQuota |
| | hadoop.fs.contract.router.TestRouterHDFSContractRootDirectorySecure |
| | hadoop.hdfs.server.federation.router.TestRouterNamenodeMonitoring |
| | hadoop.hdfs.server.federation.router.TestRouterRpcStoragePolicySatisfier |
| | hadoop.hdfs.server.federation.router.TestRouterFederationRename |
| | hadoop.fs.contract.router.TestRouterHDFSContractAppend |
| | hadoop.fs.contract.router.TestRouterHDFSContractAppendSecure |
| | hadoop.fs.contract.router.TestRouterHDFSContractCreateSecure |
| | hadoop.hdfs.server.federation.fairness.TestRouterHandlersFairness |
| | hadoop.fs.contract.router.TestRouterHDFSContractOpen |
| | hadoop.hdfs.server.federation.router.TestRouterRPCClientRetries |
| | hadoop.fs.contract.router.TestRouterHDFSContractOpenSecure |
| | hadoop.fs.contract.router.TestRouterHDFSContractRootDirectory |
| | hadoop.hdfs.server.federation.router.TestRouterMissingFolderMulti |
| | hadoop.hdfs.server.federation.router.TestRouterAllResolver |
| | hadoop.fs.contract.router.web.TestRouterWebHDFSContractMkdir |
| | hadoop.fs.contract.router.web.TestRouterWebHDFSContractCreate |
| | hadoop.hdfs.server.federation.router.TestRouterFaultTolerant |
| | hadoop.fs.contract.router.TestRouterHDFSContractDeleteSecure |
| | hadoop.fs.contract.router.TestRouterHDFSContractGetFileStatusSecure |
| | hadoop.fs.contract.router.TestRouterHDFSContractGetFileStatus |
| | hadoop.hdfs.rbfbalance.TestMountTableProcedure |
| | hadoop.fs.contract.router.web.TestRouterWebHDFSContractDelete |
| | hadoop.fs.contract.router.TestRouterHDFSContractRename |
| | hadoop.hdfs.server.federation.router.TestRouterTrash |
| | hadoop.fs.contract.router.TestRouterHDFSContractDelegationToken |
| | hadoop.hdfs.server.federation.router.TestRouterFsck |
| | hadoop.fs.contract.router.TestRouterHDFSContractRenameSecure |
| | hadoop.hdfs.server.federation.router.TestDisableNameservices |
| | hadoop.hdfs.server.federation.router.TestRouterAdmin |
| | hadoop.fs.contract.router.TestRouterHDFSContractSeek |
| | hadoop.fs.contract.router.TestRouterHDFSContractSeekSecure |
| | hadoop.fs.contract.router.TestRouterHDFSContractMkdir |
| | hadoop.fs.contract.router.web.TestRouterWebHDFSContractRename |
| | hadoop.fs.contract.router.TestRouterHDFSContractConcat |
| | hadoop.fs.contract.router.TestRouterHDFSContractSetTimesSecure |
| | hadoop.fs.contract.router.web.TestRouterWebHDFSContractRootDirectory |
| | hadoop.fs.contract.router.web.TestRouterWebHDFSContractAppend |
| | hadoop.hdfs.server.federation.router.TestRouterNetworkTopologyServlet |
| | hadoop.hdfs.rbfbalance.TestRouterDistCpProcedure |
| | hadoop.fs.contract.router.web.TestRouterWebHDFSContractConcat |
| | hadoop.hdfs.server.federation.router.TestSafeMode |
| | hadoop.fs.contract.router.TestRouterHDFSContractDelete |
| | hadoop.hdfs.server.federation.router.TestRouterRpcSingleNS |
| | hadoop.hdfs.server.federation.metrics.TestRouterClientMetrics |
| | hadoop.fs.contract.router.web.TestRouterWebHDFSContractOpen |
| | hadoop.fs.contract.router.TestRouterHDFSContractMkdirSecure |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4054/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4054 |
| JIRA Issue | HDFS-16495 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
| uname | Linux a250790c75be 4.15.0-163-generic #171-Ubuntu SMP Fri Nov 5 11:55:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / e7c0c118a0fe28f9fe78e493f9b4d9e01454a975 |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4054/1/testReport/ |
| Max. process+thread count | 3412 (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-4054/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] simbadzina commented on a change in pull request #4054: HDFS-16495: RBF should prepend the client ip rather than append it.
Posted by GitBox <gi...@apache.org>.
simbadzina commented on a change in pull request #4054:
URL: https://github.com/apache/hadoop/pull/4054#discussion_r824960593
##########
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.
Review comment:
It'd be good to update the documentation here.
--
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] simbadzina commented on a change in pull request #4054: HDFS-16495: RBF should prepend the client ip rather than append it.
Posted by GitBox <gi...@apache.org>.
simbadzina commented on a change in pull request #4054:
URL: https://github.com/apache/hadoop/pull/4054#discussion_r824960148
##########
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.
Review comment:
It'd be good to update the documentation here.
--
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 #4054: HDFS-16495: RBF should prepend the client ip rather than append it.
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #4054:
URL: https://github.com/apache/hadoop/pull/4054#issuecomment-1065811378
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 52s | | 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 2s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 28m 45s | | trunk passed |
| +1 :green_heart: | compile | 24m 23s | | trunk passed with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 20m 52s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 3m 50s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 28s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 55s | | trunk passed with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 2m 43s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 3m 55s | | trunk passed |
| +1 :green_heart: | shadedclient | 24m 19s | | 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 | 1m 33s | | the patch passed |
| +1 :green_heart: | compile | 23m 47s | | the patch passed with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javac | 23m 47s | | the patch passed |
| +1 :green_heart: | compile | 20m 56s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 20m 56s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 3m 46s | | the patch passed |
| +1 :green_heart: | mvnsite | 2m 28s | | the patch passed |
| +1 :green_heart: | javadoc | 1m 52s | | the patch passed with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 2m 42s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 4m 17s | | the patch passed |
| +1 :green_heart: | shadedclient | 24m 48s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 17m 47s | | hadoop-common in the patch passed. |
| -1 :x: | unit | 32m 7s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4054/3/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) | hadoop-hdfs-rbf in the patch passed. |
| +1 :green_heart: | asflicense | 0m 52s | | The patch does not generate ASF License warnings. |
| | | 266m 40s | | |
| 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-4054/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4054 |
| JIRA Issue | HDFS-16495 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
| uname | Linux b70e3394632d 4.15.0-163-generic #171-Ubuntu SMP Fri Nov 5 11:55:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / f414315a797dad8f4f3d8b0a4e38f7ff1aaef832 |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4054/3/testReport/ |
| Max. process+thread count | 3143 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-rbf U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4054/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] hchaverri commented on a change in pull request #4054: HDFS-16495: RBF should prepend the client ip rather than append it.
Posted by GitBox <gi...@apache.org>.
hchaverri commented on a change in pull request #4054:
URL: https://github.com/apache/hadoop/pull/4054#discussion_r824956894
##########
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:
We should try to leverage Builder#appendIfAbsent as that will do the comparison up to the KEY_VALUE_SEPARATOR. In the current form, new fields that start with "clientIp" (e.g. someone adding a field "clientIpV6" in the future) will be dropped.
--
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] omalley closed pull request #4054: HDFS-16495: RBF should prepend the client ip rather than append it.
Posted by GitBox <gi...@apache.org>.
omalley closed pull request #4054:
URL: https://github.com/apache/hadoop/pull/4054
--
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