Uploaded image for project: 'Hadoop Common'
  1. Hadoop Common
  2. HADOOP-13700

Remove unthrown IOException from TrashPolicy#initialize and #getInstance signatures

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.8.0, 3.0.0-alpha1
    • Fix Version/s: 2.8.0, 3.0.0-alpha2
    • Component/s: fs
    • Labels:
      None

      Description

      TrashPolicy is marked as public & evolving, but its public API, specifically TrashPolicy.getInstance() has been changed in an incompatible way.
      1) The path parameter is removed in 3.0
      2) A new IOException is thrown in 3.0

        Issue Links

          Activity

          Hide
          aw Allen Wittenauer added a comment -

          public API, specifically TrashPolicy.getInstance() has been changed in an incompatible way.

          The API compatibility guidelines allow for the API to change in a major release.

          Show
          aw Allen Wittenauer added a comment - public API, specifically TrashPolicy.getInstance() has been changed in an incompatible way. The API compatibility guidelines allow for the API to change in a major release.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          the new method in question HDFS-8831 (dec 2015/Hadoop 2.8); the old one deprecated in HADOOP-13538.

          so...its an API change which is coming in without any shipping ASF Hadoop release being able to use it.I think that's a bit, well, drastic. Just because we say we're allowed to remove things, doesn't mean we should —all it does is increase cost of migration.

          Show
          stevel@apache.org Steve Loughran added a comment - the new method in question HDFS-8831 (dec 2015/Hadoop 2.8); the old one deprecated in HADOOP-13538 . so...its an API change which is coming in without any shipping ASF Hadoop release being able to use it.I think that's a bit, well, drastic. Just because we say we're allowed to remove things, doesn't mean we should —all it does is increase cost of migration.
          Hide
          aw Allen Wittenauer added a comment -

          without any shipping ASF Hadoop release

          Ahh. I see! Good catch. Timeline is important here. 2.8.0 was supposed to be out months and months ago. With that branch hitting it's 1 year anniversary soon and the increasing likelihood of it being abandoned/effectively DOA, yeah, I can see that we might want a different approach. We should probably audit all of the 2.8.0 deprecations vs. the 3.0.0 removals to see how bad the damage currently is.

          Show
          aw Allen Wittenauer added a comment - without any shipping ASF Hadoop release Ahh. I see! Good catch. Timeline is important here. 2.8.0 was supposed to be out months and months ago. With that branch hitting it's 1 year anniversary soon and the increasing likelihood of it being abandoned/effectively DOA, yeah, I can see that we might want a different approach. We should probably audit all of the 2.8.0 deprecations vs. the 3.0.0 removals to see how bad the damage currently is.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          > 2.8.0 anniversary

          yeah, might be time to fork off 2.9 and release that

          Show
          stevel@apache.org Steve Loughran added a comment - > 2.8.0 anniversary yeah, might be time to fork off 2.9 and release that
          Hide
          andrew.wang Andrew Wang added a comment -

          Thanks for the discussion Steve, Allen. Here's my proposal:

          • We revert HADOOP-13534. I can open a new JIRA to do this for changelog purposes, since it's been released in 3.0.0-alpha1.
          • We remove the "throws IOException" from the initialize and getInstance methods added in HDFS-8831. This is similar to HDFS-9799. This also gets backported to branch-2 and branch-2.8; HDFS-8831 hasn't made it into a release yet, so this seems safe.
          Show
          andrew.wang Andrew Wang added a comment - Thanks for the discussion Steve, Allen. Here's my proposal: We revert HADOOP-13534 . I can open a new JIRA to do this for changelog purposes, since it's been released in 3.0.0-alpha1. We remove the "throws IOException" from the initialize and getInstance methods added in HDFS-8831 . This is similar to HDFS-9799 . This also gets backported to branch-2 and branch-2.8; HDFS-8831 hasn't made it into a release yet, so this seems safe.
          Hide
          andrew.wang Andrew Wang added a comment -

          Regarding auditing incompatible changes, I revved the JACC patch at HADOOP-13583 recently, and would appreciate a review to get that in. Running the tool and looking at the output would also be helpful.

          Show
          andrew.wang Andrew Wang added a comment - Regarding auditing incompatible changes, I revved the JACC patch at HADOOP-13583 recently, and would appreciate a review to get that in. Running the tool and looking at the output would also be helpful.
          Hide
          andrew.wang Andrew Wang added a comment -

          Trivial patch attached to remove "throws IOException" from the new methods in TrashPolicy, since they don't throw IOException.

          Show
          andrew.wang Andrew Wang added a comment - Trivial patch attached to remove "throws IOException" from the new methods in TrashPolicy, since they don't throw IOException.
          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 7m 11s trunk passed
          +1 compile 7m 23s trunk passed
          +1 checkstyle 0m 23s trunk passed
          +1 mvnsite 1m 0s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 1m 25s trunk passed
          +1 javadoc 0m 42s trunk passed
          +1 mvninstall 0m 35s the patch passed
          +1 compile 6m 43s the patch passed
          +1 javac 6m 43s the patch passed
          -0 checkstyle 0m 23s hadoop-common-project/hadoop-common: The patch generated 1 new + 4 unchanged - 1 fixed = 5 total (was 5)
          +1 mvnsite 0m 54s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 25s the patch passed
          +1 javadoc 0m 42s the patch passed
          -1 unit 16m 57s hadoop-common in the patch failed.
          +1 asflicense 0m 21s The patch does not generate ASF License warnings.
          48m 13s



          Reason Tests
          Timed out junit tests org.apache.hadoop.http.TestHttpServerLifecycle



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HADOOP-13700
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832578/HADOOP-13700.001.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 3e18fbb5f933 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 / 96b1266
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10726/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10726/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10726/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10726/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 7m 11s trunk passed +1 compile 7m 23s trunk passed +1 checkstyle 0m 23s trunk passed +1 mvnsite 1m 0s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 25s trunk passed +1 javadoc 0m 42s trunk passed +1 mvninstall 0m 35s the patch passed +1 compile 6m 43s the patch passed +1 javac 6m 43s the patch passed -0 checkstyle 0m 23s hadoop-common-project/hadoop-common: The patch generated 1 new + 4 unchanged - 1 fixed = 5 total (was 5) +1 mvnsite 0m 54s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 25s the patch passed +1 javadoc 0m 42s the patch passed -1 unit 16m 57s hadoop-common in the patch failed. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 48m 13s Reason Tests Timed out junit tests org.apache.hadoop.http.TestHttpServerLifecycle Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-13700 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832578/HADOOP-13700.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 3e18fbb5f933 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 / 96b1266 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10726/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10726/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10726/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10726/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          haibochen Haibo Chen added a comment -

          Thanks Andrew Wang for the fix. This solve most of the pain to user the new API. What about the path parameter that was in the parameter list before 3.0?

          Show
          haibochen Haibo Chen added a comment - Thanks Andrew Wang for the fix. This solve most of the pain to user the new API. What about the path parameter that was in the parameter list before 3.0?
          Hide
          andrew.wang Andrew Wang added a comment -

          Haibo Chen thanks for reviewing, see HADOOP-13705 for the Path parameter, simple revert.

          Show
          andrew.wang Andrew Wang added a comment - Haibo Chen thanks for reviewing, see HADOOP-13705 for the Path parameter, simple revert.
          Hide
          eddyxu Lei (Eddy) Xu added a comment -

          Thanks Andrew Wang. It looks safe to me. +1.

          Show
          eddyxu Lei (Eddy) Xu added a comment - Thanks Andrew Wang . It looks safe to me. +1.
          Hide
          andrew.wang Andrew Wang added a comment -

          Thanks for reviewing Eddy. I'll wait until tomorrow to commit since Allen or Steve might also want to look.

          Show
          andrew.wang Andrew Wang added a comment - Thanks for reviewing Eddy. I'll wait until tomorrow to commit since Allen or Steve might also want to look.
          Hide
          andrew.wang Andrew Wang added a comment -

          Committed to trunk, branch-2, branch-2.8. Thanks Haibo for reporting and reviewing, Eddy for the final +1, and other watchers for discussion.

          Show
          andrew.wang Andrew Wang added a comment - Committed to trunk, branch-2, branch-2.8. Thanks Haibo for reporting and reviewing, Eddy for the final +1, and other watchers for discussion.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10598 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10598/)
          HADOOP-13700. Remove unthrown IOException from TrashPolicy#initialize (wang: rev 12d739a34ba868b3f7f5adf7f37a60d4aca9061b)

          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicy.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10598 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10598/ ) HADOOP-13700 . Remove unthrown IOException from TrashPolicy#initialize (wang: rev 12d739a34ba868b3f7f5adf7f37a60d4aca9061b) (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicy.java

            People

            • Assignee:
              andrew.wang Andrew Wang
              Reporter:
              haibochen Haibo Chen
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development