Fix unreleased buffer in LimitScanResponse and evaluate client-side zero-copy OOM risk

[wuchong]

Search before asking

• I searched in the issues and found nothing similar.

Description

PR #2948 implements zero-copy for the client side (Netty buffer to application). However, LimitScanResponse is marked as isLazilyParsed but does not explicitly release the byte buffer, which may lead to memory leaks.

Concurrently, we are addressing a series of client-side OOM issues related to direct memory in #2663. Direct memory is a scarce resource; for instance, Flink TaskManagers typically allocate only tens of megabytes, making them highly susceptible to OOM errors. Therefore, in #2663, we are refining memory management and leveraging heap memory to reduce reliance on direct memory.

Regarding PR #2948, since CompleteFetch holds the Netty byte buffer until it is consumed, high traffic or backpressure scenarios could trigger client-side direct memory OOMs. Consequently, we need to re-evaluate the impact of this PR on stability. @loserwang1024 will test whether this PR introduces any OOM risks.

cc @polyzos @fresh-borzoni

Willingness to contribute

• I'm willing to submit a PR!

[polyzos]

Thank you for catching this @wuchong.
Indeed it is likely to have memory leak, but I think the fix here should be pretty straightforward and we can address it asap. WDYT? @fresh-borzoni

[wuchong]

@polyzos yes, the fix can be straightforward. But the potential direct memory OOM needs some benchmarks.

[polyzos]

@wuchong Ok so there might be a high potential risk here.
Good to know and thank you for catching this 😄🙏

[fresh-borzoni]

@wuchong @polyzos Yes, looking at the math in #2773, it does seem to be dangerous or at least unpredictable, though there is backpressure mechanism to not accumulate endless CompletedFetches.

I think #2948 can be useful as an extension for #2773 advanced usage with adjusted off-heap memory path.

Let's revert #2948 and wait until #2803 lands, WDYT?

[wuchong]

I think we can benchmark #2948 and #2803 together to see the OOM issue. Since #2803 changed the netty buffer to heap memory, so the direct memory OOM may be resolved. What do you think @loserwang1024 @fresh-borzoni ?

[fresh-borzoni]

@wuchong Agreed, benchmarking them together makes sense, #2948 avoids the deep copy, #2803 moves buffers to heap, so together the direct memory OOM risk should be eliminated while keeping the throughput benefit.

I've also opened ##3003 to fix the LimitScanResponse buffer leak mentioned.
Happy to help with the benchmark as well.