Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HDFS-17306. RBF: Router should not return nameservices that does not enable observer nodes in RpcResponseHeaderProto #6385

Merged
merged 3 commits into from
Jan 4, 2024

Conversation

LiuGuH
Copy link
Contributor

@LiuGuH LiuGuH commented Dec 28, 2023

Description of PR

JIRA: HDFS-17306. RBF: Router should not return nameservices that does not enable observer nodes in RpcResponseHeaderProto.

If a cluster has 3 nameservices: ns1, ns2,ns3, and ns1 has observer nodes, and client comminutes with nameservices via DFSRouter.

If DFS_ROUTER_OBSERVER_READ_DEFAULT_KEY enable, the client will receive all nameservices in RpcResponseHeaderProto. And ns2,ns3 statidid is Long.MIN_VALUE,that is not necessary.

We should reduce rpc response size if nameservices don't enable observer nodes.

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 6m 48s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 codespell 0m 0s codespell was not available.
+0 🆗 detsecrets 0m 0s detect-secrets was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-1 ❌ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ trunk Compile Tests _
+1 💚 mvninstall 31m 42s trunk passed
+1 💚 compile 0m 23s trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚 compile 0m 19s trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚 checkstyle 0m 18s trunk passed
+1 💚 mvnsite 0m 25s trunk passed
+1 💚 javadoc 0m 28s trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚 javadoc 0m 22s trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚 spotbugs 0m 50s trunk passed
+1 💚 shadedclient 19m 24s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+1 💚 mvninstall 0m 18s the patch passed
+1 💚 compile 0m 18s the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚 javac 0m 18s the patch passed
+1 💚 compile 0m 17s the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚 javac 0m 17s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 checkstyle 0m 11s the patch passed
+1 💚 mvnsite 0m 21s the patch passed
+1 💚 javadoc 0m 18s the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚 javadoc 0m 17s the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚 spotbugs 0m 52s the patch passed
+1 💚 shadedclient 19m 30s patch has no errors when building and testing our client artifacts.
_ Other Tests _
+1 💚 unit 19m 22s hadoop-hdfs-rbf in the patch passed.
+1 💚 asflicense 0m 25s The patch does not generate ASF License warnings.
105m 24s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6385/1/artifact/out/Dockerfile
GITHUB PR #6385
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname Linux 8e941e768c2b 5.15.0-88-generic #98-Ubuntu SMP Mon Oct 2 15:18:56 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision trunk / b423b9c
Default Java Private Build-1.8.0_392-8u392-ga-1~20.04-b08
Multi-JDK versions /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_392-8u392-ga-1~20.04-b08
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6385/1/testReport/
Max. process+thread count 2607 (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-6385/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.

@goiri goiri changed the title HDFS-17306. RBF:Router should not return nameservices that does not enable observer nodes in RpcResponseHeaderProto HDFS-17306. RBF: Router should not return nameservices that does not enable observer nodes in RpcResponseHeaderProto Jan 1, 2024
@@ -85,7 +85,11 @@ public void setResponseHeaderState(RpcResponseHeaderProto.Builder headerBuilder)
return;
}
RouterFederatedStateProto.Builder builder = RouterFederatedStateProto.newBuilder();
namespaceIdMap.forEach((k, v) -> builder.putNamespaceStateIds(k, v.get()));
namespaceIdMap.forEach((k, v) -> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a test for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, Thanks

@hadoop-yetus
Copy link

🎊 +1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 18m 16s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 codespell 0m 0s codespell was not available.
+0 🆗 detsecrets 0m 0s detect-secrets was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
+1 💚 test4tests 0m 0s The patch appears to include 1 new or modified test files.
_ trunk Compile Tests _
+1 💚 mvninstall 46m 17s trunk passed
+1 💚 compile 0m 41s trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚 compile 0m 34s trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚 checkstyle 0m 29s trunk passed
+1 💚 mvnsite 0m 41s trunk passed
+1 💚 javadoc 0m 42s trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚 javadoc 0m 30s trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚 spotbugs 1m 23s trunk passed
+1 💚 shadedclient 40m 55s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+1 💚 mvninstall 0m 35s the patch passed
+1 💚 compile 0m 35s the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚 javac 0m 35s the patch passed
+1 💚 compile 0m 30s the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚 javac 0m 30s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
-0 ⚠️ checkstyle 0m 18s /results-checkstyle-hadoop-hdfs-project_hadoop-hdfs-rbf.txt hadoop-hdfs-project/hadoop-hdfs-rbf: The patch generated 1 new + 2 unchanged - 0 fixed = 3 total (was 2)
+1 💚 mvnsite 0m 33s the patch passed
+1 💚 javadoc 0m 29s the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚 javadoc 0m 24s the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚 spotbugs 1m 28s the patch passed
+1 💚 shadedclient 38m 29s patch has no errors when building and testing our client artifacts.
_ Other Tests _
+1 💚 unit 23m 9s hadoop-hdfs-rbf in the patch passed.
+1 💚 asflicense 0m 35s The patch does not generate ASF License warnings.
181m 33s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6385/2/artifact/out/Dockerfile
GITHUB PR #6385
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname Linux 7135c90f0234 5.15.0-88-generic #98-Ubuntu SMP Mon Oct 2 15:18:56 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision trunk / b9fc917
Default Java Private Build-1.8.0_392-8u392-ga-1~20.04-b08
Multi-JDK versions /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_392-8u392-ga-1~20.04-b08
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6385/2/testReport/
Max. process+thread count 2406 (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-6385/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.

@slfan1989
Copy link
Contributor

@LiuGuH Thanks for your contribution! We need to fix checkstyle.

@LiuGuH
Copy link
Contributor Author

LiuGuH commented Jan 3, 2024

@LiuGuH Thanks for your contribution! We need to fix checkstyle.

Thanks for review. Fixed

@hadoop-yetus
Copy link

🎊 +1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 20s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 codespell 0m 1s codespell was not available.
+0 🆗 detsecrets 0m 1s detect-secrets was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
+1 💚 test4tests 0m 0s The patch appears to include 1 new or modified test files.
_ trunk Compile Tests _
+1 💚 mvninstall 32m 19s trunk passed
+1 💚 compile 0m 22s trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚 compile 0m 22s trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚 checkstyle 0m 19s trunk passed
+1 💚 mvnsite 0m 27s trunk passed
+1 💚 javadoc 0m 28s trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚 javadoc 0m 18s trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚 spotbugs 0m 50s trunk passed
+1 💚 shadedclient 19m 35s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+1 💚 mvninstall 0m 20s the patch passed
+1 💚 compile 0m 21s the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚 javac 0m 21s the patch passed
+1 💚 compile 0m 17s the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚 javac 0m 17s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 checkstyle 0m 11s the patch passed
+1 💚 mvnsite 0m 22s the patch passed
+1 💚 javadoc 0m 18s the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚 javadoc 0m 16s the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚 spotbugs 0m 50s the patch passed
+1 💚 shadedclient 19m 29s patch has no errors when building and testing our client artifacts.
_ Other Tests _
+1 💚 unit 19m 15s hadoop-hdfs-rbf in the patch passed.
+1 💚 asflicense 0m 24s The patch does not generate ASF License warnings.
99m 47s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6385/3/artifact/out/Dockerfile
GITHUB PR #6385
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname Linux da6d00d6c07f 5.15.0-88-generic #98-Ubuntu SMP Mon Oct 2 15:18:56 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision trunk / c0e9750
Default Java Private Build-1.8.0_392-8u392-ga-1~20.04-b08
Multi-JDK versions /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_392-8u392-ga-1~20.04-b08
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6385/3/testReport/
Max. process+thread count 2624 (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-6385/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.

Copy link
Member

@simbadzina simbadzina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also check the list in DFS_ROUTER_OBSERVER_READ_OVERRIDES?

If dfs.namenode.state.context.enabled is true on the namenode then this change won't have impact on calls to that nameservice, even though observer reads are disabled for it in the router.

I'm okay with saving this optimization for a separate pull request though.

@LiuGuH
Copy link
Contributor Author

LiuGuH commented Jan 4, 2024

I'm okay with saving this optimization for a separate pull request though.

Thanks for review . And I will be add a separate pull request for DFS_ROUTER_OBSERVER_READ_DEFAULT_KEY and DFS_ROUTER_OBSERVER_READ_OVERRIDES check.

@simbadzina
Copy link
Member

I'm okay with saving this optimization for a separate pull request though.

Thanks for review . And I will be add a separate pull request for DFS_ROUTER_OBSERVER_READ_DEFAULT_KEY and DFS_ROUTER_OBSERVER_READ_OVERRIDES check.

Great. Thanks for the contribution.

@simbadzina simbadzina merged commit 7d3b6a3 into apache:trunk Jan 4, 2024
4 checks passed
@LiuGuH
Copy link
Contributor Author

LiuGuH commented Jan 5, 2024

Thanks for review . And I will be add a separate pull request for DFS_ROUTER_OBSERVER_READ_DEFAULT_KEY and DFS_ROUTER_OBSERVER_READ_OVERRIDES check.

This is implemented in #6412 . Thanks for review @simbadzina

@LiuGuH LiuGuH deleted the trunk-FederatedStateToPropagate branch January 10, 2024 03:50
jiajunmao pushed a commit to jiajunmao/hadoop-MLEC that referenced this pull request Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants