[jira] [Commented] (HDFS-14610) HashMap is not thread safe. Field storageMap is typically synchronized by storageMap. However, in one place, field storageMap is not protected with synchronized.
[ https://issues.apache.org/jira/browse/HDFS-14610?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16876695#comment-16876695 ] Paul Ward commented on HDFS-14610: -- Hi Anu, Thank you for sending those links. Indeed, I am not familiar with the CR flow for Hadoop, so those links speed up a *lot* my figuring out what is where. Thanks!! > HashMap is not thread safe. Field storageMap is typically synchronized by > storageMap. However, in one place, field storageMap is not protected with > synchronized. > - > > Key: HDFS-14610 > URL: https://issues.apache.org/jira/browse/HDFS-14610 > Project: Hadoop HDFS > Issue Type: Bug >Reporter: Paul Ward >Assignee: Paul Ward >Priority: Critical > Labels: fix-provided, patch-available > Fix For: 3.3.0 > > Attachments: addingSynchronization.patch > > > I submitted a CR for this issue at: > [https://github.com/apache/hadoop/pull/1015] > The field *storageMap* (a *HashMap*) > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L155] > is typically protected by synchronization on *storageMap*, e.g., > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L294] > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L443] > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] > For a total of 9 locations. > The reason is because *HashMap* is not thread safe. > However, here: > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L455] > {{DatanodeStorageInfo storage =}} > {{ storageMap.get(report.getStorage().getStorageID());}} > It is not synchronized. > Note that in the same method: > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] > *storageMap* is again protected by synchronization: > {{synchronized (storageMap) {}} > {{ storageMapSize = storageMap.size();}} > {{}}} > > The CR I inlined above protected the above instance (line 455 ) with > synchronization > like in line 484 and in all other occurrences. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-14610) HashMap is not thread safe. Field storageMap is typically synchronized by storageMap. However, in one place, field storageMap is not protected with synchronized.
[ https://issues.apache.org/jira/browse/HDFS-14610?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16876323#comment-16876323 ] Paul Ward commented on HDFS-14610: -- Hi Anu, Thank you for explaining this convention. I was not aware of it. I will do the tagging from now on. Thanks! > HashMap is not thread safe. Field storageMap is typically synchronized by > storageMap. However, in one place, field storageMap is not protected with > synchronized. > - > > Key: HDFS-14610 > URL: https://issues.apache.org/jira/browse/HDFS-14610 > Project: Hadoop HDFS > Issue Type: Bug >Reporter: Paul Ward >Assignee: Paul Ward >Priority: Critical > Labels: fix-provided, patch-available > Attachments: addingSynchronization.patch > > > I submitted a CR for this issue at: > [https://github.com/apache/hadoop/pull/1015] > The field *storageMap* (a *HashMap*) > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L155] > is typically protected by synchronization on *storageMap*, e.g., > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L294] > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L443] > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] > For a total of 9 locations. > The reason is because *HashMap* is not thread safe. > However, here: > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L455] > {{DatanodeStorageInfo storage =}} > {{ storageMap.get(report.getStorage().getStorageID());}} > It is not synchronized. > Note that in the same method: > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] > *storageMap* is again protected by synchronization: > {{synchronized (storageMap) {}} > {{ storageMapSize = storageMap.size();}} > {{}}} > > The CR I inlined above protected the above instance (line 455 ) with > synchronization > like in line 484 and in all other occurrences. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-14610) HashMap is not thread safe. Field storageMap is typically synchronized by storageMap. However, in one place, field storageMap is not protected with synchronized.
[ https://issues.apache.org/jira/browse/HDFS-14610?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16875954#comment-16875954 ] Paul Ward commented on HDFS-14610: -- Anu, Can you please take a look at the patch, like you did for: https://issues.apache.org/jira/browse/HDFS-14618 ? I.e., I do not think those tests fail due to the patch. Thanks > HashMap is not thread safe. Field storageMap is typically synchronized by > storageMap. However, in one place, field storageMap is not protected with > synchronized. > - > > Key: HDFS-14610 > URL: https://issues.apache.org/jira/browse/HDFS-14610 > Project: Hadoop HDFS > Issue Type: Bug >Reporter: Paul Ward >Priority: Critical > Labels: fix-provided, patch-available > Attachments: addingSynchronization.patch > > > I submitted a CR for this issue at: > [https://github.com/apache/hadoop/pull/1015] > The field *storageMap* (a *HashMap*) > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L155] > is typically protected by synchronization on *storageMap*, e.g., > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L294] > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L443] > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] > For a total of 9 locations. > The reason is because *HashMap* is not thread safe. > However, here: > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L455] > {{DatanodeStorageInfo storage =}} > {{ storageMap.get(report.getStorage().getStorageID());}} > It is not synchronized. > Note that in the same method: > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] > *storageMap* is again protected by synchronization: > {{synchronized (storageMap) {}} > {{ storageMapSize = storageMap.size();}} > {{}}} > > The CR I inlined above protected the above instance (line 455 ) with > synchronization > like in line 484 and in all other occurrences. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Assigned] (HDFS-14610) HashMap is not thread safe. Field storageMap is typically synchronized by storageMap. However, in one place, field storageMap is not protected with synchronized.
[ https://issues.apache.org/jira/browse/HDFS-14610?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Ward reassigned HDFS-14610: Assignee: Anu Engineer > HashMap is not thread safe. Field storageMap is typically synchronized by > storageMap. However, in one place, field storageMap is not protected with > synchronized. > - > > Key: HDFS-14610 > URL: https://issues.apache.org/jira/browse/HDFS-14610 > Project: Hadoop HDFS > Issue Type: Bug >Reporter: Paul Ward >Assignee: Anu Engineer >Priority: Critical > Labels: fix-provided, patch-available > Attachments: addingSynchronization.patch > > > I submitted a CR for this issue at: > [https://github.com/apache/hadoop/pull/1015] > The field *storageMap* (a *HashMap*) > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L155] > is typically protected by synchronization on *storageMap*, e.g., > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L294] > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L443] > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] > For a total of 9 locations. > The reason is because *HashMap* is not thread safe. > However, here: > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L455] > {{DatanodeStorageInfo storage =}} > {{ storageMap.get(report.getStorage().getStorageID());}} > It is not synchronized. > Note that in the same method: > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] > *storageMap* is again protected by synchronization: > {{synchronized (storageMap) {}} > {{ storageMapSize = storageMap.size();}} > {{}}} > > The CR I inlined above protected the above instance (line 455 ) with > synchronization > like in line 484 and in all other occurrences. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Assigned] (HDFS-14610) HashMap is not thread safe. Field storageMap is typically synchronized by storageMap. However, in one place, field storageMap is not protected with synchronized.
[ https://issues.apache.org/jira/browse/HDFS-14610?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Ward reassigned HDFS-14610: Assignee: (was: Paul Ward) > HashMap is not thread safe. Field storageMap is typically synchronized by > storageMap. However, in one place, field storageMap is not protected with > synchronized. > - > > Key: HDFS-14610 > URL: https://issues.apache.org/jira/browse/HDFS-14610 > Project: Hadoop HDFS > Issue Type: Bug >Reporter: Paul Ward >Priority: Critical > Labels: fix-provided, patch-available > Attachments: addingSynchronization.patch > > > I submitted a CR for this issue at: > [https://github.com/apache/hadoop/pull/1015] > The field *storageMap* (a *HashMap*) > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L155] > is typically protected by synchronization on *storageMap*, e.g., > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L294] > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L443] > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] > For a total of 9 locations. > The reason is because *HashMap* is not thread safe. > However, here: > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L455] > {{DatanodeStorageInfo storage =}} > {{ storageMap.get(report.getStorage().getStorageID());}} > It is not synchronized. > Note that in the same method: > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] > *storageMap* is again protected by synchronization: > {{synchronized (storageMap) {}} > {{ storageMapSize = storageMap.size();}} > {{}}} > > The CR I inlined above protected the above instance (line 455 ) with > synchronization > like in line 484 and in all other occurrences. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-14618) Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe).
[ https://issues.apache.org/jira/browse/HDFS-14618?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16875302#comment-16875302 ] Paul Ward commented on HDFS-14618: -- Hi Anu, Btw, can you please also take a look at: https://issues.apache.org/jira/browse/HDFS-14610 I suppose there may be the same problem with non-deterministic performance. I also added there an explanation for why that patch does not introduce a deadlock Thanks. > Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe). > -- > > Key: HDFS-14618 > URL: https://issues.apache.org/jira/browse/HDFS-14618 > Project: Hadoop HDFS > Issue Type: Bug >Reporter: Paul Ward >Assignee: Paul Ward >Priority: Critical > Labels: fix-provided, patch-available > Fix For: 3.3.0 > > Attachments: race.patch > > > I submitted a CR for this issue at: > https://github.com/apache/hadoop/pull/1030 > The field {{timedOutItems}} (an {{ArrayList}}, i.e., not thread safe): > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L70 > is protected by synchronization on itself ({{timedOutItems}}): > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L167-L168 > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L267-L268 > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L178 > However, in one place: > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L133-L135 > it is (trying to be) protected by synchronized using > {{pendingReconstructions}} --- but this cannot protect {{timedOutItems}}. > Synchronized on different objects does not ensure mutual exclusion with the > other locations. > I.e., 2 code locations, one synchronized by {{pendingReconstructions}} and > the other by {{timedOutItems}} can still executed concurrently. > This CR adds the synchronized on {{timedOutItems}}. > Note that this CR keeps the synchronized on {{pendingReconstructions}}, which > is needed for a different purpose (protect {{pendingReconstructions}}) -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-14610) HashMap is not thread safe. Field storageMap is typically synchronized by storageMap. However, in one place, field storageMap is not protected with synchronized.
[ https://issues.apache.org/jira/browse/HDFS-14610?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16875301#comment-16875301 ] Paul Ward commented on HDFS-14610: -- I do not know why the tests fail and I am not familiar with Hadoop internals to debug the tests. However, note that the patch just adds a {{synchronized (storageMap) { }} at line 455 [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L455] just like the one at line 484 [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] Line 455 and 484 are in the same method body. I.e., this patch does not introduce a deadlock or such. Maybe the tests fail due to non-deterministic performance in the containers, like here ?: https://issues.apache.org/jira/browse/HDFS-14618 > HashMap is not thread safe. Field storageMap is typically synchronized by > storageMap. However, in one place, field storageMap is not protected with > synchronized. > - > > Key: HDFS-14610 > URL: https://issues.apache.org/jira/browse/HDFS-14610 > Project: Hadoop HDFS > Issue Type: Bug >Reporter: Paul Ward >Priority: Critical > Labels: fix-provided, patch-available > Attachments: addingSynchronization.patch > > > I submitted a CR for this issue at: > [https://github.com/apache/hadoop/pull/1015] > The field *storageMap* (a *HashMap*) > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L155] > is typically protected by synchronization on *storageMap*, e.g., > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L294] > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L443] > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] > For a total of 9 locations. > The reason is because *HashMap* is not thread safe. > However, here: > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L455] > {{DatanodeStorageInfo storage =}} > {{ storageMap.get(report.getStorage().getStorageID());}} > It is not synchronized. > Note that in the same method: > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] > *storageMap* is again protected by synchronization: > {{synchronized (storageMap) {}} > {{ storageMapSize = storageMap.size();}} > {{}}} > > The CR I inlined above protected the above instance (line 455 ) with > synchronization > like in line 484 and in all other occurrences. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-14618) Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe).
[ https://issues.apache.org/jira/browse/HDFS-14618?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16875294#comment-16875294 ] Paul Ward commented on HDFS-14618: -- Hi Anu, Ok, thank you for helping with those tests! > Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe). > -- > > Key: HDFS-14618 > URL: https://issues.apache.org/jira/browse/HDFS-14618 > Project: Hadoop HDFS > Issue Type: Bug >Reporter: Paul Ward >Assignee: Paul Ward >Priority: Critical > Labels: fix-provided, patch-available > Fix For: 3.3.0 > > Attachments: race.patch > > > I submitted a CR for this issue at: > https://github.com/apache/hadoop/pull/1030 > The field {{timedOutItems}} (an {{ArrayList}}, i.e., not thread safe): > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L70 > is protected by synchronization on itself ({{timedOutItems}}): > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L167-L168 > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L267-L268 > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L178 > However, in one place: > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L133-L135 > it is (trying to be) protected by synchronized using > {{pendingReconstructions}} --- but this cannot protect {{timedOutItems}}. > Synchronized on different objects does not ensure mutual exclusion with the > other locations. > I.e., 2 code locations, one synchronized by {{pendingReconstructions}} and > the other by {{timedOutItems}} can still executed concurrently. > This CR adds the synchronized on {{timedOutItems}}. > Note that this CR keeps the synchronized on {{pendingReconstructions}}, which > is needed for a different purpose (protect {{pendingReconstructions}}) -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-14618) Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe).
[ https://issues.apache.org/jira/browse/HDFS-14618?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16875251#comment-16875251 ] Paul Ward commented on HDFS-14618: -- Hi Anu, Thank you for assigning me this. I am not familiar with Hadoop internals to debug those tests. However, note that the patch grabs the locks in the same order as here: [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L257-L267] I.e., this patch does *not* introduce a deadlock. Beyond that, I don't know why this patch would cause anything to fail. Can you please take a look? Thanks > Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe). > -- > > Key: HDFS-14618 > URL: https://issues.apache.org/jira/browse/HDFS-14618 > Project: Hadoop HDFS > Issue Type: Bug >Reporter: Paul Ward >Assignee: Paul Ward >Priority: Critical > Labels: fix-provided, patch-available > Attachments: race.patch > > > I submitted a CR for this issue at: > https://github.com/apache/hadoop/pull/1030 > The field {{timedOutItems}} (an {{ArrayList}}, i.e., not thread safe): > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L70 > is protected by synchronization on itself ({{timedOutItems}}): > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L167-L168 > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L267-L268 > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L178 > However, in one place: > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L133-L135 > it is (trying to be) protected by synchronized using > {{pendingReconstructions}} --- but this cannot protect {{timedOutItems}}. > Synchronized on different objects does not ensure mutual exclusion with the > other locations. > I.e., 2 code locations, one synchronized by {{pendingReconstructions}} and > the other by {{timedOutItems}} can still executed concurrently. > This CR adds the synchronized on {{timedOutItems}}. > Note that this CR keeps the synchronized on {{pendingReconstructions}}, which > is needed for a different purpose (protect {{pendingReconstructions}}) -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Updated] (HDFS-14618) Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe).
[ https://issues.apache.org/jira/browse/HDFS-14618?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Ward updated HDFS-14618: - Attachment: (was: race.patch) > Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe). > -- > > Key: HDFS-14618 > URL: https://issues.apache.org/jira/browse/HDFS-14618 > Project: Hadoop HDFS > Issue Type: Bug >Reporter: Paul Ward >Assignee: Paul Ward >Priority: Critical > Labels: fix-provided, patch-available > Attachments: race.patch > > > I submitted a CR for this issue at: > https://github.com/apache/hadoop/pull/1030 > The field {{timedOutItems}} (an {{ArrayList}}, i.e., not thread safe): > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L70 > is protected by synchronization on itself ({{timedOutItems}}): > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L167-L168 > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L267-L268 > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L178 > However, in one place: > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L133-L135 > it is (trying to be) protected by synchronized using > {{pendingReconstructions}} --- but this cannot protect {{timedOutItems}}. > Synchronized on different objects does not ensure mutual exclusion with the > other locations. > I.e., 2 code locations, one synchronized by {{pendingReconstructions}} and > the other by {{timedOutItems}} can still executed concurrently. > This CR adds the synchronized on {{timedOutItems}}. > Note that this CR keeps the synchronized on {{pendingReconstructions}}, which > is needed for a different purpose (protect {{pendingReconstructions}}) -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Updated] (HDFS-14618) Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe).
[ https://issues.apache.org/jira/browse/HDFS-14618?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Ward updated HDFS-14618: - Attachment: race.patch Status: Patch Available (was: In Progress) > Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe). > -- > > Key: HDFS-14618 > URL: https://issues.apache.org/jira/browse/HDFS-14618 > Project: Hadoop HDFS > Issue Type: Bug >Reporter: Paul Ward >Assignee: Paul Ward >Priority: Critical > Labels: fix-provided, patch-available > Attachments: race.patch, race.patch > > > I submitted a CR for this issue at: > https://github.com/apache/hadoop/pull/1030 > The field {{timedOutItems}} (an {{ArrayList}}, i.e., not thread safe): > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L70 > is protected by synchronization on itself ({{timedOutItems}}): > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L167-L168 > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L267-L268 > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L178 > However, in one place: > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L133-L135 > it is (trying to be) protected by synchronized using > {{pendingReconstructions}} --- but this cannot protect {{timedOutItems}}. > Synchronized on different objects does not ensure mutual exclusion with the > other locations. > I.e., 2 code locations, one synchronized by {{pendingReconstructions}} and > the other by {{timedOutItems}} can still executed concurrently. > This CR adds the synchronized on {{timedOutItems}}. > Note that this CR keeps the synchronized on {{pendingReconstructions}}, which > is needed for a different purpose (protect {{pendingReconstructions}}) -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Updated] (HDFS-14618) Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe).
[ https://issues.apache.org/jira/browse/HDFS-14618?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Ward updated HDFS-14618: - Status: In Progress (was: Patch Available) > Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe). > -- > > Key: HDFS-14618 > URL: https://issues.apache.org/jira/browse/HDFS-14618 > Project: Hadoop HDFS > Issue Type: Bug >Reporter: Paul Ward >Assignee: Paul Ward >Priority: Critical > Labels: fix-provided, patch-available > Attachments: race.patch > > > I submitted a CR for this issue at: > https://github.com/apache/hadoop/pull/1030 > The field {{timedOutItems}} (an {{ArrayList}}, i.e., not thread safe): > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L70 > is protected by synchronization on itself ({{timedOutItems}}): > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L167-L168 > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L267-L268 > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L178 > However, in one place: > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L133-L135 > it is (trying to be) protected by synchronized using > {{pendingReconstructions}} --- but this cannot protect {{timedOutItems}}. > Synchronized on different objects does not ensure mutual exclusion with the > other locations. > I.e., 2 code locations, one synchronized by {{pendingReconstructions}} and > the other by {{timedOutItems}} can still executed concurrently. > This CR adds the synchronized on {{timedOutItems}}. > Note that this CR keeps the synchronized on {{pendingReconstructions}}, which > is needed for a different purpose (protect {{pendingReconstructions}}) -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Updated] (HDFS-14610) HashMap is not thread safe. Field storageMap is typically synchronized by storageMap. However, in one place, field storageMap is not protected with synchronized.
[ https://issues.apache.org/jira/browse/HDFS-14610?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Ward updated HDFS-14610: - Flags: Patch > HashMap is not thread safe. Field storageMap is typically synchronized by > storageMap. However, in one place, field storageMap is not protected with > synchronized. > - > > Key: HDFS-14610 > URL: https://issues.apache.org/jira/browse/HDFS-14610 > Project: Hadoop HDFS > Issue Type: Bug >Reporter: Paul Ward >Priority: Critical > Labels: fix-provided, patch-available > Attachments: addingSynchronization.patch > > > I submitted a CR for this issue at: > [https://github.com/apache/hadoop/pull/1015] > The field *storageMap* (a *HashMap*) > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L155] > is typically protected by synchronization on *storageMap*, e.g., > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L294] > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L443] > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] > For a total of 9 locations. > The reason is because *HashMap* is not thread safe. > However, here: > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L455] > {{DatanodeStorageInfo storage =}} > {{ storageMap.get(report.getStorage().getStorageID());}} > It is not synchronized. > Note that in the same method: > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] > *storageMap* is again protected by synchronization: > {{synchronized (storageMap) {}} > {{ storageMapSize = storageMap.size();}} > {{}}} > > The CR I inlined above protected the above instance (line 455 ) with > synchronization > like in line 484 and in all other occurrences. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Updated] (HDFS-14618) Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe).
[ https://issues.apache.org/jira/browse/HDFS-14618?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Ward updated HDFS-14618: - Labels: fix-provided patch-available (was: ) > Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe). > -- > > Key: HDFS-14618 > URL: https://issues.apache.org/jira/browse/HDFS-14618 > Project: Hadoop HDFS > Issue Type: Bug >Reporter: Paul Ward >Priority: Critical > Labels: fix-provided, patch-available > Attachments: race.patch > > > I submitted a CR for this issue at: > https://github.com/apache/hadoop/pull/1030 > The field {{timedOutItems}} (an {{ArrayList}}, i.e., not thread safe): > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L70 > is protected by synchronization on itself ({{timedOutItems}}): > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L167-L168 > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L267-L268 > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L178 > However, in one place: > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L133-L135 > it is (trying to be) protected by synchronized using > {{pendingReconstructions}} --- but this cannot protect {{timedOutItems}}. > Synchronized on different objects does not ensure mutual exclusion with the > other locations. > I.e., 2 code locations, one synchronized by {{pendingReconstructions}} and > the other by {{timedOutItems}} can still executed concurrently. > This CR adds the synchronized on {{timedOutItems}}. > Note that this CR keeps the synchronized on {{pendingReconstructions}}, which > is needed for a different purpose (protect {{pendingReconstructions}}) -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Updated] (HDFS-14610) HashMap is not thread safe. Field storageMap is typically synchronized by storageMap. However, in one place, field storageMap is not protected with synchronized.
[ https://issues.apache.org/jira/browse/HDFS-14610?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Ward updated HDFS-14610: - Labels: fix-provided patch-available (was: fix-provided) > HashMap is not thread safe. Field storageMap is typically synchronized by > storageMap. However, in one place, field storageMap is not protected with > synchronized. > - > > Key: HDFS-14610 > URL: https://issues.apache.org/jira/browse/HDFS-14610 > Project: Hadoop HDFS > Issue Type: Bug >Reporter: Paul Ward >Priority: Critical > Labels: fix-provided, patch-available > Attachments: addingSynchronization.patch > > > I submitted a CR for this issue at: > [https://github.com/apache/hadoop/pull/1015] > The field *storageMap* (a *HashMap*) > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L155] > is typically protected by synchronization on *storageMap*, e.g., > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L294] > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L443] > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] > For a total of 9 locations. > The reason is because *HashMap* is not thread safe. > However, here: > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L455] > {{DatanodeStorageInfo storage =}} > {{ storageMap.get(report.getStorage().getStorageID());}} > It is not synchronized. > Note that in the same method: > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] > *storageMap* is again protected by synchronization: > {{synchronized (storageMap) {}} > {{ storageMapSize = storageMap.size();}} > {{}}} > > The CR I inlined above protected the above instance (line 455 ) with > synchronization > like in line 484 and in all other occurrences. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Updated] (HDFS-14618) Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe).
[ https://issues.apache.org/jira/browse/HDFS-14618?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Ward updated HDFS-14618: - Attachment: (was: race.diff) > Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe). > -- > > Key: HDFS-14618 > URL: https://issues.apache.org/jira/browse/HDFS-14618 > Project: Hadoop HDFS > Issue Type: Bug >Reporter: Paul Ward >Priority: Critical > Attachments: race.patch > > > I submitted a CR for this issue at: > https://github.com/apache/hadoop/pull/1030 > The field {{timedOutItems}} (an {{ArrayList}}, i.e., not thread safe): > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L70 > is protected by synchronization on itself ({{timedOutItems}}): > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L167-L168 > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L267-L268 > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L178 > However, in one place: > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L133-L135 > it is (trying to be) protected by synchronized using > {{pendingReconstructions}} --- but this cannot protect {{timedOutItems}}. > Synchronized on different objects does not ensure mutual exclusion with the > other locations. > I.e., 2 code locations, one synchronized by {{pendingReconstructions}} and > the other by {{timedOutItems}} can still executed concurrently. > This CR adds the synchronized on {{timedOutItems}}. > Note that this CR keeps the synchronized on {{pendingReconstructions}}, which > is needed for a different purpose (protect {{pendingReconstructions}}) -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Updated] (HDFS-14618) Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe).
[ https://issues.apache.org/jira/browse/HDFS-14618?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Ward updated HDFS-14618: - Attachment: race.patch > Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe). > -- > > Key: HDFS-14618 > URL: https://issues.apache.org/jira/browse/HDFS-14618 > Project: Hadoop HDFS > Issue Type: Bug >Reporter: Paul Ward >Priority: Critical > Attachments: race.patch > > > I submitted a CR for this issue at: > https://github.com/apache/hadoop/pull/1030 > The field {{timedOutItems}} (an {{ArrayList}}, i.e., not thread safe): > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L70 > is protected by synchronization on itself ({{timedOutItems}}): > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L167-L168 > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L267-L268 > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L178 > However, in one place: > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L133-L135 > it is (trying to be) protected by synchronized using > {{pendingReconstructions}} --- but this cannot protect {{timedOutItems}}. > Synchronized on different objects does not ensure mutual exclusion with the > other locations. > I.e., 2 code locations, one synchronized by {{pendingReconstructions}} and > the other by {{timedOutItems}} can still executed concurrently. > This CR adds the synchronized on {{timedOutItems}}. > Note that this CR keeps the synchronized on {{pendingReconstructions}}, which > is needed for a different purpose (protect {{pendingReconstructions}}) -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Updated] (HDFS-14610) HashMap is not thread safe. Field storageMap is typically synchronized by storageMap. However, in one place, field storageMap is not protected with synchronized.
[ https://issues.apache.org/jira/browse/HDFS-14610?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Ward updated HDFS-14610: - Description: I submitted a CR for this issue at: [https://github.com/apache/hadoop/pull/1015] The field *storageMap* (a *HashMap*) [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L155] is typically protected by synchronization on *storageMap*, e.g., [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L294] [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L443] [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] For a total of 9 locations. The reason is because *HashMap* is not thread safe. However, here: [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L455] {{DatanodeStorageInfo storage =}} {{ storageMap.get(report.getStorage().getStorageID());}} It is not synchronized. Note that in the same method: [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] *storageMap* is again protected by synchronization: {{synchronized (storageMap) {}} {{ storageMapSize = storageMap.size();}} {{}}} The CR I inlined above protected the above instance (line 455 ) with synchronization like in line 484 and in all other occurrences. was: The field *storageMap* (a *HashMap*) [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L155] is typically protected by synchronization on *storageMap*, e.g., [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L294] [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L443] [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] For a total of 9 locations. The reason is because *HashMap* is not thread safe. However, here: [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L455] {{DatanodeStorageInfo storage =}} {{ storageMap.get(report.getStorage().getStorageID());}} It is not synchronized. Note that in the same method: [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] *storageMap* is again protected by synchronization: {{synchronized (storageMap) {}} {{ storageMapSize = storageMap.size();}} {{}}} I submitted a CR: [https://github.com/apache/hadoop/pull/1015] This CR protected the above instance (line 455 ) with synchronization like in line 484 and in all other occurrences. > HashMap is not thread safe. Field storageMap is typically synchronized by > storageMap. However, in one place, field storageMap is not protected with > synchronized. > - > > Key: HDFS-14610 > URL: https://issues.apache.org/jira/browse/HDFS-14610 > Project: Hadoop HDFS > Issue Type: Bug >Reporter: Paul Ward >Priority: Critical > Labels: fix-provided > Attachments: addingSynchronization.patch > > > I submitted a CR for this issue at: > [https://github.com/apache/hadoop/pull/1015] > The field *storageMap* (a *HashMap*) > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L155] > is typically protected by synchronization on *storageMap*, e.g., > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L294] > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L443] >
[jira] [Updated] (HDFS-14610) HashMap is not thread safe. Field storageMap is typically synchronized by storageMap. However, in one place, field storageMap is not protected with synchronized.
[ https://issues.apache.org/jira/browse/HDFS-14610?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Ward updated HDFS-14610: - Attachment: addingSynchronization.patch Status: Patch Available (was: Open) This is the same patch I submitted in the CR I inlined in the text > HashMap is not thread safe. Field storageMap is typically synchronized by > storageMap. However, in one place, field storageMap is not protected with > synchronized. > - > > Key: HDFS-14610 > URL: https://issues.apache.org/jira/browse/HDFS-14610 > Project: Hadoop HDFS > Issue Type: Bug >Reporter: Paul Ward >Priority: Critical > Labels: fix-provided > Attachments: addingSynchronization.patch > > > The field *storageMap* (a *HashMap*) > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L155] > is typically protected by synchronization on *storageMap*, e.g., > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L294] > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L443] > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] > For a total of 9 locations. > The reason is because *HashMap* is not thread safe. > However, here: > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L455] > {{DatanodeStorageInfo storage =}} > {{ storageMap.get(report.getStorage().getStorageID());}} > It is not synchronized. > Note that in the same method: > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] > *storageMap* is again protected by synchronization: > {{synchronized (storageMap) {}} > {{ storageMapSize = storageMap.size();}} > {{}}} > I submitted a CR: > > [https://github.com/apache/hadoop/pull/1015] > > This CR protected the above instance (line 455 ) with synchronization > like in line 484 and in all other occurrences. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Updated] (HDFS-14618) Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe).
[ https://issues.apache.org/jira/browse/HDFS-14618?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Ward updated HDFS-14618: - Attachment: race.diff Status: Patch Available (was: Open) This diff is the same from the Pull request I inlined > Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe). > -- > > Key: HDFS-14618 > URL: https://issues.apache.org/jira/browse/HDFS-14618 > Project: Hadoop HDFS > Issue Type: Bug >Reporter: Paul Ward >Priority: Critical > Attachments: race.diff > > > I submitted a CR for this issue at: > https://github.com/apache/hadoop/pull/1030 > The field {{timedOutItems}} (an {{ArrayList}}, i.e., not thread safe): > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L70 > is protected by synchronization on itself ({{timedOutItems}}): > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L167-L168 > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L267-L268 > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L178 > However, in one place: > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L133-L135 > it is (trying to be) protected by synchronized using > {{pendingReconstructions}} --- but this cannot protect {{timedOutItems}}. > Synchronized on different objects does not ensure mutual exclusion with the > other locations. > I.e., 2 code locations, one synchronized by {{pendingReconstructions}} and > the other by {{timedOutItems}} can still executed concurrently. > This CR adds the synchronized on {{timedOutItems}}. > Note that this CR keeps the synchronized on {{pendingReconstructions}}, which > is needed for a different purpose (protect {{pendingReconstructions}}) -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Updated] (HDFS-14618) Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe).
[ https://issues.apache.org/jira/browse/HDFS-14618?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Ward updated HDFS-14618: - Description: I submitted a CR for this issue at: https://github.com/apache/hadoop/pull/1030 The field {{timedOutItems}} (an {{ArrayList}}, i.e., not thread safe): https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L70 is protected by synchronization on itself ({{timedOutItems}}): https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L167-L168 https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L267-L268 https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L178 However, in one place: https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L133-L135 it is (trying to be) protected by synchronized using {{pendingReconstructions}} --- but this cannot protect {{timedOutItems}}. Synchronized on different objects does not ensure mutual exclusion with the other locations. I.e., 2 code locations, one synchronized by {{pendingReconstructions}} and the other by {{timedOutItems}} can still executed concurrently. This CR adds the synchronized on {{timedOutItems}}. Note that this CR keeps the synchronized on {{pendingReconstructions}}, which is needed for a different purpose (protect {{pendingReconstructions}}) was: I submitted a CR for this issue at: https://github.com/apache/hadoop/pull/1030 The field ```timedOutItems``` (an ```ArrayList```, i.e., not thread safe): https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L70 is protected by synchronization on itself (```timedOutItems```): https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L167-L168 https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L267-L268 https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L178 However, in one place: https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L133-L135 it is (trying to be) protected by synchronized using ```pendingReconstructions``` --- but this cannot protect ```timedOutItems```. Synchronized on different objects does not ensure mutual exclusion with the other locations. I.e., 2 code locations, one synchronized by ```pendingReconstructions``` and the other by ```timedOutItems``` can still executed concurrently. This CR adds the synchronized on ```timedOutItems```. Note that this CR keeps the synchronized on ```pendingReconstructions```, which is needed for a different purpose (protect ```pendingReconstructions```) > Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe). > -- > > Key: HDFS-14618 > URL: https://issues.apache.org/jira/browse/HDFS-14618 > Project: Hadoop HDFS > Issue Type: Bug >Reporter: Paul Ward >Priority: Critical > > I submitted a CR for this issue at: > https://github.com/apache/hadoop/pull/1030 > The field {{timedOutItems}} (an {{ArrayList}}, i.e., not thread safe): > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L70 > is protected by synchronization on itself ({{timedOutItems}}): > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L167-L168 > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L267-L268 > https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L178 > However, in one place: >
[jira] [Created] (HDFS-14618) Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe).
Paul Ward created HDFS-14618: Summary: Incorrect synchronization of ArrayList field (ArrayList is thread-unsafe). Key: HDFS-14618 URL: https://issues.apache.org/jira/browse/HDFS-14618 Project: Hadoop HDFS Issue Type: Bug Reporter: Paul Ward I submitted a CR for this issue at: https://github.com/apache/hadoop/pull/1030 The field ```timedOutItems``` (an ```ArrayList```, i.e., not thread safe): https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L70 is protected by synchronization on itself (```timedOutItems```): https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L167-L168 https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L267-L268 https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L178 However, in one place: https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReconstructionBlocks.java#L133-L135 it is (trying to be) protected by synchronized using ```pendingReconstructions``` --- but this cannot protect ```timedOutItems```. Synchronized on different objects does not ensure mutual exclusion with the other locations. I.e., 2 code locations, one synchronized by ```pendingReconstructions``` and the other by ```timedOutItems``` can still executed concurrently. This CR adds the synchronized on ```timedOutItems```. Note that this CR keeps the synchronized on ```pendingReconstructions```, which is needed for a different purpose (protect ```pendingReconstructions```) -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Updated] (HDFS-14610) HashMap is not thread safe. Field storageMap is typically synchronized by storageMap. However, in one place, field storageMap is not protected with synchronized.
[ https://issues.apache.org/jira/browse/HDFS-14610?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Paul Ward updated HDFS-14610: - Description: The field *storageMap* (a *HashMap*) [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L155] is typically protected by synchronization on *storageMap*, e.g., [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L294] [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L443] [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] For a total of 9 locations. The reason is because *HashMap* is not thread safe. However, here: [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L455] {{DatanodeStorageInfo storage =}} {{ storageMap.get(report.getStorage().getStorageID());}} It is not synchronized. Note that in the same method: [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] *storageMap* is again protected by synchronization: {{synchronized (storageMap) {}} {{ storageMapSize = storageMap.size();}} {{}}} I submitted a CR: [https://github.com/apache/hadoop/pull/1015] This CR protected the above instance (line 455 ) with synchronization like in line 484 and in all other occurrences. was: The field *storageMap* (a *HashMap*) [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L155] is typically protected by synchronization on *storageMap*, e.g., [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L294] [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L443] [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] For a total of 9 locations. The reason is because *HashMap* is not thread safe. However, here: [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L455] {{ DatanodeStorageInfo storage =}} {{ storageMap.get(report.getStorage().getStorageID());}} It is not synchronized. Note that in the same method: [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] *storageMap* is again protected by synchronization: {{ synchronized (storageMap) {}} {{ storageMapSize = storageMap.size();}} {{ }}} I submitted a CR: [https://github.com/apache/hadoop/pull/1015] This CR protected the above instance (line 455 ) with synchronization like in line 484 and in all other occurrences. > HashMap is not thread safe. Field storageMap is typically synchronized by > storageMap. However, in one place, field storageMap is not protected with > synchronized. > - > > Key: HDFS-14610 > URL: https://issues.apache.org/jira/browse/HDFS-14610 > Project: Hadoop HDFS > Issue Type: Bug >Reporter: Paul Ward >Priority: Critical > > The field *storageMap* (a *HashMap*) > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L155] > is typically protected by synchronization on *storageMap*, e.g., > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L294] > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L443] > [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] > For a total of 9 locations. > The reason is because
[jira] [Created] (HDFS-14610) HashMap is not thread safe. Field storageMap is typically synchronized by storageMap. However, in one place, field storageMap is not protected with synchronized.
Paul Ward created HDFS-14610: Summary: HashMap is not thread safe. Field storageMap is typically synchronized by storageMap. However, in one place, field storageMap is not protected with synchronized. Key: HDFS-14610 URL: https://issues.apache.org/jira/browse/HDFS-14610 Project: Hadoop HDFS Issue Type: Bug Reporter: Paul Ward The field *storageMap* (a *HashMap*) [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L155] is typically protected by synchronization on *storageMap*, e.g., [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L294] [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L443] [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] For a total of 9 locations. The reason is because *HashMap* is not thread safe. However, here: [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L455] {{ DatanodeStorageInfo storage =}} {{ storageMap.get(report.getStorage().getStorageID());}} It is not synchronized. Note that in the same method: [https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java#L484] *storageMap* is again protected by synchronization: {{ synchronized (storageMap) {}} {{ storageMapSize = storageMap.size();}} {{ }}} I submitted a CR: [https://github.com/apache/hadoop/pull/1015] This CR protected the above instance (line 455 ) with synchronization like in line 484 and in all other occurrences. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org