andygrove commented on PR #5543:
URL: 
https://github.com/apache/datafusion-comet/pull/5543#issuecomment-5689046173

   Thanks @sunchao — both are fixed in b978e5447, with the details on the 
individual threads. Two things worth pulling up here.
   
   The leak is real, and the fix is not local to the failure point: 
`VectorUnloader` accumulates the compressed buffers into a list inside 
`getRecordBatch`, so nothing a caller holds can reach them once it throws. 
`serialize` now unloads plain and compresses in a new 
`CachedBatchIpc.compressed`, the same shape `decompressed` already uses on the 
read side. I mutated the fix away to check the new test earns its place: it 
then fails with 2,176 bytes still allocated after the input batch is closed, 
which is your 17.5 MiB at a test-sized batch rather than a 4M-value one.
   
   The lint failure was the redundant `s` prefix you found. All five failing 
checks were the same scalafix run across profiles, so that one line covers 
them; I scanned the rest of the PR's files for a second instance and there 
isn't one.
   
   Verified on the default profile: `CometInMemoryCacheSuite` 45/45, 
`CometInMemoryCacheKryoSuite` and `UtilsSuite` green, `spotless:check` and 
`scalastyle:check` clean, and test-compiles against Spark 3.5 and 4.0 as well 
as 4.1.


-- 
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]

Reply via email to