lhotari opened a new pull request, #25843:
URL: https://github.com/apache/pulsar/pull/25843

   ### Motivation
   
   Fix flaky `testSkipRedeliverTemporally` in 
`PersistentStickyKeyDispatcherMultipleConsumersTest`.
   
   The test was flaky because the `asyncReadEntriesWithSkipOrWait` mock 
filtered entries with `!alreadySent.contains(...)`. With the slow consumer at 0 
permits, the entries it owned were never marked as sent, so the mock kept 
returning them on every normal read, creating an infinite normal-read loop. The 
dispatcher therefore never entered the replay-read path needed to deliver to 
the slow consumer once its permits were raised.
   
   In a real cursor, `asyncReadEntriesWithSkipOrWait` advances the read 
position — entries can only be re-read through `asyncReplayEntries`. The mock 
didn't simulate this behavior.
   
   After raising the slow consumer's permits, the test also relied on the 
dispatcher self-rescheduling within 5 seconds, which is not deterministic.
   
   This is the same root cause and fix as #25385, applied to the non-Classic 
variant.
   
   ### Modifications
   
   1. **Realistic cursor mock** — `asyncReadEntriesWithSkipOrWait` now tracks 
which entries it has already returned via `normalReadReturned` and doesn't 
return them again (simulating cursor read-position advancement). When no new 
entries exist, the callback is not invoked (simulating "OrWait" behavior), 
which stops the infinite normal-read loop.
   
   2. **Explicit `readMoreEntriesAsync()` call** — After setting the slow 
consumer's permits, the test triggers a new dispatch cycle. This mimics what 
`consumerFlow` does in production when a consumer sends more permits. With the 
loop stopped and `isDispatcherStuckOnReplays` never set, the dispatcher 
correctly enters the replay-read path and delivers messages to the slow 
consumer.
   
   ### Verifying this change
   
   - [ ] Make sure that the change passes the CI checks.
   
   This change is already covered by existing tests, such as 
`PersistentStickyKeyDispatcherMultipleConsumersTest.testSkipRedeliverTemporally`.
 Verified locally with `@Test(invocationCount = 10)`: 10/10 passes.
   
   ### Does this pull request potentially affect one of the following parts:
   
   - [ ] 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
   
   ### Documentation
   
   - [x] `doc-not-needed`
   
   ### Matching PR in forked repository
   
   PR in forked repository: 
https://github.com/lhotari/pulsar/tree/lh-fix-flaky-skipRedeliverTemporally


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

Reply via email to