dwsmith1983 commented on PR #5613: URL: https://github.com/apache/datafusion-comet/pull/5613#issuecomment-5679049370
> Now that `take_missing_anchor` exists, would it make sense to drop the acquire in `with_bridge` and let the first `try_grow` take the anchor? ... `try_new` and `create_memory_pool` could go back to handing the pool back directly. Done. Creating the pool makes no JVM call, `new` returns the pool directly and `create_memory_pool` no longer unwraps it, so a scan-filter-project plan never touches the task memory manager and never counts as an active task there. Pinned by a test that a pool which never grows makes no acquire and no release, and the registry race test now asserts neither create calls Spark and only the survivor's first grow takes the byte. > Would it work to move `take_missing_anchor` below the reserve-under-lock step and above the bridge acquire, rolling back with `finish_acquire(0, additional)` if it errors? Done, and the anchor attempt sits under the same `catch_unwind` as the request so a panic inside its JNI frame rolls the reservation back too. `over_limit_grow_without_an_anchor_never_reaches_spark` pins that an over-limit grow with no anchor makes zero JVM calls, and the vanished-entry test now drives the parked anchor request from a first grow and asserts the reservation is back to zero when it fails. > one line in the description reads more strongly than the test supports. Reworded: the description now says the second-acquire test pins the Rust side only, since `TaskMemoryManager` serializes a task's acquires on its own monitor, and the setup paragraph describes the lazy anchor. Pool tests are at 36 (two new, two reshaped), all of `memory_pools` looped ten times in release with no failure, full core suite and clippy clean. -- 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]
