No need to add null checks. Thanks for explanation. Please update the patch with latest trunk code and this makes me easy to test the code.
Best Regards Kelly Zhang/Zhang,Liyun -----Original Message----- From: Nandor Kollar [mailto:nore...@reviews.apache.org] On Behalf Of Nandor Kollar Sent: Tuesday, June 13, 2017 3:52 PM To: Rohini Palaniswamy <rohini.adi...@gmail.com>; Zhang, Liyun <liyun.zh...@intel.com>; Adam Szita <sz...@cloudera.com> Cc: Zhang, Liyun <liyun.zh...@intel.com>; pig <dev@pig.apache.org>; Nandor Kollar <nkol...@cloudera.com> Subject: Re: Review Request 59530: PIG-5157 Upgrade to Spark 2.0 > On June 13, 2017, 7:33 a.m., kelly zhang wrote: > > src/org/apache/pig/tools/pigstats/spark/SparkJobStats1.java > > Lines 60-63 (patched) > > <https://reviews.apache.org/r/59530/diff/4/?file=1749088#file1749088line60> > > > > why > > inputMetricExists,outputMetricExist,shuffleReadMetricExist,shuffleWriteMetricExist > > are deleted in SparkJobStats2.java? These metrics are no longer Optional, but have an initialized value. For example this is the shuffleWriteMetrics: val shuffleWriteMetrics: ShuffleWriteMetrics = new ShuffleWriteMetrics(); I don't think we need to check for the existence of these metrics, they are initialized all the time, but if you think it is better to check for null before using them, I can add null checks. for it. - Nandor ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59530/#review177699 ----------------------------------------------------------- On June 12, 2017, 9:20 p.m., Nandor Kollar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59530/ > ----------------------------------------------------------- > > (Updated June 12, 2017, 9:20 p.m.) > > > Review request for pig, liyun zhang, Rohini Palaniswamy, and Adam Szita. > > > Repository: pig-git > > > Description > ------- > > Upgrade to Spark 2.1 API using shims. > > > Diffs > ----- > > build.xml 4040fcec8f88d448ed7442461fbf0dea8cd1136e > ivy.xml 971724380f086d214ce62c7ab7879b08b6926802 > ivy/libraries.properties a0eb00acd2df42324540df4a9d762c64c608a6d3 > > src/org/apache/pig/backend/hadoop/executionengine/spark/FlatMapFunctionAdapter.java > PRE-CREATION > > src/org/apache/pig/backend/hadoop/executionengine/spark/JobMetricsListener.java > f81341233447203abc4800cc7b22a4f419e10262 > > src/org/apache/pig/backend/hadoop/executionengine/spark/PairFlatMapFunctionAdapter.java > PRE-CREATION > src/org/apache/pig/backend/hadoop/executionengine/spark/SparkLauncher.java > c6351e01a48f297ea2e432401ffd65c4f27f8078 > src/org/apache/pig/backend/hadoop/executionengine/spark/SparkShim.java > PRE-CREATION > src/org/apache/pig/backend/hadoop/executionengine/spark/SparkShim1.java > PRE-CREATION > src/org/apache/pig/backend/hadoop/executionengine/spark/SparkShim2.java > PRE-CREATION > > src/org/apache/pig/backend/hadoop/executionengine/spark/converter/CollectedGroupConverter.java > 83311dfa5bb25209a5366c2db7e8d483c31d94cd > > src/org/apache/pig/backend/hadoop/executionengine/spark/converter/FRJoinConverter.java > 382258e7ff9105aa397c5a2888df0c11e9562ec9 > > src/org/apache/pig/backend/hadoop/executionengine/spark/converter/ForEachConverter.java > b58415e7e18ca4cf1331beef06e9214600a51424 > > src/org/apache/pig/backend/hadoop/executionengine/spark/converter/GlobalRearrangeConverter.java > f571b808839c2de9415a3e8e4b229a7f4b2eebd7 > > src/org/apache/pig/backend/hadoop/executionengine/spark/converter/LimitConverter.java > fe1b54c8f128661d7d19c276d3bb2de7874d3086 > > src/org/apache/pig/backend/hadoop/executionengine/spark/converter/MergeCogroupConverter.java > adf78ecab0da10d3b1a7fdde8af2b42dd899810f > > src/org/apache/pig/backend/hadoop/executionengine/spark/converter/MergeJoinConverter.java > d1c43b1e06adc4c9fe45a83b81103333402e3756 > > src/org/apache/pig/backend/hadoop/executionengine/spark/converter/PoissonSampleConverter.java > e003bbd95763b2d189ff9ec540c89abe52592420 > > src/org/apache/pig/backend/hadoop/executionengine/spark/converter/SecondaryKeySortUtil.java > 00d29b44848546ed16dde2baa8c61b36939971b2 > > src/org/apache/pig/backend/hadoop/executionengine/spark/converter/SkewedJoinConverter.java > c55ba3145495a53d69db2dd56434dcc9b3bf8ed5 > > src/org/apache/pig/backend/hadoop/executionengine/spark/converter/SortConverter.java > baabfa090323e3bef087e259ce19df2e4c34dd63 > > src/org/apache/pig/backend/hadoop/executionengine/spark/converter/SparkSampleSortConverter.java > 3166fdc31745c013380492e089c83f3e853a3e6e > > src/org/apache/pig/backend/hadoop/executionengine/spark/converter/StreamConverter.java > 3a50d485cfd54b9f3b9c1a982e6c30497a4c85fc > src/org/apache/pig/tools/pigstats/spark/SparkJobStats.java > c8cc03194b223d2ee181d73c6b651a6872cac6b6 > src/org/apache/pig/tools/pigstats/spark/SparkJobStats1.java PRE-CREATION > src/org/apache/pig/tools/pigstats/spark/SparkJobStats2.java PRE-CREATION > src/org/apache/pig/tools/pigstats/spark/SparkPigStats.java > 61ccbcc9fd723f6e2e578a8476230c42d5587dfe > test/org/apache/pig/test/TestPigRunner.java > 1b1b11b0ccaca2cddeb3ad8ddc659a1c8a75722c > > > Diff: https://reviews.apache.org/r/59530/diff/4/ > > > Testing > ------- > > > Thanks, > > Nandor Kollar > >