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

Optimize insertion/removal of replica in ShortCircuitCache

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 3.0.0-alpha2
    • Fix Version/s: 2.8.0, 3.0.0-alpha2
    • Component/s: hdfs-client
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed
    • Tags:
      ShortCircuitCache

      Description

      Currently in ShortCircuitCache, two TreeMap objects are used to track the cached replicas.

      private final TreeMap<Long, ShortCircuitReplica> evictable = new TreeMap<>();
      private final TreeMap<Long, ShortCircuitReplica> evictableMmapped = new TreeMap<>();

      TreeMap employs Red-Black tree for sorting. This isn't an issue when using traditional HDD. But when using high-performance SSD/PCIe Flash, the cost inserting/removing an entry becomes considerable.

      To mitigate it, we designed a new list-based for replica tracking.

      The list is a double-linked FIFO. FIFO is time-based, thus insertion is a very low cost operation. On the other hand, list is not lookup-friendly. To address this issue, we introduce two references into ShortCircuitReplica object.

      ShortCircuitReplica next = null;
      ShortCircuitReplica prev = null;

      In this way, lookup is not needed when removing a replica from the list. We only need to modify its predecessor's and successor's references in the lists.

      Our tests showed up to 15-50% performance improvement when using PCIe flash as storage media.

      The original patch is against 2.6.4, now I am porting to Hadoop trunk, and patch will be posted soon.

      1. HDFS-10690.008.patch
        12 kB
        Fenghua Hu
      2. HDFS-10690.007.patch
        11 kB
        Fenghua Hu
      3. HDFS-10690.006.patch
        8 kB
        Fenghua Hu
      4. HDFS-10690.005.patch
        8 kB
        Fenghua Hu
      5. HDFS-10690.004.patch
        8 kB
        Fenghua Hu
      6. HDFS-10690.003.patch
        7 kB
        Fenghua Hu
      7. ShortCircuitCache_LinkedMap.patch
        7 kB
        Xiaoyu Yao
      8. HDFS-10690.002.patch
        23 kB
        Fenghua Hu
      9. HDFS-10690.001.patch
        22 kB
        Fenghua Hu

        Issue Links

          Activity

          Hide
          fenghua_hu Fenghua Hu added a comment -

          Some comments about the patch:
          1. LruList.java implements a double-linked list to track the cached replicainfo objects.
          2.Two references are added to ShortCircuitReplica object to eliminate the lookup in the list.

          Show
          fenghua_hu Fenghua Hu added a comment - Some comments about the patch: 1. LruList.java implements a double-linked list to track the cached replicainfo objects. 2.Two references are added to ShortCircuitReplica object to eliminate the lookup in the list.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 14s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 0m 8s Maven dependency ordering for branch
          +1 mvninstall 6m 45s trunk passed
          +1 compile 1m 31s trunk passed
          +1 checkstyle 0m 36s trunk passed
          +1 mvnsite 1m 33s trunk passed
          +1 mvneclipse 0m 29s trunk passed
          +1 findbugs 3m 13s trunk passed
          +1 javadoc 1m 23s trunk passed
          0 mvndep 0m 7s Maven dependency ordering for patch
          +1 mvninstall 1m 23s the patch passed
          +1 compile 1m 25s the patch passed
          +1 javac 1m 25s the patch passed
          -0 checkstyle 0m 30s hadoop-hdfs-project: The patch generated 17 new + 196 unchanged - 2 fixed = 213 total (was 198)
          +1 mvnsite 1m 32s the patch passed
          +1 mvneclipse 0m 23s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          -1 findbugs 1m 55s hadoop-hdfs-project/hadoop-hdfs-client generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
          +1 javadoc 1m 20s the patch passed
          +1 unit 1m 0s hadoop-hdfs-client in the patch passed.
          -1 unit 76m 37s hadoop-hdfs in the patch failed.
          -1 asflicense 0m 19s The patch generated 1 ASF License warnings.
          106m 8s



          Reason Tests
          FindBugs module:hadoop-hdfs-project/hadoop-hdfs-client
            Suspicious comparison of Long references in org.apache.hadoop.hdfs.shortcircuit.LruList.containsKey(Long) At LruList.java:in org.apache.hadoop.hdfs.shortcircuit.LruList.containsKey(Long) At LruList.java:[line 153]
          Failed junit tests hadoop.hdfs.TestRollingUpgrade
            hadoop.hdfs.server.datanode.TestDataNodeErasureCodingMetrics



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12820708/lrulist.patch
          JIRA Issue HDFS-10690
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux f13ea27f1665 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 7f3c306
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16236/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project.txt
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16236/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs-client.html
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16236/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16236/testReport/
          asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/16236/artifact/patchprocess/patch-asflicense-problems.txt
          modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16236/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 14s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 0m 8s Maven dependency ordering for branch +1 mvninstall 6m 45s trunk passed +1 compile 1m 31s trunk passed +1 checkstyle 0m 36s trunk passed +1 mvnsite 1m 33s trunk passed +1 mvneclipse 0m 29s trunk passed +1 findbugs 3m 13s trunk passed +1 javadoc 1m 23s trunk passed 0 mvndep 0m 7s Maven dependency ordering for patch +1 mvninstall 1m 23s the patch passed +1 compile 1m 25s the patch passed +1 javac 1m 25s the patch passed -0 checkstyle 0m 30s hadoop-hdfs-project: The patch generated 17 new + 196 unchanged - 2 fixed = 213 total (was 198) +1 mvnsite 1m 32s the patch passed +1 mvneclipse 0m 23s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 1m 55s hadoop-hdfs-project/hadoop-hdfs-client generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 javadoc 1m 20s the patch passed +1 unit 1m 0s hadoop-hdfs-client in the patch passed. -1 unit 76m 37s hadoop-hdfs in the patch failed. -1 asflicense 0m 19s The patch generated 1 ASF License warnings. 106m 8s Reason Tests FindBugs module:hadoop-hdfs-project/hadoop-hdfs-client   Suspicious comparison of Long references in org.apache.hadoop.hdfs.shortcircuit.LruList.containsKey(Long) At LruList.java:in org.apache.hadoop.hdfs.shortcircuit.LruList.containsKey(Long) At LruList.java: [line 153] Failed junit tests hadoop.hdfs.TestRollingUpgrade   hadoop.hdfs.server.datanode.TestDataNodeErasureCodingMetrics Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12820708/lrulist.patch JIRA Issue HDFS-10690 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux f13ea27f1665 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 7f3c306 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16236/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project.txt findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16236/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs-client.html unit https://builds.apache.org/job/PreCommit-HDFS-Build/16236/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16236/testReport/ asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/16236/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16236/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 0m 18s Maven dependency ordering for branch
          +1 mvninstall 7m 18s trunk passed
          +1 compile 1m 28s trunk passed
          +1 checkstyle 0m 32s trunk passed
          +1 mvnsite 1m 29s trunk passed
          +1 mvneclipse 0m 24s trunk passed
          +1 findbugs 3m 18s trunk passed
          +1 javadoc 1m 16s trunk passed
          0 mvndep 0m 6s Maven dependency ordering for patch
          +1 mvninstall 1m 21s the patch passed
          +1 compile 1m 24s the patch passed
          +1 javac 1m 24s the patch passed
          -0 checkstyle 0m 30s hadoop-hdfs-project: The patch generated 17 new + 197 unchanged - 2 fixed = 214 total (was 199)
          +1 mvnsite 1m 23s the patch passed
          +1 mvneclipse 0m 21s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          -1 findbugs 1m 38s hadoop-hdfs-project/hadoop-hdfs-client generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
          +1 javadoc 1m 12s the patch passed
          +1 unit 0m 55s hadoop-hdfs-client in the patch passed.
          -1 unit 61m 17s hadoop-hdfs in the patch failed.
          -1 asflicense 0m 32s The patch generated 1 ASF License warnings.
          90m 16s



          Reason Tests
          FindBugs module:hadoop-hdfs-project/hadoop-hdfs-client
            Suspicious comparison of Long references in org.apache.hadoop.hdfs.shortcircuit.LruList.containsKey(Long) At LruList.java:in org.apache.hadoop.hdfs.shortcircuit.LruList.containsKey(Long) At LruList.java:[line 153]
          Failed junit tests hadoop.hdfs.server.namenode.ha.TestFailureToReadEdits



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12820709/HDFS-10690.001.patch
          JIRA Issue HDFS-10690
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux a209889e560a 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 7f3c306
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16237/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project.txt
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16237/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs-client.html
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16237/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16237/testReport/
          asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/16237/artifact/patchprocess/patch-asflicense-problems.txt
          modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16237/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 0m 18s Maven dependency ordering for branch +1 mvninstall 7m 18s trunk passed +1 compile 1m 28s trunk passed +1 checkstyle 0m 32s trunk passed +1 mvnsite 1m 29s trunk passed +1 mvneclipse 0m 24s trunk passed +1 findbugs 3m 18s trunk passed +1 javadoc 1m 16s trunk passed 0 mvndep 0m 6s Maven dependency ordering for patch +1 mvninstall 1m 21s the patch passed +1 compile 1m 24s the patch passed +1 javac 1m 24s the patch passed -0 checkstyle 0m 30s hadoop-hdfs-project: The patch generated 17 new + 197 unchanged - 2 fixed = 214 total (was 199) +1 mvnsite 1m 23s the patch passed +1 mvneclipse 0m 21s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 1m 38s hadoop-hdfs-project/hadoop-hdfs-client generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 javadoc 1m 12s the patch passed +1 unit 0m 55s hadoop-hdfs-client in the patch passed. -1 unit 61m 17s hadoop-hdfs in the patch failed. -1 asflicense 0m 32s The patch generated 1 ASF License warnings. 90m 16s Reason Tests FindBugs module:hadoop-hdfs-project/hadoop-hdfs-client   Suspicious comparison of Long references in org.apache.hadoop.hdfs.shortcircuit.LruList.containsKey(Long) At LruList.java:in org.apache.hadoop.hdfs.shortcircuit.LruList.containsKey(Long) At LruList.java: [line 153] Failed junit tests hadoop.hdfs.server.namenode.ha.TestFailureToReadEdits Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12820709/HDFS-10690.001.patch JIRA Issue HDFS-10690 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux a209889e560a 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 7f3c306 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16237/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project.txt findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16237/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs-client.html unit https://builds.apache.org/job/PreCommit-HDFS-Build/16237/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16237/testReport/ asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/16237/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16237/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          xyao Xiaoyu Yao added a comment -

          Fenghua Hu, thanks for reporting the issue and posting the fix. If the goal is to avoid the TreeMap overhead, can we use LinkedHashMap with the following constructor (accessOrder=true) to simplify the code changes?

          public LinkedHashMap(int initialCapacity,
                       float loadFactor,
                       boolean accessOrder)
          Constructs an empty LinkedHashMap instance with the specified initial capacity, load factor and ordering mode.
          Parameters:
          initialCapacity - the initial capacity
          loadFactor - the load factor
          accessOrder - the ordering mode - true for access-order, false for insertion-order
          
          
          Show
          xyao Xiaoyu Yao added a comment - Fenghua Hu , thanks for reporting the issue and posting the fix. If the goal is to avoid the TreeMap overhead, can we use LinkedHashMap with the following constructor ( accessOrder=true ) to simplify the code changes? public LinkedHashMap( int initialCapacity, float loadFactor, boolean accessOrder) Constructs an empty LinkedHashMap instance with the specified initial capacity, load factor and ordering mode. Parameters: initialCapacity - the initial capacity loadFactor - the load factor accessOrder - the ordering mode - true for access-order, false for insertion-order
          Hide
          fenghua_hu Fenghua Hu added a comment -

          Xiaoyu, thanks for the suggestion. LinkedHashMap is another good choice for performance. I considered it before but didn't implement thus had no performance. LinkedHashMap needs to do two things: 1. calculate key and insert hashmap, 2, insert into a linked list, hence in theory, it does more things than LruList in this patch. Yes, LinkedHashMap does make code cleaner, but its performances could be compromised. Please correct me if i am wrong.

          Show
          fenghua_hu Fenghua Hu added a comment - Xiaoyu, thanks for the suggestion. LinkedHashMap is another good choice for performance. I considered it before but didn't implement thus had no performance. LinkedHashMap needs to do two things: 1. calculate key and insert hashmap, 2, insert into a linked list, hence in theory, it does more things than LruList in this patch. Yes, LinkedHashMap does make code cleaner, but its performances could be compromised. Please correct me if i am wrong.
          Hide
          fenghua_hu Fenghua Hu added a comment -

          Patch updated.

          Show
          fenghua_hu Fenghua Hu added a comment - Patch updated.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 14s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 0m 31s Maven dependency ordering for branch
          +1 mvninstall 7m 46s trunk passed
          +1 compile 1m 37s trunk passed
          +1 checkstyle 0m 35s trunk passed
          +1 mvnsite 1m 33s trunk passed
          +1 mvneclipse 0m 25s trunk passed
          +1 findbugs 3m 26s trunk passed
          +1 javadoc 1m 20s trunk passed
          0 mvndep 0m 7s Maven dependency ordering for patch
          +1 mvninstall 1m 26s the patch passed
          +1 compile 1m 32s the patch passed
          +1 javac 1m 32s the patch passed
          +1 checkstyle 0m 31s hadoop-hdfs-project: The patch generated 0 new + 197 unchanged - 2 fixed = 197 total (was 199)
          +1 mvnsite 1m 30s the patch passed
          +1 mvneclipse 0m 21s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 42s the patch passed
          +1 javadoc 1m 15s the patch passed
          +1 unit 0m 59s hadoop-hdfs-client in the patch passed.
          +1 unit 63m 14s hadoop-hdfs in the patch passed.
          +1 asflicense 0m 23s The patch does not generate ASF License warnings.
          93m 56s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12820875/HDFS-10690.002.patch
          JIRA Issue HDFS-10690
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 79bbad446325 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 8ebf2e9
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16247/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16247/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 14s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 0m 31s Maven dependency ordering for branch +1 mvninstall 7m 46s trunk passed +1 compile 1m 37s trunk passed +1 checkstyle 0m 35s trunk passed +1 mvnsite 1m 33s trunk passed +1 mvneclipse 0m 25s trunk passed +1 findbugs 3m 26s trunk passed +1 javadoc 1m 20s trunk passed 0 mvndep 0m 7s Maven dependency ordering for patch +1 mvninstall 1m 26s the patch passed +1 compile 1m 32s the patch passed +1 javac 1m 32s the patch passed +1 checkstyle 0m 31s hadoop-hdfs-project: The patch generated 0 new + 197 unchanged - 2 fixed = 197 total (was 199) +1 mvnsite 1m 30s the patch passed +1 mvneclipse 0m 21s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 42s the patch passed +1 javadoc 1m 15s the patch passed +1 unit 0m 59s hadoop-hdfs-client in the patch passed. +1 unit 63m 14s hadoop-hdfs in the patch passed. +1 asflicense 0m 23s The patch does not generate ASF License warnings. 93m 56s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12820875/HDFS-10690.002.patch JIRA Issue HDFS-10690 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 79bbad446325 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 8ebf2e9 Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16247/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16247/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          fenghua_hu Fenghua Hu added a comment - - edited

          Performance test result against hadoop-2.6.4:
          Test configuration:

          • 1 name node + 1 data node
          • Datanode: 1 PCIe SSD, 1 SATA HDD, OS page cache: on, read ahead for PCIe SSD: 0
          • Hbase 1.1.2
          • Table: key: 10 bytes, value: 1KB, total table size: 320GB, region count: 30, storage policy: ALL_SSD, blocksize: 8K
          • YCSB 1.1.2, 4 clients, 16 processes / client, in total 64 processes

          Test steps:
          Run GET queries with 64 YCSB processes for 30 minutes, record the QPS for each process。
          Total QPS:
          w/o patch: 95K
          w/ patch: 135K

          The performance gain is (135 - 95) / 95 = 42%.

          Show
          fenghua_hu Fenghua Hu added a comment - - edited Performance test result against hadoop-2.6.4: Test configuration: 1 name node + 1 data node Datanode: 1 PCIe SSD, 1 SATA HDD, OS page cache: on, read ahead for PCIe SSD: 0 Hbase 1.1.2 Table: key: 10 bytes, value: 1KB, total table size: 320GB, region count: 30, storage policy: ALL_SSD, blocksize: 8K YCSB 1.1.2, 4 clients, 16 processes / client, in total 64 processes Test steps: Run GET queries with 64 YCSB processes for 30 minutes, record the QPS for each process。 Total QPS: w/o patch: 95K w/ patch: 135K The performance gain is (135 - 95) / 95 = 42%.
          Hide
          xyao Xiaoyu Yao added a comment - - edited

          Fenghua Hu, thanks for updating the patch with the perf numbers.

          LinkedHashMap needs to do two things: 1. calculate key and insert hashmap, 2, insert into a linked list, hence in theory, it does more things than LruList in this patch. Yes, LinkedHashMap does make code cleaner, but its performances could be compromised. Please correct me if i am wrong.

          I think LinkedHashMap is a better choice based on the following reasons unless the perf number shows significant difference.

          1. We prefer not to reinvent the wheel, which adds unnecessary test/maintenance cost for the project. Search though the Hadoop code base, we can see many LRU implementation based on well-tested LinkedHashMap. This can not only improve the performance but also simplify the existing code in ShortCuicuitCache.

          2. The hashmap inside LinkedHashMap does have some overhead of key calculation. But it offers faster lookup than a pure double link list, which is also very important for ShortCircuitCache.

          Show
          xyao Xiaoyu Yao added a comment - - edited Fenghua Hu , thanks for updating the patch with the perf numbers. LinkedHashMap needs to do two things: 1. calculate key and insert hashmap, 2, insert into a linked list, hence in theory, it does more things than LruList in this patch. Yes, LinkedHashMap does make code cleaner, but its performances could be compromised. Please correct me if i am wrong. I think LinkedHashMap is a better choice based on the following reasons unless the perf number shows significant difference. 1. We prefer not to reinvent the wheel, which adds unnecessary test/maintenance cost for the project. Search though the Hadoop code base, we can see many LRU implementation based on well-tested LinkedHashMap. This can not only improve the performance but also simplify the existing code in ShortCuicuitCache. 2. The hashmap inside LinkedHashMap does have some overhead of key calculation. But it offers faster lookup than a pure double link list, which is also very important for ShortCircuitCache.
          Hide
          fenghua_hu Fenghua Hu added a comment -

          Xiaoyu, thanks for the reply. Regarding the bulletin 2, look like i didn't explain the design very well. Sorry for the misleading. I'd like to clarify here. This design won't need lookup in link list, because there are two references in ShortCircuitReplica object. If we want to remove a ShortCircuitReplica object from the list, just directly access its references and unlink itself. That's why it can improve performance.

          Show
          fenghua_hu Fenghua Hu added a comment - Xiaoyu, thanks for the reply. Regarding the bulletin 2, look like i didn't explain the design very well. Sorry for the misleading. I'd like to clarify here. This design won't need lookup in link list, because there are two references in ShortCircuitReplica object. If we want to remove a ShortCircuitReplica object from the list, just directly access its references and unlink itself. That's why it can improve performance.
          Hide
          fenghua_hu Fenghua Hu added a comment - - edited

          Xiaoyu,

          Xiaoyu YaoI tried to replace TreeMap with linkedHashMap, but found LinkedHashMap lacks of function "ceilingEntry" or similar alternative, which is key to implement LRU-based replacement algorithm. LinkedHashMap also can't provide getYoungest or getEldest or similar functions. That's to say, if we want to use LinkedHashMap, we actually need to rewrite it. Any comments? Thanks.

          Finally i found the correct email for you

          Show
          fenghua_hu Fenghua Hu added a comment - - edited Xiaoyu, Xiaoyu Yao I tried to replace TreeMap with linkedHashMap, but found LinkedHashMap lacks of function "ceilingEntry" or similar alternative, which is key to implement LRU-based replacement algorithm. LinkedHashMap also can't provide getYoungest or getEldest or similar functions. That's to say, if we want to use LinkedHashMap, we actually need to rewrite it. Any comments? Thanks. Finally i found the correct email for you
          Hide
          carp84 Yu Li added a comment -

          Run GET queries with 64 YCSB processes for 30 minutes, record the QPS for each process。
          Total QPS:
          w/o patch: 95K
          w/ patch: 135K
          The performance gain is (135 - 95) / 95 = 42%.

          I think 42% is quite a big performance gain and people using fast disks like PCIe-SSD could benefit a lot. Mighty committers, mind further review and help make this in? Thanks.

          Show
          carp84 Yu Li added a comment - Run GET queries with 64 YCSB processes for 30 minutes, record the QPS for each process。 Total QPS: w/o patch: 95K w/ patch: 135K The performance gain is (135 - 95) / 95 = 42%. I think 42% is quite a big performance gain and people using fast disks like PCIe-SSD could benefit a lot. Mighty committers, mind further review and help make this in? Thanks.
          Hide
          xyao Xiaoyu Yao added a comment -

          I tried to replace TreeMap with linkedHashMap, but found LinkedHashMap lacks of function "ceilingEntry" or similar alternative, which is key to implement LRU-based replacement algorithm. LinkedHashMap also can't provide getYoungest or getEldest or similar functions. That's to say, if we want to use LinkedHashMap, we actually need to rewrite it. Any comments? Thanks.

          Fenghua Hu/Yu Li, thanks again for working on this. I think we can use org.apache.commons.collections.map.LinkedMap here which provides both firstKey(eldest) and lastKey(youngest). I highlight eldest/youngest here because the firstKey/lastKey in the Javadoc is wrong. Attach a patch for illustration (PS: I have not tested it myself), feel free to modify/benchmark it and let us know the results. Hope that helps.

          Show
          xyao Xiaoyu Yao added a comment - I tried to replace TreeMap with linkedHashMap, but found LinkedHashMap lacks of function "ceilingEntry" or similar alternative, which is key to implement LRU-based replacement algorithm. LinkedHashMap also can't provide getYoungest or getEldest or similar functions. That's to say, if we want to use LinkedHashMap, we actually need to rewrite it. Any comments? Thanks. Fenghua Hu / Yu Li , thanks again for working on this. I think we can use org.apache.commons.collections.map.LinkedMap here which provides both firstKey( eldest ) and lastKey( youngest ). I highlight eldest/youngest here because the firstKey/lastKey in the Javadoc is wrong. Attach a patch for illustration (PS: I have not tested it myself), feel free to modify/benchmark it and let us know the results. Hope that helps.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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 mvninstall 6m 54s trunk passed
          +1 compile 0m 31s trunk passed
          +1 checkstyle 0m 16s trunk passed
          +1 mvnsite 0m 37s trunk passed
          +1 mvneclipse 0m 14s trunk passed
          +1 findbugs 1m 27s trunk passed
          +1 javadoc 0m 23s trunk passed
          +1 mvninstall 0m 32s the patch passed
          +1 compile 0m 28s the patch passed
          -1 javac 0m 28s hadoop-hdfs-project_hadoop-hdfs-client generated 1 new + 13 unchanged - 1 fixed = 14 total (was 14)
          -0 checkstyle 0m 13s hadoop-hdfs-project/hadoop-hdfs-client: The patch generated 7 new + 12 unchanged - 4 fixed = 19 total (was 16)
          +1 mvnsite 0m 33s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 32s the patch passed
          +1 javadoc 0m 17s the patch passed
          +1 unit 0m 51s hadoop-hdfs-client in the patch passed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          16m 51s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12823090/ShortCircuitCache_LinkedMap.patch
          JIRA Issue HDFS-10690
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 365dc7873691 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / d00d3ad
          Default Java 1.8.0_101
          findbugs v3.0.0
          javac https://builds.apache.org/job/PreCommit-HDFS-Build/16385/artifact/patchprocess/diff-compile-javac-hadoop-hdfs-project_hadoop-hdfs-client.txt
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16385/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs-client.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16385/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16385/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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 mvninstall 6m 54s trunk passed +1 compile 0m 31s trunk passed +1 checkstyle 0m 16s trunk passed +1 mvnsite 0m 37s trunk passed +1 mvneclipse 0m 14s trunk passed +1 findbugs 1m 27s trunk passed +1 javadoc 0m 23s trunk passed +1 mvninstall 0m 32s the patch passed +1 compile 0m 28s the patch passed -1 javac 0m 28s hadoop-hdfs-project_hadoop-hdfs-client generated 1 new + 13 unchanged - 1 fixed = 14 total (was 14) -0 checkstyle 0m 13s hadoop-hdfs-project/hadoop-hdfs-client: The patch generated 7 new + 12 unchanged - 4 fixed = 19 total (was 16) +1 mvnsite 0m 33s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 32s the patch passed +1 javadoc 0m 17s the patch passed +1 unit 0m 51s hadoop-hdfs-client in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 16m 51s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12823090/ShortCircuitCache_LinkedMap.patch JIRA Issue HDFS-10690 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 365dc7873691 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / d00d3ad Default Java 1.8.0_101 findbugs v3.0.0 javac https://builds.apache.org/job/PreCommit-HDFS-Build/16385/artifact/patchprocess/diff-compile-javac-hadoop-hdfs-project_hadoop-hdfs-client.txt checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16385/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs-client.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16385/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16385/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          fenghua_hu Fenghua Hu added a comment -

          Xiaoyu,
          Xiaoyu YaoThanks for the suggestion. I'll test the patch once I get test enviornment ready.

          Show
          fenghua_hu Fenghua Hu added a comment - Xiaoyu, Xiaoyu Yao Thanks for the suggestion. I'll test the patch once I get test enviornment ready.
          Hide
          fenghua_hu Fenghua Hu added a comment - - edited

          Xiaoyu Yao, I have a question about the patch:

          According to https://commons.apache.org/proper/commons-collections/jacoco/org.apache.commons.collections4.map/LinkedMap.java.html

          public K get(final int index)

          { return getEntry(index).getKey(); }

          public V remove(final int index)

          { return remove(get(index)); }

          note that the parameter is index.
          In the patch,
          ...
          + ShortCircuitReplica replica = (ShortCircuitReplica)evictableMmapped.get
          + (eldestKey);
          ...
          + ShortCircuitReplica removed = (ShortCircuitReplica)map.remove
          + (evictableTimeNs);

          Here eldestKey and evictableTimeNs actually are the key, not the index, thus they should respectively been changed to
          evictableMmapped.getValue(map.indexOf(eldestKey));
          and
          map.remove(map.indexOf(evictableTimeNs);

          But indexOf() is a O( n ) operation and LinkedMap's performance for remove won't compete with TreeMap().

          Correct me if i am wrong. Thanks.

          Show
          fenghua_hu Fenghua Hu added a comment - - edited Xiaoyu Yao , I have a question about the patch: According to https://commons.apache.org/proper/commons-collections/jacoco/org.apache.commons.collections4.map/LinkedMap.java.html public K get(final int index) { return getEntry(index).getKey(); } public V remove(final int index) { return remove(get(index)); } note that the parameter is index. In the patch, ... + ShortCircuitReplica replica = (ShortCircuitReplica)evictableMmapped.get + (eldestKey); ... + ShortCircuitReplica removed = (ShortCircuitReplica)map.remove + (evictableTimeNs); Here eldestKey and evictableTimeNs actually are the key, not the index, thus they should respectively been changed to evictableMmapped.getValue(map.indexOf(eldestKey)); and map.remove(map.indexOf(evictableTimeNs); But indexOf() is a O( n ) operation and LinkedMap's performance for remove won't compete with TreeMap(). Correct me if i am wrong. Thanks.
          Hide
          xyao Xiaoyu Yao added a comment -

          Thanks Fenghua Hu for correcting the mix usage of index/key in my previous patch. Your proposed change looks good to me.
          I don't worry about the LinkedMap perf compared with TreeMap() because indexOf() is O( n ) in general lookup but always O(1) for finding the eldest in the use case here.

          A cleaner way would be extending LinkedMap with a firstEntry method and use LinkedLruMap#firstEntry().

          package org.apache.hadoop.hdfs.shortcircuit;
          
          import org.apache.commons.collections.map.LinkedMap;
          
          import java.util.NoSuchElementException;
          
          public class LinkedLruMap extends LinkedMap {
            protected Object firstEntry() {
              if (size == 0) {
                throw new NoSuchElementException("Map is empty");
              }
              return entryAfter(header).getValue();
            }
          }
          
          Show
          xyao Xiaoyu Yao added a comment - Thanks Fenghua Hu for correcting the mix usage of index/key in my previous patch. Your proposed change looks good to me. I don't worry about the LinkedMap perf compared with TreeMap() because indexOf() is O( n ) in general lookup but always O(1) for finding the eldest in the use case here. A cleaner way would be extending LinkedMap with a firstEntry method and use LinkedLruMap#firstEntry(). package org.apache.hadoop.hdfs.shortcircuit; import org.apache.commons.collections.map.LinkedMap; import java.util.NoSuchElementException; public class LinkedLruMap extends LinkedMap { protected Object firstEntry() { if (size == 0) { throw new NoSuchElementException( "Map is empty" ); } return entryAfter(header).getValue(); } }
          Hide
          fenghua_hu Fenghua Hu added a comment -

          Xiaoyu Yao,

          Thanks for your support!

          I would like to clarify that the removal could happen in two cases. The first case is to remove the eldest, just like you mentioned. The other is that when an entry in map is accessed/activated again, it's promoted from cache to memory, we need to remove it. I think the latter is more common. For this case, it's a O( n ) operation. Sorry for the confusion.

          Show
          fenghua_hu Fenghua Hu added a comment - Xiaoyu Yao , Thanks for your support! I would like to clarify that the removal could happen in two cases. The first case is to remove the eldest, just like you mentioned. The other is that when an entry in map is accessed/activated again, it's promoted from cache to memory, we need to remove it. I think the latter is more common. For this case, it's a O( n ) operation. Sorry for the confusion.
          Hide
          fenghua_hu Fenghua Hu added a comment - - edited

          I would like to explain the solution here.

          Currently, TreeMap is used to track all the ShortCircuitReplica entries in the order of being inserted. These entries could be removed from TreeMap in two cases. The first is when the entry is accessed again, it will be removed from TreeMap. Please note that the entry could be anyone in the TreeMap. The other is when the entry is evicted due to treemap size limitation, in this case, only the eldest entry will be removed.

          Removal is a costly operation for the first case, because looking up ShortCircuitReplica is needed, in TreeMap, it's O(log n) operation. To improve it, we design a new data structure LruList, which entirely eliminates costly look-up operation.
          --------------- --------------- ----------------

          Replica 1 ----Next---> Replica 2 ----Next---> Replica 3
          Replica 1 <----Prev--- Replica 2 <----Prev--- Replica 3

          --------------- --------------- ----------------

          We introduced two references in ShortCircuitReplica objects. Reference Next points to the elder ShortCircuitReplica and Prev points to the younger one. All the replicas are doubly linked in order of insertion time. The youngest is always at the head of the linked list, and the eldest is always at the tail. Removing the entries between the head and the tail doesn't need any lookup, because the replica knows its position in linked list by next and prev, thus remove is simple: change it's precedessor's and its succssor's next and prev. The order of operation is always O(1).

          For insertion, the youngest entry is always be added to the head, thus the operation is also O(1).

          Existing classes, including LinkedHashMap, LinkedMap, can't provide O(1) operation for insertion/lookup/removal.

          Here comes a brief test result:

          Run GET queries with 64 YCSB processes for 30 minutes, record the QPS for each process。
          Total QPS:
          w/o patch: 95K
          w/ patch: 135K

          The performance gain is (135 - 95) / 95 = 42%.

          Suggestions/comments are very welcomed.

          Show
          fenghua_hu Fenghua Hu added a comment - - edited I would like to explain the solution here. Currently, TreeMap is used to track all the ShortCircuitReplica entries in the order of being inserted. These entries could be removed from TreeMap in two cases. The first is when the entry is accessed again, it will be removed from TreeMap. Please note that the entry could be anyone in the TreeMap. The other is when the entry is evicted due to treemap size limitation, in this case, only the eldest entry will be removed. Removal is a costly operation for the first case, because looking up ShortCircuitReplica is needed, in TreeMap, it's O(log n) operation. To improve it, we design a new data structure LruList, which entirely eliminates costly look-up operation. --------------- --------------- ---------------- Replica 1 ---- Next ---> Replica 2 ---- Next ---> Replica 3 Replica 1 <---- Prev --- Replica 2 <---- Prev --- Replica 3 --------------- --------------- ---------------- We introduced two references in ShortCircuitReplica objects. Reference Next points to the elder ShortCircuitReplica and Prev points to the younger one. All the replicas are doubly linked in order of insertion time. The youngest is always at the head of the linked list, and the eldest is always at the tail. Removing the entries between the head and the tail doesn't need any lookup, because the replica knows its position in linked list by next and prev, thus remove is simple: change it's precedessor's and its succssor's next and prev. The order of operation is always O(1). For insertion, the youngest entry is always be added to the head, thus the operation is also O(1). Existing classes, including LinkedHashMap, LinkedMap, can't provide O(1) operation for insertion/lookup/removal. Here comes a brief test result: Run GET queries with 64 YCSB processes for 30 minutes, record the QPS for each process。 Total QPS: w/o patch: 95K w/ patch: 135K The performance gain is (135 - 95) / 95 = 42%. Suggestions/comments are very welcomed.
          Hide
          xyao Xiaoyu Yao added a comment - - edited

          Fenghua Hu, I understand the tradeoff between memory footprint and the insert/delete performance here. Note the ShortCircuitCache#replicaInfoMap is a HashMap too, we are O( n) worst case and O(1) on average anyway for ref()/unref() case. So I don't think that will change the time complexity of ref()/unref() case.

          After taking another look at the get() usage issue you mentioned earlier, I don't think we need the extra indexOf(). A LinkedMap is extended from AbstractLinkedMap, which extended from AbstractHashedMap. Because of that, we are calling into the AbstractHashedMap#get(Object Key) here according to https://commons.apache.org/proper/commonscollections/jacoco/org.apache.commons.collections4.map/AbstractHashedMap.java.html. This will still be O(1) on average and O( n) worst case.

          I would appreciate if you could post the result of the simpler approach that I proposed under the same test environment. If the difference is significant, we can look at alternatives as you suggested. Let me know your thoughts. Thanks!

          Show
          xyao Xiaoyu Yao added a comment - - edited Fenghua Hu , I understand the tradeoff between memory footprint and the insert/delete performance here. Note the ShortCircuitCache#replicaInfoMap is a HashMap too, we are O( n) worst case and O(1) on average anyway for ref()/unref() case. So I don't think that will change the time complexity of ref()/unref() case. After taking another look at the get() usage issue you mentioned earlier, I don't think we need the extra indexOf(). A LinkedMap is extended from AbstractLinkedMap, which extended from AbstractHashedMap. Because of that, we are calling into the AbstractHashedMap#get(Object Key) here according to https://commons.apache.org/proper/commonscollections/jacoco/org.apache.commons.collections4.map/AbstractHashedMap.java.html . This will still be O(1) on average and O( n) worst case. I would appreciate if you could post the result of the simpler approach that I proposed under the same test environment. If the difference is significant, we can look at alternatives as you suggested. Let me know your thoughts. Thanks!
          Hide
          fenghua_hu Fenghua Hu added a comment -

          Xiaoyu Yao, thanks for your comments. I'll write a micro benchmark to compare LinkedMap with TreeMap and get back to you.

          Show
          fenghua_hu Fenghua Hu added a comment - Xiaoyu Yao , thanks for your comments. I'll write a micro benchmark to compare LinkedMap with TreeMap and get back to you.
          Hide
          xyao Xiaoyu Yao added a comment -

          I'll write a micro benchmark to compare LinkedMap with TreeMap and get back to you.

          We have a lot of variations here: key, hash function, percentage of insert/delete/lookup operations, etc. There are quite some micro benchmark for TreeMap/LinkedHashMap(similar to LinkedMap from apache common) if you search online. Instead of tuning the parameter to simulate the workloads with micro benchmark, I would suggest we run the same YCSB benchmark with the last patch using LinkedMap for easy comparison.

          Show
          xyao Xiaoyu Yao added a comment - I'll write a micro benchmark to compare LinkedMap with TreeMap and get back to you. We have a lot of variations here: key, hash function, percentage of insert/delete/lookup operations, etc. There are quite some micro benchmark for TreeMap/LinkedHashMap(similar to LinkedMap from apache common) if you search online. Instead of tuning the parameter to simulate the workloads with micro benchmark, I would suggest we run the same YCSB benchmark with the last patch using LinkedMap for easy comparison.
          Hide
          fenghua_hu Fenghua Hu added a comment -

          I got you. I'll run YCSB.

          Regarding the patch, what do you mean "I don't think we need the extra indexOf()."? I think the LinkedMap patch needs revision, am i right?

          Show
          fenghua_hu Fenghua Hu added a comment - I got you. I'll run YCSB. Regarding the patch, what do you mean "I don't think we need the extra indexOf()."? I think the LinkedMap patch needs revision, am i right?
          Hide
          xyao Xiaoyu Yao added a comment -

          I mean the LinkedMap patch should just work without revision. LinkedMap#get(Object obj) and LinkedMap#remove(Object obj) will work with key (Object type) instead of index (int type) because it is inherited from AbstractHashedMap as I mentioned in comment..

          Show
          xyao Xiaoyu Yao added a comment - I mean the LinkedMap patch should just work without revision. LinkedMap#get(Object obj) and LinkedMap#remove(Object obj) will work with key (Object type) instead of index (int type) because it is inherited from AbstractHashedMap as I mentioned in comment. .
          Hide
          fenghua_hu Fenghua Hu added a comment -

          Xiaoyu Yao,

          I just got environment to test your patch. Below is the result.
          1. LruList: 122K 122K 123K
          2. LinkedMap: 118K 119K 117K QPS

          In general, LinkedMap's performance is 3%-5% worse than LruList. I think it's reasonable, because LinkedMap needs hash and in the worst case it needs more comparison.

          I still prefer to LruList implementation for the minor performance improvement, but consider that the difference is not big, I am open to LinkedMap. What do you think?

          Again, thank you for your review and great suggestions!

          Show
          fenghua_hu Fenghua Hu added a comment - Xiaoyu Yao , I just got environment to test your patch. Below is the result. 1. LruList: 122K 122K 123K 2. LinkedMap: 118K 119K 117K QPS In general, LinkedMap's performance is 3%-5% worse than LruList. I think it's reasonable, because LinkedMap needs hash and in the worst case it needs more comparison. I still prefer to LruList implementation for the minor performance improvement, but consider that the difference is not big, I am open to LinkedMap. What do you think? Again, thank you for your review and great suggestions!
          Hide
          xyao Xiaoyu Yao added a comment -

          Thanks Fenghua Hu for providing the new numbers. Considering the perf difference and what we have done before for similar TreeMap performance issue in HDFS-7433, HDFS-8793, I would prefer a LinkedMap (or HashMap) based solution with lower risk of regression.

          Show
          xyao Xiaoyu Yao added a comment - Thanks Fenghua Hu for providing the new numbers. Considering the perf difference and what we have done before for similar TreeMap performance issue in HDFS-7433 , HDFS-8793 , I would prefer a LinkedMap (or HashMap) based solution with lower risk of regression.
          Hide
          fenghua_hu Fenghua Hu added a comment -

          hi Xiaoyu Yao, based on your suggestion, i have updated the patch which fixes a few unit test issues. Could you please review it? Thanks.

          Show
          fenghua_hu Fenghua Hu added a comment - hi Xiaoyu Yao , based on your suggestion, i have updated the patch which fixes a few unit test issues. Could you please review it? Thanks.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 1s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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 mvninstall 6m 42s trunk passed
          +1 compile 0m 31s trunk passed
          +1 checkstyle 0m 14s trunk passed
          +1 mvnsite 0m 33s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 1m 23s trunk passed
          +1 javadoc 0m 19s trunk passed
          +1 mvninstall 0m 30s the patch passed
          +1 compile 0m 27s the patch passed
          -1 javac 0m 27s hadoop-hdfs-project_hadoop-hdfs-client generated 1 new + 13 unchanged - 1 fixed = 14 total (was 14)
          -0 checkstyle 0m 11s hadoop-hdfs-project/hadoop-hdfs-client: The patch generated 7 new + 12 unchanged - 4 fixed = 19 total (was 16)
          +1 mvnsite 0m 31s the patch passed
          +1 mvneclipse 0m 9s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          -1 findbugs 1m 31s hadoop-hdfs-project/hadoop-hdfs-client generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
          +1 javadoc 0m 17s the patch passed
          +1 unit 0m 52s hadoop-hdfs-client in the patch passed.
          +1 asflicense 0m 17s The patch does not generate ASF License warnings.
          16m 6s



          Reason Tests
          FindBugs module:hadoop-hdfs-project/hadoop-hdfs-client
            Null passed for non-null parameter of purge(ShortCircuitReplica) in org.apache.hadoop.hdfs.shortcircuit.ShortCircuitCache.trimEvictionMaps() Method invoked at ShortCircuitCache.java:of purge(ShortCircuitReplica) in org.apache.hadoop.hdfs.shortcircuit.ShortCircuitCache.trimEvictionMaps() Method invoked at ShortCircuitCache.java:[line 554]



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10690
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12830242/HDFS-10690.003.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 84314e41ed87 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 5707f88
          Default Java 1.8.0_101
          findbugs v3.0.0
          javac https://builds.apache.org/job/PreCommit-HDFS-Build/16859/artifact/patchprocess/diff-compile-javac-hadoop-hdfs-project_hadoop-hdfs-client.txt
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16859/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs-client.txt
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16859/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs-client.html
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16859/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16859/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 1s The patch does not contain any @author tags. -1 test4tests 0m 0s 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 mvninstall 6m 42s trunk passed +1 compile 0m 31s trunk passed +1 checkstyle 0m 14s trunk passed +1 mvnsite 0m 33s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 23s trunk passed +1 javadoc 0m 19s trunk passed +1 mvninstall 0m 30s the patch passed +1 compile 0m 27s the patch passed -1 javac 0m 27s hadoop-hdfs-project_hadoop-hdfs-client generated 1 new + 13 unchanged - 1 fixed = 14 total (was 14) -0 checkstyle 0m 11s hadoop-hdfs-project/hadoop-hdfs-client: The patch generated 7 new + 12 unchanged - 4 fixed = 19 total (was 16) +1 mvnsite 0m 31s the patch passed +1 mvneclipse 0m 9s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 1m 31s hadoop-hdfs-project/hadoop-hdfs-client generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 javadoc 0m 17s the patch passed +1 unit 0m 52s hadoop-hdfs-client in the patch passed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 16m 6s Reason Tests FindBugs module:hadoop-hdfs-project/hadoop-hdfs-client   Null passed for non-null parameter of purge(ShortCircuitReplica) in org.apache.hadoop.hdfs.shortcircuit.ShortCircuitCache.trimEvictionMaps() Method invoked at ShortCircuitCache.java:of purge(ShortCircuitReplica) in org.apache.hadoop.hdfs.shortcircuit.ShortCircuitCache.trimEvictionMaps() Method invoked at ShortCircuitCache.java: [line 554] Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10690 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12830242/HDFS-10690.003.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 84314e41ed87 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 5707f88 Default Java 1.8.0_101 findbugs v3.0.0 javac https://builds.apache.org/job/PreCommit-HDFS-Build/16859/artifact/patchprocess/diff-compile-javac-hadoop-hdfs-project_hadoop-hdfs-client.txt checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16859/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs-client.txt findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16859/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs-client.html Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16859/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16859/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          fenghua_hu Fenghua Hu added a comment -

          Fixed two minor issues.

          Show
          fenghua_hu Fenghua Hu added a comment - Fixed two minor issues.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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 mvninstall 7m 16s trunk passed
          +1 compile 0m 34s trunk passed
          +1 checkstyle 0m 15s trunk passed
          +1 mvnsite 0m 37s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 1m 24s trunk passed
          +1 javadoc 0m 22s trunk passed
          +1 mvninstall 0m 31s the patch passed
          +1 compile 0m 28s the patch passed
          +1 javac 0m 28s the patch passed
          -0 checkstyle 0m 12s hadoop-hdfs-project/hadoop-hdfs-client: The patch generated 7 new + 12 unchanged - 4 fixed = 19 total (was 16)
          +1 mvnsite 0m 32s the patch passed
          +1 mvneclipse 0m 9s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 31s the patch passed
          +1 javadoc 0m 18s the patch passed
          +1 unit 0m 55s hadoop-hdfs-client in the patch passed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          17m 5s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10690
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12830243/HDFS-10690.004.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 36b15e8d3198 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 5707f88
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16860/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs-client.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16860/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16860/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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 mvninstall 7m 16s trunk passed +1 compile 0m 34s trunk passed +1 checkstyle 0m 15s trunk passed +1 mvnsite 0m 37s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 24s trunk passed +1 javadoc 0m 22s trunk passed +1 mvninstall 0m 31s the patch passed +1 compile 0m 28s the patch passed +1 javac 0m 28s the patch passed -0 checkstyle 0m 12s hadoop-hdfs-project/hadoop-hdfs-client: The patch generated 7 new + 12 unchanged - 4 fixed = 19 total (was 16) +1 mvnsite 0m 32s the patch passed +1 mvneclipse 0m 9s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 31s the patch passed +1 javadoc 0m 18s the patch passed +1 unit 0m 55s hadoop-hdfs-client in the patch passed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 17m 5s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10690 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12830243/HDFS-10690.004.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 36b15e8d3198 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 5707f88 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16860/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs-client.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16860/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16860/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          fenghua_hu Fenghua Hu added a comment -

          Existing unit tests have been able to cover all the necessary tests, no new case needs to be introduced.

          Show
          fenghua_hu Fenghua Hu added a comment - Existing unit tests have been able to cover all the necessary tests, no new case needs to be introduced.
          Hide
          fenghua_hu Fenghua Hu added a comment -

          Fixed a few coding style issues.

          Show
          fenghua_hu Fenghua Hu added a comment - Fixed a few coding style issues.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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 mvninstall 8m 41s trunk passed
          +1 compile 0m 29s trunk passed
          +1 checkstyle 0m 16s trunk passed
          +1 mvnsite 0m 35s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 1m 28s trunk passed
          +1 javadoc 0m 20s trunk passed
          +1 mvninstall 0m 31s the patch passed
          +1 compile 0m 32s the patch passed
          +1 javac 0m 32s the patch passed
          +1 checkstyle 0m 13s hadoop-hdfs-project/hadoop-hdfs-client: The patch generated 0 new + 13 unchanged - 4 fixed = 13 total (was 17)
          +1 mvnsite 0m 32s the patch passed
          +1 mvneclipse 0m 9s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 34s the patch passed
          +1 javadoc 0m 18s the patch passed
          +1 unit 0m 56s hadoop-hdfs-client in the patch passed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          18m 32s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10690
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12830246/HDFS-10690.005.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux d6905057204e 3.13.0-92-generic #139-Ubuntu SMP Tue Jun 28 20:42:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 5707f88
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16861/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16861/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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 mvninstall 8m 41s trunk passed +1 compile 0m 29s trunk passed +1 checkstyle 0m 16s trunk passed +1 mvnsite 0m 35s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 28s trunk passed +1 javadoc 0m 20s trunk passed +1 mvninstall 0m 31s the patch passed +1 compile 0m 32s the patch passed +1 javac 0m 32s the patch passed +1 checkstyle 0m 13s hadoop-hdfs-project/hadoop-hdfs-client: The patch generated 0 new + 13 unchanged - 4 fixed = 13 total (was 17) +1 mvnsite 0m 32s the patch passed +1 mvneclipse 0m 9s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 34s the patch passed +1 javadoc 0m 18s the patch passed +1 unit 0m 56s hadoop-hdfs-client in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 18m 32s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10690 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12830246/HDFS-10690.005.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux d6905057204e 3.13.0-92-generic #139-Ubuntu SMP Tue Jun 28 20:42:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 5707f88 Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16861/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16861/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          xyao Xiaoyu Yao added a comment -

          Fenghua Hu, thanks for updating the patch. The patch v05 looks good to me overall. Nice catch for the NoSuchElementException instead null returned from LinkedMap#firstKey(). Two minor issues:
          1. Can we wrap this NoSuchElementException handling below into a helper function that returns null when NoSuchElementException is thrown from firstKey() call?

          2. Do we need if (eldestKey == null) after the try/catch? I think it is needed only if we wrap the try/catch with a helper function, which returns null when the list is empty.

                  Object eldestKey;
          897	        try {
          898	          eldestKey = evictableMmapped.firstKey();
          899	        } catch (NoSuchElementException e) {
          900	          break;
          901	        }
          902	        if (eldestKey == null) {
          903	          break;
          904	        }
          
          Show
          xyao Xiaoyu Yao added a comment - Fenghua Hu , thanks for updating the patch. The patch v05 looks good to me overall. Nice catch for the NoSuchElementException instead null returned from LinkedMap#firstKey(). Two minor issues: 1. Can we wrap this NoSuchElementException handling below into a helper function that returns null when NoSuchElementException is thrown from firstKey() call? 2. Do we need if (eldestKey == null) after the try/catch? I think it is needed only if we wrap the try/catch with a helper function, which returns null when the list is empty. Object eldestKey; 897 try { 898 eldestKey = evictableMmapped.firstKey(); 899 } catch (NoSuchElementException e) { 900 break ; 901 } 902 if (eldestKey == null ) { 903 break ; 904 }
          Hide
          fenghua_hu Fenghua Hu added a comment -

          Thanks Xiaoyu Yao for your suggestion. How about we just remove "if (eldestKey == null)

          { break; }

          for bulletin 2? If so we won't need an extra helper function for bulletin 1.

          Show
          fenghua_hu Fenghua Hu added a comment - Thanks Xiaoyu Yao for your suggestion. How about we just remove "if (eldestKey == null) { break; } for bulletin 2? If so we won't need an extra helper function for bulletin 1.
          Hide
          fenghua_hu Fenghua Hu added a comment -

          Removed unnessary "if (eldestKey == null)" statement, and updated the patch.

          Show
          fenghua_hu Fenghua Hu added a comment - Removed unnessary "if (eldestKey == null)" statement, and updated the patch.
          Hide
          xyao Xiaoyu Yao added a comment -

          Thanks Fenghua Hu for updating the patch. +1 for patch v6 pending Jenkins.

          Show
          xyao Xiaoyu Yao added a comment - Thanks Fenghua Hu for updating the patch. +1 for patch v6 pending Jenkins.
          Hide
          fenghua_hu Fenghua Hu added a comment -

          looks like patch v6 hasn't been built and verified by Jenkins. Anything else i should do?Xiaoyu Yao

          Show
          fenghua_hu Fenghua Hu added a comment - looks like patch v6 hasn't been built and verified by Jenkins. Anything else i should do? Xiaoyu Yao
          Hide
          fenghua_hu Fenghua Hu added a comment -

          Re-submit patch v6 so as to trigger Jenkins build.

          Show
          fenghua_hu Fenghua Hu added a comment - Re-submit patch v6 so as to trigger Jenkins build.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 19s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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 mvninstall 8m 21s trunk passed
          +1 compile 0m 35s trunk passed
          +1 checkstyle 0m 16s trunk passed
          +1 mvnsite 0m 39s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 1m 40s trunk passed
          +1 javadoc 0m 22s trunk passed
          +1 mvninstall 0m 34s the patch passed
          +1 compile 0m 29s the patch passed
          +1 javac 0m 29s the patch passed
          +1 checkstyle 0m 12s hadoop-hdfs-project/hadoop-hdfs-client: The patch generated 0 new + 12 unchanged - 4 fixed = 12 total (was 16)
          +1 mvnsite 0m 32s the patch passed
          +1 mvneclipse 0m 9s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 32s the patch passed
          +1 javadoc 0m 17s the patch passed
          +1 unit 0m 53s hadoop-hdfs-client in the patch passed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          18m 31s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10690
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12830623/HDFS-10690.006.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 167e21fdd8c5 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 6437ba1
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16898/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16898/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 19s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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 mvninstall 8m 21s trunk passed +1 compile 0m 35s trunk passed +1 checkstyle 0m 16s trunk passed +1 mvnsite 0m 39s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 40s trunk passed +1 javadoc 0m 22s trunk passed +1 mvninstall 0m 34s the patch passed +1 compile 0m 29s the patch passed +1 javac 0m 29s the patch passed +1 checkstyle 0m 12s hadoop-hdfs-project/hadoop-hdfs-client: The patch generated 0 new + 12 unchanged - 4 fixed = 12 total (was 16) +1 mvnsite 0m 32s the patch passed +1 mvneclipse 0m 9s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 32s the patch passed +1 javadoc 0m 17s the patch passed +1 unit 0m 53s hadoop-hdfs-client in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 18m 31s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10690 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12830623/HDFS-10690.006.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 167e21fdd8c5 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 6437ba1 Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16898/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16898/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          fenghua_hu Fenghua Hu added a comment -

          This fix doesn't change any interface, and I have run the unit test for ShortCircuitCache.

          Show
          fenghua_hu Fenghua Hu added a comment - This fix doesn't change any interface, and I have run the unit test for ShortCircuitCache.
          Hide
          xyao Xiaoyu Yao added a comment -

          Fenghua Hu: the patch v06 looks good. I plan to commit it by EOD tomorrow.

          Show
          xyao Xiaoyu Yao added a comment - Fenghua Hu : the patch v06 looks good. I plan to commit it by EOD tomorrow.
          Hide
          fenghua_hu Fenghua Hu added a comment -

          Xiaoyu Yao, thanks for the help!

          Show
          fenghua_hu Fenghua Hu added a comment - Xiaoyu Yao , thanks for the help!
          Hide
          stack stack added a comment -

          Skimmed. Patch LGTM. Unfortunate we leave behind some perf but agree on avoiding custom data structure unless large benefit. Nice work.

          Show
          stack stack added a comment - Skimmed. Patch LGTM. Unfortunate we leave behind some perf but agree on avoiding custom data structure unless large benefit. Nice work.
          Hide
          fenghua_hu Fenghua Hu added a comment -

          stack thanks for reviewing the patch!

          Show
          fenghua_hu Fenghua Hu added a comment - stack thanks for reviewing the patch!
          Hide
          xyao Xiaoyu Yao added a comment - - edited

          Fenghua Hu, we will need to update the CacheVisistor instances in TestEnhancedByteBufferAccess.java and TestShortCircuitCache.java now that the CacheVisitor interface has been changed from using Map to LinkedMap. Otherwise, it will fail with compiler error like below.

          Not sure why Jenkins run did not catch this, but will file separate infra JIRA for that.

          [ERROR] /hadoop/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestEnhancedByteBufferAccess.java:[291,18] org.apache.hadoop.fs.TestEnhancedByteBufferAccess.CountingVisitor is not abstract and does not override abstract method visit(int,java.util.Map<org.apache.hadoop.hdfs.ExtendedBlockId,org.apache.hadoop.hdfs.shortcircuit.ShortCircuitReplica>,java.util.Map<org.apache.hadoop.hdfs.ExtendedBlockId,org.apache.hadoop.security.token.SecretManager.InvalidToken>,org.apache.commons.collections.map.LinkedMap,org.apache.commons.collections.map.LinkedMap) in org.apache.hadoop.hdfs.shortcircuit.ShortCircuitCache.CacheVisitor
          [ERROR] /hadoop/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestEnhancedByteBufferAccess.java:[306,5] method does not override or implement a method from a supertype
          
          Show
          xyao Xiaoyu Yao added a comment - - edited Fenghua Hu , we will need to update the CacheVisistor instances in TestEnhancedByteBufferAccess.java and TestShortCircuitCache.java now that the CacheVisitor interface has been changed from using Map to LinkedMap. Otherwise, it will fail with compiler error like below. Not sure why Jenkins run did not catch this, but will file separate infra JIRA for that. [ERROR] /hadoop/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestEnhancedByteBufferAccess.java:[291,18] org.apache.hadoop.fs.TestEnhancedByteBufferAccess.CountingVisitor is not abstract and does not override abstract method visit( int ,java.util.Map<org.apache.hadoop.hdfs.ExtendedBlockId,org.apache.hadoop.hdfs.shortcircuit.ShortCircuitReplica>,java.util.Map<org.apache.hadoop.hdfs.ExtendedBlockId,org.apache.hadoop.security.token.SecretManager.InvalidToken>,org.apache.commons.collections.map.LinkedMap,org.apache.commons.collections.map.LinkedMap) in org.apache.hadoop.hdfs.shortcircuit.ShortCircuitCache.CacheVisitor [ERROR] /hadoop/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestEnhancedByteBufferAccess.java:[306,5] method does not override or implement a method from a supertype
          Hide
          fenghua_hu Fenghua Hu added a comment -

          Hi Xiaoyu Yao, I should had run a clean compilation, sorry for your inconvenience. I will update the patch soon.

          Show
          fenghua_hu Fenghua Hu added a comment - Hi Xiaoyu Yao , I should had run a clean compilation, sorry for your inconvenience. I will update the patch soon.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 13s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 26s Maven dependency ordering for branch
          +1 mvninstall 7m 2s trunk passed
          +1 compile 1m 26s trunk passed
          +1 checkstyle 0m 31s trunk passed
          +1 mvnsite 1m 28s trunk passed
          +1 mvneclipse 0m 25s trunk passed
          +1 findbugs 3m 18s trunk passed
          +1 javadoc 1m 21s trunk passed
          0 mvndep 0m 7s Maven dependency ordering for patch
          -1 mvninstall 0m 45s hadoop-hdfs in the patch failed.
          -1 compile 1m 6s hadoop-hdfs-project in the patch failed.
          -1 javac 1m 6s hadoop-hdfs-project in the patch failed.
          +1 checkstyle 0m 29s hadoop-hdfs-project: The patch generated 0 new + 90 unchanged - 4 fixed = 90 total (was 94)
          -1 mvnsite 0m 44s hadoop-hdfs in the patch failed.
          +1 mvneclipse 0m 20s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          -1 findbugs 0m 21s hadoop-hdfs in the patch failed.
          +1 javadoc 1m 12s the patch passed
          +1 unit 0m 58s hadoop-hdfs-client in the patch passed.
          -1 unit 0m 47s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 18s The patch does not generate ASF License warnings.
          27m 21s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10690
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831205/HDFS-10690.007.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 9c586e03e52d 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 89bd6d2
          Default Java 1.8.0_101
          findbugs v3.0.0
          mvninstall https://builds.apache.org/job/PreCommit-HDFS-Build/16964/artifact/patchprocess/patch-mvninstall-hadoop-hdfs-project_hadoop-hdfs.txt
          compile https://builds.apache.org/job/PreCommit-HDFS-Build/16964/artifact/patchprocess/patch-compile-hadoop-hdfs-project.txt
          javac https://builds.apache.org/job/PreCommit-HDFS-Build/16964/artifact/patchprocess/patch-compile-hadoop-hdfs-project.txt
          mvnsite https://builds.apache.org/job/PreCommit-HDFS-Build/16964/artifact/patchprocess/patch-mvnsite-hadoop-hdfs-project_hadoop-hdfs.txt
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16964/artifact/patchprocess/patch-findbugs-hadoop-hdfs-project_hadoop-hdfs.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16964/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16964/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16964/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 13s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 26s Maven dependency ordering for branch +1 mvninstall 7m 2s trunk passed +1 compile 1m 26s trunk passed +1 checkstyle 0m 31s trunk passed +1 mvnsite 1m 28s trunk passed +1 mvneclipse 0m 25s trunk passed +1 findbugs 3m 18s trunk passed +1 javadoc 1m 21s trunk passed 0 mvndep 0m 7s Maven dependency ordering for patch -1 mvninstall 0m 45s hadoop-hdfs in the patch failed. -1 compile 1m 6s hadoop-hdfs-project in the patch failed. -1 javac 1m 6s hadoop-hdfs-project in the patch failed. +1 checkstyle 0m 29s hadoop-hdfs-project: The patch generated 0 new + 90 unchanged - 4 fixed = 90 total (was 94) -1 mvnsite 0m 44s hadoop-hdfs in the patch failed. +1 mvneclipse 0m 20s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 0m 21s hadoop-hdfs in the patch failed. +1 javadoc 1m 12s the patch passed +1 unit 0m 58s hadoop-hdfs-client in the patch passed. -1 unit 0m 47s hadoop-hdfs in the patch failed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 27m 21s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10690 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831205/HDFS-10690.007.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 9c586e03e52d 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 89bd6d2 Default Java 1.8.0_101 findbugs v3.0.0 mvninstall https://builds.apache.org/job/PreCommit-HDFS-Build/16964/artifact/patchprocess/patch-mvninstall-hadoop-hdfs-project_hadoop-hdfs.txt compile https://builds.apache.org/job/PreCommit-HDFS-Build/16964/artifact/patchprocess/patch-compile-hadoop-hdfs-project.txt javac https://builds.apache.org/job/PreCommit-HDFS-Build/16964/artifact/patchprocess/patch-compile-hadoop-hdfs-project.txt mvnsite https://builds.apache.org/job/PreCommit-HDFS-Build/16964/artifact/patchprocess/patch-mvnsite-hadoop-hdfs-project_hadoop-hdfs.txt findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16964/artifact/patchprocess/patch-findbugs-hadoop-hdfs-project_hadoop-hdfs.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/16964/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16964/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16964/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          fenghua_hu Fenghua Hu added a comment -

          Fixed unit test issue.

          Show
          fenghua_hu Fenghua Hu added a comment - Fixed unit test issue.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 19s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 0m 31s Maven dependency ordering for branch
          +1 mvninstall 7m 7s trunk passed
          +1 compile 1m 22s trunk passed
          +1 checkstyle 0m 33s trunk passed
          +1 mvnsite 1m 23s trunk passed
          +1 mvneclipse 0m 24s trunk passed
          +1 findbugs 3m 4s trunk passed
          +1 javadoc 1m 15s trunk passed
          0 mvndep 0m 6s Maven dependency ordering for patch
          +1 mvninstall 1m 16s the patch passed
          +1 compile 1m 21s the patch passed
          +1 javac 1m 21s the patch passed
          +1 checkstyle 0m 29s hadoop-hdfs-project: The patch generated 0 new + 188 unchanged - 4 fixed = 188 total (was 192)
          +1 mvnsite 1m 20s the patch passed
          +1 mvneclipse 0m 19s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 14s the patch passed
          +1 javadoc 1m 11s the patch passed
          +1 unit 0m 52s hadoop-hdfs-client in the patch passed.
          -1 unit 59m 53s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 18s The patch does not generate ASF License warnings.
          87m 39s



          Reason Tests
          Failed junit tests hadoop.fs.viewfs.TestViewFsHdfs
            hadoop.hdfs.server.blockmanagement.TestPendingInvalidateBlock



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10690
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831219/HDFS-10690.008.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux b66539b4b006 3.13.0-92-generic #139-Ubuntu SMP Tue Jun 28 20:42:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / fe9ebe2
          Default Java 1.8.0_101
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16967/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16967/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16967/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 19s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 31s Maven dependency ordering for branch +1 mvninstall 7m 7s trunk passed +1 compile 1m 22s trunk passed +1 checkstyle 0m 33s trunk passed +1 mvnsite 1m 23s trunk passed +1 mvneclipse 0m 24s trunk passed +1 findbugs 3m 4s trunk passed +1 javadoc 1m 15s trunk passed 0 mvndep 0m 6s Maven dependency ordering for patch +1 mvninstall 1m 16s the patch passed +1 compile 1m 21s the patch passed +1 javac 1m 21s the patch passed +1 checkstyle 0m 29s hadoop-hdfs-project: The patch generated 0 new + 188 unchanged - 4 fixed = 188 total (was 192) +1 mvnsite 1m 20s the patch passed +1 mvneclipse 0m 19s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 14s the patch passed +1 javadoc 1m 11s the patch passed +1 unit 0m 52s hadoop-hdfs-client in the patch passed. -1 unit 59m 53s hadoop-hdfs in the patch failed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 87m 39s Reason Tests Failed junit tests hadoop.fs.viewfs.TestViewFsHdfs   hadoop.hdfs.server.blockmanagement.TestPendingInvalidateBlock Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10690 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831219/HDFS-10690.008.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux b66539b4b006 3.13.0-92-generic #139-Ubuntu SMP Tue Jun 28 20:42:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / fe9ebe2 Default Java 1.8.0_101 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/16967/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16967/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16967/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 14s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 0m 29s Maven dependency ordering for branch
          +1 mvninstall 6m 56s trunk passed
          +1 compile 1m 23s trunk passed
          +1 checkstyle 0m 32s trunk passed
          +1 mvnsite 1m 23s trunk passed
          +1 mvneclipse 0m 25s trunk passed
          +1 findbugs 3m 5s trunk passed
          +1 javadoc 1m 15s trunk passed
          0 mvndep 0m 6s Maven dependency ordering for patch
          +1 mvninstall 1m 16s the patch passed
          +1 compile 1m 20s the patch passed
          +1 javac 1m 20s the patch passed
          +1 checkstyle 0m 30s hadoop-hdfs-project: The patch generated 0 new + 187 unchanged - 4 fixed = 187 total (was 191)
          +1 mvnsite 1m 19s the patch passed
          +1 mvneclipse 0m 19s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 14s the patch passed
          +1 javadoc 1m 10s the patch passed
          +1 unit 0m 53s hadoop-hdfs-client in the patch passed.
          -1 unit 67m 33s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          95m 2s



          Reason Tests
          Failed junit tests hadoop.hdfs.TestDFSShell
          Timed out junit tests org.apache.hadoop.hdfs.TestLeaseRecovery2



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10690
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831252/HDFS-10690.008.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 5b89cdb9bf4f 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / fe9ebe2
          Default Java 1.8.0_101
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16970/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16970/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16970/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 14s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 29s Maven dependency ordering for branch +1 mvninstall 6m 56s trunk passed +1 compile 1m 23s trunk passed +1 checkstyle 0m 32s trunk passed +1 mvnsite 1m 23s trunk passed +1 mvneclipse 0m 25s trunk passed +1 findbugs 3m 5s trunk passed +1 javadoc 1m 15s trunk passed 0 mvndep 0m 6s Maven dependency ordering for patch +1 mvninstall 1m 16s the patch passed +1 compile 1m 20s the patch passed +1 javac 1m 20s the patch passed +1 checkstyle 0m 30s hadoop-hdfs-project: The patch generated 0 new + 187 unchanged - 4 fixed = 187 total (was 191) +1 mvnsite 1m 19s the patch passed +1 mvneclipse 0m 19s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 14s the patch passed +1 javadoc 1m 10s the patch passed +1 unit 0m 53s hadoop-hdfs-client in the patch passed. -1 unit 67m 33s hadoop-hdfs in the patch failed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 95m 2s Reason Tests Failed junit tests hadoop.hdfs.TestDFSShell Timed out junit tests org.apache.hadoop.hdfs.TestLeaseRecovery2 Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10690 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831252/HDFS-10690.008.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 5b89cdb9bf4f 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / fe9ebe2 Default Java 1.8.0_101 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/16970/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16970/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16970/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          fenghua_hu Fenghua Hu added a comment - - edited

          Failed cases have nothing to do with the patch, actually they passed on my test bed. Xiaoyu Yao

          Show
          fenghua_hu Fenghua Hu added a comment - - edited Failed cases have nothing to do with the patch, actually they passed on my test bed. Xiaoyu Yao
          Hide
          xyao Xiaoyu Yao added a comment -

          Thanks Fenghua Hu for the contribution and all the reviewers for the discussion. I've commit the patch to trunk, branch-2 and branch-2.8.

          Show
          xyao Xiaoyu Yao added a comment - Thanks Fenghua Hu for the contribution and all the reviewers for the discussion. I've commit the patch to trunk, branch-2 and branch-2.8.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10533 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10533/)
          HDFS-10690. Optimize insertion/removal of replica in ShortCircuitCache. (xyao: rev 607705c488fa5263d851cee578a2d319e6e52ecd)

          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/shortcircuit/TestShortCircuitCache.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestEnhancedByteBufferAccess.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/shortcircuit/ShortCircuitCache.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10533 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10533/ ) HDFS-10690 . Optimize insertion/removal of replica in ShortCircuitCache. (xyao: rev 607705c488fa5263d851cee578a2d319e6e52ecd) (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/shortcircuit/TestShortCircuitCache.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestEnhancedByteBufferAccess.java (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/shortcircuit/ShortCircuitCache.java
          Hide
          fenghua_hu Fenghua Hu added a comment -

          Xiaoyu Yao, thank you for the great help!

          Show
          fenghua_hu Fenghua Hu added a comment - Xiaoyu Yao , thank you for the great help!

            People

            • Assignee:
              fenghua_hu Fenghua Hu
              Reporter:
              fenghua_hu Fenghua Hu
            • Votes:
              0 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 336h
                336h
                Remaining:
                Remaining Estimate - 336h
                336h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development