[ 
https://issues.apache.org/jira/browse/HDFS-14986?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16980017#comment-16980017
 ] 

Yiqun Lin edited comment on HDFS-14986 at 11/22/19 9:57 AM:
------------------------------------------------------------

Hi [~Aiphag0], the latest patch can still be simplified. Actually we don't need 
to add {{runImmediately}} for RefreshThread. Only one thing we need to do is 
that we skip init refresh in FSCachingGetSpaceUsed. Here init refresh just 
means first refresh operation. Hope you have got my idea.
 So the change can simplified to this
{noformat}
--- 
a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CachingGetSpaceUsed.java
+++ 
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CachingGetSpaceUsed.java
@@ -47,6 +47,7 @@
   private final long jitter;
   private final String dirPath;
   private Thread refreshUsed;
+  private boolean shouldInitRefresh;

   /**
    * This is the constructor used by the builder.
@@ -79,12 +80,15 @@ public CachingGetSpaceUsed(CachingGetSpaceUsed.Builder 
builder)
     this.refreshInterval = interval;
     this.jitter = jitter;
     this.used.set(initialUsed);
+    this.shouldInitRefresh = true;
   }

   void init() {
     if (used.get() < 0) {
       used.set(0);
-      refresh();
+      if (shouldInitRefresh) {
+       refresh();
+      }
     }

     if (refreshInterval > 0) {
@@ -145,6 +149,14 @@ protected void setUsed(long usedValue) {
     this.used.set(usedValue);
   }

+  /**
+   * Reset that if we need to do the initial refresh.
+   * @param shouldInitRefresh The flag value to set.
+   */
+  protected void setShouldInitRefresh(boolean shouldInitRefresh) {
+    this.shouldInitRefresh = shouldInitRefresh;
+  }
+
   @Override
   public void close() throws IOException {
     running.set(false);
diff --git 
a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/FSCachingGetSpaceUsed.java
 
b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/FSCachingGetSpaceUsed.java
index a5f350860f3..0a922b2d22f 100644
--- 
a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/FSCachingGetSpaceUsed.java
+++ 
b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/FSCachingGetSpaceUsed.java
@@ -39,6 +39,7 @@

   public FSCachingGetSpaceUsed(Builder builder) throws IOException {
     super(builder);
+    this.setShouldInitRefresh(false);
   }
{noformat}
 


was (Author: linyiqun):
Hi [~Aiphag0], the latest patch can still be simplified. Actually we don't need 
to add {{runImmediately}} for RefreshThread. Only one thing we need to do is 
that we skip init refresh in FSCachingGetSpaceUsed.  Hope you have got my idea.
 So the change can simplified to this
{noformat}
--- 
a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CachingGetSpaceUsed.java
+++ 
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CachingGetSpaceUsed.java
@@ -47,6 +47,7 @@
   private final long jitter;
   private final String dirPath;
   private Thread refreshUsed;
+  private boolean shouldInitRefresh;

   /**
    * This is the constructor used by the builder.
@@ -79,12 +80,15 @@ public CachingGetSpaceUsed(CachingGetSpaceUsed.Builder 
builder)
     this.refreshInterval = interval;
     this.jitter = jitter;
     this.used.set(initialUsed);
+    this.shouldInitRefresh = true;
   }

   void init() {
     if (used.get() < 0) {
       used.set(0);
-      refresh();
+      if (shouldInitRefresh) {
+       refresh();
+      }
     }

     if (refreshInterval > 0) {
@@ -145,6 +149,14 @@ protected void setUsed(long usedValue) {
     this.used.set(usedValue);
   }

+  /**
+   * Reset that if we need to do the initial refresh.
+   * @param shouldInitRefresh The flag value to set.
+   */
+  protected void setShouldInitRefresh(boolean shouldInitRefresh) {
+    this.shouldInitRefresh = shouldInitRefresh;
+  }
+
   @Override
   public void close() throws IOException {
     running.set(false);
diff --git 
a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/FSCachingGetSpaceUsed.java
 
b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/FSCachingGetSpaceUsed.java
index a5f350860f3..0a922b2d22f 100644
--- 
a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/FSCachingGetSpaceUsed.java
+++ 
b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/FSCachingGetSpaceUsed.java
@@ -39,6 +39,7 @@

   public FSCachingGetSpaceUsed(Builder builder) throws IOException {
     super(builder);
+    this.setShouldInitRefresh(false);
   }
{noformat}
 

> ReplicaCachingGetSpaceUsed throws  ConcurrentModificationException
> ------------------------------------------------------------------
>
>                 Key: HDFS-14986
>                 URL: https://issues.apache.org/jira/browse/HDFS-14986
>             Project: Hadoop HDFS
>          Issue Type: Bug
>          Components: datanode, performance
>            Reporter: Ryan Wu
>            Assignee: Aiphago
>            Priority: Major
>         Attachments: HDFS-14986.001.patch, HDFS-14986.002.patch, 
> HDFS-14986.003.patch, HDFS-14986.004.patch
>
>
> Running DU across lots of disks is very expensive . We applied the patch 
> HDFS-14313 to get  used space from ReplicaInfo in memory.However, new du 
> threads throw the exception
> {code:java}
> // 2019-11-08 18:07:13,858 ERROR 
> [refreshUsed-/home/vipshop/hard_disk/7/dfs/dn/current/BP-1203969992-XXXX-1450855658517]
>  
> org.apache.hadoop.hdfs.server.datanode.fsdataset.impl.ReplicaCachingGetSpaceUsed:
>  ReplicaCachingGetSpaceUsed refresh error
> java.util.ConcurrentModificationException: Tree has been modified outside of 
> iterator    
> at 
> org.apache.hadoop.hdfs.util.FoldedTreeSet$TreeSetIterator.checkForModification(FoldedTreeSet.java:311)
>     
> at 
> org.apache.hadoop.hdfs.util.FoldedTreeSet$TreeSetIterator.hasNext(FoldedTreeSet.java:256)
>     
> at java.util.AbstractCollection.addAll(AbstractCollection.java:343)    
> at java.util.HashSet.<init>(HashSet.java:120)    
> at 
> org.apache.hadoop.hdfs.server.datanode.fsdataset.impl.FsDatasetImpl.deepCopyReplica(FsDatasetImpl.java:1052)
>     
> at 
> org.apache.hadoop.hdfs.server.datanode.fsdataset.impl.ReplicaCachingGetSpaceUsed.refresh(ReplicaCachingGetSpaceUsed.java:73)
>     
> at 
> org.apache.hadoop.fs.CachingGetSpaceUsed$RefreshThread.run(CachingGetSpaceUsed.java:178)
>    
> at java.lang.Thread.run(Thread.java:748)
> {code}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to