kosiew commented on code in PR #24993:
URL: https://github.com/apache/datafusion/pull/24993#discussion_r4024468591
##########
datafusion/execution/src/memory_pool/pool.rs:
##########
@@ -340,8 +340,9 @@ impl TrackedConsumer {
/// Grows the tracked consumer's reserved size,
/// should be called after the pool has successfully performed the grow().
fn grow(&self, additional: usize) {
Review Comment:
Could we add a bounded, deterministic regression test for concurrent peak
accounting across multiple reservations of the same consumer? Ideally, the test
would force this interleaving: grow A updates `reserved` but pauses before
recording `peak`; grow and shrink B; then resume A. We can then assert that
`metrics()` reports zero reserved memory and the combined high-water `peak`.
A barrier-only test where all grows complete before asserting and shrinking
would not quite cover this race, since it would also pass with the old
`fetch_add` followed by a separate `reserved()` load. A test-only hook or
another deterministic orchestration mechanism would be preferable to relying on
timing or thread scheduling.
--
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]
