----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20973/#review41939 -----------------------------------------------------------
Few comments already in jira. src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/MRTaskContext.java <https://reviews.apache.org/r/20973/#comment75596> In what case would counter be null after a getCounter() call? It seems to always create a counter if it does not exist. Do we need this check in all methods? Can it be removed? src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PigRecordReader.java <https://reviews.apache.org/r/20973/#comment75598> If counter was initialized to -1, then lot other places will have problem (Spill counters for example). We were reporting counters wrong all along by a count of 1? Looking at GenericCounter.java in hadoop it is initialized to 0 only. I haven't stepped through and checked if GenericCounter is being used though it looks like that will be the case from a glance at the code. src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POStore.java <https://reviews.apache.org/r/20973/#comment75597> if (countername != null) - Rohini Palaniswamy On May 1, 2014, 7:37 p.m., Cheolsoo Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20973/ > ----------------------------------------------------------- > > (Updated May 1, 2014, 7:37 p.m.) > > > Review request for pig, Daniel Dai and Rohini Palaniswamy. > > > Bugs: PIG-3914 > https://issues.apache.org/jira/browse/PIG-3914 > > > Repository: pig-git > > > Description > ------- > > This is a follow-up of PIG-3860. > > The patch includes the following changes- > 1) Converts TaskContext to an abstract class and adds MRTaskContext, > FetchTaskContext, and TezTaskContext as subclasses. > 2) Changes "PigStatusReporter.getCounter() && counter.increment()" to > "PigStatusReporter.incrCounter()" for MULTI_INPUTS_RECORD_COUNTER and > MULTI_STORE_RECORD_COUNTER. > > > Diffs > ----- > > > shims/src/hadoop20/org/apache/pig/backend/hadoop/executionengine/shims/TaskContext.java > 6276ccd > > shims/src/hadoop23/org/apache/pig/backend/hadoop/executionengine/shims/TaskContext.java > 46cb8f4 > src/org/apache/pig/backend/hadoop/executionengine/TaskContext.java e69de29 > src/org/apache/pig/backend/hadoop/executionengine/fetch/FetchLauncher.java > 275ae50 > > src/org/apache/pig/backend/hadoop/executionengine/fetch/FetchTaskContext.java > e69de29 > > src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/MRTaskContext.java > e69de29 > > src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/MapReducePOStoreImpl.java > 5746b41 > > src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PigCombiner.java > 776096b > > src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PigGenericMapBase.java > c2b51d8 > > src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PigGenericMapReduce.java > 94dcbdd > > src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PigRecordReader.java > 4b17228 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POStore.java > 5b288dd > src/org/apache/pig/backend/hadoop/executionengine/tez/PigProcessor.java > 0cfee16 > src/org/apache/pig/backend/hadoop/executionengine/tez/TezTaskContext.java > e69de29 > src/org/apache/pig/tools/pigstats/PigStatusReporter.java 2e4abbf > > Diff: https://reviews.apache.org/r/20973/diff/ > > > Testing > ------- > > All unit tests pass. > > > Thanks, > > Cheolsoo Park > >
