andygrove commented on PR #5652: URL: https://github.com/apache/datafusion-comet/pull/5652#issuecomment-5559970385
Thanks for the controlled-heap repro — that's a much sharper statement of the problem than the thread it's attached to, and it pushed me off the design I had. Rather than keep the native guard armed across the JNI handoff, I made the JVM's knowledge of the written files independent of the decoder. The native operator emits the locations as a second Binary column beside the manifest, framed as a big-endian count then a length plus UTF-8 bytes per location, which the JVM walks with a `ByteBuffer`. `doExecute` registers the failure listener before pulling the payload, owning nothing at first; `drainNativePayload` hands it the locations off that column before it copies the manifest bytes out of the off-heap batch. So your boundary is covered on both sides of it: the manifest `Array[Byte]` copy and the Avro decode both happen with the listener already owning the files. I went this way rather than holding the native guard until the JVM acknowledges because the ack has to cross JNI, and the native guard's lifetime is tied to plan release — which happens at task end, after failure listeners run, so the two would double-delete and the disarm-on-success point would have to be a new JNI entry point. Reporting the locations makes the handoff a single volatile store with nothing fallible in between. Ownership is now explicit and non-overlapping: native until the output batch reaches the JVM (which now also covers manifest encoding and output-batch construction — see the sibling thread), JVM from then on. The one thing I did not close is building the location list itself: if that allocation is what OOMs, nobody deletes. That's a few thousand short strings against the decoder's full `DataFile` objects plus metrics maps, so it is a much smaller target, but it isn't zero and I'd rather say so than claim otherwise. Also rebased onto main to clear the conflict. -- 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]
