Skip to content

[SPARK-58975][CORE] Add failure messages to MemoryPool require checks - #58251

Open
uros-b wants to merge 3 commits into
apache:masterfrom
uros-b:memorypool-require-messages
Open

[SPARK-58975][CORE] Add failure messages to MemoryPool require checks#58251
uros-b wants to merge 3 commits into
apache:masterfrom
uros-b:memorypool-require-messages

Conversation

@uros-b

@uros-b uros-b commented Aug 24, 2026

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

MemoryPool.incrementPoolSize and decrementPoolSize guard their pool-size accounting invariants with four require(...) checks that carry no failure message. When one fails, the thrown IllegalArgumentException reads only requirement failed, with no indication of which invariant was violated or the values involved.

This adds descriptive messages to those four checks, interpolating the offending values, for example:

require(delta >= 0, s"cannot increment the pool size by a negative amount: $delta")

The sibling MemoryManager already attaches messages to all of its require calls; this brings MemoryPool in line.

Why are the changes needed?

These checks guard internal memory-bookkeeping invariants. A bare requirement failed is hard to act on when one trips; including the invariant and the actual delta / pool size / used-memory values makes such failures diagnosable.

Does this PR introduce any user-facing change?

No. MemoryPool is private[memory]; this only changes the message of an IllegalArgumentException thrown on an internal invariant violation.

How was this patch tested?

Existing tests. The change only adds messages to existing require checks; the conditions themselves are unchanged.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Opus 4.8)

@uros-b uros-b left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@srowen Please review.

@uros-b
uros-b requested a review from srowen August 24, 2026 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant