HDFS-17343. Revert "HDFS-16016. BPServiceActor to provide new thread to handle IBR (#2998)" #6457

slfan1989 · 2024-01-17T06:38:35Z

Description of PR

JIRA: HDFS-17343. Revert HDFS-16016. BPServiceActor to provide new thread to handle IBR.

Revert "HDFS-16016. BPServiceActor to provide new thread to handle IBR (#2998)"
This reverts commit c1bf3cb

How was this patch tested?

For code changes:

Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
If applicable, have you updated the LICENSE, LICENSE-binary, NOTICE-binary files?

apache#2998)" This reverts commit c1bf3cb

yuanboliu · 2024-01-17T07:07:26Z

@slfan1989
Why is HDFS-16016 reverted? any bug describe?

tasanuma · 2024-01-17T07:07:33Z

@slfan1989 Thank you for creating the revert PR. Considering HDFS-16016 has already been released in 3.3.6, it might be better to create a new JIRA to manage this revert PR, rather than reopening HDFS-16016.

tasanuma · 2024-01-17T07:11:19Z

@yuanboliu HDFS-17129 was caused by HDFS-16016. Please read the discussion in #6244.

slfan1989 · 2024-01-17T07:20:29Z

@slfan1989 Thank you for creating the revert PR. Considering HDFS-16016 has already been released in 3.3.6, it might be better to create a new JIRA to manage this revert PR, rather than reopening HDFS-16016.

@tasanuma Thanks for your suggestion! I will create a new JIRA to complete the revert and update HDFS-16016 back.

yuanboliu · 2024-01-17T07:35:41Z

@tasanuma @slfan1989
we have HDFS-16016 and HDFS-17129 both in our production and HDFS-16898 as well. I understand that HDFS-16898 can reduce the impaction of heartbeat for ibr, but separating ibr from heartbeat is still a good idea as when massive deleting commands come from nn, it still will block the ibr for a long time.

tasanuma · 2024-01-17T08:58:39Z

@yuanboliu
I don't object to fixing HDFS-16016 and HDFS-17129. However, no conclusion has been reached on HDFS-17129, and it is acting as a blocker for 3.4.0, which we are about to release. I believe that in order to merge HDFS-17129, we first need to create a unit test and then obtain one or more approvals from the committers. That may take some time.

hadoop-yetus · 2024-01-17T09:05:11Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 33s		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 2 new or modified test files.
			_ trunk Compile Tests _
+1 💚	mvninstall	41m 36s		trunk passed
+1 💚	compile	1m 19s		trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚	compile	1m 13s		trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	checkstyle	1m 7s		trunk passed
+1 💚	mvnsite	1m 22s		trunk passed
+1 💚	javadoc	1m 6s		trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚	javadoc	1m 39s		trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	spotbugs	3m 16s		trunk passed
+1 💚	shadedclient	39m 6s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	1m 23s		the patch passed
+1 💚	compile	1m 29s		the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚	javac	1m 29s		the patch passed
+1 💚	compile	1m 23s		the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	javac	1m 23s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	1m 13s		the patch passed
+1 💚	mvnsite	1m 32s		the patch passed
+1 💚	javadoc	1m 4s		the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚	javadoc	1m 50s		the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	spotbugs	4m 17s		the patch passed
-1 ❌	shadedclient	39m 15s		patch has errors when building and testing our client artifacts.
			_ Other Tests _
-1 ❌	unit	0m 26s	/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt	hadoop-hdfs in the patch failed.
+0 🆗	asflicense	0m 26s		ASF License check generated no output?
		145m 22s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6457/1/artifact/out/Dockerfile
GITHUB PR	#6457
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 1c73a3fb55f3 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 / `a25f1e5`
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-6457/1/testReport/
Max. process+thread count	555 (vs. ulimit of 5500)
modules	C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6457/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.

slfan1989 · 2024-01-17T13:51:15Z

@yuanboliu
I don't object to fixing HDFS-16016 and HDFS-17129. However, no conclusion has been reached on HDFS-17129, and it is acting as a blocker for 3.4.0, which we are about to release. I believe that in order to merge HDFS-17129, we first need to create a unit test and then obtain one or more approvals from the committers. That may take some time.

I agree with @tasanuma's idea.

hadoop-yetus · 2024-01-17T20:02:49Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 32s		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 2 new or modified test files.
			_ trunk Compile Tests _
+1 💚	mvninstall	41m 13s		trunk passed
+1 💚	compile	1m 21s		trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚	compile	1m 13s		trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	checkstyle	1m 7s		trunk passed
+1 💚	mvnsite	1m 21s		trunk passed
+1 💚	javadoc	1m 5s		trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚	javadoc	1m 38s		trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	spotbugs	3m 15s		trunk passed
+1 💚	shadedclient	34m 52s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	1m 10s		the patch passed
+1 💚	compile	1m 12s		the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚	javac	1m 12s		the patch passed
+1 💚	compile	1m 4s		the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	javac	1m 4s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 54s		the patch passed
+1 💚	mvnsite	1m 12s		the patch passed
+1 💚	javadoc	0m 51s		the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚	javadoc	1m 29s		the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	spotbugs	3m 15s		the patch passed
+1 💚	shadedclient	34m 18s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	215m 20s		hadoop-hdfs in the patch passed.
+1 💚	asflicense	0m 44s		The patch does not generate ASF License warnings.
		349m 29s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6457/2/artifact/out/Dockerfile
GITHUB PR	#6457
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux b5fe36a2048b 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 / `70583a0`
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-6457/2/testReport/
Max. process+thread count	3329 (vs. ulimit of 5500)
modules	C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6457/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 · 2024-01-18T06:23:46Z

