[
https://issues.apache.org/jira/browse/HDFS-14986?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16983153#comment-16983153
]
Yiqun Lin edited comment on HDFS-14986 at 11/27/19 4:04 AM:
------------------------------------------------------------
Comment makes sense to me. I have minor suggestion for {{shouldInitRefresh}}.
The meaning of shouldInitRefresh should be that 'should do init refresh
operation', so the default value should be true rather than false. So I prefer
to change this and add comment:
{code}
void init() {
if (used.get() < 0) {
used.set(0);
// Skip initial refresh operation, so we need to do first refresh
// operation immediately in refresh thread.
+ if (!shouldInitRefresh) {
+ initRefeshThread(true);
+ return;
+ }
refresh();
}
+ initRefeshThread(false);
+ }
{code}
Can we move this method into CachingGetSpaceUsed? The variable is under
CachingGetSpaceUsed, it will be better also put the set operation in this class
and let sub-classes to use this.
{code}
+ /**
+ * 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;
+ }
{code}
Can you update method name and comment for this method?
{code}
void initRefeshThread (boolean runImmediately)
{code}
to
{code}
// add comment here.
private void initRefeshThread (boolean runImmediately)
{code}
was (Author: linyiqun):
Comment makes sense to me. I have minor suggestion for {{shouldInitRefresh}}.
The meaning of shouldInitRefresh should be that 'should do init refresh
operation', so the default value should be true rather than false. So I prefer
to change this and add comment:
{code}
void init() {
if (used.get() < 0) {
used.set(0);
// Skip initial refresh operation, so we need to do first refresh
// operation immediately in refresh thread.
+ if (!shouldInitRefresh) {
+ initRefeshThread(true);
+ return;
+ }
refresh();
}
+ initRefeshThread(false);
+ }
{code}
Can we move this method into CachingGetSpaceUsed? The variable is under
CachingGetSpaceUsed, it will be better also put the set operation in this class
and let sub-classes to use this.
{code}
+ /**
+ * 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;
+ }
{code}
> 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]
