Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-3485

DataTransferThrottler will over-throttle when currentTimeMillis jumps

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.0-alpha
    • Fix Version/s: 2.0.2-alpha
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      When the system clock is set backwards, DataTransferThrottler will simply pause until the clock reaches the end of the previously calculated transfer period:

          this.curPeriodStart = System.currentTimeMillis();
      ...
          while (curReserve <= 0) {
            long now = System.currentTimeMillis();
            long curPeriodEnd = curPeriodStart + period;
            if ( now < curPeriodEnd ) {
              try {
                wait( curPeriodEnd - now );
      

      Instead of using currentTimeMillis() which is affected by system-clock-changes, this code should use nanoTime which ticks forward monotonically.

      1. hdfs-3485.1.patch
        4 kB
        Andy Isaacson
      2. hdfs-3485.patch
        2 kB
        Andy Isaacson
      3. hdfs-3485-2.patch
        6 kB
        Andy Isaacson

        Activity

        Hide
        Colin Patrick McCabe added a comment -

        Hmm, nice find.

        It's kind of unfortunate that java.lang.System conflates the concepts of "nanosecond accuracy" and "monotonic time." Linux has a coarse-grained monotonic timer, but there's probably no way to really get at it through Java6.

        Show
        Colin Patrick McCabe added a comment - Hmm, nice find. It's kind of unfortunate that java.lang.System conflates the concepts of "nanosecond accuracy" and "monotonic time." Linux has a coarse-grained monotonic timer, but there's probably no way to really get at it through Java6.
        Hide
        Andy Isaacson added a comment -

        Yes, it is a little unfortunate, there is a tiny optimization that we miss out on here, we could save a few hundred clock cycles if we could use CLOCK_MONOTONIC_COARSE.

        But, on modern systems the overhead of CLOCK_MONOTONIC (which presumably backs nanoTime()) is not that much greater than CLOCK_MONOTONIC_COARSE, and is probably the same as gettimeofday() which presumably backs currentTimeMillis().

        Show
        Andy Isaacson added a comment - Yes, it is a little unfortunate, there is a tiny optimization that we miss out on here, we could save a few hundred clock cycles if we could use CLOCK_MONOTONIC_COARSE. But, on modern systems the overhead of CLOCK_MONOTONIC (which presumably backs nanoTime()) is not that much greater than CLOCK_MONOTONIC_COARSE, and is probably the same as gettimeofday() which presumably backs currentTimeMillis().
        Hide
        Hadoop QA added a comment -

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

        +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. The javadoc tool did not generate any warning messages.

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

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) 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.

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

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2563//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2563//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/12530484/hdfs-3485.patch against trunk revision . +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. The javadoc tool did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) 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. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2563//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2563//console This message is automatically generated.
        Hide
        Andy Isaacson added a comment -

        Expand patch to update TestBlockReplacement to junit 4. Note that TestBlockReplacement already tests this codepath so a regression test is not needed.

        Show
        Andy Isaacson added a comment - Expand patch to update TestBlockReplacement to junit 4. Note that TestBlockReplacement already tests this codepath so a regression test is not needed.
        Hide
        Hadoop QA added a comment -

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

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

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

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

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

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

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) 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.

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

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2571//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2571//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/12530582/hdfs-3485.1.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The javadoc tool did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) 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. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2571//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2571//console This message is automatically generated.
        Hide
        Todd Lipcon added a comment -

        Looks pretty good. One small nit: can you move monotonicMillis() to o.a.h.hdfs.server.common.Util, so it's next to the "now()" function we've already got there? Then, folks are more likely to see it and use the right method. Also worth adding a note to the javadoc for this function like: "Whenever time is only used to calculate a duration or interval, this method should be used instead of

        {@link #now()}

        so that the timer is not affected by changes in the system time."

        Show
        Todd Lipcon added a comment - Looks pretty good. One small nit: can you move monotonicMillis() to o.a.h.hdfs.server.common.Util, so it's next to the "now()" function we've already got there? Then, folks are more likely to see it and use the right method. Also worth adding a note to the javadoc for this function like: "Whenever time is only used to calculate a duration or interval, this method should be used instead of {@link #now()} so that the timer is not affected by changes in the system time."
        Hide
        Andy Isaacson added a comment -

        Thanks for the comments, Todd! I've renamed the helper to monotonicNow() and moved it to Util. I also added a note to the now() javadoc noting when it should not be used.

        Show
        Andy Isaacson added a comment - Thanks for the comments, Todd! I've renamed the helper to monotonicNow() and moved it to Util. I also added a note to the now() javadoc noting when it should not be used.
        Hide
        Hadoop QA added a comment -

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

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

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

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

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

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

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) 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.

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

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2573//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2573//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/12530616/hdfs-3485-2.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The javadoc tool did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) 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. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2573//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2573//console This message is automatically generated.
        Hide
        Todd Lipcon added a comment -

        +1, looks good to me. Thanks Andy.

        Show
        Todd Lipcon added a comment - +1, looks good to me. Thanks Andy.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk-Commit #2331 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2331/)
        HDFS-3485. DataTransferThrottler will over-throttle when currentTimeMillis jumps. Contributed by Andy Isaacson. (Revision 1347751)

        Result = SUCCESS
        todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1347751
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Util.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/DataTransferThrottler.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockReplacement.java
        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #2331 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2331/ ) HDFS-3485 . DataTransferThrottler will over-throttle when currentTimeMillis jumps. Contributed by Andy Isaacson. (Revision 1347751) Result = SUCCESS todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1347751 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Util.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/DataTransferThrottler.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockReplacement.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk-Commit #2404 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2404/)
        HDFS-3485. DataTransferThrottler will over-throttle when currentTimeMillis jumps. Contributed by Andy Isaacson. (Revision 1347751)

        Result = SUCCESS
        todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1347751
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Util.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/DataTransferThrottler.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockReplacement.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #2404 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2404/ ) HDFS-3485 . DataTransferThrottler will over-throttle when currentTimeMillis jumps. Contributed by Andy Isaacson. (Revision 1347751) Result = SUCCESS todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1347751 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Util.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/DataTransferThrottler.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockReplacement.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk-Commit #2350 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2350/)
        HDFS-3485. DataTransferThrottler will over-throttle when currentTimeMillis jumps. Contributed by Andy Isaacson. (Revision 1347751)

        Result = SUCCESS
        todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1347751
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Util.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/DataTransferThrottler.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockReplacement.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #2350 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2350/ ) HDFS-3485 . DataTransferThrottler will over-throttle when currentTimeMillis jumps. Contributed by Andy Isaacson. (Revision 1347751) Result = SUCCESS todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1347751 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Util.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/DataTransferThrottler.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockReplacement.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #1071 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1071/)
        HDFS-3485. DataTransferThrottler will over-throttle when currentTimeMillis jumps. Contributed by Andy Isaacson. (Revision 1347751)

        Result = SUCCESS
        todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1347751
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Util.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/DataTransferThrottler.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockReplacement.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1071 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1071/ ) HDFS-3485 . DataTransferThrottler will over-throttle when currentTimeMillis jumps. Contributed by Andy Isaacson. (Revision 1347751) Result = SUCCESS todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1347751 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Util.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/DataTransferThrottler.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockReplacement.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #1104 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1104/)
        HDFS-3485. DataTransferThrottler will over-throttle when currentTimeMillis jumps. Contributed by Andy Isaacson. (Revision 1347751)

        Result = FAILURE
        todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1347751
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Util.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/DataTransferThrottler.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockReplacement.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1104 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1104/ ) HDFS-3485 . DataTransferThrottler will over-throttle when currentTimeMillis jumps. Contributed by Andy Isaacson. (Revision 1347751) Result = FAILURE todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1347751 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Util.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/DataTransferThrottler.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockReplacement.java

          People

          • Assignee:
            Andy Isaacson
            Reporter:
            Andy Isaacson
          • Votes:
            0 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development