-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HDFS-16521. DFS API to retrieve slow datanodes #4107
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,6 +54,8 @@ public class DataNodePeerMetrics { | |
|
||
private final String name; | ||
|
||
// Strictly to be used by test code only. Source code is not supposed to use this. | ||
private Map<String, Double> testOutlier = null; | ||
|
||
private final OutlierDetector slowNodeDetector; | ||
|
||
|
@@ -142,14 +144,28 @@ public void collectThreadLocalStates() { | |
* than their peers. | ||
*/ | ||
public Map<String, Double> getOutliers() { | ||
// This maps the metric name to the aggregate latency. | ||
// The metric name is the datanode ID. | ||
final Map<String, Double> stats = | ||
sendPacketDownstreamRollingAverages.getStats( | ||
minOutlierDetectionSamples); | ||
LOG.trace("DataNodePeerMetrics: Got stats: {}", stats); | ||
|
||
return slowNodeDetector.getOutliers(stats); | ||
// outlier must be null for source code. | ||
if (testOutlier == null) { | ||
// This maps the metric name to the aggregate latency. | ||
// The metric name is the datanode ID. | ||
final Map<String, Double> stats = | ||
sendPacketDownstreamRollingAverages.getStats(minOutlierDetectionSamples); | ||
LOG.trace("DataNodePeerMetrics: Got stats: {}", stats); | ||
return slowNodeDetector.getOutliers(stats); | ||
} else { | ||
// this happens only for test code. | ||
return testOutlier; | ||
} | ||
} | ||
|
||
/** | ||
* Strictly to be used by test code only. Source code is not supposed to use this. This method | ||
* directly sets outlier mapping so that aggregate latency metrics are not calculated for tests. | ||
* | ||
* @param outlier outlier directly set by tests. | ||
*/ | ||
public void setTestOutliers(Map<String, Double> outlier) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a little awkward? Add comment on the testOutlier data member that it is for test only? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah it's very difficult to reproduce the actual slow node in UT, hence had to do this way. Sure, added comment on |
||
this.testOutlier = outlier; | ||
} | ||
|
||
public MutableRollingAverages getSendPacketDownstreamRollingAverages() { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just want to check with every one that it is okay to have an array of objects as the return value.
I think it's fine but just want to check with every one, because once we decide the the interface it can't be changed later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought List is also fine but kept it Array to keep the API contract in line with
getDatanodeReport()
so that both APIs can use same underlying utility methods (e.g. getDatanodeInfoFromDescriptors() ).