[fix] [client] chunked message uuid-queue never removes entries on successful processing

[programmerahul]

Fixes: #26676

Motivation

uuid-queue pendingChunkedMessageUuidQueue doesn't remove entries after a chunked message is processed. So after processing 1M chunked messages, this queue has 1M entries.
This can be reproduced easily by sending chunked message and on consumer end simply acking those. So once the consumer processs 1M message , this queue also reached that size.

The larger issue with this is that , after processing of first chunked message, the expiry mechanism removeExpireIncompleteChunkedMessages() can't work for any incomplete message, because the logic will always find the first uuid from blockingQueue, but not in the chunkedMessageMap.

Modifications

Remove the uuid from quue when the chunked message is completely received

Verifying this change

• Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change added tests and can be verified as follows:

• Added a test-case for this

If the box was checked, please highlight the changes

• Dependencies (add or upgrade a dependency)
• The public API
• The schema
• The default values of configurations
• The threading model
• The binary protocol
• The REST endpoints
• The admin CLI options
• The metrics
• Anything that affects deployment

[programmerahul]

@lhotari Please review

[programmerahul]

The larger issue with this is that , after processing of first chunked message, the expiry mechanism removeExpireIncompleteChunkedMessages() can't work for any incomplete message, because the logic will always find the first uuid from blockingQueue, but not in the chunkedMessageMap.

[programmerahul]

@poorbarcode @void-ptr974 @merlimat @codelipenghui @BewareMyPower @Demogorgon314 please review this PR. This is easily reproducible bug, and as a side-effect of it, the expiry mechanism doesn't work

[programmerahul]

The larger issue with this is that , after processing of first chunked message, the expiry mechanism removeExpireIncompleteChunkedMessages() can't work for any incomplete message, because the logic will always find the first uuid from blockingQueue, but not in the chunkedMessageMap.

I have added a testcase for this scenario

[lhotari]

Thanks for tracking this down and for including the follow-on observation about expiry.

The bug is real and the fix is correct for the normal completion path: ConsumerImpl.java:1548 now drops the completed uuid from pendingChunkedMessageUuidQueue. I removed that line locally and both new tests fail (queue stays at 50 entries; the incomplete "stuck" context is never expired), and with it they pass. GrowableArrayBlockingQueue.remove(Object) takes both queue locks so it is safe against the expiry thread, and the O(n) scan is bounded by the number of in-flight chunked messages, so I am not concerned about cost.

One point I would like addressed or discussed: this fixes one way a completed/discarded uuid can be left at the head of the queue, but removeExpireIncompleteChunkedMessages still returns on the first uuid whose context is gone (ConsumerImpl.java:3207-3216), so any other path that leaves a stale uuid behind reintroduces the exact symptom described in the PR. See the inline comment for the two paths I confirmed. Making the expiry loop drop stale heads and continue would address the whole class, and the two small cases could be covered by tests.

Minor: the PR description says the test is added but does not mention the expiry effect that the second test covers; worth adding to the Motivation.