Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-781

Metrics PendingDeletionBlocks is not decremented

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 0.20.2, 0.21.0, 0.22.0
    • Fix Version/s: 0.20.2, 0.21.0
    • Component/s: namenode
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Release Note:
      Correct PendingDeletionBlocks metric to properly decrement counts.

      Description

      PendingDeletionBlocks is not decremented decremented when blocks pending deletion in BlockManager.recentInvalidateSets are sent to datanode for deletion. This results in invalid value in the metrics.

      1. hdfs-781.rel20.patch
        7 kB
        Suresh Srinivas
      2. hdfs-781.rel20.1.patch
        7 kB
        Suresh Srinivas
      3. hdfs-781.patch
        4 kB
        Suresh Srinivas
      4. hdfs-781.4.patch
        7 kB
        Suresh Srinivas
      5. hdfs-781.3.patch
        7 kB
        Suresh Srinivas
      6. hdfs-781.2.patch
        4 kB
        Suresh Srinivas
      7. hdfs-781.1.patch
        4 kB
        Suresh Srinivas

        Activity

        Hide
        Suresh Srinivas added a comment -

        Attached patch fixes a bug in decrementing the metrics PendingDeletionBlocks.

        Show
        Suresh Srinivas added a comment - Attached patch fixes a bug in decrementing the metrics PendingDeletionBlocks.
        Hide
        Eli Collins added a comment -

        Hey Suresh, the attached patch looks unrelated, wrong one?

        Show
        Eli Collins added a comment - Hey Suresh, the attached patch looks unrelated, wrong one?
        Hide
        Suresh Srinivas added a comment -

        Attaching the right file.

        Show
        Suresh Srinivas added a comment - Attaching the right file.
        Hide
        Eli Collins added a comment -

        Would be good to replace the 4000 with symbolic constants, eg (NUM_PERIODS_PENDING_DEL + 1) * DFS_HEARTBEAT_INTERVAL, otherwise the patch looks great.

        Show
        Eli Collins added a comment - Would be good to replace the 4000 with symbolic constants, eg (NUM_PERIODS_PENDING_DEL + 1) * DFS_HEARTBEAT_INTERVAL, otherwise the patch looks great.
        Hide
        Suresh Srinivas added a comment -

        Patch addresses the comments from Eli.

        Show
        Suresh Srinivas added a comment - Patch addresses the comments from Eli.
        Hide
        Suresh Srinivas added a comment -

        Converting time from second to milliseconds in sleep.

        Show
        Suresh Srinivas added a comment - Converting time from second to milliseconds in sleep.
        Hide
        Eli Collins added a comment -

        Patch looks great. +1

        Show
        Eli Collins added a comment - Patch looks great. +1
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12426739/hdfs-781.2.patch
        against trunk revision 886322.

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

        +1 tests included. The patch appears to include 7 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

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

        +1 findbugs. The patch does not introduce any new Findbugs warnings.

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

        +1 core tests. The patch passed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/129/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/129/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/129/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/129/console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12426739/hdfs-781.2.patch against trunk revision 886322. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 7 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/129/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/129/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/129/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/129/console This message is automatically generated.
        Hide
        Jakob Homan added a comment -

        Patch looks good except that, if we're (correctly) improving the test directory name, we should also fix its writing directly to /tmp rather than using the build property (

        System.getProperty("test.build.data","/tmp")

        ) (HADOOP-5916).

        Show
        Jakob Homan added a comment - Patch looks good except that, if we're (correctly) improving the test directory name, we should also fix its writing directly to /tmp rather than using the build property ( System .getProperty( "test.build.data" , "/tmp" ) ) ( HADOOP-5916 ).
        Hide
        Suresh Srinivas added a comment -

        Files created in tests were renamed to identify the test that is being run in the logs. Attaching a file that does not use /tmp for creating test files.

        Show
        Suresh Srinivas added a comment - Files created in tests were renamed to identify the test that is being run in the logs. Attaching a file that does not use /tmp for creating test files.
        Hide
        Jakob Homan added a comment -

        Looks good. +1, pending Hudson.

        Show
        Jakob Homan added a comment - Looks good. +1, pending Hudson.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12426821/hdfs-781.3.patch
        against trunk revision 886322.

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

        +1 tests included. The patch appears to include 4 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

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

        +1 findbugs. The patch does not introduce any new Findbugs warnings.

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

        -1 core tests. The patch failed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/80/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/80/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/80/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/80/console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12426821/hdfs-781.3.patch against trunk revision 886322. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 4 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/80/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/80/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/80/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/80/console This message is automatically generated.
        Hide
        Suresh Srinivas added a comment -

        With the change in the file path to incorporate "test.build.data" property, the filesTotal metrics calculation changes. Previous patch passed on my system. However it fails on Hudson as the test directory path is different from test setup. Changing the test not to use hardcoded expected filesTotal; instead use the path of file being created to calculate the expected filesTotal metrics.

        Show
        Suresh Srinivas added a comment - With the change in the file path to incorporate "test.build.data" property, the filesTotal metrics calculation changes. Previous patch passed on my system. However it fails on Hudson as the test directory path is different from test setup. Changing the test not to use hardcoded expected filesTotal; instead use the path of file being created to calculate the expected filesTotal metrics.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12426857/hdfs-781.4.patch
        against trunk revision 886322.

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

        +1 tests included. The patch appears to include 4 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

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

        +1 findbugs. The patch does not introduce any new Findbugs warnings.

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

        +1 core tests. The patch passed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/132/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/132/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/132/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/132/console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12426857/hdfs-781.4.patch against trunk revision 886322. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 4 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/132/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/132/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/132/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/132/console This message is automatically generated.
        Hide
        Suresh Srinivas added a comment -

        Attaching patch for branch 20. This patch has one difference from the trunk version - in TestNameNodeMetrics.testFileAdd(), after deletion of the file, the metrics "blocksTotal" is not expected to be set to 0. In 21 and trunk block that belongs to a file is deleted immediately. In 20 this is done later on receiving block report from the datanode.

        Show
        Suresh Srinivas added a comment - Attaching patch for branch 20. This patch has one difference from the trunk version - in TestNameNodeMetrics.testFileAdd(), after deletion of the file, the metrics "blocksTotal" is not expected to be set to 0. In 21 and trunk block that belongs to a file is deleted immediately. In 20 this is done later on receiving block report from the datanode.
        Hide
        Jakob Homan added a comment -

        +1 for both new patches.

        Show
        Jakob Homan added a comment - +1 for both new patches.
        Hide
        Suresh Srinivas added a comment -

        Earlier release 20 patch was created using git diff. Creating patch with git diff --no-prefix.

        Show
        Suresh Srinivas added a comment - Earlier release 20 patch was created using git diff. Creating patch with git diff --no-prefix.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk-Commit #130 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/130/)
        . Namenode metrics PendingDeletionBlocks is not decremented. Contributed by Suresh Srinivas.

        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #130 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/130/ ) . Namenode metrics PendingDeletionBlocks is not decremented. Contributed by Suresh Srinivas.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #161 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk/161/)
        . Namenode metrics PendingDeletionBlocks is not decremented. Contributed by Suresh Srinivas.

        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #161 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk/161/ ) . Namenode metrics PendingDeletionBlocks is not decremented. Contributed by Suresh Srinivas.
        Hide
        Suresh Srinivas added a comment -

        Just committed this patch to trunk and branch 0.21.

        Show
        Suresh Srinivas added a comment - Just committed this patch to trunk and branch 0.21.
        Hide
        Hudson added a comment -

        Integrated in Hdfs-Patch-h5.grid.sp2.yahoo.net #135 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/135/)

        Show
        Hudson added a comment - Integrated in Hdfs-Patch-h5.grid.sp2.yahoo.net #135 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/135/ )
        Hide
        Hudson added a comment -

        Integrated in Hdfs-Patch-h2.grid.sp2.yahoo.net #83 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/83/)

        Show
        Hudson added a comment - Integrated in Hdfs-Patch-h2.grid.sp2.yahoo.net #83 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/83/ )
        Hide
        sravankorumilli added a comment -

        There is a possibility that if the condition if (!it.hasNext()) is met in the method invalidateWorkForOneNode()
        We are reducing the pendingDeletionBlocksCount twice
        1.first time in the method removeFromInvalidates(firstNodeId);
        2.second time as per the patch pendingDeletionBlocksCount -= blocksToInvalidate.size()

        Show
        sravankorumilli added a comment - There is a possibility that if the condition if (!it.hasNext()) is met in the method invalidateWorkForOneNode() We are reducing the pendingDeletionBlocksCount twice 1.first time in the method removeFromInvalidates(firstNodeId); 2.second time as per the patch pendingDeletionBlocksCount -= blocksToInvalidate.size()

          People

          • Assignee:
            Suresh Srinivas
            Reporter:
            Suresh Srinivas
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development