Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 0.21.0
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Release Note:
      labeled the method @Deprecated as suggested by dhruba

      Description

      DFSClient#getBlockSize is unused. Since it's a public class internal to HDFS we just remove it? If not then we should add a unit test.

      1. HDFS-929.patch
        4 kB
        Harsh J
      2. HDFS-929-take1.txt
        0.5 kB
        Jim Plush

        Activity

        Eli Collins created issue -
        Hide
        dhruba borthakur added a comment -

        Maybe you can mark it as deprecated?

        Show
        dhruba borthakur added a comment - Maybe you can mark it as deprecated?
        Hide
        Jim Plush added a comment -

        Added the @Deprecated tag to the method as suggested.

        Show
        Jim Plush added a comment - Added the @Deprecated tag to the method as suggested.
        Jim Plush made changes -
        Field Original Value New Value
        Attachment HDFS-929-take1.txt [ 12483825 ]
        Jim Plush made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Release Note labeled the method @Deprecated as suggested by dhruba
        Affects Version/s 0.21.0 [ 12314046 ]
        Assignee Jim Plush [ jimplush ]
        Fix Version/s 0.23.0 [ 12315571 ]
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12483825/HDFS-929-take1.txt
        against trunk revision 1139473.

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

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

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

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

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

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

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

        +1 system test framework. The patch passed system test framework compile.

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/841//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/841//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/841//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12483825/HDFS-929-take1.txt against trunk revision 1139473. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/841//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/841//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/841//console This message is automatically generated.
        Hide
        dhruba borthakur added a comment -

        any motivation for doing this? Since this is a public api, other software might be using it maybe? is there any downside of keeping this API as it is?

        Show
        dhruba borthakur added a comment - any motivation for doing this? Since this is a public api, other software might be using it maybe? is there any downside of keeping this API as it is?
        Hide
        Jim Plush added a comment -

        apologies, I read your comment "Maybe you can mark it as deprecated?" as you wanted it marked deprecated.

        Show
        Jim Plush added a comment - apologies, I read your comment "Maybe you can mark it as deprecated?" as you wanted it marked deprecated.
        Arun C Murthy made changes -
        Fix Version/s 0.24.0 [ 12317653 ]
        Fix Version/s 0.23.0 [ 12315571 ]
        Hide
        Harsh J added a comment -

        Thanks for the careful eye Jim!

        This is provided by the ClientProtocol. If it is being deprecated, it should be deprecated all the way, and removed at a subsequent release. This'd incur an unwanted change in compatibility and will make HDFS require major version upgrade. So far, I only see server-side mechanisms use this via the INodeFile#getPreferredBlockSize(…) method that eventually gets called, and this is called pretty directly.

        I do not know why its published into the client (seems to be available since very long ago) but there's no harm in keeping it as DFSClient was never meant for user consumption. Perhaps a feature that may get added later may use it?

        If you agree with this assessment, lets resolve this one.

        Show
        Harsh J added a comment - Thanks for the careful eye Jim! This is provided by the ClientProtocol. If it is being deprecated, it should be deprecated all the way, and removed at a subsequent release. This'd incur an unwanted change in compatibility and will make HDFS require major version upgrade. So far, I only see server-side mechanisms use this via the INodeFile#getPreferredBlockSize(…) method that eventually gets called, and this is called pretty directly. I do not know why its published into the client (seems to be available since very long ago) but there's no harm in keeping it as DFSClient was never meant for user consumption. Perhaps a feature that may get added later may use it? If you agree with this assessment, lets resolve this one.
        Hide
        Eli Collins added a comment -

        Let's mark as deprecated and add a unit test.

        Show
        Eli Collins added a comment - Let's mark as deprecated and add a unit test.
        Hide
        Harsh J added a comment -

        Patch that deprecates the ClientProtocol calls.

        For tests, INodeFile already has a test for its use internally, should we add a test with ClientProtocol use in mind?

        Show
        Harsh J added a comment - Patch that deprecates the ClientProtocol calls. For tests, INodeFile already has a test for its use internally, should we add a test with ClientProtocol use in mind?
        Harsh J made changes -
        Attachment HDFS-929.patch [ 12509272 ]
        Hide
        Hadoop QA added a comment -

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

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        -1 javadoc. The javadoc tool appears to have generated 20 warning messages.

        -1 javac. The applied patch generated 41 javac compiler warnings (more than the trunk's current 39 warnings).

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

        -1 findbugs. The patch appears to introduce 1 new Findbugs (version 1.3.9) warnings.

        -1 release audit. The applied patch generated 1 release audit warnings (more than the trunk's current 0 warnings).

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

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

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/1748//testReport/
        Release audit warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/1748//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/1748//artifact/trunk/hadoop-hdfs-project/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/1748//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12509272/HDFS-929.patch against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 javadoc. The javadoc tool appears to have generated 20 warning messages. -1 javac. The applied patch generated 41 javac compiler warnings (more than the trunk's current 39 warnings). +1 eclipse:eclipse. The patch built with eclipse:eclipse. -1 findbugs. The patch appears to introduce 1 new Findbugs (version 1.3.9) warnings. -1 release audit. The applied patch generated 1 release audit warnings (more than the trunk's current 0 warnings). +1 core tests. The patch passed unit tests in . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/1748//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/1748//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/1748//artifact/trunk/hadoop-hdfs-project/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/1748//console This message is automatically generated.
        Allen Wittenauer made changes -
        Fix Version/s 0.24.0 [ 12317653 ]
        Allen Wittenauer made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Hide
        Allen Wittenauer added a comment -

        Do we want to actually deprecate this now or just let it live on forever?

        Show
        Allen Wittenauer added a comment - Do we want to actually deprecate this now or just let it live on forever?
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Patch Available Patch Available
        512d 17h 38m 1 Jim Plush 25/Jun/11 20:03
        Patch Available Patch Available Open Open
        1130d 2h 55m 1 Allen Wittenauer 29/Jul/14 22:58

          People

          • Assignee:
            Jim Plush
            Reporter:
            Eli Collins
          • Votes:
            0 Vote for this issue
            Watchers:
            10 Start watching this issue

            Dates

            • Created:
              Updated:

              Development