[
https://issues.apache.org/jira/browse/HDFS-17878?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18070295#comment-18070295
]
ASF GitHub Bot commented on HDFS-17878:
---------------------------------------
ZanderXu commented on code in PR #8220:
URL: https://github.com/apache/hadoop/pull/8220#discussion_r3021262310
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java:
##########
@@ -1534,15 +1562,19 @@ void setNumStaleStorages(int numStaleStorages) {
/** Fetch live and dead datanodes. */
public void fetchDatanodes(final List<DatanodeDescriptor> live,
- final List<DatanodeDescriptor> dead, final boolean
removeDecommissionNode) {
+ final List<DatanodeDescriptor> dead, final boolean
removeDecommissionNode, boolean useCache) {
Review Comment:
please add a method without many changes.
```
fetchDatanodes(final List<DatanodeDescriptor> live,
final List<DatanodeDescriptor> dead, final boolean
removeDecommissionNode) {
fetchDatanodes(live, dead, removeDecommisionNode, false);
}
```
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java:
##########
@@ -1635,6 +1667,37 @@ private DatanodeID parseDNFromHostsEntry(String
hostLine) {
return dnId;
}
+ public void refreshDatanodeListSnapshot(long newExpirationMs) {
Review Comment:
It's better to support this hot-reload operations in a new PR if you think
it's necessary.
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java:
##########
@@ -230,6 +233,9 @@ public class DatanodeManager {
private final boolean randomNodeOrderEnabled;
+ /** Cached map of DatanodeReportType -> list of DatanodeDescriptor for
metrics purposes. */
+ private volatile Cache<DatanodeReportType, List<DatanodeDescriptor>>
datanodeListSnapshots = null;
Review Comment:
`volatile` is unnecessary?
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java:
##########
@@ -945,6 +963,11 @@ void addDatanode(final DatanodeDescriptor node) {
synchronized(this) {
host2DatanodeMap.remove(datanodeMap.put(node.getDatanodeUuid(), node));
}
+ Cache<DatanodeReportType, List<DatanodeDescriptor>>
tmpDatanodeListSnapshots =
Review Comment:
how about remove this `invalidateAll`.
If you want to remove the dead datanode from the cache in time, we also need
to update the decommission status, right? So we can make it more simple, just
updated the cache with a fixed interval.
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java:
##########
@@ -1722,6 +1722,9 @@ public void verifyReplication(String src,
public boolean isSufficientlyReplicated(BlockInfo b) {
// Compare against the lesser of the minReplication and number of live DNs.
final int liveReplicas = countNodes(b).liveReplicas();
+ if (liveReplicas == 0) {
Review Comment:
Perhaps we can make this PR more focused, simply providing a cache machine
for a list of DataNodes, without making any other modifications.
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java:
##########
@@ -1635,6 +1667,37 @@ private DatanodeID parseDNFromHostsEntry(String
hostLine) {
return dnId;
}
+ public void refreshDatanodeListSnapshot(long newExpirationMs) {
+ if (newExpirationMs <= 0) {
+ LOG.info("New config is non-positive ({}), disabling DN list cache",
newExpirationMs);
+ datanodeListSnapshots = null;
+ } else {
+ LOG.info("Resetting DN list cache with new expiration time {}ms",
newExpirationMs);
+ datanodeListSnapshots = CacheBuilder.newBuilder()
+ .expireAfterWrite(newExpirationMs, TimeUnit.MILLISECONDS)
+ .build();
+ }
+ }
+
+ /**
+ * Low impact version of {@link #getDatanodeListForReport} with possible
stale
+ * data for low impact usage (metrics).
+ */
+ public List<DatanodeDescriptor> getDatanodeListSnapshotForReport(
Review Comment:
`getDatanodeListForReportWithCache`
> Reduce frequency of getDatanodeListForReport calls for metrics
> --------------------------------------------------------------
>
> Key: HDFS-17878
> URL: https://issues.apache.org/jira/browse/HDFS-17878
> Project: Hadoop HDFS
> Issue Type: Improvement
> Reporter: Felix N
> Assignee: Felix N
> Priority: Minor
> Labels: pull-request-available
>
> getDatanodeListForReport is called by a lot of metrics method while holding
> synchronized lock, interfering with more critical ops like datanodeReport
> while not having critical data (metrics). Best to reduce the frequency of
> calls to this method.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
