> On 2012-02-07 19:01:36, Alan Gates wrote: > > src/java/org/apache/hadoop/mapred/HCatMapRedUtil.java, line 1 > > <https://reviews.apache.org/r/3775/diff/1/?file=72848#file72848line1> > > > > It seems odd to put this into the o.a.hadoop.mapred package. Are you > > calling some package scoped function or something?
Sadly yes. mapred.JobContext and mapred.TaskAttemptContext both have package-private constructor. Nasty but ound no other way around it. > On 2012-02-07 19:01:36, Alan Gates wrote: > > src/java/org/apache/hcatalog/common/HCatUtil.java, line 465 > > <https://reviews.apache.org/r/3775/diff/1/?file=72850#file72850line465> > > > > Some javadoc here on what should be passed into each of these values > > would be helpful. They are class names I assume? Yep. Will add javadoc in the next patch. As I've mentioned in the jira there are a few things that are missing. I'll make sure this gets documented. > On 2012-02-07 19:01:36, Alan Gates wrote: > > src/java/org/apache/hcatalog/common/HCatUtil.java, line 509 > > <https://reviews.apache.org/r/3775/diff/1/?file=72850#file72850line509> > > > > We shouldn't need code at all to pull out ISD and OSD info, since we're > > removing them, correct? So I'm confused why a new function is being added > > here to pull that info out. Yeah I'll remove it. OutputStorageDriver cleansing is only partially done since so a lot of things will break. Probably good to do that once everything is committed and working. > On 2012-02-07 19:01:36, Alan Gates wrote: > > src/java/org/apache/hcatalog/mapreduce/FileRecordWriterContainer.java, line > > 161 > > <https://reviews.apache.org/r/3775/diff/1/?file=72857#file72857line161> > > > > What is better that needs to be put here? I'll be more elaborate. We shouldn't pass a NULL reporter as much as possible or else the underlying recorderWriter won't be able to report progress to the task tracker. One of the things I still have to do. > On 2012-02-07 19:01:36, Alan Gates wrote: > > src/java/org/apache/hcatalog/mapreduce/FileRecordWriterContainer.java, line > > 264 > > <https://reviews.apache.org/r/3775/diff/1/?file=72857#file72857line264> > > > > We should add this before we commit the patch, as it should speed > > things up quite a bit. I was hoping to make that change during integration with Sushanth's HCatRecord patch. Should be easy enough to do before that as well. > On 2012-02-07 19:01:36, Alan Gates wrote: > > src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java, line 27 > > <https://reviews.apache.org/r/3775/diff/1/?file=72858#file72858line27> > > > > Some javadoc on the purpose of this class would be good. I'm assuming > > the purpose is to be a stand in StorageHandler in the HDFS storage case > > where there isn't a storage handler. > > > > Once we have moved getAuthorizationProvider from HCatStorageHandler to > > HiveStorageHandler will this class extend HiveStorageHandler? Correct, it's a StorageHandler for orphan serDe,IF,OF groups. There are four methods the HCatStorageHandler add 1. configureOuputJobProperties() 2. configureInputJobProperties() 3. getAuthorizationProvider() 4. getOuputFormatContainer() The first three methods HCatStorageHandler is only standing in for HiveStorageHandler. The 4th is used to determine what type of storage system the storageHandler represents. Hive doesn't have this and that's why they had to do a lot of hacky things to get HBase working (ie have HBaseSplit extend FileSplit) even though they have some notion of containers (ie HiveOutputFormat). We can have this as a stand in for now until we decide on the final solution HiveStorageHandler will implement. - Francis ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3775/#review4867 ----------------------------------------------------------- On 2012-02-07 17:27:58, Francis Liu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3775/ > ----------------------------------------------------------- > > (Updated 2012-02-07 17:27:58) > > > Review request for hcatalog, Alan Gates and Sushanth Sowmyan. > > > Summary > ------- > > First drop. See HCATALOG-240. > > > This addresses bug HCATALOG-240. > https://issues.apache.org/jira/browse/HCATALOG-240 > > > Diffs > ----- > > src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java > f37c5fd > src/java/org/apache/hcatalog/mapreduce/DefaultOutputFormatContainer.java > 13fa8ac > src/java/org/apache/hcatalog/mapreduce/DefaultRecordWriterContainer.java > 7c90737 > src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java > 9c5fdd3 > src/java/org/apache/hadoop/mapred/HCatMapRedUtil.java PRE-CREATION > src/java/org/apache/hcatalog/cli/SemanticAnalysis/CreateTableHook.java > 9d98f50 > src/java/org/apache/hcatalog/common/HCatUtil.java 13b56fd > src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java > e91ed0f > src/java/org/apache/hcatalog/mapreduce/FileOutputStorageDriver.java 6596264 > src/java/org/apache/hcatalog/mapreduce/FileRecordWriterContainer.java > c72cb4f > src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java > PRE-CREATION > src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1b2f9a4 > src/java/org/apache/hcatalog/mapreduce/HCatEximInputFormat.java 90c5671 > src/java/org/apache/hcatalog/mapreduce/HCatEximInputFormat.java.broken > PRE-CREATION > src/java/org/apache/hcatalog/mapreduce/HCatEximOutputCommitter.java 1ba8ccd > src/java/org/apache/hcatalog/mapreduce/HCatEximOutputCommitter.java.broken > PRE-CREATION > src/java/org/apache/hcatalog/mapreduce/HCatEximOutputFormat.java ed1ba66 > src/java/org/apache/hcatalog/mapreduce/HCatEximOutputFormat.java.broken > PRE-CREATION > src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java d7eab9e > src/java/org/apache/hcatalog/mapreduce/HCatOutputStorageDriver.java 566943f > src/java/org/apache/hcatalog/mapreduce/HCatStorageHandler.java PRE-CREATION > src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java e65f1d0 > src/java/org/apache/hcatalog/mapreduce/OutputCommitterContainer.java > 5f0585a > src/java/org/apache/hcatalog/mapreduce/OutputFormatContainer.java f73a6dc > src/java/org/apache/hcatalog/mapreduce/OutputJobInfo.java 9830190 > src/java/org/apache/hcatalog/mapreduce/ProgressReporter.java PRE-CREATION > src/java/org/apache/hcatalog/mapreduce/RecordWriterContainer.java 9a93bfa > src/java/org/apache/hcatalog/mapreduce/StorerInfo.java 8785ffb > src/java/org/apache/hcatalog/pig/HCatEximLoader.java a36f808 > src/java/org/apache/hcatalog/pig/HCatEximLoader.java.broken PRE-CREATION > src/java/org/apache/hcatalog/pig/HCatEximStorer.java b7bee44 > src/java/org/apache/hcatalog/pig/HCatEximStorer.java.broken PRE-CREATION > src/test/org/apache/hcatalog/cli/DummyStorageHandler.java 0e9565c > src/test/org/apache/hcatalog/cli/TestStorageHandlerProperties.java df8f5a5 > src/test/org/apache/hcatalog/cli/TestStorageHandlerProperties.java.broken > PRE-CREATION > src/test/org/apache/hcatalog/mapreduce/TestHCatEximInputFormat.java 9d94c7b > src/test/org/apache/hcatalog/mapreduce/TestHCatEximInputFormat.java.broken > PRE-CREATION > src/test/org/apache/hcatalog/mapreduce/TestHCatEximOutputFormat.java > 99ec02b > src/test/org/apache/hcatalog/mapreduce/TestHCatEximOutputFormat.java.broken > PRE-CREATION > src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 5df0146 > src/test/org/apache/hcatalog/pig/TestHCatEximLoader.java 7691fb9 > src/test/org/apache/hcatalog/pig/TestHCatEximLoader.java.broken > PRE-CREATION > src/test/org/apache/hcatalog/pig/TestHCatEximStorer.java c3ba19d > src/test/org/apache/hcatalog/pig/TestHCatEximStorer.java.broken > PRE-CREATION > storage-drivers/build.xml efd26d0 > > Diff: https://reviews.apache.org/r/3775/diff > > > Testing > ------- > > > Thanks, > > Francis > >
