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.

Sign up for GitHub

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

Jump to bottom

HDFS-16016. BPServiceActor to provide new thread to handle IBR #2998

Merged
merged 2 commits into from
Jun 14, 2021
Merged

HDFS-16016. BPServiceActor to provide new thread to handle IBR #2998

merged 2 commits into from
Jun 14, 2021

Conversation

virajjasani
Copy link
Contributor

@virajjasani virajjasani commented May 11, 2021 •
edited by smengcl
Loading

@hadoop-yetus

This comment has been minimized.

Sign in to view
@hadoop-yetus

This comment has been minimized.

Sign in to view
@virajjasani virajjasani force-pushed the HDFS-16016-trunk branch 2 times, most recently from 0f58570 to 0832df5 Compare May 12, 2021 16:11
@virajjasani virajjasani reopened this May 12, 2021
@hadoop-yetus

This comment has been minimized.

Sign in to view
@hadoop-yetus

This comment has been minimized.

Sign in to view
@hadoop-yetus

This comment has been minimized.

Sign in to view
@hadoop-yetus

This comment has been minimized.

Sign in to view
@hadoop-yetus

This comment has been minimized.

Sign in to view
@hadoop-yetus

This comment has been minimized.

Sign in to view
@virajjasani
Copy link
Contributor Author

virajjasani commented May 16, 2021 •
edited
Loading

Surprisingly I am not able to repro TestDecommissioningStatus and TestDecommissioningStatusWithBackoffMonitor test failures locally at all.

@virajjasani
Copy link
Contributor Author

@jojochuang @smengcl would you like to take a look?

@virajjasani
Copy link
Contributor Author

@liuml07 would you like to take a look?

@liuml07
Copy link
Member

liuml07 commented May 25, 2021

May take a look next week if no one reviews.

@hadoop-yetus

This comment has been minimized.

Sign in to view
@virajjasani
Copy link
Contributor Author

FYI @liuml07 if you get some bandwidth. Thanks

smengcl
smengcl requested changes Jun 8, 2021
View reviewed changes
Copy link
Contributor

@smengcl smengcl left a comment

Choose a reason for hiding this comment

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

Thanks for the patch @virajjasani . Overall looks good. Pls find comments inline.

...project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BPServiceActor.java
}
// There is no work to do; sleep until hearbeat timer elapses,
// or work arrives, and then iterate again.
ibrManager.waitTillNextIBR(scheduler.getHeartbeatWaitTime());
Copy link
Contributor

Choose a reason for hiding this comment

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

This implies this IBR timer will expire around the same time as the FBR one (offerService()). This should be fine.

With IBR separated in a new thread, maybe later we could have a new config key that controls IBR interval separately, or add a configurable constant offset (from the FBR timer) to the IBR timer. This isn't something we need to add to this jira. Just a thought.

Just in case I miss anything, @sodonnel would you like to take a quick look at 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.

With IBR separated in a new thread, maybe later we could have a new config key that controls IBR interval separately, or add a configurable constant offset (from the FBR timer) to the IBR timer. This isn't something we need to add to this jira. Just a thought.

I agree. We can add new config or continue with IBR/FBR expiring around same time for some time.

...project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BPServiceActor.java Outdated Show resolved Hide resolved
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDatanodeReport.java Outdated Show resolved Hide resolved
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDatanodeReport.java Outdated Show resolved Hide resolved
...p-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestIncrementalBlockReports.java Outdated Show resolved Hide resolved
@hadoop-yetus

This comment has been minimized.

Sign in to view
@hadoop-yetus

This comment has been minimized.

Sign in to view
@hadoop-yetus

This comment has been minimized.

Sign in to view
@virajjasani virajjasani requested a review from smengcl June 11, 2021 05:14
@virajjasani
Copy link
Contributor Author

Thanks for the review @smengcl. I have addressed your concerns.

smengcl
smengcl approved these changes Jun 14, 2021
View reviewed changes
Copy link
Contributor

@smengcl smengcl left a comment

Choose a reason for hiding this comment

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

+1

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDatanodeReport.java Outdated Show resolved Hide resolved
@virajjasani @smengcl
Update hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoo…
0503dfd 
…p/hdfs/TestDatanodeReport.java

Co-authored-by: Siyao Meng <[email protected]>
@smengcl
Copy link
Contributor

smengcl commented Jun 14, 2021

Thanks @virajjasani for patch. Will merge shortly.

