Review Board 1.7.22


PIG-3567 LogicalPlanPrinter throws OOM for large scripts

Review Request #15565 - Created Nov. 15, 2013 and updated

Aniket Mokashi
trunk
PIG-3567
Reviewers
pig
cheolsoo, daijy, julien, rohini
pig
Changed LPPrinter recursion to directly write to stream so that memory footprint is reduced.
Diff'ed explain plan of few existing scripts. No diffs found. I will try to submit a testcase to show that LP print of before after is same (test need not be committed).
Total:
1
Open:
1
Resolved:
0
Dropped:
0
Status:
From:
Description From Last Updated Status
will remove this white space before commit. Aniket Mokashi Nov. 15, 2013, 5:26 a.m. Open
Posted (Nov. 15, 2013, 5:26 a.m.)

   

  
will remove this white space before commit.
Ship it!
Posted (Nov. 15, 2013, 9:53 p.m.)
Didn't go through line by line, but general approach looks good. You can add a unit test to check the memory footprint. 
  1. We had few scripts failing due to OOM because of this issue. With this change, they work now. I will add a test to show plan print before the change and after a change is same.
  2. I tested this change against a few production scripts and I see the plan printing properly. It's hard to come with a backward compatibility test. I will commit this if there are no objections.