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

Remove the long deprecated BlockReaderRemote

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.0.0-alpha1
    • Component/s: hdfs
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Incompatible change, Reviewed
    • Release Note:
      This removes the configuration property {{dfs.client.use.legacy.blockreader}}, since the legacy remote block reader class has been removed from the codebase.

      Description

      To lessen the maintain burden like raised in HDFS-8901, suggest we remove BlockReaderRemote class that's deprecated very long time ago.

      From BlockReaderRemote header:

      • @deprecated this is an old implementation that is being left around
      • in case any issues spring up with the new
        Unknown macro: {@link BlockReaderRemote2}
      • implementation.
      • It will be removed in the next release.

      From BlockReaderRemote2 class header:

      • This is a new implementation introduced in Hadoop 0.23 which
      • is more efficient and simpler than the older BlockReader
      • implementation. It should be renamed to BlockReaderRemote
      • once we are confident in it.

      So even further, after getting rid of the old class, we could rename as the comment suggested: BlockReaderRemote2 => BlockReaderRemote.

      1. HDFS-10548-v1.patch
        25 kB
        Kai Zheng
      2. HDFS-10548-v2.patch
        62 kB
        Kai Zheng
      3. HDFS-10548-v3.patch
        62 kB
        Kai Zheng

        Activity

        Hide
        drankye Kai Zheng added a comment -

        Colin P. McCabe, how would you like this if we do it targeting the 3.0 version? Thanks!

        Show
        drankye Kai Zheng added a comment - Colin P. McCabe , how would you like this if we do it targeting the 3.0 version? Thanks!
        Hide
        cmccabe Colin P. McCabe added a comment -

        I would love to see this class go away. It is truly a relic of another time, which lasted much longer than it should. I think the only remaining use case for it is when using sockets that don't have associated channels (SOCKS sockets don't, I think?) We should be able to create adaptors for those, though, assuming anyone even uses SOCKS with the DN any more. Unfortunately I don't have a lot of time to review this at the moment, though.

        Show
        cmccabe Colin P. McCabe added a comment - I would love to see this class go away. It is truly a relic of another time, which lasted much longer than it should. I think the only remaining use case for it is when using sockets that don't have associated channels (SOCKS sockets don't, I think?) We should be able to create adaptors for those, though, assuming anyone even uses SOCKS with the DN any more. Unfortunately I don't have a lot of time to review this at the moment, though.
        Hide
        drankye Kai Zheng added a comment -

        Thanks Colin for the nice comment. I checked the old class again and looks like we can safely remove it. Will upload a patch in case you could review it some time in future.

        Show
        drankye Kai Zheng added a comment - Thanks Colin for the nice comment. I checked the old class again and looks like we can safely remove it. Will upload a patch in case you could review it some time in future.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 23s 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 7s Maven dependency ordering for branch
        +1 mvninstall 6m 22s trunk passed
        +1 compile 1m 30s trunk passed
        +1 checkstyle 0m 33s trunk passed
        +1 mvnsite 1m 34s trunk passed
        +1 mvneclipse 0m 29s trunk passed
        +1 findbugs 3m 10s trunk passed
        +1 javadoc 1m 19s trunk passed
        0 mvndep 0m 6s Maven dependency ordering for patch
        +1 mvninstall 1m 18s the patch passed
        +1 compile 1m 23s the patch passed
        +1 javac 1m 23s the patch passed
        +1 checkstyle 0m 29s hadoop-hdfs-project: The patch generated 0 new + 51 unchanged - 25 fixed = 51 total (was 76)
        +1 mvnsite 1m 21s the patch passed
        +1 mvneclipse 0m 20s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 3m 45s the patch passed
        +1 javadoc 1m 21s the patch passed
        +1 unit 1m 5s hadoop-hdfs-client in the patch passed.
        -1 unit 73m 18s hadoop-hdfs in the patch failed.
        +1 asflicense 0m 22s The patch does not generate ASF License warnings.
        101m 56s



        Reason Tests
        Failed junit tests hadoop.hdfs.server.namenode.TestNameNodeMetadataConsistency
          hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer
          hadoop.hdfs.server.namenode.TestNamenodeRetryCache
          hadoop.hdfs.server.namenode.ha.TestRetryCacheWithHA
          hadoop.hdfs.server.namenode.TestCacheDirectives



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:e2f6409
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12812120/HDFS-10548-v1.patch
        JIRA Issue HDFS-10548
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 1c35c03ea566 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 / 46f1602
        Default Java 1.8.0_91
        findbugs v3.0.0
        unit https://builds.apache.org/job/PreCommit-HDFS-Build/15843/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
        unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15843/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15843/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/15843/console
        Powered by Apache Yetus 0.3.0 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 23s 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 7s Maven dependency ordering for branch +1 mvninstall 6m 22s trunk passed +1 compile 1m 30s trunk passed +1 checkstyle 0m 33s trunk passed +1 mvnsite 1m 34s trunk passed +1 mvneclipse 0m 29s trunk passed +1 findbugs 3m 10s trunk passed +1 javadoc 1m 19s trunk passed 0 mvndep 0m 6s Maven dependency ordering for patch +1 mvninstall 1m 18s the patch passed +1 compile 1m 23s the patch passed +1 javac 1m 23s the patch passed +1 checkstyle 0m 29s hadoop-hdfs-project: The patch generated 0 new + 51 unchanged - 25 fixed = 51 total (was 76) +1 mvnsite 1m 21s the patch passed +1 mvneclipse 0m 20s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 45s the patch passed +1 javadoc 1m 21s the patch passed +1 unit 1m 5s hadoop-hdfs-client in the patch passed. -1 unit 73m 18s hadoop-hdfs in the patch failed. +1 asflicense 0m 22s The patch does not generate ASF License warnings. 101m 56s Reason Tests Failed junit tests hadoop.hdfs.server.namenode.TestNameNodeMetadataConsistency   hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer   hadoop.hdfs.server.namenode.TestNamenodeRetryCache   hadoop.hdfs.server.namenode.ha.TestRetryCacheWithHA   hadoop.hdfs.server.namenode.TestCacheDirectives Subsystem Report/Notes Docker Image:yetus/hadoop:e2f6409 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12812120/HDFS-10548-v1.patch JIRA Issue HDFS-10548 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 1c35c03ea566 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 / 46f1602 Default Java 1.8.0_91 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/15843/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15843/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15843/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/15843/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        andrew.wang Andrew Wang added a comment -

        Hi Kai, do you want to also rename BlockReaderRemote2 to just BlockReaderRemote per the class javadoc in BRR2?

        Show
        andrew.wang Andrew Wang added a comment - Hi Kai, do you want to also rename BlockReaderRemote2 to just BlockReaderRemote per the class javadoc in BRR2?
        Hide
        drankye Kai Zheng added a comment -

        Sure, Andrew! I will do that shortly.

        Show
        drankye Kai Zheng added a comment - Sure, Andrew! I will do that shortly.
        Hide
        drankye Kai Zheng added a comment -

        Updated the patch doing the rename and also cleaning up the related tests.

        Show
        drankye Kai Zheng added a comment - Updated the patch doing the rename and also cleaning up the related tests.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 26s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 5 new or modified test files.
        0 mvndep 0m 16s Maven dependency ordering for branch
        +1 mvninstall 12m 19s trunk passed
        +1 compile 1m 43s trunk passed
        +1 checkstyle 0m 36s trunk passed
        +1 mvnsite 1m 36s trunk passed
        +1 mvneclipse 0m 25s trunk passed
        +1 findbugs 3m 17s trunk passed
        +1 javadoc 1m 23s trunk passed
        0 mvndep 0m 8s Maven dependency ordering for patch
        +1 mvninstall 1m 28s the patch passed
        +1 compile 1m 34s the patch passed
        +1 javac 1m 34s the patch passed
        -0 checkstyle 0m 29s hadoop-hdfs-project: The patch generated 4 new + 71 unchanged - 30 fixed = 75 total (was 101)
        +1 mvnsite 1m 35s the patch passed
        +1 mvneclipse 0m 22s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 3m 46s the patch passed
        +1 javadoc 1m 16s the patch passed
        +1 unit 0m 54s hadoop-hdfs-client in the patch passed.
        -1 unit 80m 50s hadoop-hdfs in the patch failed.
        +1 asflicense 0m 27s The patch does not generate ASF License warnings.
        116m 16s



        Reason Tests
        Failed junit tests hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer
          hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure
          hadoop.hdfs.server.datanode.TestDirectoryScanner



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:85209cc
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12814360/HDFS-10548-v2.patch
        JIRA Issue HDFS-10548
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 0f83f938f654 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 / 77031a9
        Default Java 1.8.0_91
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/15938/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project.txt
        unit https://builds.apache.org/job/PreCommit-HDFS-Build/15938/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15938/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/15938/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 26s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 5 new or modified test files. 0 mvndep 0m 16s Maven dependency ordering for branch +1 mvninstall 12m 19s trunk passed +1 compile 1m 43s trunk passed +1 checkstyle 0m 36s trunk passed +1 mvnsite 1m 36s trunk passed +1 mvneclipse 0m 25s trunk passed +1 findbugs 3m 17s trunk passed +1 javadoc 1m 23s trunk passed 0 mvndep 0m 8s Maven dependency ordering for patch +1 mvninstall 1m 28s the patch passed +1 compile 1m 34s the patch passed +1 javac 1m 34s the patch passed -0 checkstyle 0m 29s hadoop-hdfs-project: The patch generated 4 new + 71 unchanged - 30 fixed = 75 total (was 101) +1 mvnsite 1m 35s the patch passed +1 mvneclipse 0m 22s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 46s the patch passed +1 javadoc 1m 16s the patch passed +1 unit 0m 54s hadoop-hdfs-client in the patch passed. -1 unit 80m 50s hadoop-hdfs in the patch failed. +1 asflicense 0m 27s The patch does not generate ASF License warnings. 116m 16s Reason Tests Failed junit tests hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer   hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure   hadoop.hdfs.server.datanode.TestDirectoryScanner Subsystem Report/Notes Docker Image:yetus/hadoop:85209cc JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12814360/HDFS-10548-v2.patch JIRA Issue HDFS-10548 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 0f83f938f654 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 / 77031a9 Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/15938/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/15938/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15938/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/15938/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        drankye Kai Zheng added a comment -

        Updated the patch to clean up the checking styles.

        Show
        drankye Kai Zheng added a comment - Updated the patch to clean up the checking styles.
        Hide
        drankye Kai Zheng added a comment -

        Updated the patch to clean up the checking styles.

        Show
        drankye Kai Zheng added a comment - Updated the patch to clean up the checking styles.
        Hide
        andrew.wang Andrew Wang added a comment -

        LGTM, thanks Kai! I kicked the precommit build so we can test the v3 patch.

        Show
        andrew.wang Andrew Wang added a comment - LGTM, thanks Kai! I kicked the precommit build so we can test the v3 patch.
        Hide
        andrew.wang Andrew Wang added a comment -

        To be clear, +1 pending the precommit run

        Show
        andrew.wang Andrew Wang added a comment - To be clear, +1 pending the precommit run
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 45s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 5 new or modified test files.
        0 mvndep 0m 11s Maven dependency ordering for branch
        +1 mvninstall 6m 56s trunk passed
        +1 compile 1m 35s trunk passed
        +1 checkstyle 0m 30s trunk passed
        +1 mvnsite 1m 26s trunk passed
        +1 mvneclipse 0m 24s trunk passed
        +1 findbugs 3m 11s trunk passed
        +1 javadoc 1m 15s trunk passed
        0 mvndep 0m 7s Maven dependency ordering for patch
        +1 mvninstall 1m 17s the patch passed
        +1 compile 1m 21s the patch passed
        +1 javac 1m 21s the patch passed
        -0 checkstyle 0m 28s hadoop-hdfs-project: The patch generated 1 new + 71 unchanged - 30 fixed = 72 total (was 101)
        +1 mvnsite 1m 23s the patch passed
        +1 mvneclipse 0m 20s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 3m 18s the patch passed
        +1 javadoc 1m 11s the patch passed
        +1 unit 0m 54s hadoop-hdfs-client in the patch passed.
        -1 unit 73m 43s hadoop-hdfs in the patch failed.
        +1 asflicense 0m 21s The patch does not generate ASF License warnings.
        102m 2s



        Reason Tests
        Failed junit tests hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer
          hadoop.hdfs.server.namenode.snapshot.TestOpenFilesWithSnapshot



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:85209cc
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12814620/HDFS-10548-v3.patch
        JIRA Issue HDFS-10548
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 58e8147eeb33 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 / 991c946
        Default Java 1.8.0_91
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/15949/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project.txt
        unit https://builds.apache.org/job/PreCommit-HDFS-Build/15949/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15949/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/15949/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 45s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 5 new or modified test files. 0 mvndep 0m 11s Maven dependency ordering for branch +1 mvninstall 6m 56s trunk passed +1 compile 1m 35s trunk passed +1 checkstyle 0m 30s trunk passed +1 mvnsite 1m 26s trunk passed +1 mvneclipse 0m 24s trunk passed +1 findbugs 3m 11s trunk passed +1 javadoc 1m 15s trunk passed 0 mvndep 0m 7s Maven dependency ordering for patch +1 mvninstall 1m 17s the patch passed +1 compile 1m 21s the patch passed +1 javac 1m 21s the patch passed -0 checkstyle 0m 28s hadoop-hdfs-project: The patch generated 1 new + 71 unchanged - 30 fixed = 72 total (was 101) +1 mvnsite 1m 23s the patch passed +1 mvneclipse 0m 20s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 18s the patch passed +1 javadoc 1m 11s the patch passed +1 unit 0m 54s hadoop-hdfs-client in the patch passed. -1 unit 73m 43s hadoop-hdfs in the patch failed. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 102m 2s Reason Tests Failed junit tests hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer   hadoop.hdfs.server.namenode.snapshot.TestOpenFilesWithSnapshot Subsystem Report/Notes Docker Image:yetus/hadoop:85209cc JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12814620/HDFS-10548-v3.patch JIRA Issue HDFS-10548 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 58e8147eeb33 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 / 991c946 Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/15949/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/15949/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15949/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/15949/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        drankye Kai Zheng added a comment -

        Thanks Andrew Wang for the reviewing! The only check style isn't suitable for fix. The TestOfflineEditsViewer test failure was not relevant to this change, will report it separately. TestOpenFilesWithSnapshot can pass locally.

        Show
        drankye Kai Zheng added a comment - Thanks Andrew Wang for the reviewing! The only check style isn't suitable for fix. The TestOfflineEditsViewer test failure was not relevant to this change, will report it separately. TestOpenFilesWithSnapshot can pass locally.
        Hide
        andrew.wang Andrew Wang added a comment -

        SGTM, feel free to commit Kai. +1.

        Show
        andrew.wang Andrew Wang added a comment - SGTM, feel free to commit Kai. +1.
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Hadoop-trunk-Commit #10047 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10047/)
        HDFS-10548. Remove the long deprecated BlockReaderRemote. Contributed by (kai.zheng: rev 8b281bce85474501868d68f8d5590a6086abb7b7)

        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/client/impl/TestBlockReaderRemote.java
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/client/impl/TestBlockReaderBase.java
        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/impl/BlockReaderRemote.java
        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/impl/DfsClientConf.java
        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/impl/BlockReaderFactory.java
        • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/impl/BlockReaderRemote2.java
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/client/impl/TestClientBlockVerification.java
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/shortcircuit/TestShortCircuitLocalRead.java
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/client/impl/TestBlockReaderRemote2.java
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/erasurecode/StripedBlockReader.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #10047 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10047/ ) HDFS-10548 . Remove the long deprecated BlockReaderRemote. Contributed by (kai.zheng: rev 8b281bce85474501868d68f8d5590a6086abb7b7) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/client/impl/TestBlockReaderRemote.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/client/impl/TestBlockReaderBase.java hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/impl/BlockReaderRemote.java hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/impl/DfsClientConf.java hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/impl/BlockReaderFactory.java hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/impl/BlockReaderRemote2.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/client/impl/TestClientBlockVerification.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/shortcircuit/TestShortCircuitLocalRead.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/client/impl/TestBlockReaderRemote2.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/erasurecode/StripedBlockReader.java
        Hide
        drankye Kai Zheng added a comment -

        Thanks Andrew Wang! I just committed this to trunk, targeting for 3.0

        Show
        drankye Kai Zheng added a comment - Thanks Andrew Wang ! I just committed this to trunk, targeting for 3.0
        Hide
        cmccabe Colin P. McCabe added a comment -

        Thanks for tackling this, guys. It is good to see this code duplication finally go away. Next target: BlockReaderLocalLegacy?

        I do think renaming BlockReaderRemote2 will make merging code back to branch-2 more difficult-- you might want to reconsider that.

        Show
        cmccabe Colin P. McCabe added a comment - Thanks for tackling this, guys. It is good to see this code duplication finally go away. Next target: BlockReaderLocalLegacy ? I do think renaming BlockReaderRemote2 will make merging code back to branch-2 more difficult-- you might want to reconsider that.
        Hide
        andrew.wang Andrew Wang added a comment -

        I don't think there's too much activity on BlockReaderRemote2 these days, so I think now is as good as any to pull the trigger.

        Is there a JIRA for removing BRLocalLegacy too? IIRC Windows used to need it since they didn't support passing the fd via domain socket, but maybe that's changed.

        Show
        andrew.wang Andrew Wang added a comment - I don't think there's too much activity on BlockReaderRemote2 these days, so I think now is as good as any to pull the trigger. Is there a JIRA for removing BRLocalLegacy too? IIRC Windows used to need it since they didn't support passing the fd via domain socket, but maybe that's changed.
        Hide
        drankye Kai Zheng added a comment -

        This was done targeting 3.0, but if we need this for branch-2 as well, I can check and post a patch for the branch separately if necessary.

        Show
        drankye Kai Zheng added a comment - This was done targeting 3.0, but if we need this for branch-2 as well, I can check and post a patch for the branch separately if necessary.
        Hide
        andrew.wang Andrew Wang added a comment -

        I think what Colin meant is that (with the rename) if someone changes BRR in trunk, that change needs to be reapplied to BRR2 for the branch-2 backport. So the backports won't be clean.

        Recommend that we not backport this to branch-2 for compatibility reasons.

        Show
        andrew.wang Andrew Wang added a comment - I think what Colin meant is that (with the rename) if someone changes BRR in trunk, that change needs to be reapplied to BRR2 for the branch-2 backport. So the backports won't be clean. Recommend that we not backport this to branch-2 for compatibility reasons.
        Hide
        andrew.wang Andrew Wang added a comment -

        I also realized on second inspection that the LEGACY_BLOCKREADER config key is still present in HdfsClientConfigKeys and DFSConfigKeys. How do we feel about removing it?

        Show
        andrew.wang Andrew Wang added a comment - I also realized on second inspection that the LEGACY_BLOCKREADER config key is still present in HdfsClientConfigKeys and DFSConfigKeys. How do we feel about removing it?
        Hide
        drankye Kai Zheng added a comment -

        Is there a JIRA for removing BRLocalLegacy too? IIRC Windows used to need it since they didn't support passing the fd via domain socket, but maybe that's changed.

        Ping: Chris Nauroth and Steve Loughran. Hope this to be clarified or confirmed, and if sounds good I can do it as well.

        How do we feel about removing it?

        Sure Andrew Wang, I will find a chance do it.

        Show
        drankye Kai Zheng added a comment - Is there a JIRA for removing BRLocalLegacy too? IIRC Windows used to need it since they didn't support passing the fd via domain socket, but maybe that's changed. Ping: Chris Nauroth and Steve Loughran . Hope this to be clarified or confirmed, and if sounds good I can do it as well. How do we feel about removing it? Sure Andrew Wang , I will find a chance do it.
        Hide
        stevel@apache.org Steve Loughran added a comment -

        I have no opinions; Chris is on vacation this week, so best to hold off until he returns and can have his input on this

        Show
        stevel@apache.org Steve Loughran added a comment - I have no opinions; Chris is on vacation this week, so best to hold off until he returns and can have his input on this
        Hide
        drankye Kai Zheng added a comment -

        Thanks Steve for the sync up ...

        Show
        drankye Kai Zheng added a comment - Thanks Steve for the sync up ...

          People

          • Assignee:
            drankye Kai Zheng
            Reporter:
            drankye Kai Zheng
          • Votes:
            0 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development