Github user JoshRosen commented on the issue:

    https://github.com/apache/spark/pull/15043
  
    I agree that we should add more off-heap tests, but I'd like to do it in 
another patch so that we can get this one merged faster to unblock the 2.0.1 RC.
    
    In terms of testing off-heap, I think that one of the best high-level tests 
/ asserts would be to strengthen the `releaseUnrollMemory()` checks so that 
inappropriately releasing unroll memory _during_ a task throws an exception 
during tests. Today there are some circumstances where unroll memory can only 
be released at the end of a task (such as an iterator backed by an unrolled 
block that is only partially consumed before the task ends), so the calls to 
release unroll memory have been tolerant of too much memory being released (it 
just releases `min(actualMemory, requestedToRelease)`). However, this is only 
appropriate to do at the end of the task so we should strengthen the asserts to 
only allow it there; this would have caught the memory mode mixup that I fixed 
here.
    
    I'm going to retest this and if it passes tests then I'll merge to master 
and branch-2.0. I'll add the new tests described above in a followup.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

Reply via email to