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

      Siddharth Seth created issue -
      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 made changes -
      Field Original Value New Value
      Attachment MR2500.patch [ 12479404 ]
      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 -
      Attachment MR2500_2.patch [ 12479407 ]
      Hide
      Luke Lu added a comment -

      +1, lgtm.

      Show
      Luke Lu added a comment - +1, lgtm.
      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
      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.
      Siddharth Seth made changes -
      Attachment MR2500_3.patch [ 12479412 ]
      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!
      Mahadev konar made changes -
      Status Open [ 1 ] Resolved [ 5 ]
      Hadoop Flags [Reviewed]
      Resolution Fixed [ 1 ]
      Arun C Murthy made changes -
      Status Resolved [ 5 ] Closed [ 6 ]

        People

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

          Dates

          • Created:
            Updated:
            Resolved:

            Development