Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.23.0
    • Component/s: mrv2
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    1. MR2500_3.patch
      19 kB
      Siddharth Seth
    2. MR2500_2.patch
      16 kB
      Siddharth Seth
    3. MR2500.patch
      14 kB
      Siddharth Seth

      Activity

      Transition Time In Source Status Execution Times Last Executer Last Execution Date
      Open Open Resolved Resolved
      1d 20m 1 Mahadev konar 18/May/11 00:07
      Resolved Resolved Closed Closed
      181d 42m 1 Arun C Murthy 15/Nov/11 00:49
      Arun C Murthy made changes -
      Status Resolved [ 5 ] Closed [ 6 ]
      Mahadev konar made changes -
      Status Open [ 1 ] Resolved [ 5 ]
      Hadoop Flags [Reviewed]
      Resolution Fixed [ 1 ]
      Hide
      Mahadev konar added a comment -

      I just pushed this to MR-279. thanks sid and luke!

      Show
      Mahadev konar added a comment - I just pushed this to MR-279. thanks sid and luke!
      Siddharth Seth made changes -
      Attachment MR2500_3.patch [ 12479412 ]
      Hide
      Siddharth Seth added a comment -

      Thanks for taking a look Luke.

      Patch with the suggested changes.

      Show
      Siddharth Seth added a comment - Thanks for taking a look Luke. Patch with the suggested changes.
      Hide
      Luke Lu added a comment -

      Minor nits:

      1. The ctor lookup code probably should save the lookup result instead make another cache.get call.
      2. You can probably get rid of the null in clazz.getConstructor(null) in RecordFactoryPBImpl to avoid the vararg warning.
      Show
      Luke Lu added a comment - Minor nits: The ctor lookup code probably should save the lookup result instead make another cache.get call. You can probably get rid of the null in clazz.getConstructor(null) in RecordFactoryPBImpl to avoid the vararg warning.
      Hide
      Luke Lu added a comment -

      +1, lgtm.

      Show
      Luke Lu added a comment - +1, lgtm.
      Siddharth Seth made changes -
      Attachment MR2500_2.patch [ 12479407 ]
      Hide
      Siddharth Seth added a comment -

      Same as the previous patch - with the apache license added.

      Show
      Siddharth Seth added a comment - Same as the previous patch - with the apache license added.
      Siddharth Seth made changes -
      Field Original Value New Value
      Attachment MR2500.patch [ 12479404 ]
      Hide
      Siddharth Seth added a comment -

      Factories changed to use a ConcurrentMap - doesn't really matter if two parallel calls end up creating their own instances (instead of using a cached copy).

      Changed FactoryProvider to use a static configuration object if none is provided (instead of creating a configuration each time)

      Changed TypeConverter to use a static RecordFactory. (To be changed later to use a provided recordFactory)

      Show
      Siddharth Seth added a comment - Factories changed to use a ConcurrentMap - doesn't really matter if two parallel calls end up creating their own instances (instead of using a cached copy). Changed FactoryProvider to use a static configuration object if none is provided (instead of creating a configuration each time) Changed TypeConverter to use a static RecordFactory. (To be changed later to use a provided recordFactory)
      Siddharth Seth created issue -

        People

        • Assignee:
          Siddharth Seth
          Reporter:
          Siddharth Seth
        • Votes:
          0 Vote for this issue
          Watchers:
          1 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development