sunchao commented on code in PR #56293:
URL: https://github.com/apache/spark/pull/56293#discussion_r3364960433
##########
core/src/main/java/org/apache/spark/memory/TaskMemoryManager.java:
##########
@@ -114,7 +117,14 @@ public class TaskMemoryManager {
/**
* The amount of memory that is acquired but not used.
*/
- private volatile long acquiredButNotUsed = 0L;
+ @GuardedBy("this")
+ private long acquiredButNotUsed = 0L;
+
+ /**
+ * Prevent nested page allocations while spilling from recursively entering
allocator recovery.
+ */
+ private final ThreadLocal<Boolean> inPageAllocationRecovery =
Review Comment:
Thanks for checking this. Both guarded helpers return immediately when the
flag is already true, so any invocation that reaches `set(true)` necessarily
has a prior value of false. A nested invocation cannot enter its own
`set`/`finally` scope and therefore cannot clear the outer flag. I prefer
keeping `remove()` here because it also removes the ThreadLocal entry from the
pooled thread rather than retaining a false value. If the call structure
changes to permit nesting in the future, we should introduce a truly scoped
helper or depth counter then.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