@tasanuma @Hexiaoqiao @virajjasani Can you help review this PR? Thank you very much!

tasanuma

+1. Thanks for working on this!

Hexiaoqiao

LGTM. +1 from my side.

virajjasani · 2024-01-18T16:20:17Z

Thanks for working on the revert to unblock 3.4.0!

slfan1989 · 2024-01-18T23:36:55Z

@ayushtkn Can you help review this PR? Thank you very much!

ayushtkn · 2024-01-19T08:31:04Z

Revert doesn't need approval, just revert and remove 3.4.0 from the fix version in the original ticket and mention it has been reverted as part of this ticket for this reason

tasanuma · 2024-01-19T12:46:56Z

Creating another ticket is my request, as I commented here. We also want to revert HDFS-16016 from branch-3.3, but we cannot remove 3.3.6 from the fix version. So, if it has been released once or more, I would request another JIRA for reverting it.

ayushtkn

yep, the released versions shouldn't be removed, just the unreleased versions should be removed from the fix version & just link this ticket & put a comment on the original ticket for posterity "that this ticket got reverted as part of X ticket"
& We are all set

slfan1989 · 2024-01-19T23:53:14Z

@tasanuma @Hexiaoqiao @ayushtkn @virajjasani Thanks for reviewing the code! merged into trunk.

#2998)" (#6457) Contributed by Shilun Fan. This reverts commit c1bf3cb. Reviewed-by: Takanobu Asanuma <tasanuma@apache.org> Reviewed-by: He Xiaoqiao <hexiaoqiao@apache.org> Reviewed-by: Ayush Saxena <ayushsaxena@apache.org> Reviewed-by: Viraj Jasani <vjasani@apache.org> Signed-off-by: Shilun Fan <slfan1989@apache.org>

apache#2998)" (apache#6457) Contributed by Shilun Fan. This reverts commit c1bf3cb. Reviewed-by: Takanobu Asanuma <tasanuma@apache.org> Reviewed-by: He Xiaoqiao <hexiaoqiao@apache.org> Reviewed-by: Ayush Saxena <ayushsaxena@apache.org> Reviewed-by: Viraj Jasani <vjasani@apache.org> Signed-off-by: Shilun Fan <slfan1989@apache.org>

Revert "HDFS-16016. BPServiceActor to provide new thread to handle IBR (

a25f1e5

apache#2998)" This reverts commit c1bf3cb

github-actions bot added HDFS trunk labels Jan 17, 2024

Merge branch 'apache:trunk' into HDFS-16016

70583a0

slfan1989 changed the title ~~Revert "HDFS-16016. BPServiceActor to provide new thread to handle IBR (#2998)"~~ HDFS-17343. Revert "HDFS-16016. BPServiceActor to provide new thread to handle IBR (#2998)" Jan 18, 2024

tasanuma approved these changes Jan 18, 2024

View reviewed changes

Hexiaoqiao approved these changes Jan 18, 2024

View reviewed changes

ayushtkn approved these changes Jan 19, 2024

View reviewed changes

slfan1989 merged commit 15e1789 into apache:trunk Jan 19, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HDFS-17343. Revert "HDFS-16016. BPServiceActor to provide new thread to handle IBR (#2998)" #6457

HDFS-17343. Revert "HDFS-16016. BPServiceActor to provide new thread to handle IBR (#2998)" #6457

slfan1989 commented Jan 17, 2024 •

edited

Loading

yuanboliu commented Jan 17, 2024

tasanuma commented Jan 17, 2024

tasanuma commented Jan 17, 2024

slfan1989 commented Jan 17, 2024

yuanboliu commented Jan 17, 2024

tasanuma commented Jan 17, 2024

hadoop-yetus commented Jan 17, 2024

slfan1989 commented Jan 17, 2024

hadoop-yetus commented Jan 17, 2024

slfan1989 commented Jan 18, 2024

tasanuma left a comment

Hexiaoqiao left a comment

virajjasani commented Jan 18, 2024

slfan1989 commented Jan 18, 2024

ayushtkn commented Jan 19, 2024

tasanuma commented Jan 19, 2024

ayushtkn left a comment

slfan1989 commented Jan 19, 2024

HDFS-17343. Revert "HDFS-16016. BPServiceActor to provide new thread to handle IBR (#2998)" #6457

HDFS-17343. Revert "HDFS-16016. BPServiceActor to provide new thread to handle IBR (#2998)" #6457

Conversation

slfan1989 commented Jan 17, 2024 • edited Loading

Description of PR

How was this patch tested?

For code changes:

yuanboliu commented Jan 17, 2024

tasanuma commented Jan 17, 2024

tasanuma commented Jan 17, 2024

slfan1989 commented Jan 17, 2024

yuanboliu commented Jan 17, 2024

tasanuma commented Jan 17, 2024

hadoop-yetus commented Jan 17, 2024

slfan1989 commented Jan 17, 2024

hadoop-yetus commented Jan 17, 2024

slfan1989 commented Jan 18, 2024

tasanuma left a comment

Choose a reason for hiding this comment

Hexiaoqiao left a comment

Choose a reason for hiding this comment

virajjasani commented Jan 18, 2024

slfan1989 commented Jan 18, 2024

ayushtkn commented Jan 19, 2024

tasanuma commented Jan 19, 2024

ayushtkn left a comment

Choose a reason for hiding this comment

slfan1989 commented Jan 19, 2024

slfan1989 commented Jan 17, 2024 •

edited

Loading