HDFS-16426. Fix nextBlockReportTime when trigger full block report force #3887
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tomscut
reviewed
Jan 13, 2022
| // If we have sent the first set of block reports, then wait a random | ||
| // time before we start the periodic block reports. | ||
| if (resetBlockReportTime) { | ||
| nextBlockReportTime.getAndSet(monotonicNow() + | ||
| ThreadLocalRandom.current().nextInt((int) (blockReportIntervalMs))); | ||
| resetBlockReportTime = false; | ||
| } else if (forceFullBr) { | ||
| nextBlockReportTime.getAndSet(monotonicNow() + blockReportIntervalMs); |
There was a problem hiding this comment.
If many datanodes of a large cluster is triggered in batches, the FBR time of these datanodes will be concentrated in the future, which may cause great pressure on NN. Maybe we also need to add a random value here.
There was a problem hiding this comment.
I think that in most of cases, we want to know the exactly nextBlockReportTime if we trigger block report by force.
As for trigger full block report in batches, I think it is a dangerous behavior because it may cause some datanode heartbeat timeout.
There was a problem hiding this comment.
I think that in most of cases, we want to know the exactly nextBlockReportTime if we trigger block report by force. As for trigger full block report in batches, I think it is a dangerous behavior because it may cause some datanode heartbeat timeout.
I mean, if we trigger the FBR on some nodes in a short period of time, it will affect the original randomness.
There was a problem hiding this comment.
@liubingxing @tomscut
Thanks for the discussion. IMHO, I prefer to use a random time to keep the randomness.
I think HDFS-15167 causes this problem. Before HDFS-15167, resetBlockReportTime is true when triggering full block report by force, and it used a random time.
|
🎊 +1 overall
This message was automatically generated. |
|
@tasanuma @Hexiaoqiao Please take a look. |
liubingxing
force-pushed
the
HDFS-16426
branch
from
1a64e33
to
6965a47
Compare
|
@tasanuma Thanks for your advice, I update the PR to set resetBlockReportTime=true when triggering full block report by force. Please take a look. |
|
💔 -1 overall
This message was automatically generated. |
|
Thanks for your contribution, @liubingxing. Thanks for your review, @tomscut. |
bogthe
pushed a commit
to bogthe/hadoop
that referenced
this pull request
Feb 2, 2022
…rce (apache#3887) (cherry picked from commit fcb1076)
HarshitGupta11
pushed a commit
to HarshitGupta11/hadoop
that referenced
this pull request
Nov 28, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
jira: HDFS-16426
When we trigger full block report force by command line, the next block report time will be set like this:
nextBlockReportTime.getAndAdd(blockReportIntervalMs);
nextBlockReportTime will larger than blockReportIntervalMs.
If we trigger full block report twice, the nextBlockReportTime will larger than 2 * blockReportIntervalMs. This is obviously not what we want.
We fix the nextBlockReportTime = now + blockReportIntervalMs after full block report trigger by command line.