[GitHub] [spark] attilapiros opened a new pull request #35559: [SPARK-33206] Fix shuffle index cache weight calculation for small index files

[GitBox]

attilapiros opened a new pull request #35559:
URL: https://github.com/apache/spark/pull/35559


   ### What changes were proposed in this pull request?
   
   Increasing the shuffle index weight with a constant number to avoid 
underestimating retained memory size caused by the bookkeeping objects 
(including Guava cache internal objects, the `File `object and the 
`ShuffleIndexInformation` object itself).
   
   ### Why are the changes needed?
   
   Underestimating cache entry size easily can cause OOM in the Yarn 
NodeManager. 
   In the following analyses of a prod issue (HPROF file) we can see the leak 
suspect Guava's `LocalCache$Segment` objects:
   
   <img width="943" alt="Screenshot 2022-02-17 at 18 55 40" 
src="https://user-images.githubusercontent.com/2017933/154541995-44014212-2046-41d6-ba7f-99369ca7d739.png";>
   
   Going further we can see a `ShuffleIndexInformation` for a small index file 
(16 bytes) but the retained heap memory is 1192 bytes (by the Guava internal 
object used for bookkeeping):
   
   <img width="1635" alt="Screenshot 2022-02-17 at 19 01 33" 
src="https://user-images.githubusercontent.com/2017933/154547083-60b48ee7-9c79-4f4c-97d8-72a96cfb453b.png";>
   
   Finally we can see this is very common within this heap dump (using MAT's 
Object Query Language):
    
   <img width="1418" alt="image" 
src="https://user-images.githubusercontent.com/2017933/154547678-44c8af34-1765-4e14-b71a-dc03d1a304aa.png";>
   
   I have even exported the data to a CSV and done some calculations with `awk`:
   
   ```
   $ tail -n+2 export.csv | awk -F, 'BEGIN { numUnderEstimated=0; } { 
sumOldSize += $1; corrected=$1 + 1176; sumCorrectedSize += corrected; 
sumRetainedMem += $2; if (corrected < $2) numUnderEstimated+=1; } END { print 
"sum old size: " sumOldSize / 1024 / 1024   " MB, sum corrected size: " 
sumCorrectedSize / 1024 / 1024 " MB, sum retained memory:" sumRetainedMem / 
1024 / 1024  " MB, num under estimated: " numUnderEstimated }'
   ```
   
   It gives the followings:
   ```
   sum old size: 76.8785 MB, sum corrected size: 1066.93 MB, sum retained 
memory:1064.47 MB, num under estimated: 0
   ```
   
   So using the old calculation we were at 7.6.8 MB way under the default cache 
limit (100 MB). 
   Using the correction (applying 1176 as increment to the size) we are at 
1066.93 MB (~1GB) which is close to the real retained sum heap: 1064.47 MB 
(~1GB) and there is no entry which was underestimated.
   
   ### Does this PR introduce _any_ user-facing change?
   
   No.
   
   ### How was this patch tested?
   
   With the calculations above.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org