Hadoop Common
  1. Hadoop Common
  2. HADOOP-1010

getReordReader methof of InputFormat class should handle null reporter argument

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.12.0
    • Component/s: None
    • Labels:
      None

      Description

      In some cases, I need to create a record reader object in the config method of mappers.
      At that time, reporter is not available yet. And logically, the reporter should not be a required for getRecordReader anyway.

        Activity

        Runping Qi created issue -
        Hide
        Doug Cutting added a comment -

        Alternately, we could provide a Reprter.NULL constant that could be passed in such cases.

        public static final Reporter NULL = new Reporter() { public void setStatus(String s) {} };

        Show
        Doug Cutting added a comment - Alternately, we could provide a Reprter.NULL constant that could be passed in such cases. public static final Reporter NULL = new Reporter() { public void setStatus(String s) {} };
        Runping Qi made changes -
        Field Original Value New Value
        Assignee Runping Qi [ runping ]
        Hide
        Runping Qi added a comment -

        This patch adds a NULL constant to Reporter interface.

        Show
        Runping Qi added a comment - This patch adds a NULL constant to Reporter interface.
        Runping Qi made changes -
        Attachment hadoop-1010.patch [ 12350967 ]
        Hide
        Runping Qi added a comment -

        Added Reporter.NULL constant.

        Show
        Runping Qi added a comment - Added Reporter.NULL constant.
        Runping Qi made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Milind Bhandarkar added a comment -

        I think the right way to do this is by supplying a reporter in all methods of tasks (configure, map/reduce, close). (See HADOOP-403).

        Show
        Milind Bhandarkar added a comment - I think the right way to do this is by supplying a reporter in all methods of tasks (configure, map/reduce, close). (See HADOOP-403 ).
        Hide
        Hadoop QA added a comment -

        +1, because http://issues.apache.org/jira/secure/attachment/12350967/hadoop-1010.patch applied and successfully tested against trunk revision r505557.

        Show
        Hadoop QA added a comment - +1, because http://issues.apache.org/jira/secure/attachment/12350967/hadoop-1010.patch applied and successfully tested against trunk revision r505557.
        Hide
        Doug Cutting added a comment -

        > the right way to do this is by supplying a reporter in all methods of tasks

        I'm not sure exactly how that addresses this issue.

        I think we ought to add a Context parameters to most methods on core interfaces, that we can then effectively add and remove/deprecate parameters from these interfaces without breaking implementations. For example, Owen made this a part of his proposal in HADOOP-904. So, as we next modify each interface, I think we ought to convert to this style. And Reporter could then be added as a getReporter() method to the context parameter used by most interfaces, removing it from the explicit parameter list, and solving this issue. But that's a long-term fix. The short term fix Runping attached is probably a good idea in the meantime, no?

        Show
        Doug Cutting added a comment - > the right way to do this is by supplying a reporter in all methods of tasks I'm not sure exactly how that addresses this issue. I think we ought to add a Context parameters to most methods on core interfaces, that we can then effectively add and remove/deprecate parameters from these interfaces without breaking implementations. For example, Owen made this a part of his proposal in HADOOP-904 . So, as we next modify each interface, I think we ought to convert to this style. And Reporter could then be added as a getReporter() method to the context parameter used by most interfaces, removing it from the explicit parameter list, and solving this issue. But that's a long-term fix. The short term fix Runping attached is probably a good idea in the meantime, no?
        Hide
        Milind Bhandarkar added a comment -

        The way I see it, reporter is just another output stream coming out of the task. It should get a uniform treatment just like other streams. Context is a good abstraction, since we would be able to hang non-Strings (such as the task's stream-set) to it.

        Show
        Milind Bhandarkar added a comment - The way I see it, reporter is just another output stream coming out of the task. It should get a uniform treatment just like other streams. Context is a good abstraction, since we would be able to hang non-Strings (such as the task's stream-set) to it.
        Hide
        Owen O'Malley added a comment -

        Yeah, I agree with Doug that Runping's fix is the right one for the short term and the context parameter is the right long term solution.

        Show
        Owen O'Malley added a comment - Yeah, I agree with Doug that Runping's fix is the right one for the short term and the context parameter is the right long term solution.
        Hide
        Doug Cutting added a comment -

        I just committed this. Thanks, Runping!

        Show
        Doug Cutting added a comment - I just committed this. Thanks, Runping!
        Doug Cutting made changes -
        Resolution Fixed [ 1 ]
        Fix Version/s 0.12.0 [ 12312293 ]
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Doug Cutting made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Owen O'Malley made changes -
        Component/s mapred [ 12310690 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Patch Available Patch Available
        32m 35s 1 Runping Qi 12/Feb/07 21:11
        Patch Available Patch Available Resolved Resolved
        2h 33m 1 Doug Cutting 12/Feb/07 23:44
        Resolved Resolved Closed Closed
        17d 23h 17m 1 Doug Cutting 02/Mar/07 23:02

          People

          • Assignee:
            Runping Qi
            Reporter:
            Runping Qi
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development