Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-7261

storageMap is accessed without synchronization in DatanodeDescriptor#updateHeartbeatState()

Details

    • Bug
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • None
    • 2.8.0, 3.0.0-alpha1
    • None
    • None

    Description

      Here is the code:

            failedStorageInfos = new HashSet<DatanodeStorageInfo>(
                storageMap.values());
      

      In other places, the lock on "DatanodeDescriptor.storageMap" is held:

          synchronized (storageMap) {
            final Collection<DatanodeStorageInfo> storages = storageMap.values();
            return storages.toArray(new DatanodeStorageInfo[storages.size()]);
          }
      

      Attachments

        1. HDFS-7261.patch
          0.9 kB
          Brahma Reddy Battula
        2. HDFS-7261-001.patch
          2 kB
          Brahma Reddy Battula
        3. HDFS-7261-002.patch
          3 kB
          Brahma Reddy Battula

        Activity

          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12702819/HDFS-7261.patch
          against trunk revision 1b67209.

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.TestLeaseRecovery2
          org.apache.hadoop.hdfs.server.namenode.ha.TestPipelinesFailover
          org.apache.hadoop.hdfs.server.blockmanagement.TestDatanodeManager

          The following test timeouts occurred in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.TestAppendSnapshotTruncate

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/9750//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9750//console

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12702819/HDFS-7261.patch against trunk revision 1b67209. +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.TestLeaseRecovery2 org.apache.hadoop.hdfs.server.namenode.ha.TestPipelinesFailover org.apache.hadoop.hdfs.server.blockmanagement.TestDatanodeManager The following test timeouts occurred in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.TestAppendSnapshotTruncate Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/9750//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9750//console This message is automatically generated.
          cmccabe Colin McCabe added a comment -

          Thanks, Brahma.

          It looks like storageMap is also being accessed without synchronization here:

              if (storageMap.size() != reports.length) {
                pruneStorageMap(reports);
              }
           

          and here:

            private void pruneStorageMap(final StorageReport[] reports) {
              if (LOG.isDebugEnabled()) {
                LOG.debug("Number of storages reported in heartbeat=" + reports.length +
                              "; Number of storages in storageMap=" + storageMap.size());
              }
          
              HashMap<String, DatanodeStorageInfo> excessStorages;
          
              synchronized (storageMap) {
                // Init excessStorages with all known storages.
          

          We should be able to avoid the lack of synchronization, and avoid holding the lock for a long time, by getting the size() before we release the lock.

          cmccabe Colin McCabe added a comment - Thanks, Brahma. It looks like storageMap is also being accessed without synchronization here: if (storageMap.size() != reports.length) { pruneStorageMap(reports); } and here: private void pruneStorageMap( final StorageReport[] reports) { if (LOG.isDebugEnabled()) { LOG.debug( " Number of storages reported in heartbeat=" + reports.length + "; Number of storages in storageMap=" + storageMap.size()); } HashMap< String , DatanodeStorageInfo> excessStorages; synchronized (storageMap) { // Init excessStorages with all known storages. We should be able to avoid the lack of synchronization, and avoid holding the lock for a long time, by getting the size() before we release the lock.

          cmccabe Thanks a lot for looking into this issue...Yes, I missed above two..Updated the patch,Kindly review..

          brahmareddy Brahma Reddy Battula added a comment - cmccabe Thanks a lot for looking into this issue...Yes, I missed above two..Updated the patch,Kindly review..
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12703042/HDFS-7261-001.patch
          against trunk revision 95bfd08.

          -1 patch. Trunk compilation may be broken.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9774//console

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12703042/HDFS-7261-001.patch against trunk revision 95bfd08. -1 patch . Trunk compilation may be broken. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9774//console This message is automatically generated.
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12703042/HDFS-7261-001.patch
          against trunk revision 24db081.

          -1 patch. Trunk compilation may be broken.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9782//console

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12703042/HDFS-7261-001.patch against trunk revision 24db081. -1 patch . Trunk compilation may be broken. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9782//console This message is automatically generated.

          cmccabe Kindly review the patch and can you please kick jenkin once..?

          brahmareddy Brahma Reddy Battula added a comment - cmccabe Kindly review the patch and can you please kick jenkin once..?
          cmccabe Colin McCabe added a comment -

          thanks, brahmareddy.

          403 synchronized (storageMap)

          { 404 failedStorageInfos = new HashSet<DatanodeStorageInfo>( 404 storageMap.values()); 405 storageMap.values()); 406 }

          the indentation is off on line 404

          similarly, the indentation is off in pruneStorageMap

          cmccabe Colin McCabe added a comment - thanks, brahmareddy . 403 synchronized (storageMap) { 404 failedStorageInfos = new HashSet<DatanodeStorageInfo>( 404 storageMap.values()); 405 storageMap.values()); 406 } the indentation is off on line 404 similarly, the indentation is off in pruneStorageMap

          Thanks a lot for review.. will update the patch..

          brahmareddy Brahma Reddy Battula added a comment - Thanks a lot for review.. will update the patch..
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12705313/HDFS-7261-002.patch
          against trunk revision 3411732.

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/9957//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9957//console

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12705313/HDFS-7261-002.patch against trunk revision 3411732. +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/9957//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9957//console This message is automatically generated.
          cmccabe Colin McCabe added a comment -

          +1. Thanks, brahmareddy.

          cmccabe Colin McCabe added a comment - +1. Thanks, brahmareddy .
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #7463 (See https://builds.apache.org/job/Hadoop-trunk-Commit/7463/)
          HDFS-7261. storageMap is accessed without synchronization in DatanodeDescriptor#updateHeartbeatState() (Brahma Reddy Battula via Colin P. McCabe) (cmccabe: rev 1feb9569f366a29ecb43592d71ee21023162c18f)

          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #7463 (See https://builds.apache.org/job/Hadoop-trunk-Commit/7463/ ) HDFS-7261 . storageMap is accessed without synchronization in DatanodeDescriptor#updateHeartbeatState() (Brahma Reddy Battula via Colin P. McCabe) (cmccabe: rev 1feb9569f366a29ecb43592d71ee21023162c18f) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java

          Thanks a lot cmccabe

          brahmareddy Brahma Reddy Battula added a comment - Thanks a lot cmccabe
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #149 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/149/)
          HDFS-7261. storageMap is accessed without synchronization in DatanodeDescriptor#updateHeartbeatState() (Brahma Reddy Battula via Colin P. McCabe) (cmccabe: rev 1feb9569f366a29ecb43592d71ee21023162c18f)

          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #149 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/149/ ) HDFS-7261 . storageMap is accessed without synchronization in DatanodeDescriptor#updateHeartbeatState() (Brahma Reddy Battula via Colin P. McCabe) (cmccabe: rev 1feb9569f366a29ecb43592d71ee21023162c18f) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Yarn-trunk #883 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/883/)
          HDFS-7261. storageMap is accessed without synchronization in DatanodeDescriptor#updateHeartbeatState() (Brahma Reddy Battula via Colin P. McCabe) (cmccabe: rev 1feb9569f366a29ecb43592d71ee21023162c18f)

          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #883 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/883/ ) HDFS-7261 . storageMap is accessed without synchronization in DatanodeDescriptor#updateHeartbeatState() (Brahma Reddy Battula via Colin P. McCabe) (cmccabe: rev 1feb9569f366a29ecb43592d71ee21023162c18f) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk #2081 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2081/)
          HDFS-7261. storageMap is accessed without synchronization in DatanodeDescriptor#updateHeartbeatState() (Brahma Reddy Battula via Colin P. McCabe) (cmccabe: rev 1feb9569f366a29ecb43592d71ee21023162c18f)

          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2081 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2081/ ) HDFS-7261 . storageMap is accessed without synchronization in DatanodeDescriptor#updateHeartbeatState() (Brahma Reddy Battula via Colin P. McCabe) (cmccabe: rev 1feb9569f366a29ecb43592d71ee21023162c18f) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #140 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/140/)
          HDFS-7261. storageMap is accessed without synchronization in DatanodeDescriptor#updateHeartbeatState() (Brahma Reddy Battula via Colin P. McCabe) (cmccabe: rev 1feb9569f366a29ecb43592d71ee21023162c18f)

          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #140 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/140/ ) HDFS-7261 . storageMap is accessed without synchronization in DatanodeDescriptor#updateHeartbeatState() (Brahma Reddy Battula via Colin P. McCabe) (cmccabe: rev 1feb9569f366a29ecb43592d71ee21023162c18f) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #149 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/149/)
          HDFS-7261. storageMap is accessed without synchronization in DatanodeDescriptor#updateHeartbeatState() (Brahma Reddy Battula via Colin P. McCabe) (cmccabe: rev 1feb9569f366a29ecb43592d71ee21023162c18f)

          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #149 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/149/ ) HDFS-7261 . storageMap is accessed without synchronization in DatanodeDescriptor#updateHeartbeatState() (Brahma Reddy Battula via Colin P. McCabe) (cmccabe: rev 1feb9569f366a29ecb43592d71ee21023162c18f) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk #2099 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2099/)
          HDFS-7261. storageMap is accessed without synchronization in DatanodeDescriptor#updateHeartbeatState() (Brahma Reddy Battula via Colin P. McCabe) (cmccabe: rev 1feb9569f366a29ecb43592d71ee21023162c18f)

          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2099 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2099/ ) HDFS-7261 . storageMap is accessed without synchronization in DatanodeDescriptor#updateHeartbeatState() (Brahma Reddy Battula via Colin P. McCabe) (cmccabe: rev 1feb9569f366a29ecb43592d71ee21023162c18f) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeDescriptor.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt

          People

            brahmareddy Brahma Reddy Battula
            yuzhihong@gmail.com Ted Yu
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: