Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: conf, io, ipc
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      quote Daryn Sharp from HADOOP-11772:

      CacheBuilder is obscenely expensive for concurrent map, and it requires generating unnecessary garbage even just to look up a key. Replace it with ConcurrentHashMap.

      I identified this issue that impaired my own perf testing under load. The slowdown isn't just the sync. It's the expensive of Connection's ctor stalling other connections. The expensive of ConnectionId#equals causes delays. Synch'ing on connections causes unfair contention unlike a sync'ed method. Concurrency simply hides this.

      BTW, guava Cache is heavyweight. Per local test, ConcurrentHashMap has better overal performance.

      1. HADOOP-12475.01.patch
        5 kB
        Walter Su
      2. HADOOP-12475.02.patch
        5 kB
        Walter Su
      3. HADOOP-12475.03.patch
        5 kB
        Walter Su

        Activity

        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 18m 8s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 tests included 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 javac 8m 6s There were no new javac warning messages.
        +1 javadoc 10m 36s There were no new javadoc warning messages.
        +1 release audit 0m 25s The applied patch does not increase the total number of release audit warnings.
        -1 checkstyle 1m 9s The applied patch generated 1 new checkstyle issues (total was 100, now 101).
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 30s mvn install still works.
        +1 eclipse:eclipse 0m 34s The patch built with eclipse:eclipse.
        -1 findbugs 1m 57s The patch appears to introduce 1 new Findbugs (version 3.0.0) warnings.
        -1 common tests 6m 50s Tests failed in hadoop-common.
            49m 19s  



        Reason Tests
        FindBugs module:hadoop-common
        Failed unit tests hadoop.ipc.TestRPC
          hadoop.metrics2.impl.TestGangliaMetrics
          hadoop.net.TestDNS



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12766476/HADOOP-12475.01.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / 2a98724
        checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/7810/artifact/patchprocess/diffcheckstylehadoop-common.txt
        Findbugs warnings https://builds.apache.org/job/PreCommit-HADOOP-Build/7810/artifact/patchprocess/newPatchFindbugsWarningshadoop-common.html
        hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7810/artifact/patchprocess/testrun_hadoop-common.txt
        Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7810/testReport/
        Java 1.7.0_55
        uname Linux asf900.gq1.ygridcore.net 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
        Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7810/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 18m 8s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 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 javac 8m 6s There were no new javac warning messages. +1 javadoc 10m 36s There were no new javadoc warning messages. +1 release audit 0m 25s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 1m 9s The applied patch generated 1 new checkstyle issues (total was 100, now 101). +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 30s mvn install still works. +1 eclipse:eclipse 0m 34s The patch built with eclipse:eclipse. -1 findbugs 1m 57s The patch appears to introduce 1 new Findbugs (version 3.0.0) warnings. -1 common tests 6m 50s Tests failed in hadoop-common.     49m 19s   Reason Tests FindBugs module:hadoop-common Failed unit tests hadoop.ipc.TestRPC   hadoop.metrics2.impl.TestGangliaMetrics   hadoop.net.TestDNS Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12766476/HADOOP-12475.01.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 2a98724 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/7810/artifact/patchprocess/diffcheckstylehadoop-common.txt Findbugs warnings https://builds.apache.org/job/PreCommit-HADOOP-Build/7810/artifact/patchprocess/newPatchFindbugsWarningshadoop-common.html hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7810/artifact/patchprocess/testrun_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7810/testReport/ Java 1.7.0_55 uname Linux asf900.gq1.ygridcore.net 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 Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7810/console This message was automatically generated.
        Hide
        stevel@apache.org Steve Loughran added a comment -

        TestDNS is probably a regression from a patch that went in last night (filed HADOOP-12476). Resubmitting

        Show
        stevel@apache.org Steve Loughran added a comment - TestDNS is probably a regression from a patch that went in last night (filed HADOOP-12476 ). Resubmitting
        Hide
        Apache9 Duo Zhang added a comment -
        Client.java
        1483	      connection = connections.get(remoteId);
        1484	      if (connection == null) {
        1485	        connection = new Connection(remoteId, serviceClass);
        1486	        connections.put(remoteId, connection);
        1487	      }
        

        I think the behavior here is not same with original code? The guava cache will prevent creating multiple connections for the same remoteId, but the CHM version will not and I think there will be connection leak...

        Show
        Apache9 Duo Zhang added a comment - Client.java 1483 connection = connections.get(remoteId); 1484 if (connection == null ) { 1485 connection = new Connection(remoteId, serviceClass); 1486 connections.put(remoteId, connection); 1487 } I think the behavior here is not same with original code? The guava cache will prevent creating multiple connections for the same remoteId, but the CHM version will not and I think there will be connection leak...
        Hide
        sjlee0 Sangjin Lee added a comment -

        +1 on Duo Zhang's comment. A more correct way of using the ConcurrentHashMap is to utilize its putIfAbsent() method:

        connection = connections.get(remoteId);
        if (connection == null) {
          connection = new Connection(remoteId, serviceClass);
          Connection existing = connections.putIfAbsent(remoteId, connection);
          if (existing != null) {
            // ditch the one you just created as there is already a connection
            connection.close();
            connection = existing;
          }
        }
        
        Show
        sjlee0 Sangjin Lee added a comment - +1 on Duo Zhang 's comment. A more correct way of using the ConcurrentHashMap is to utilize its putIfAbsent() method: connection = connections.get(remoteId); if (connection == null ) { connection = new Connection(remoteId, serviceClass); Connection existing = connections.putIfAbsent(remoteId, connection); if (existing != null ) { // ditch the one you just created as there is already a connection connection.close(); connection = existing; } }
        Hide
        walter.k.su Walter Su added a comment -

        1. Connection leak means an opened connection which forgets to close. It doesn't happen here. Connection is opened at setupIOstreams. And it will be closed even it's not in the cache.

        2. It's true a race condition can cause creating multiple connections for the same remoteId. I replace it with putIfAbsent. Thanks Duo Zhang, Sangjin Lee

        3. There is another race condition about removing. It's caused by HADOOP-11772.
        Before HADOOP-11772, Client used HashTable for caching. The removing logic is

        1175	      synchronized (connections) {		
        1176	        if (connections.get(remoteId) == this) {		
        1177	          connections.remove(remoteId);		
        1178	        }		
        1179	      }
        

        It bothers me a while why a thread-safe HashTable need synchronized. Then I know this connection is closed, should be removed. But other thread could have already known this closedConnection, and replace it with a new connection.

        I use conditional remove ConcurrentHashMap.remove(key,value) to address this.

        I think before HADOOP-11772, synchronized (connections) (which locks the whole table and HashTable also locks whole table) is the reason why the invokers choke up.

        Uploaded 02 patch.

        Show
        walter.k.su Walter Su added a comment - 1. Connection leak means an opened connection which forgets to close. It doesn't happen here. Connection is opened at setupIOstreams . And it will be closed even it's not in the cache. 2. It's true a race condition can cause creating multiple connections for the same remoteId. I replace it with putIfAbsent . Thanks Duo Zhang , Sangjin Lee 3. There is another race condition about removing. It's caused by HADOOP-11772 . Before HADOOP-11772 , Client used HashTable for caching. The removing logic is 1175 synchronized (connections) { 1176 if (connections.get(remoteId) == this ) { 1177 connections.remove(remoteId); 1178 } 1179 } It bothers me a while why a thread-safe HashTable need synchronized . Then I know this connection is closed, should be removed. But other thread could have already known this closedConnection, and replace it with a new connection. I use conditional remove ConcurrentHashMap.remove(key,value) to address this. I think before HADOOP-11772 , synchronized (connections) (which locks the whole table and HashTable also locks whole table) is the reason why the invokers choke up. Uploaded 02 patch.
        Hide
        Apache9 Duo Zhang added a comment -

        The new patch is OK for me.

        And in java8, there is a computeIfAbsent method in ConcurrentHashMap which does the same thing with guava cache. I think we could add a comment here like 'use computeIfAbsent instead if we decide to drop support of java7'?

        Thanks.

        Show
        Apache9 Duo Zhang added a comment - The new patch is OK for me. And in java8, there is a computeIfAbsent method in ConcurrentHashMap which does the same thing with guava cache. I think we could add a comment here like 'use computeIfAbsent instead if we decide to drop support of java7'? Thanks.
        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 23m 13s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 tests included 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 javac 9m 51s There were no new javac warning messages.
        +1 javadoc 10m 44s There were no new javadoc warning messages.
        -1 release audit 0m 20s The applied patch generated 1 release audit warnings.
        -1 checkstyle 1m 22s The applied patch generated 1 new checkstyle issues (total was 100, now 101).
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 53s mvn install still works.
        +1 eclipse:eclipse 0m 35s The patch built with eclipse:eclipse.
        +1 findbugs 1m 55s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
        -1 common tests 6m 38s Tests failed in hadoop-common.
            56m 38s  



        Reason Tests
        Failed unit tests hadoop.net.TestDNS



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12766710/HADOOP-12475.02.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / c80b3a8
        Release Audit https://builds.apache.org/job/PreCommit-HADOOP-Build/7816/artifact/patchprocess/patchReleaseAuditProblems.txt
        checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/7816/artifact/patchprocess/diffcheckstylehadoop-common.txt
        hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7816/artifact/patchprocess/testrun_hadoop-common.txt
        Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7816/testReport/
        Java 1.7.0_55
        uname Linux asf900.gq1.ygridcore.net 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
        Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7816/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 23m 13s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 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 javac 9m 51s There were no new javac warning messages. +1 javadoc 10m 44s There were no new javadoc warning messages. -1 release audit 0m 20s The applied patch generated 1 release audit warnings. -1 checkstyle 1m 22s The applied patch generated 1 new checkstyle issues (total was 100, now 101). +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 53s mvn install still works. +1 eclipse:eclipse 0m 35s The patch built with eclipse:eclipse. +1 findbugs 1m 55s The patch does not introduce any new Findbugs (version 3.0.0) warnings. -1 common tests 6m 38s Tests failed in hadoop-common.     56m 38s   Reason Tests Failed unit tests hadoop.net.TestDNS Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12766710/HADOOP-12475.02.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / c80b3a8 Release Audit https://builds.apache.org/job/PreCommit-HADOOP-Build/7816/artifact/patchprocess/patchReleaseAuditProblems.txt checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/7816/artifact/patchprocess/diffcheckstylehadoop-common.txt hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7816/artifact/patchprocess/testrun_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7816/testReport/ Java 1.7.0_55 uname Linux asf900.gq1.ygridcore.net 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 Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7816/console This message was automatically generated.
        Hide
        sjlee0 Sangjin Lee added a comment -

        The latest patch looks good for the most part.

        A couple of nits: IMO it's generally a good practice to declare types with interfaces rather than concrete implementations:

        129	  private ConcurrentHashMap<ConnectionId, Connection> connections =
        130	    new ConcurrentHashMap<>();
        

        I would use ConcurrentMap instead of ConcurrentHashMap as the type. Also line 130 has the checkstyle violation. It'd be good to fix it too.

        Show
        sjlee0 Sangjin Lee added a comment - The latest patch looks good for the most part. A couple of nits: IMO it's generally a good practice to declare types with interfaces rather than concrete implementations: 129 private ConcurrentHashMap<ConnectionId, Connection> connections = 130 new ConcurrentHashMap<>(); I would use ConcurrentMap instead of ConcurrentHashMap as the type. Also line 130 has the checkstyle violation. It'd be good to fix it too.
        Hide
        walter.k.su Walter Su added a comment -

        And in java8, there is a computeIfAbsent method in ConcurrentHashMap which does the same thing with guava cache. I think we could add a comment here like 'use computeIfAbsent instead if we decide to drop support of java7'?

        Done. I didn't know it. Thanks! It is much cleaner.

        I would use ConcurrentMap instead of ConcurrentHashMap as the type. Also line 130 has the checkstyle violation. It'd be good to fix it too.

        Done.

        Thanks again, Duo Zhang, Sangjin Lee! And TestDNS failed recently and not related. Uploaded 03 patch.

        Show
        walter.k.su Walter Su added a comment - And in java8, there is a computeIfAbsent method in ConcurrentHashMap which does the same thing with guava cache. I think we could add a comment here like 'use computeIfAbsent instead if we decide to drop support of java7'? Done. I didn't know it. Thanks! It is much cleaner. I would use ConcurrentMap instead of ConcurrentHashMap as the type. Also line 130 has the checkstyle violation. It'd be good to fix it too. Done. Thanks again, Duo Zhang , Sangjin Lee ! And TestDNS failed recently and not related. Uploaded 03 patch.
        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 19m 20s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 tests included 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 javac 8m 33s There were no new javac warning messages.
        +1 javadoc 11m 26s There were no new javadoc warning messages.
        -1 release audit 0m 21s The applied patch generated 1 release audit warnings.
        +1 checkstyle 1m 10s There were no new checkstyle issues.
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 42s mvn install still works.
        +1 eclipse:eclipse 0m 38s The patch built with eclipse:eclipse.
        +1 findbugs 2m 1s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
        +1 common tests 6m 52s Tests passed in hadoop-common.
            52m 8s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12766732/HADOOP-12475.03.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / c80b3a8
        Release Audit https://builds.apache.org/job/PreCommit-HADOOP-Build/7818/artifact/patchprocess/patchReleaseAuditProblems.txt
        hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7818/artifact/patchprocess/testrun_hadoop-common.txt
        Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7818/testReport/
        Java 1.7.0_55
        uname Linux asf902.gq1.ygridcore.net 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
        Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7818/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 19m 20s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 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 javac 8m 33s There were no new javac warning messages. +1 javadoc 11m 26s There were no new javadoc warning messages. -1 release audit 0m 21s The applied patch generated 1 release audit warnings. +1 checkstyle 1m 10s There were no new checkstyle issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 42s mvn install still works. +1 eclipse:eclipse 0m 38s The patch built with eclipse:eclipse. +1 findbugs 2m 1s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 common tests 6m 52s Tests passed in hadoop-common.     52m 8s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12766732/HADOOP-12475.03.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / c80b3a8 Release Audit https://builds.apache.org/job/PreCommit-HADOOP-Build/7818/artifact/patchprocess/patchReleaseAuditProblems.txt hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7818/artifact/patchprocess/testrun_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7818/testReport/ Java 1.7.0_55 uname Linux asf902.gq1.ygridcore.net 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 Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7818/console This message was automatically generated.
        Hide
        sjlee0 Sangjin Lee added a comment -

        The latest patch LGTM. Committing it shortly (unless there is an objection).

        Show
        sjlee0 Sangjin Lee added a comment - The latest patch LGTM. Committing it shortly (unless there is an objection).
        Hide
        sjlee0 Sangjin Lee added a comment -

        Committed to trunk and branch-2. Thanks Walter Su for your contribution! Thanks Duo Zhang for your review.

        Show
        sjlee0 Sangjin Lee added a comment - Committed to trunk and branch-2. Thanks Walter Su for your contribution! Thanks Duo Zhang for your review.
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-trunk-Commit #8647 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8647/)
        HADOOP-12475. Replace guava Cache with ConcurrentHashMap for caching (sjlee: rev 8d2d3eb7bb938cc06ea3cc74040cfe0be13a2ba8)

        • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
        • hadoop-common-project/hadoop-common/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #8647 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8647/ ) HADOOP-12475 . Replace guava Cache with ConcurrentHashMap for caching (sjlee: rev 8d2d3eb7bb938cc06ea3cc74040cfe0be13a2ba8) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java hadoop-common-project/hadoop-common/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk #1276 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1276/)
        HADOOP-12475. Replace guava Cache with ConcurrentHashMap for caching (sjlee: rev 8d2d3eb7bb938cc06ea3cc74040cfe0be13a2ba8)

        • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
        • hadoop-common-project/hadoop-common/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #1276 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1276/ ) HADOOP-12475 . Replace guava Cache with ConcurrentHashMap for caching (sjlee: rev 8d2d3eb7bb938cc06ea3cc74040cfe0be13a2ba8) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java hadoop-common-project/hadoop-common/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #553 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/553/)
        HADOOP-12475. Replace guava Cache with ConcurrentHashMap for caching (sjlee: rev 8d2d3eb7bb938cc06ea3cc74040cfe0be13a2ba8)

        • hadoop-common-project/hadoop-common/CHANGES.txt
        • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #553 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/553/ ) HADOOP-12475 . Replace guava Cache with ConcurrentHashMap for caching (sjlee: rev 8d2d3eb7bb938cc06ea3cc74040cfe0be13a2ba8) hadoop-common-project/hadoop-common/CHANGES.txt hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk #2488 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2488/)
        HADOOP-12475. Replace guava Cache with ConcurrentHashMap for caching (sjlee: rev 8d2d3eb7bb938cc06ea3cc74040cfe0be13a2ba8)

        • hadoop-common-project/hadoop-common/CHANGES.txt
        • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2488 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2488/ ) HADOOP-12475 . Replace guava Cache with ConcurrentHashMap for caching (sjlee: rev 8d2d3eb7bb938cc06ea3cc74040cfe0be13a2ba8) hadoop-common-project/hadoop-common/CHANGES.txt hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #539 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/539/)
        HADOOP-12475. Replace guava Cache with ConcurrentHashMap for caching (sjlee: rev 8d2d3eb7bb938cc06ea3cc74040cfe0be13a2ba8)

        • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
        • hadoop-common-project/hadoop-common/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #539 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/539/ ) HADOOP-12475 . Replace guava Cache with ConcurrentHashMap for caching (sjlee: rev 8d2d3eb7bb938cc06ea3cc74040cfe0be13a2ba8) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java hadoop-common-project/hadoop-common/CHANGES.txt
        Hide
        cnauroth Chris Nauroth added a comment -

        Just FYI, I have committed a correction to CHANGES.txt on trunk. It had HADOOP-12475 listed in the trunk section, and I moved it to the 2.8.0 section. This was not a problem on branch-2, where it was already listed under 2.8.0.

        Show
        cnauroth Chris Nauroth added a comment - Just FYI, I have committed a correction to CHANGES.txt on trunk. It had HADOOP-12475 listed in the trunk section, and I moved it to the 2.8.0 section. This was not a problem on branch-2, where it was already listed under 2.8.0.
        Hide
        sjlee0 Sangjin Lee added a comment -

        Thanks Chris Nauroth!

        I'm actually somewhat confused by this. When we commit a patch onto trunk, do we add the attribution to the trunk section or the latest unreleased major release (in this case 2.8.0)? I see a whole bunch of JIRAs in both sections, and I assumed that I would add it to the trunk section for trunk and 2.8.0 for branch-2. The commit guide doesn't seem to mention it clearly either...

        Show
        sjlee0 Sangjin Lee added a comment - Thanks Chris Nauroth ! I'm actually somewhat confused by this. When we commit a patch onto trunk, do we add the attribution to the trunk section or the latest unreleased major release (in this case 2.8.0)? I see a whole bunch of JIRAs in both sections, and I assumed that I would add it to the trunk section for trunk and 2.8.0 for branch-2. The commit guide doesn't seem to mention it clearly either...
        Hide
        cnauroth Chris Nauroth added a comment -

        Sangjin Lee, don't worry. It's not just you. Hence all of the discussions about getting rid of CHANGES.txt and automating it instead.

        Attribution in CHANGES.txt should fall under the earliest release that is receiving the patch, and it should be consistent across all live branches. If the patch is targeted to 2.8.0, then its CHANGES.txt entry would go in the 2.8.0 section on trunk and branch-2. If the patch is targeted to 2.7.2, then its CHANGES.txt entry would go in the 2.7.2 section on trunk, branch-2 and branch-2.7.

        Another interesting case is when a patch gets selected for backporting to a maintenance line. Suppose a patch initially targets 2.8.0, but then later becomes a candidate for 2.7.2. On the initial commit, it would have been listed under the 2.8.0 section on trunk and branch-2. After the decision to backport to 2.7.2, we'd have to go back and update CHANGES.txt on all branches to match reality, so we'd have to move it to the 2.7.2 section on trunk, branch-2 and branch-2.7.

        I hope this helps.

        Show
        cnauroth Chris Nauroth added a comment - Sangjin Lee , don't worry. It's not just you. Hence all of the discussions about getting rid of CHANGES.txt and automating it instead. Attribution in CHANGES.txt should fall under the earliest release that is receiving the patch, and it should be consistent across all live branches. If the patch is targeted to 2.8.0, then its CHANGES.txt entry would go in the 2.8.0 section on trunk and branch-2. If the patch is targeted to 2.7.2, then its CHANGES.txt entry would go in the 2.7.2 section on trunk, branch-2 and branch-2.7. Another interesting case is when a patch gets selected for backporting to a maintenance line. Suppose a patch initially targets 2.8.0, but then later becomes a candidate for 2.7.2. On the initial commit, it would have been listed under the 2.8.0 section on trunk and branch-2. After the decision to backport to 2.7.2, we'd have to go back and update CHANGES.txt on all branches to match reality, so we'd have to move it to the 2.7.2 section on trunk, branch-2 and branch-2.7. I hope this helps.
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-trunk-Commit #8648 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8648/)
        HADOOP-12475. Move attribution to 2.8.0 section of CHANGES.txt. (cnauroth: rev 5f3f0e0f707ca94c62955e57c6b94c43a6321ea6)

        • hadoop-common-project/hadoop-common/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #8648 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8648/ ) HADOOP-12475 . Move attribution to 2.8.0 section of CHANGES.txt. (cnauroth: rev 5f3f0e0f707ca94c62955e57c6b94c43a6321ea6) hadoop-common-project/hadoop-common/CHANGES.txt
        Hide
        sjlee0 Sangjin Lee added a comment -

        Thanks much for the detailed explanation! It would be awesome if that is in the how-to-commit wiki page. It seems like I don't have the permission to edit it. I'd greatly appreciate it if you could add that to the wiki page. Thanks again!

        Show
        sjlee0 Sangjin Lee added a comment - Thanks much for the detailed explanation! It would be awesome if that is in the how-to-commit wiki page . It seems like I don't have the permission to edit it. I'd greatly appreciate it if you could add that to the wiki page. Thanks again!
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk #2440 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2440/)
        HADOOP-12475. Replace guava Cache with ConcurrentHashMap for caching (sjlee: rev 8d2d3eb7bb938cc06ea3cc74040cfe0be13a2ba8)

        • hadoop-common-project/hadoop-common/CHANGES.txt
        • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2440 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2440/ ) HADOOP-12475 . Replace guava Cache with ConcurrentHashMap for caching (sjlee: rev 8d2d3eb7bb938cc06ea3cc74040cfe0be13a2ba8) hadoop-common-project/hadoop-common/CHANGES.txt hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #540 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/540/)
        HADOOP-12475. Move attribution to 2.8.0 section of CHANGES.txt. (cnauroth: rev 5f3f0e0f707ca94c62955e57c6b94c43a6321ea6)

        • hadoop-common-project/hadoop-common/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #540 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/540/ ) HADOOP-12475 . Move attribution to 2.8.0 section of CHANGES.txt. (cnauroth: rev 5f3f0e0f707ca94c62955e57c6b94c43a6321ea6) hadoop-common-project/hadoop-common/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #554 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/554/)
        HADOOP-12475. Move attribution to 2.8.0 section of CHANGES.txt. (cnauroth: rev 5f3f0e0f707ca94c62955e57c6b94c43a6321ea6)

        • hadoop-common-project/hadoop-common/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #554 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/554/ ) HADOOP-12475 . Move attribution to 2.8.0 section of CHANGES.txt. (cnauroth: rev 5f3f0e0f707ca94c62955e57c6b94c43a6321ea6) hadoop-common-project/hadoop-common/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #503 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/503/)
        HADOOP-12475. Replace guava Cache with ConcurrentHashMap for caching (sjlee: rev 8d2d3eb7bb938cc06ea3cc74040cfe0be13a2ba8)

        • hadoop-common-project/hadoop-common/CHANGES.txt
        • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #503 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/503/ ) HADOOP-12475 . Replace guava Cache with ConcurrentHashMap for caching (sjlee: rev 8d2d3eb7bb938cc06ea3cc74040cfe0be13a2ba8) hadoop-common-project/hadoop-common/CHANGES.txt hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk #1277 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1277/)
        HADOOP-12475. Move attribution to 2.8.0 section of CHANGES.txt. (cnauroth: rev 5f3f0e0f707ca94c62955e57c6b94c43a6321ea6)

        • hadoop-common-project/hadoop-common/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #1277 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1277/ ) HADOOP-12475 . Move attribution to 2.8.0 section of CHANGES.txt. (cnauroth: rev 5f3f0e0f707ca94c62955e57c6b94c43a6321ea6) hadoop-common-project/hadoop-common/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk #2489 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2489/)
        HADOOP-12475. Move attribution to 2.8.0 section of CHANGES.txt. (cnauroth: rev 5f3f0e0f707ca94c62955e57c6b94c43a6321ea6)

        • hadoop-common-project/hadoop-common/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2489 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2489/ ) HADOOP-12475 . Move attribution to 2.8.0 section of CHANGES.txt. (cnauroth: rev 5f3f0e0f707ca94c62955e57c6b94c43a6321ea6) hadoop-common-project/hadoop-common/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk #2441 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2441/)
        HADOOP-12475. Move attribution to 2.8.0 section of CHANGES.txt. (cnauroth: rev 5f3f0e0f707ca94c62955e57c6b94c43a6321ea6)

        • hadoop-common-project/hadoop-common/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2441 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2441/ ) HADOOP-12475 . Move attribution to 2.8.0 section of CHANGES.txt. (cnauroth: rev 5f3f0e0f707ca94c62955e57c6b94c43a6321ea6) hadoop-common-project/hadoop-common/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #504 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/504/)
        HADOOP-12475. Move attribution to 2.8.0 section of CHANGES.txt. (cnauroth: rev 5f3f0e0f707ca94c62955e57c6b94c43a6321ea6)

        • hadoop-common-project/hadoop-common/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #504 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/504/ ) HADOOP-12475 . Move attribution to 2.8.0 section of CHANGES.txt. (cnauroth: rev 5f3f0e0f707ca94c62955e57c6b94c43a6321ea6) hadoop-common-project/hadoop-common/CHANGES.txt

          People

          • Assignee:
            walter.k.su Walter Su
            Reporter:
            walter.k.su Walter Su
          • Votes:
            0 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development