qqeasonchen opened a new pull request, #5427:
URL: https://github.com/apache/eventmesh/pull/5427
### What this fixes
`develop` went red on `51b918b` (the only non-green run in the last 30):
```
MemoryStorageE2EIntegrationTest > publishSubscribePollAckOverMemoryStorage()
FAILED
org.opentest4j.AssertionFailedError at
MemoryStorageE2EIntegrationTest.java:94
349 tests completed, 1 failed
```
Line 94 is the test's ACK assertion, and the macOS leg of the same commit
was green — a timing-dependent failure, not a functional one. It is a real
product race, not a flaky assertion.
### Root cause
`PushService.offer()` made the delivery visible to subscribers before it
made it ACKable:
```java
queue.offer(new BufferedEvent(deliveryId, event)); // poll() can return
this now
callbacksByDeliveryId.put(deliveryId, callback); // callback registered
only after
```
A subscriber parked in `poll()` is woken by that `queue.offer` and can call
`ack(deliveryId)` before the callback is registered. `PushService.ack()` then
finds no callback and returns `false` ("unknown delivery"), even though the
event was handed out a few instructions earlier. The delivery stays in flight
and is redelivered when the 30s ACK window expires.
Reproduced locally with an offer/poll/ack loop (one producer, one poller):
**36 unackable ACKs out of 5,000 deliveries** on the old ordering, **0** after
the fix.
### The fix
Register the ACK callback *before* enqueueing, and roll the registration
back if the queue ever refuses the event:
- normal path -> `enqueue(..)` registers then offers;
- `DROP_OLDEST` overflow path -> same helper (after dropping the oldest
entry and nacking its callback);
- invariant documented on `offer()`: a delivery must be ACKable the moment
it becomes visible to `poll()`.
### Tests
- New `PushServiceTest#everyPolledDeliveryIsImmediatelyAckable` (2,000
rounds, bounded by `@Timeout`). It fails on the old ordering ("a delivery must
be ACKable the moment it becomes visible to poll()") and passes with the fix.
- `:eventmesh-runtime:test` locally: **350 tests, 0 failures** (includes the
previously flaky `MemoryStorageE2EIntegrationTest`).
- `checkstyleMain`/`checkstyleTest` and `spotbugsMain` clean.
Fixes #5426.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
