Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/9241#discussion_r43461668
--- Diff:
core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java ---
@@ -647,16 +707,24 @@ public boolean putNewKey(
* Acquire a new page from the memory manager.
* @return whether there is enough space to allocate the new page.
*/
- private boolean acquireNewPage() {
- MemoryBlock newPage = taskMemoryManager.allocatePage(pageSizeBytes);
- if (newPage == null) {
- logger.debug("Failed to acquire {} bytes of memory", pageSizeBytes);
+ private boolean acquireNewPage(long required) {
+ try {
+ currentPage = allocatePage(required);
+ dataPages.add(currentPage);
+ Platform.putInt(currentPage.getBaseObject(),
currentPage.getBaseOffset(), 0);
+ pageCursor = 4;
+ return true;
+ } catch (OutOfMemoryError e) {
--- End diff --
If we want to catch OOME here, I think that we should do it at a much
smaller scope (in the assignment to `currentPage` but not for adding to
`dataPages` or modifying the page cursor. Given the risks of catching OOME that
I mentioned above, the scope of the catch should be as narrow as possible.
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
