This is a friendly reminder about the urgent need for review of PR #23352, the first part of implementing "PIP-379: Key_Shared Draining Hashes for Improved Message Ordering".
PR: https://github.com/apache/pulsar/pull/23352 Target: 4.0.0-preview.2 release, scheduled for Monday Oct 7th. Your prompt review would be greatly appreciated, ideally by end of day today. This will allow time for addressing feedback over the weekend. Thank you for your attention to this matter. -Lari On 2024/10/02 21:43:53 Lari Hotari wrote: > Dear Pulsar Community, > > I am excited to announce that the first PR implementing "PIP-379: > Key_Shared Draining Hashes for Improved Message Ordering" is now ready > for review. This PR contains the core implementation as described in > the PIP-379 document. PIP-379 is currently in voting. > > Since there is a goal to include this implementation in the > 4.0.0-preview.2 release of 4.0.0, there is urgency to get the review > started so that we can complete it by EOD on Monday, October 7th. I > hope to receive review feedback as soon as possible so that I will > have time to address the review comments. > > PR for review: https://github.com/apache/pulsar/pull/23352 > Please read the PIP-379 document to get the context. The link is in > the PR description. > > Some notable details: > > 1. The implementation focuses on maintaining the key invariant for > Key_Shared subscriptions: "Messages with the same key are delivered > and allowed to be in an unacknowledged state to only one consumer at a > time." > > 2. More tests will be added to cover the new logic in classes such as > DrainingHashesTracker, HashRanges (utility class), and PendingAcksMap. > These tests will be added to the current PR asap. > > 3. Observability features will be implemented in a subsequent PR after > this one is completed. > > 4. The vote for PIP-379 is currently open at: > https://lists.apache.org/thread/z1kgo34qfkkvdnn3l007bdvjr3qqf4rw > > Important implementation notes: > > - While working on this solution, I discovered a potential memory leak > in the current pending ack implementations. I do not think that the > current implementation in Pulsar contains a solution for removing > pending acks if the mark delete position moves forward as a result of > cursor reset (or any other reason where it moves forward without > individual acks). The PIP-379 implementation addresses this issue in > the PendingAcksMap.removeAllUpTo method, which gets called from the > PersistentDispatcherMultipleConsumers.readMoreEntries method. > > - Significant improvements have been made to ensure the consistency of > pending acks, which is crucial for maintaining the Key_Shared > subscription invariant. > > I kindly request the community to review the first implementation PR: > https://github.com/apache/pulsar/pull/23352. Your feedback and > suggestions are crucial for ensuring the quality and correctness of > this important improvement to Pulsar's Key_Shared subscriptions. > > Thank you for your time and feedback, > > Lari >
