Uh oh!
There was an error while loading. Please reload this page.
feat: Expose used MemoryPool details in ResourcesExhausted error messages - #20387
Conversation
75af7db to
168b51aCompare168b51a to
73bcc70Compareb24f6c8 to
897b01cCompareMemoryPool in ResourcesExhausted error messagesMemoryPool in ResourcesExhausted error messagesMemoryPool details in ResourcesExhausted error messages200940a to
10d6c19Compare
nathanb9
left a comment
There was a problem hiding this comment.
Nice. This makes sense to me
Uh oh!
There was an error while loading. Please reload this page.
ff7b82a to
9abbd3aCompare| write!( | ||
| f, | ||
| "{}", | ||
| format_args!( |
There was a problem hiding this comment.
Fixed by the last commit. Thanks for the review.
| } | ||
| } | ||
| impl Display for FairSpillPool { |
There was a problem hiding this comment.
how about adding used in FairSpillPool Display
There was a problem hiding this comment.
Currently, both GreedyMemoryPool and UnboundedMemoryPool track used memory size. FairSpillPool can also be extended to track it. I think it can be useful extension. Does it make sense to address by a follow-up issue? If so, i can create and work on it.
There was a problem hiding this comment.
#21753 has been created for follow-up effort. Thanks for quick feedback.
9abbd3a to
5b181a1CompareUh oh!
There was an error while loading. Please reload this page.
Which issue does this PR close?
MemoryPooldetails inResourcesExhaustederror messages #20386.Rationale for this change
memory_limit(RuntimeEnvBuilder::new().with_memory_limit()) configuration usesgreedymemory pool asdefault. However, ifmemory_pool(RuntimeEnvBuilder::new().with_memory_pool()) is set, it overrides by expectedmemory_poolconfig such asfair. Also, if bothmemory_limitandmemory_poolconfigs are not set,unboundedmemory pool will be used so it can be useful to exposeultimately used/selected poolas part ofResourcesExhaustederror message for the end user awareness and the user may need to switch used memory pool (greedy,fair,unbounded),greedyandfairmemory pools runtime behaviors and this addition can help for this kind of comparison table by exposing used memory pool info as part of native logs.Please find following example use-cases by
datafusion-cli:Case1: datafusion-cli result when
memory-limitandtop-memory-consumers > 0are set:Case2: datafusion-cli result when
memory-limitandtop-memory-consumers = 0(disabling top memory consumers logging) are set:Case3: datafusion-cli result when only
memory-limit,memory-poolandtop-memory-consumers > 0are set:What changes are included in this PR?
Are these changes tested?
Yes and updating existing test cases.
Are there any user-facing changes?
Yes, being updated Resources Exhausted error messages.