Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.3
    • Fix Version/s: 0.3
    • Component/s: None
    • Labels:

      Description

      Some table sources have no need for paths (ie HBase) and thus JobInfo.location may be null. Looks like commitJob was trying to do a test before acting on the location but buggy. abortJob does not have such check.

      1. HCatalog-60.patch
        135 kB
        Francis Liu
      2. HCatalog-60_rev2.patch
        141 kB
        Francis Liu
      3. HCatalog-60_rev3.patch
        141 kB
        Francis Liu
      4. HCatalog-60_rev4.patch
        141 kB
        Francis Liu
      5. HCatalog-60_rev5.patch
        145 kB
        Francis Liu
      6. HCatalog-60_rev6.patch
        145 kB
        Francis Liu

        Issue Links

          Activity

          Hide
          Alan Gates added a comment -

          Issue closed with 0.4 release.

          Show
          Alan Gates added a comment - Issue closed with 0.4 release.
          Hide
          Francis Liu added a comment -

          Awesome, thanks guys!

          Show
          Francis Liu added a comment - Awesome, thanks guys!
          Hide
          Alan Gates added a comment -

          Patch checked in. Woohoo!

          Show
          Alan Gates added a comment - Patch checked in. Woohoo!
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1861/
          -----------------------------------------------------------

          (Updated 2011-09-30 14:36:34.780294)

          Review request for hcatalog and Sushanth Sowmyan.

          Changes
          -------

          new patch created with --no-prefix

          Summary
          -------

          review of HCatalog-60, refactor HCatalog output stuff to support non file-based storage drivers

          This addresses bug HCatalog-60.
          https://issues.apache.org/jira/browse/HCatalog-60

          Diffs (updated)


          src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/DefaultOutputFormatContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/DefaultRecordWriterContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/FileOutputStorageDriver.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/FileRecordWriterContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputCommitter.java 8516e1a
          src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java d289714
          src/java/org/apache/hcatalog/mapreduce/HCatEximOutputCommitter.java 4e5c4d1
          src/java/org/apache/hcatalog/mapreduce/HCatEximOutputFormat.java 47ddae3
          src/java/org/apache/hcatalog/mapreduce/HCatOutputCommitter.java 973a4b6
          src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 7a99caa
          src/java/org/apache/hcatalog/mapreduce/HCatOutputStorageDriver.java 8ef0d25
          src/java/org/apache/hcatalog/mapreduce/HCatRecordWriter.java e71b9af
          src/java/org/apache/hcatalog/mapreduce/OutputCommitterContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/OutputFormatContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/RecordWriterContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/pig/HCatEximStorer.java 3f1db06
          src/java/org/apache/hcatalog/pig/HCatStorer.java 90d14c0
          src/java/org/apache/hcatalog/rcfile/RCFileOutputDriver.java 6efc4d9
          src/test/org/apache/hcatalog/mapreduce/HCatMapReduceTest.java 2dee111
          src/test/org/apache/hcatalog/mapreduce/TestHCatDynamicPartitioned.java dfc7a53
          src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 506ed4e
          src/test/org/apache/hcatalog/rcfile/TestRCFileOutputStorageDriver.java 4c4acfe

          Diff: https://reviews.apache.org/r/1861/diff

          Testing
          -------

          all unit tests passed

          Thanks,

          Francis

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1861/ ----------------------------------------------------------- (Updated 2011-09-30 14:36:34.780294) Review request for hcatalog and Sushanth Sowmyan. Changes ------- new patch created with --no-prefix Summary ------- review of HCatalog-60, refactor HCatalog output stuff to support non file-based storage drivers This addresses bug HCatalog-60. https://issues.apache.org/jira/browse/HCatalog-60 Diffs (updated) src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/DefaultOutputFormatContainer.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/DefaultRecordWriterContainer.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/FileOutputStorageDriver.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/FileRecordWriterContainer.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputCommitter.java 8516e1a src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java d289714 src/java/org/apache/hcatalog/mapreduce/HCatEximOutputCommitter.java 4e5c4d1 src/java/org/apache/hcatalog/mapreduce/HCatEximOutputFormat.java 47ddae3 src/java/org/apache/hcatalog/mapreduce/HCatOutputCommitter.java 973a4b6 src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 7a99caa src/java/org/apache/hcatalog/mapreduce/HCatOutputStorageDriver.java 8ef0d25 src/java/org/apache/hcatalog/mapreduce/HCatRecordWriter.java e71b9af src/java/org/apache/hcatalog/mapreduce/OutputCommitterContainer.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/OutputFormatContainer.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/RecordWriterContainer.java PRE-CREATION src/java/org/apache/hcatalog/pig/HCatEximStorer.java 3f1db06 src/java/org/apache/hcatalog/pig/HCatStorer.java 90d14c0 src/java/org/apache/hcatalog/rcfile/RCFileOutputDriver.java 6efc4d9 src/test/org/apache/hcatalog/mapreduce/HCatMapReduceTest.java 2dee111 src/test/org/apache/hcatalog/mapreduce/TestHCatDynamicPartitioned.java dfc7a53 src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 506ed4e src/test/org/apache/hcatalog/rcfile/TestRCFileOutputStorageDriver.java 4c4acfe Diff: https://reviews.apache.org/r/1861/diff Testing ------- all unit tests passed Thanks, Francis
          Hide
          Francis Liu added a comment -

          sorry about that. new patch created with --no-prefix

          Show
          Francis Liu added a comment - sorry about that. new patch created with --no-prefix
          Hide
          Alan Gates added a comment -

          I tried to apply the patch but it was not created with --no-prefix, so it won't apply properly.

          Show
          Alan Gates added a comment - I tried to apply the patch but it was not created with --no-prefix, so it won't apply properly.
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1861/
          -----------------------------------------------------------

          (Updated 2011-09-29 22:35:16.427726)

          Review request for hcatalog and Sushanth Sowmyan.

          Changes
          -------

          Added javadoc comments to new classes and new un-overriden methods. Added "Container" suffix to container class implementations.

          Summary
          -------

          review of HCatalog-60, refactor HCatalog output stuff to support non file-based storage drivers

          This addresses bug HCatalog-60.
          https://issues.apache.org/jira/browse/HCatalog-60

          Diffs (updated)


          src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/DefaultOutputFormatContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/DefaultRecordWriterContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/FileOutputStorageDriver.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/FileRecordWriterContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputCommitter.java 8516e1a
          src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java d289714
          src/java/org/apache/hcatalog/mapreduce/HCatEximOutputCommitter.java 4e5c4d1
          src/java/org/apache/hcatalog/mapreduce/HCatEximOutputFormat.java 47ddae3
          src/java/org/apache/hcatalog/mapreduce/HCatOutputCommitter.java 973a4b6
          src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 7a99caa
          src/java/org/apache/hcatalog/mapreduce/HCatOutputStorageDriver.java 8ef0d25
          src/java/org/apache/hcatalog/mapreduce/HCatRecordWriter.java e71b9af
          src/java/org/apache/hcatalog/mapreduce/OutputCommitterContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/OutputFormatContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/RecordWriterContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/pig/HCatEximStorer.java 3f1db06
          src/java/org/apache/hcatalog/pig/HCatStorer.java 90d14c0
          src/java/org/apache/hcatalog/rcfile/RCFileOutputDriver.java 6efc4d9
          src/test/org/apache/hcatalog/mapreduce/HCatMapReduceTest.java 2dee111
          src/test/org/apache/hcatalog/mapreduce/TestHCatDynamicPartitioned.java dfc7a53
          src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 506ed4e
          src/test/org/apache/hcatalog/rcfile/TestRCFileOutputStorageDriver.java 4c4acfe

          Diff: https://reviews.apache.org/r/1861/diff

          Testing
          -------

          all unit tests passed

          Thanks,

          Francis

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1861/ ----------------------------------------------------------- (Updated 2011-09-29 22:35:16.427726) Review request for hcatalog and Sushanth Sowmyan. Changes ------- Added javadoc comments to new classes and new un-overriden methods. Added "Container" suffix to container class implementations. Summary ------- review of HCatalog-60, refactor HCatalog output stuff to support non file-based storage drivers This addresses bug HCatalog-60. https://issues.apache.org/jira/browse/HCatalog-60 Diffs (updated) src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/DefaultOutputFormatContainer.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/DefaultRecordWriterContainer.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/FileOutputStorageDriver.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/FileRecordWriterContainer.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputCommitter.java 8516e1a src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java d289714 src/java/org/apache/hcatalog/mapreduce/HCatEximOutputCommitter.java 4e5c4d1 src/java/org/apache/hcatalog/mapreduce/HCatEximOutputFormat.java 47ddae3 src/java/org/apache/hcatalog/mapreduce/HCatOutputCommitter.java 973a4b6 src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 7a99caa src/java/org/apache/hcatalog/mapreduce/HCatOutputStorageDriver.java 8ef0d25 src/java/org/apache/hcatalog/mapreduce/HCatRecordWriter.java e71b9af src/java/org/apache/hcatalog/mapreduce/OutputCommitterContainer.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/OutputFormatContainer.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/RecordWriterContainer.java PRE-CREATION src/java/org/apache/hcatalog/pig/HCatEximStorer.java 3f1db06 src/java/org/apache/hcatalog/pig/HCatStorer.java 90d14c0 src/java/org/apache/hcatalog/rcfile/RCFileOutputDriver.java 6efc4d9 src/test/org/apache/hcatalog/mapreduce/HCatMapReduceTest.java 2dee111 src/test/org/apache/hcatalog/mapreduce/TestHCatDynamicPartitioned.java dfc7a53 src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 506ed4e src/test/org/apache/hcatalog/rcfile/TestRCFileOutputStorageDriver.java 4c4acfe Diff: https://reviews.apache.org/r/1861/diff Testing ------- all unit tests passed Thanks, Francis
          Hide
          Francis Liu added a comment -

          Alan, thanks for the comments. I've update the classes with Javadoc comments. Also I've add a Conatiner suffix to the class names for clarity. Let me know if I missed anything.

          Show
          Francis Liu added a comment - Alan, thanks for the comments. I've update the classes with Javadoc comments. Also I've add a Conatiner suffix to the class names for clarity. Let me know if I missed anything.
          Hide
          Alan Gates added a comment -

          Before being checked in the new classes in this patch need javadoc. There should be class level comments on all the new classes, particularly the new abstract classes, that explains where they fit in the class tree and what they do. New methods in these classes that aren't overriding super class methods need javadoc too.

          I don't understand the distinction between Default*Container and File*Container. Perhaps the javadocs requested above will resolve this.

          FileOutputFormat is a bad name for the default file handling output format, since a very well known class of that name exists already in MapReduce. It should be called HCatFileOutputFormat for clarity.

          Show
          Alan Gates added a comment - Before being checked in the new classes in this patch need javadoc. There should be class level comments on all the new classes, particularly the new abstract classes, that explains where they fit in the class tree and what they do. New methods in these classes that aren't overriding super class methods need javadoc too. I don't understand the distinction between Default*Container and File*Container. Perhaps the javadocs requested above will resolve this. FileOutputFormat is a bad name for the default file handling output format, since a very well known class of that name exists already in MapReduce. It should be called HCatFileOutputFormat for clarity.
          Hide
          Francis Liu added a comment -

          thanks for the update. BTW will we need to have the concept of containers for the input side as well for uniformity?

          Show
          Francis Liu added a comment - thanks for the update. BTW will we need to have the concept of containers for the input side as well for uniformity?
          Hide
          Sushanth Sowmyan added a comment -

          (As an update, I'm okay with this patch, I've just having a bit of difficulty setting up the e2e test harness ( in dir src/test/e2e/hcatalog/ ) right now (which is important to run for any major changes), and as soon as that is done, I should be able to +1 this)

          Show
          Sushanth Sowmyan added a comment - (As an update, I'm okay with this patch, I've just having a bit of difficulty setting up the e2e test harness ( in dir src/test/e2e/hcatalog/ ) right now (which is important to run for any major changes), and as soon as that is done, I should be able to +1 this)
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1861/
          -----------------------------------------------------------

          (Updated 2011-09-21 20:12:54.861369)

          Review request for hcatalog and Sushanth Sowmyan.

          Changes
          -------

          fixed a typo in a class name

          Summary
          -------

          review of HCatalog-60, refactor HCatalog output stuff to support non file-based storage drivers

          This addresses bug HCatalog-60.
          https://issues.apache.org/jira/browse/HCatalog-60

          Diffs (updated)


          src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitter.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/DefaultOutputFormat.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/DefaultRecordWriter.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/FileOutputCommitter.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/FileOutputFormat.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/FileOutputStorageDriver.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/FileRecordWriter.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputCommitter.java 8516e1a
          src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java d289714
          src/java/org/apache/hcatalog/mapreduce/HCatEximOutputCommitter.java 4e5c4d1
          src/java/org/apache/hcatalog/mapreduce/HCatEximOutputFormat.java 47ddae3
          src/java/org/apache/hcatalog/mapreduce/HCatOutputCommitter.java 973a4b6
          src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 7a99caa
          src/java/org/apache/hcatalog/mapreduce/HCatOutputStorageDriver.java 8ef0d25
          src/java/org/apache/hcatalog/mapreduce/HCatRecordWriter.java e71b9af
          src/java/org/apache/hcatalog/mapreduce/OutputCommitterContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/OutputFormatContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/RecordWriterContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/pig/HCatEximStorer.java 3f1db06
          src/java/org/apache/hcatalog/pig/HCatStorer.java 90d14c0
          src/java/org/apache/hcatalog/rcfile/RCFileOutputDriver.java 6efc4d9
          src/test/org/apache/hcatalog/mapreduce/HCatMapReduceTest.java 2dee111
          src/test/org/apache/hcatalog/mapreduce/TestHCatDynamicPartitioned.java dfc7a53
          src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 506ed4e
          src/test/org/apache/hcatalog/rcfile/TestRCFileOutputStorageDriver.java 4c4acfe

          Diff: https://reviews.apache.org/r/1861/diff

          Testing
          -------

          all unit tests passed

          Thanks,

          Francis

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1861/ ----------------------------------------------------------- (Updated 2011-09-21 20:12:54.861369) Review request for hcatalog and Sushanth Sowmyan. Changes ------- fixed a typo in a class name Summary ------- review of HCatalog-60, refactor HCatalog output stuff to support non file-based storage drivers This addresses bug HCatalog-60. https://issues.apache.org/jira/browse/HCatalog-60 Diffs (updated) src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitter.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/DefaultOutputFormat.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/DefaultRecordWriter.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/FileOutputCommitter.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/FileOutputFormat.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/FileOutputStorageDriver.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/FileRecordWriter.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputCommitter.java 8516e1a src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java d289714 src/java/org/apache/hcatalog/mapreduce/HCatEximOutputCommitter.java 4e5c4d1 src/java/org/apache/hcatalog/mapreduce/HCatEximOutputFormat.java 47ddae3 src/java/org/apache/hcatalog/mapreduce/HCatOutputCommitter.java 973a4b6 src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 7a99caa src/java/org/apache/hcatalog/mapreduce/HCatOutputStorageDriver.java 8ef0d25 src/java/org/apache/hcatalog/mapreduce/HCatRecordWriter.java e71b9af src/java/org/apache/hcatalog/mapreduce/OutputCommitterContainer.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/OutputFormatContainer.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/RecordWriterContainer.java PRE-CREATION src/java/org/apache/hcatalog/pig/HCatEximStorer.java 3f1db06 src/java/org/apache/hcatalog/pig/HCatStorer.java 90d14c0 src/java/org/apache/hcatalog/rcfile/RCFileOutputDriver.java 6efc4d9 src/test/org/apache/hcatalog/mapreduce/HCatMapReduceTest.java 2dee111 src/test/org/apache/hcatalog/mapreduce/TestHCatDynamicPartitioned.java dfc7a53 src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 506ed4e src/test/org/apache/hcatalog/rcfile/TestRCFileOutputStorageDriver.java 4c4acfe Diff: https://reviews.apache.org/r/1861/diff Testing ------- all unit tests passed Thanks, Francis
          Hide
          Francis Liu added a comment -

          fixed a typo in RecordWriterContainer classname

          Show
          Francis Liu added a comment - fixed a typo in RecordWriterContainer classname
          Hide
          Francis Liu added a comment -

          Minutes of meeting:

          -For the OutputFormatContainer having HCatRecord as the Value type. Agreed as correct since this is what users are expected to pass and StorageDriver.convert() is exptected to consume.
          -Description of this refactoring in a nutshell is:HCatOutputFormat, HCatRecordWriter, HCatOutputCommitter has been renamed to FileOutputFormatContainer,etc and a new thin HCatOutputFormat encapsulates the Container class delegating all calls to the underlying container (ie getRecordWriter, getOutputCommitter)
          -code path is essentially identical except for some changes such as moving output directory validation to OutputFormat.checkOutputSpecs()

          Show
          Francis Liu added a comment - Minutes of meeting: -For the OutputFormatContainer having HCatRecord as the Value type. Agreed as correct since this is what users are expected to pass and StorageDriver.convert() is exptected to consume. -Description of this refactoring in a nutshell is:HCatOutputFormat, HCatRecordWriter, HCatOutputCommitter has been renamed to FileOutputFormatContainer,etc and a new thin HCatOutputFormat encapsulates the Container class delegating all calls to the underlying container (ie getRecordWriter, getOutputCommitter) -code path is essentially identical except for some changes such as moving output directory validation to OutputFormat.checkOutputSpecs()
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1861/
          -----------------------------------------------------------

          (Updated 2011-09-21 17:33:45.934922)

          Review request for hcatalog and Sushanth Sowmyan.

          Changes
          -------

          rebased onto updates in trunk

          Summary
          -------

          review of HCatalog-60, refactor HCatalog output stuff to support non file-based storage drivers

          This addresses bug HCatalog-60.
          https://issues.apache.org/jira/browse/HCatalog-60

          Diffs (updated)


          src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitter.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/DefaultOutputFormat.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/DefaultRecordWriter.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/FileOutputCommitter.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/FileOutputFormat.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/FileOutputStorageDriver.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/FileRecordWriter.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputCommitter.java 8516e1a
          src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java d289714
          src/java/org/apache/hcatalog/mapreduce/HCatEximOutputCommitter.java 4e5c4d1
          src/java/org/apache/hcatalog/mapreduce/HCatEximOutputFormat.java 47ddae3
          src/java/org/apache/hcatalog/mapreduce/HCatOutputCommitter.java 973a4b6
          src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 7a99caa
          src/java/org/apache/hcatalog/mapreduce/HCatOutputStorageDriver.java 8ef0d25
          src/java/org/apache/hcatalog/mapreduce/HCatRecordWriter.java e71b9af
          src/java/org/apache/hcatalog/mapreduce/OutputCommitterContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/OutputFormatContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/RecordWriterBaseContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/pig/HCatEximStorer.java 3f1db06
          src/java/org/apache/hcatalog/pig/HCatStorer.java 90d14c0
          src/java/org/apache/hcatalog/rcfile/RCFileOutputDriver.java 6efc4d9
          src/test/org/apache/hcatalog/mapreduce/HCatMapReduceTest.java 2dee111
          src/test/org/apache/hcatalog/mapreduce/TestHCatDynamicPartitioned.java dfc7a53
          src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 506ed4e
          src/test/org/apache/hcatalog/rcfile/TestRCFileOutputStorageDriver.java 4c4acfe

          Diff: https://reviews.apache.org/r/1861/diff

          Testing
          -------

          all unit tests passed

          Thanks,

          Francis

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1861/ ----------------------------------------------------------- (Updated 2011-09-21 17:33:45.934922) Review request for hcatalog and Sushanth Sowmyan. Changes ------- rebased onto updates in trunk Summary ------- review of HCatalog-60, refactor HCatalog output stuff to support non file-based storage drivers This addresses bug HCatalog-60. https://issues.apache.org/jira/browse/HCatalog-60 Diffs (updated) src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitter.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/DefaultOutputFormat.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/DefaultRecordWriter.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/FileOutputCommitter.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/FileOutputFormat.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/FileOutputStorageDriver.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/FileRecordWriter.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputCommitter.java 8516e1a src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java d289714 src/java/org/apache/hcatalog/mapreduce/HCatEximOutputCommitter.java 4e5c4d1 src/java/org/apache/hcatalog/mapreduce/HCatEximOutputFormat.java 47ddae3 src/java/org/apache/hcatalog/mapreduce/HCatOutputCommitter.java 973a4b6 src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 7a99caa src/java/org/apache/hcatalog/mapreduce/HCatOutputStorageDriver.java 8ef0d25 src/java/org/apache/hcatalog/mapreduce/HCatRecordWriter.java e71b9af src/java/org/apache/hcatalog/mapreduce/OutputCommitterContainer.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/OutputFormatContainer.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/RecordWriterBaseContainer.java PRE-CREATION src/java/org/apache/hcatalog/pig/HCatEximStorer.java 3f1db06 src/java/org/apache/hcatalog/pig/HCatStorer.java 90d14c0 src/java/org/apache/hcatalog/rcfile/RCFileOutputDriver.java 6efc4d9 src/test/org/apache/hcatalog/mapreduce/HCatMapReduceTest.java 2dee111 src/test/org/apache/hcatalog/mapreduce/TestHCatDynamicPartitioned.java dfc7a53 src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 506ed4e src/test/org/apache/hcatalog/rcfile/TestRCFileOutputStorageDriver.java 4c4acfe Diff: https://reviews.apache.org/r/1861/diff Testing ------- all unit tests passed Thanks, Francis
          Hide
          Francis Liu added a comment -

          updated patch to match changes in trunk

          Show
          Francis Liu added a comment - updated patch to match changes in trunk
          Hide
          Francis Liu added a comment -

          Yes, let's document what we decide on. I'll email you about availability.

          As for the HCatRecord. I don't see this as an issue since this is the top level "record" class and users are expected to read and write using HCatRecords.

          Show
          Francis Liu added a comment - Yes, let's document what we decide on. I'll email you about availability. As for the HCatRecord. I don't see this as an issue since this is the top level "record" class and users are expected to read and write using HCatRecords.
          Hide
          Sushanth Sowmyan added a comment -

          Hi Francis,

          For most part, the patch looks okay, I've not been rigourous enough..

          The only comment I had regarding your patch is that I see that you use HCatRecord as your Value type for specification of underlying OutputFormatContainer , and that is probably problematic, and should have just been Writable - still need to check my instinct on this though.

          Yes, it's probably better if we do meet and go through this in person, although we do want to capture the results of that on the jira.

          Show
          Sushanth Sowmyan added a comment - Hi Francis, For most part, the patch looks okay, I've not been rigourous enough.. The only comment I had regarding your patch is that I see that you use HCatRecord as your Value type for specification of underlying OutputFormatContainer , and that is probably problematic, and should have just been Writable - still need to check my instinct on this though. Yes, it's probably better if we do meet and go through this in person, although we do want to capture the results of that on the jira.
          Hide
          Francis Liu added a comment -

          Sushanth, just following this up, would it be beneficial for you to if I walk you through the changes in person?

          Show
          Francis Liu added a comment - Sushanth, just following this up, would it be beneficial for you to if I walk you through the changes in person?
          Hide
          Francis Liu added a comment -

          Created a reviewboard request, I used the hcatalog-git repository instead. I understand they are mirrors so things should be ok. Let me know if this is not satisfactory.

          https://reviews.apache.org/r/1861/

          Show
          Francis Liu added a comment - Created a reviewboard request, I used the hcatalog-git repository instead. I understand they are mirrors so things should be ok. Let me know if this is not satisfactory. https://reviews.apache.org/r/1861/
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1861/
          -----------------------------------------------------------

          Review request for hcatalog and Sushanth Sowmyan.

          Summary
          -------

          review of HCatalog-60, refactor HCatalog output stuff to support non file-based storage drivers

          This addresses bug HCatalog-60.
          https://issues.apache.org/jira/browse/HCatalog-60

          Diffs


          src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitter.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/DefaultOutputFormat.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/DefaultRecordWriter.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/FileOutputCommitter.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/FileOutputFormat.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/FileOutputStorageDriver.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/FileRecordWriter.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputCommitter.java 8516e1a
          src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java d289714
          src/java/org/apache/hcatalog/mapreduce/HCatEximOutputCommitter.java 4e5c4d1
          src/java/org/apache/hcatalog/mapreduce/HCatEximOutputFormat.java 47ddae3
          src/java/org/apache/hcatalog/mapreduce/HCatOutputCommitter.java 973a4b6
          src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 7a99caa
          src/java/org/apache/hcatalog/mapreduce/HCatOutputStorageDriver.java 8ef0d25
          src/java/org/apache/hcatalog/mapreduce/HCatRecordWriter.java e71b9af
          src/java/org/apache/hcatalog/mapreduce/OutputCommitterContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/OutputFormatContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/mapreduce/RecordWriterBaseContainer.java PRE-CREATION
          src/java/org/apache/hcatalog/pig/HCatEximStorer.java 3f1db06
          src/java/org/apache/hcatalog/pig/HCatStorer.java 90d14c0
          src/java/org/apache/hcatalog/rcfile/RCFileOutputDriver.java 6efc4d9
          src/test/org/apache/hcatalog/mapreduce/HCatMapReduceTest.java 2dee111
          src/test/org/apache/hcatalog/mapreduce/TestHCatDynamicPartitioned.java dfc7a53
          src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 506ed4e
          src/test/org/apache/hcatalog/rcfile/TestRCFileOutputStorageDriver.java 4c4acfe

          Diff: https://reviews.apache.org/r/1861/diff

          Testing
          -------

          all unit tests passed

          Thanks,

          Francis

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1861/ ----------------------------------------------------------- Review request for hcatalog and Sushanth Sowmyan. Summary ------- review of HCatalog-60, refactor HCatalog output stuff to support non file-based storage drivers This addresses bug HCatalog-60. https://issues.apache.org/jira/browse/HCatalog-60 Diffs src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitter.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/DefaultOutputFormat.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/DefaultRecordWriter.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/FileOutputCommitter.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/FileOutputFormat.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/FileOutputStorageDriver.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/FileRecordWriter.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputCommitter.java 8516e1a src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java d289714 src/java/org/apache/hcatalog/mapreduce/HCatEximOutputCommitter.java 4e5c4d1 src/java/org/apache/hcatalog/mapreduce/HCatEximOutputFormat.java 47ddae3 src/java/org/apache/hcatalog/mapreduce/HCatOutputCommitter.java 973a4b6 src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 7a99caa src/java/org/apache/hcatalog/mapreduce/HCatOutputStorageDriver.java 8ef0d25 src/java/org/apache/hcatalog/mapreduce/HCatRecordWriter.java e71b9af src/java/org/apache/hcatalog/mapreduce/OutputCommitterContainer.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/OutputFormatContainer.java PRE-CREATION src/java/org/apache/hcatalog/mapreduce/RecordWriterBaseContainer.java PRE-CREATION src/java/org/apache/hcatalog/pig/HCatEximStorer.java 3f1db06 src/java/org/apache/hcatalog/pig/HCatStorer.java 90d14c0 src/java/org/apache/hcatalog/rcfile/RCFileOutputDriver.java 6efc4d9 src/test/org/apache/hcatalog/mapreduce/HCatMapReduceTest.java 2dee111 src/test/org/apache/hcatalog/mapreduce/TestHCatDynamicPartitioned.java dfc7a53 src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 506ed4e src/test/org/apache/hcatalog/rcfile/TestRCFileOutputStorageDriver.java 4c4acfe Diff: https://reviews.apache.org/r/1861/diff Testing ------- all unit tests passed Thanks, Francis
          Hide
          Francis Liu added a comment -

          Sushanth, while trying to upload the patch up getting this error:

          "Something broke! (Error 500)

          It appears something broke when you tried to go to here. This is either a bug in Review Board or a server configuration error. Please report this to your administrator."

          Apart from this patch this is the config I used:

          Repository: hcatalog
          base directory: /trunk

          Does this mean there's something wrong with my patch?

          Show
          Francis Liu added a comment - Sushanth, while trying to upload the patch up getting this error: "Something broke! (Error 500) It appears something broke when you tried to go to here. This is either a bug in Review Board or a server configuration error. Please report this to your administrator." Apart from this patch this is the config I used: Repository: hcatalog base directory: /trunk Does this mean there's something wrong with my patch?
          Hide
          Sushanth Sowmyan added a comment -

          (Updating to let you know that I'm looking through this - In the meanwhile, could you please also create a reviewboard submission for this? Makes community reviews easier : https://reviews.apache.org/ ( You can use https://reviews.apache.org/r/1197/ as a template ) )

          Show
          Sushanth Sowmyan added a comment - (Updating to let you know that I'm looking through this - In the meanwhile, could you please also create a reviewboard submission for this? Makes community reviews easier : https://reviews.apache.org/ ( You can use https://reviews.apache.org/r/1197/ as a template ) )
          Hide
          Francis Liu added a comment -

          updated patch

          Show
          Francis Liu added a comment - updated patch
          Hide
          Francis Liu added a comment -

          I tried to tread carefully but a lot of the logic was in HCatOutputCommitter and HCatRecordWriter. I added logic to make the OutputCommitter and RecordWriter returned by HCatOutputFormat pluggable. In light of that I added FileOutputFormat and DefaultOutputFormat. The former used for file based storage. There were some things that could be refactored but I tried to transfer as is a lot of the logic from the HCat* classes to the File* classes and the FileOuputStorageDriver. All of these changes are not exposed to the user except for FileOutputStorageDriver which file-based output storage drivers should extend.

          Hopefully you guys can take a look at this soon since this is a big change as well as a blocker for the hbase drivers.

          Show
          Francis Liu added a comment - I tried to tread carefully but a lot of the logic was in HCatOutputCommitter and HCatRecordWriter. I added logic to make the OutputCommitter and RecordWriter returned by HCatOutputFormat pluggable. In light of that I added FileOutputFormat and DefaultOutputFormat. The former used for file based storage. There were some things that could be refactored but I tried to transfer as is a lot of the logic from the HCat* classes to the File* classes and the FileOuputStorageDriver. All of these changes are not exposed to the user except for FileOutputStorageDriver which file-based output storage drivers should extend. Hopefully you guys can take a look at this soon since this is a big change as well as a blocker for the hbase drivers.
          Hide
          Francis Liu added a comment -

          this was the original patch which was agreed upon not to use.

          Show
          Francis Liu added a comment - this was the original patch which was agreed upon not to use.
          Hide
          Francis Liu added a comment -

          Following this up. Need a solution since this is blocking HBase storage driver development.

          We can also go for the temporary solution as originally described (checki if getLocation returns null).

          Show
          Francis Liu added a comment - Following this up. Need a solution since this is blocking HBase storage driver development. We can also go for the temporary solution as originally described (checki if getLocation returns null).
          Hide
          Francis Liu added a comment -

          Here is one idea I have for solving this issue. We create wrapper storage drivers which file-based storage drivers should subclass if they'd like to take advantage of already implemented file-based features/behavior.

          public class FileInputStorageDriverWrapper {
          public FileInputStorageDriverWrapper(InputStorageDriver is)

          { ... }

          //implement file based features here, if need be creating wrappers for InputFormat, OutputCommiter, etc
          //methods are final so that they don't get overriden
          }

          User wishing to create a file-based driver extends the wrapper

          public class RCFileInputStorageDriver extends FileInputStorageDriverWrapper {
          public RCFileInputStorageDriver()

          { super(new ActualRCFileInputStorageDriver()); }

          public static class ActualRCFileInputStorageDriver

          { //actual implementation }

          }

          Another approach would be to have a way for HCatalog to determine the type (ie file-based) of storage driver and include the necessary behavior for such a driver. This approach to me requires a better grasp of the current and future storage drivers that will be implemented and hence it feels a bit to early to go for such a solution. Let me know what you guys think.

          Show
          Francis Liu added a comment - Here is one idea I have for solving this issue. We create wrapper storage drivers which file-based storage drivers should subclass if they'd like to take advantage of already implemented file-based features/behavior. public class FileInputStorageDriverWrapper { public FileInputStorageDriverWrapper(InputStorageDriver is) { ... } //implement file based features here, if need be creating wrappers for InputFormat, OutputCommiter, etc //methods are final so that they don't get overriden } User wishing to create a file-based driver extends the wrapper public class RCFileInputStorageDriver extends FileInputStorageDriverWrapper { public RCFileInputStorageDriver() { super(new ActualRCFileInputStorageDriver()); } public static class ActualRCFileInputStorageDriver { //actual implementation } } Another approach would be to have a way for HCatalog to determine the type (ie file-based) of storage driver and include the necessary behavior for such a driver. This approach to me requires a better grasp of the current and future storage drivers that will be implemented and hence it feels a bit to early to go for such a solution. Let me know what you guys think.
          Hide
          Ashutosh Chauhan added a comment -

          BTW where I can read about dynamic partitioning?

          If you do ant docs that will generate documentation and then you can find src/docs/build/site/dynpartition.html

          Show
          Ashutosh Chauhan added a comment - BTW where I can read about dynamic partitioning? If you do ant docs that will generate documentation and then you can find src/docs/build/site/dynpartition.html
          Hide
          Francis Liu added a comment -

          These all look like behaviors for file-based storage systems. Wouldn't it be possible to push these behaviors down to a storage driver base class (ie FileStorageDriverSkeleton, FileStorageDriverSkeleton.OutputCommiter, etc) which contains all the file related logic? File-based storage drivers looking to take advantage of such features can inherit from that?

          BTW where I can read about dynamic partitioning?

          Show
          Francis Liu added a comment - These all look like behaviors for file-based storage systems. Wouldn't it be possible to push these behaviors down to a storage driver base class (ie FileStorageDriverSkeleton, FileStorageDriverSkeleton.OutputCommiter, etc) which contains all the file related logic? File-based storage drivers looking to take advantage of such features can inherit from that? BTW where I can read about dynamic partitioning?
          Hide
          Sushanth Sowmyan added a comment -

          Additional comment here - the underlying location is a bit too munged in right now, and needs to be treated with care. I can think of cases beyond the ones Ashutosh mentioned:

          a) Unpartitioned table write - uses a temporary location and copies in
          b) Dynamic partitioning anything - writes out to a temporary location and then moves - heck, the scan for what got written out previously bases on this
          c) Checks to see if pre-existing partitions exist (minor, and irrelevant in cases where it isn't a FS)

          On top of this, when we send the location to the hive metastore when we register the partition, hive can/will at least instantiate a FileSystem on the given location uri, so it needs to be valid at the time we do add_partition.

          Cassandra seems to get around that by implementing a CassandraFS implementation of FileSystem so that hive works seamlessly on top of it. That approach would work here too, but I'm not convinced that that's ideal either. What's needed here is not a special usecase for RCFile alone, it's something that I'd like to see a bit of discussion on. It's why I tried to initially introduce a concept of a PostProcessor for har in HCATALOG-42 before finally pulling it out due to it being not fleshed out enough.

          Show
          Sushanth Sowmyan added a comment - Additional comment here - the underlying location is a bit too munged in right now, and needs to be treated with care. I can think of cases beyond the ones Ashutosh mentioned: a) Unpartitioned table write - uses a temporary location and copies in b) Dynamic partitioning anything - writes out to a temporary location and then moves - heck, the scan for what got written out previously bases on this c) Checks to see if pre-existing partitions exist (minor, and irrelevant in cases where it isn't a FS) On top of this, when we send the location to the hive metastore when we register the partition, hive can/will at least instantiate a FileSystem on the given location uri, so it needs to be valid at the time we do add_partition. Cassandra seems to get around that by implementing a CassandraFS implementation of FileSystem so that hive works seamlessly on top of it. That approach would work here too, but I'm not convinced that that's ideal either. What's needed here is not a special usecase for RCFile alone, it's something that I'd like to see a bit of discussion on. It's why I tried to initially introduce a concept of a PostProcessor for har in HCATALOG-42 before finally pulling it out due to it being not fleshed out enough.
          Hide
          Olga Natkovich added a comment -

          I don't think it is resolved as somebody need to do the work of moving that code around. Francis, would you be able to submit the patch for this?

          Show
          Olga Natkovich added a comment - I don't think it is resolved as somebody need to do the work of moving that code around. Francis, would you be able to submit the patch for this?
          Hide
          Francis Liu added a comment -

          Excellent, that would be ideal. Should I mark this as resolved then?

          Show
          Francis Liu added a comment - Excellent, that would be ideal. Should I mark this as resolved then?
          Hide
          Ashutosh Chauhan added a comment -

          Ideally, HCatOutputCommitter should not do anything special in commitJob/abortJob api as it is doing currently, as this special handling makes assumption about underlying storage. Deletion as it is currently done should be a responsibility of underlying baseCommitter not of HCatOutputCommitter. Since, FileOutputCommitter don't do this, we are doing it here. I will suggest to move all of this deletion code to RCFileOutputCommitter and keep this one clean here. This way we need not to worry about location string anymore. Similarly, in commitJob we should rely on underlying committer to create _SUCCESS file and not on HCatOutputCommitter. So, here as well we shall move this code to RCFileOutputCommitter.

          Show
          Ashutosh Chauhan added a comment - Ideally, HCatOutputCommitter should not do anything special in commitJob/abortJob api as it is doing currently, as this special handling makes assumption about underlying storage. Deletion as it is currently done should be a responsibility of underlying baseCommitter not of HCatOutputCommitter. Since, FileOutputCommitter don't do this, we are doing it here. I will suggest to move all of this deletion code to RCFileOutputCommitter and keep this one clean here. This way we need not to worry about location string anymore. Similarly, in commitJob we should rely on underlying committer to create _SUCCESS file and not on HCatOutputCommitter. So, here as well we shall move this code to RCFileOutputCommitter.
          Hide
          Francis Liu added a comment -

          previous patch contained other changes not for this bug

          Show
          Francis Liu added a comment - previous patch contained other changes not for this bug
          Hide
          Francis Liu added a comment -

          submitted a patch taken based on source code from trunk

          Show
          Francis Liu added a comment - submitted a patch taken based on source code from trunk

            People

            • Assignee:
              Francis Liu
              Reporter:
              Francis Liu
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development