Re: [PR] [fix][broker] Don't redeliver individually acknowledged messages from the compacted ledger [pulsar]

["merlimat (via GitHub)" <gi...@apache.org>]

merlimat commented on PR #23495:
URL: https://github.com/apache/pulsar/pull/23495#issuecomment-6349106451

   @summeriiii thanks for the fix and the test, and sorry this sat for so long. I've picked it up: I merged master (#24741 and #26190 reworked the compacted read path) and pushed a reworked version to your branch.
   
   - The filter uses `isMessageDeleted` but only drops entries after the mark-delete position, so `ManagedCursor` doesn't need a new method. As you found, a plain `isMessageDeleted` breaks `testCompactionWithTTL`, because a read from the earliest position must still return the entries before the mark-delete position.
   - The dropped entries are now released.
   - The test is deterministic and also covers `redeliverUnacknowledgedMessages()`. A negative ack or an ack timeout on an Exclusive or Failover subscription sends that same redelivery, and it hits the same read through `rewind(true)`.
   
   @coderzc on the compaction concern: the `__compaction` subscription only accepts cumulative acks (`PulsarCompactorSubscription`), so it never has individually deleted entries and the filter doesn't change what the compactor reads. Cumulative acks are the better fit for compacted topics, but individual acks are allowed on Exclusive and Failover subscriptions, so I think the broker should not resend them. Your backlog point still holds: with individual acks, the messages that compaction removed stay in the backlog. That's a separate issue, and this PR doesn't change it.
   
   PTAL.
   


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org