@smengcl smengcl merged commit c1bf3cb into apache:trunk Jun 14, 2021
@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 50s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 1s No case conflicting files found.
+0 🆗 codespell 0m 0s codespell 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 30m 48s trunk passed
+1 💚 compile 1m 24s trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚 compile 1m 19s trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚 checkstyle 1m 1s trunk passed
+1 💚 mvnsite 1m 25s trunk passed
+1 💚 javadoc 0m 58s trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚 javadoc 1m 29s trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚 spotbugs 3m 12s trunk passed
+1 💚 shadedclient 16m 9s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+1 💚 mvninstall 1m 14s the patch passed
+1 💚 compile 1m 13s the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚 javac 1m 13s the patch passed
+1 💚 compile 1m 8s the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚 javac 1m 8s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 checkstyle 0m 53s the patch passed
+1 💚 mvnsite 1m 14s the patch passed
+1 💚 javadoc 0m 47s the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚 javadoc 1m 21s the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚 spotbugs 3m 7s the patch passed
+1 💚 shadedclient 15m 56s patch has no errors when building and testing our client artifacts.
_ Other Tests _
-1 ❌ unit 355m 13s /patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt hadoop-hdfs in the patch passed.
+1 💚 asflicense 0m 47s The patch does not generate ASF License warnings.
439m 29s
Reason Tests
Failed junit tests hadoop.hdfs.web.TestWebHdfsFileSystemContract
hadoop.hdfs.server.namenode.TestDecommissioningStatusWithBackoffMonitor
hadoop.hdfs.server.namenode.ha.TestBootstrapStandby
hadoop.hdfs.server.namenode.TestDecommissioningStatus
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2998/25/artifact/out/Dockerfile
GITHUB PR #2998
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell
uname Linux 09ad53fd21e2 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision trunk / 0503dfd
Default Java Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
Multi-JDK versions /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2998/25/testReport/
Max. process+thread count 2757 (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-2998/25/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.

@virajjasani virajjasani deleted the HDFS-16016-trunk branch June 15, 2021 07:01
kiran-maturi pushed a commit to kiran-maturi/hadoop that referenced this pull request Nov 24, 2021
@virajjasani
HDFS-16016. BPServiceActor to provide new thread to handle IBR (apach…
89df9e4 
…e#2998)

Contributed by Viraj Jasani
tasanuma pushed a commit that referenced this pull request Oct 24, 2022
@virajjasani @tasanuma
HDFS-16016. BPServiceActor to provide new thread to handle IBR (#2998)
36a0e81 
Contributed by Viraj Jasani

(cherry picked from commit c1bf3cb)
jojochuang pushed a commit to jojochuang/hadoop that referenced this pull request May 23, 2023
@virajjasani
CDPD-45291. HDFS-16016. BPServiceActor to provide new thread to handl…
63d3917 
…e IBR (apache#2998)

Contributed by Viraj Jasani

(cherry picked from commit c1bf3cb)
Change-Id: Iedabb8f14c8165c62431916ae34320a5f63f1977
(cherry picked from commit 49c412895085c996ebf77352aee7e5961fe91aef)
@slfan1989 slfan1989 mentioned this pull request Jan 15, 2024
Open
@slfan1989
Copy link
Contributor

@virajjasani We plan to revert this pr. There have been user reports indicating that it may cause dn ibr and fbr to be mis-order. we can refer to #6244 for more details.

slfan1989 added a commit to slfan1989/hadoop that referenced this pull request Jan 17, 2024
@slfan1989
Revert "HDFS-16016. BPServiceActor to provide new thread to handle IBR (
a25f1e5 
apache#2998)"

This reverts commit c1bf3cb
@slfan1989 slfan1989 mentioned this pull request Jan 17, 2024
Merged
4 tasks
slfan1989 added a commit that referenced this pull request Jan 19, 2024
@slfan1989
Revert "HDFS-16016. BPServiceActor to provide new thread to handle IBR (
15e1789 
#2998)" (#6457) Contributed by Shilun Fan.

This reverts commit c1bf3cb.

Reviewed-by: Takanobu Asanuma <[email protected]>
Reviewed-by: He Xiaoqiao <[email protected]>
Reviewed-by: Ayush Saxena <[email protected]>
Reviewed-by: Viraj Jasani <[email protected]>
Signed-off-by: Shilun Fan <[email protected]>
slfan1989 added a commit that referenced this pull request Jan 20, 2024
@slfan1989
Revert "HDFS-16016. BPServiceActor to provide new thread to handle IBR (
76887c1 
#2998)" (#6457) Contributed by Shilun Fan.

This reverts commit c1bf3cb.

Reviewed-by: Takanobu Asanuma <[email protected]>
Reviewed-by: He Xiaoqiao <[email protected]>
Reviewed-by: Ayush Saxena <[email protected]>
Reviewed-by: Viraj Jasani <[email protected]>
Signed-off-by: Shilun Fan <[email protected]>
slfan1989 added a commit that referenced this pull request Jan 20, 2024
@slfan1989
Revert "HDFS-16016. BPServiceActor to provide new thread to handle IBR (
10197ac 
#2998)" (#6457) Contributed by Shilun Fan.

This reverts commit c1bf3cb.

Reviewed-by: Takanobu Asanuma <[email protected]>
Reviewed-by: He Xiaoqiao <[email protected]>
Reviewed-by: Ayush Saxena <[email protected]>
Reviewed-by: Viraj Jasani <[email protected]>
Signed-off-by: Shilun Fan <[email protected]>
jiajunmao pushed a commit to jiajunmao/hadoop-MLEC that referenced this pull request Feb 6, 2024
@slfan1989 @jiajunmao
Revert "HDFS-16016. BPServiceActor to provide new thread to handle IBR (
d7e4ed9 
apache#2998)" (apache#6457) Contributed by Shilun Fan.

This reverts commit c1bf3cb.

Reviewed-by: Takanobu Asanuma <[email protected]>
Reviewed-by: He Xiaoqiao <[email protected]>
Reviewed-by: Ayush Saxena <[email protected]>
Reviewed-by: Viraj Jasani <[email protected]>
Signed-off-by: Shilun Fan <[email protected]>
LiuGuH pushed a commit to LiuGuH/hadoop that referenced this pull request Mar 26, 2024
@virajjasani
HDFS-16016. BPServiceActor to provide new thread to handle IBR (apach…
ca27467 
…e#2998)

Contributed by Viraj Jasani
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smengcl smengcl smengcl approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@virajjasani @hadoop-yetus @liuml07 @smengcl @slfan1989