----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69107/#review209935 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMapRecordHandler.java Line 171 (original), 170 (patched) <https://reviews.apache.org/r/69107/#comment294574> move to first line of method ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java Line 49 (original), 52 (patched) <https://reviews.apache.org/r/69107/#comment294578> i think volatile long is sufficient here and is probably cheaper. atomics might be expensive when done per row ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java Line 50 (original), 53 (patched) <https://reviews.apache.org/r/69107/#comment294580> this need to be volatile since it is modified by the timer task ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java Line 52 (original), 55 (patched) <https://reviews.apache.org/r/69107/#comment294575> Lets set the max to 15 minutes ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java Lines 104 (patched) <https://reviews.apache.org/r/69107/#comment294576> I think you can remove these debug statements. They don't look like they add much value. ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java Lines 105 (patched) <https://reviews.apache.org/r/69107/#comment294579> might make more sense to schedule the task at the end of the method ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java Lines 117 (patched) <https://reviews.apache.org/r/69107/#comment294581> nit: remove this ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java Lines 105-106 (original), 129-130 (patched) <https://reviews.apache.org/r/69107/#comment294582> this looks like the same code that is called in the `MemoryInfoLogger` can it be abstracted into its own method? ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkReduceRecordHandler.java Line 601 (original), 597 (patched) <https://reviews.apache.org/r/69107/#comment294577> move to top of method - Sahil Takiar On Oct. 20, 2018, 7:13 p.m., Bharathkrishna Guruvayoor Murali wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69107/ > ----------------------------------------------------------- > > (Updated Oct. 20, 2018, 7:13 p.m.) > > > Review request for hive, Antal Sinkovits, Sahil Takiar, and Vihang > Karajgaonkar. > > > Repository: hive-git > > > Description > ------- > > Improve record and memory usage logging in SparkRecordHandler > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMapRecordHandler.java > 88dd12c05ade417aca4cdaece4448d31d4e1d65f > > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkMergeFileRecordHandler.java > 8880bb604e088755dcfb0bcb39689702fab0cb77 > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkRecordHandler.java > cb5bd7ada2d5ad4f1f654cf80ddaf4504be5d035 > > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkReduceRecordHandler.java > 20e7ea0f4e8d4ff79dddeaab0406fc7350d22bd7 > > > Diff: https://reviews.apache.org/r/69107/diff/1/ > > > Testing > ------- > > > Thanks, > > Bharathkrishna Guruvayoor Murali > >
