Hadoop Common
  1. Hadoop Common
  2. HADOOP-5823

Handling javac "deprecated" warning for using UTF8

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.20.0
    • Fix Version/s: 0.21.0
    • Component/s: build
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      o.a.h.io.UTF8 is deprecated but is still used in multiple places. FSEditLog.java has 40 UTF8 related warnings. I don't think it is feasible to avoid using UTF8 in FSEditLog.java.

      Two options to get rid of these warnings :
      1. use @SupressWarnings at each use of UTF or for enclosing class.
      2. define a wrapper class DeprecatedUTF8 that is not @deprecated.

      I prefer the second option in this case since it keeps FSEditLog.java and other places clean and still makes it explicit that a deprecated class is used.

      This is part of spring cleaning effort to remove warnings in javac. I will attach a patch for the second option.

      1. HADOOP-5823.patch
        8 kB
        Raghu Angadi
      2. HADOOP-5823.patch
        8 kB
        Raghu Angadi

        Issue Links

          Activity

          Hide
          Raghu Angadi added a comment -

          I am not sure about the other warnings. I thought others might want to use the wrapper where UTF8 is still used. I doubt if use of UTF8 outside HDFS will be removed anytime soon.

          I filed HADOOP-5866 to move the new class to hdfs.

          Show
          Raghu Angadi added a comment - I am not sure about the other warnings. I thought others might want to use the wrapper where UTF8 is still used. I doubt if use of UTF8 outside HDFS will be removed anytime soon. I filed HADOOP-5866 to move the new class to hdfs.
          Hide
          Doug Cutting added a comment -

          > UTF8 is currently used in many places.

          Yes, but FSEditLog has been identified as a place that UTF8 will continue to be used for a long time yet, long-enough that we want to squash the warnings there. Are there other places where it should continue to be used, whose warnings should also be squashed? If not, then moving it to be private in HDFS makes sense.

          > We could move DeprecatedUTF8 to hdfs as a package private class.

          +1

          Show
          Doug Cutting added a comment - > UTF8 is currently used in many places. Yes, but FSEditLog has been identified as a place that UTF8 will continue to be used for a long time yet, long-enough that we want to squash the warnings there. Are there other places where it should continue to be used, whose warnings should also be squashed? If not, then moving it to be private in HDFS makes sense. > We could move DeprecatedUTF8 to hdfs as a package private class. +1
          Hide
          Raghu Angadi added a comment -

          We could move DeprecatedUTF8 to hdfs as a package private class.

          Show
          Raghu Angadi added a comment - We could move DeprecatedUTF8 to hdfs as a package private class.
          Hide
          Raghu Angadi added a comment -

          UTF8 is currently used in many places. FSEditLog.java is just an example. That said, I am not opposed to any changes related this.

          Show
          Raghu Angadi added a comment - UTF8 is currently used in many places. FSEditLog.java is just an example. That said, I am not opposed to any changes related this.
          Hide
          Doug Cutting added a comment -

          Since the use of this new class is confined to FSEditLog, why not make it package-private there, rather than pollute the public, documented namespace with a new class? For that matter, if FSEditLog is really the only user, we could move UTF8 to a private class in its package.

          Show
          Doug Cutting added a comment - Since the use of this new class is confined to FSEditLog, why not make it package-private there, rather than pollute the public, documented namespace with a new class? For that matter, if FSEditLog is really the only user, we could move UTF8 to a private class in its package.
          Hide
          Raghu Angadi added a comment -

          I just committed this.

          Show
          Raghu Angadi added a comment - I just committed this.
          Hide
          Raghu Angadi added a comment -

          ant test-patch :

               [exec] -1 overall.  
               [exec] 
               [exec]     +1 @author.  The patch does not contain any @author tags.
               [exec] 
               [exec]     -1 tests included.  The patch doesn't appear to include any new or modified tests.
               [exec]                         Please justify why no tests are needed for this patch.
               [exec] 
               [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
               [exec] 
               [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
               [exec] 
               [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
               [exec] 
               [exec]     +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
               [exec] 
               [exec]     +1 release audit.  The applied patch does not increase the total number of release audit warnings.
               [exec] 
          
          Show
          Raghu Angadi added a comment - ant test-patch : [exec] -1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] -1 tests included. The patch doesn't appear to include any new or modified tests. [exec] Please justify why no tests are needed for this patch. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. [exec]
          Hide
          Tsz Wo Nicholas Sze added a comment -

          +1 for the second option

          It would be great if we can complete remove UTF8 in the near future.

          Show
          Tsz Wo Nicholas Sze added a comment - +1 for the second option It would be great if we can complete remove UTF8 in the near future.
          Hide
          Raghu Angadi added a comment -

          > Some comments that indicates methods overridden from JournalStream are deleted. Not sure if that is intentional.

          Updated patch removes these changes since they are not related to UTF8 warnings.

          Show
          Raghu Angadi added a comment - > Some comments that indicates methods overridden from JournalStream are deleted. Not sure if that is intentional. Updated patch removes these changes since they are not related to UTF8 warnings.
          Hide
          Raghu Angadi added a comment -

          > Some comments that indicates methods overridden from JournalStream are deleted.
          yes. I removed "@Override" since these methods were "implementing" abstract methods rather than overriding. My eclipse flagged them as errors. I could remove those.

          I still could not get rid of one warning in DeprecatedUTF8.java. "@SupressWarnings("deprecated")" didn't work for {{extends UTF8} part.

          Show
          Raghu Angadi added a comment - > Some comments that indicates methods overridden from JournalStream are deleted. yes. I removed "@Override" since these methods were "implementing" abstract methods rather than overriding. My eclipse flagged them as errors. I could remove those. I still could not get rid of one warning in DeprecatedUTF8.java. "@SupressWarnings("deprecated")" didn't work for {{extends UTF8} part.
          Hide
          Suresh Srinivas added a comment -

          Some comments that indicates methods overridden from JournalStream are deleted. Not sure if that is intentional. +1 other than that.

          Show
          Suresh Srinivas added a comment - Some comments that indicates methods overridden from JournalStream are deleted. Not sure if that is intentional. +1 other than that.
          Hide
          Raghu Angadi added a comment -

          Patch for the second option. FSEditLog.java is modified as a demo of the use case.

          Show
          Raghu Angadi added a comment - Patch for the second option. FSEditLog.java is modified as a demo of the use case.

            People

            • Assignee:
              Raghu Angadi
              Reporter:
              Raghu Angadi
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development