Repository navigation
Conversation
…ition scope ReliableDispatcher#ackDelivery ran the partition-ownership guard for every delivery. UniIngressService#deliverLocal routes a locally delivered event with partition = -1, the "no partition scope" marker, and the ownership view only ever holds real partition indexes, so owned.contains(-1) was always false: every ACK of such a delivery was rejected with StaleOwnerException. The exception then escaped the HTTP handler, so the connection was closed without any response, and because PushService#ack removed the callback before invoking it, the client's retry was answered with 404 "unknown deliveryId". The subscriber could neither acknowledge the event nor find out what happened, and its offset never advanced. - ReliableDispatcher: run the ownership guard only when the delivery is bound to a partition (partition >= 0). - PushService: keep the ACK callback registered unless the ACK succeeds, so a failed ACK stays ackable. - UniHttpServer: answer a fenced ACK with 409 (topic/partition in the message) instead of letting the exception escape the handler. Fixes apache#5432 Signed-off-by: Lyin-OTTF <htao08203@gmail.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is the bug
ReliableDispatcher#ackDeliveryruns the partition ownership guard for every delivery:UniIngressService#deliverLocalroutes a locally delivered event withpartition = -1— the "no partition scope" marker. The ownership view only ever holds real partition indexes, soowned.contains(-1)is alwaysfalseand every ACK of such a delivery was rejected. Fencing is a per-partition concept; it does not apply to a delivery that has no partition.The consequence chain:
StaleOwnerExceptionescaped the HTTP handler, so the connection was closed without any response.PushService#ackremoved the callback before invoking it, so the client's retry was answered with404 unknown deliveryId.What the fix does
ReliableDispatcher— run the ownership guard only for a delivery that is bound to a partition (partition >= 0). Partition-less deliveries are not fenced.PushService#ack— keep the ACK callback registered unless the ACK actually succeeds, so a failed ACK stays ackable instead of turning into a 404.UniHttpServer— answer a fenced ACK with409including the topic/partition, instead of letting the exception escape the handler. The delivery is dropped on the fenced instance and the broker redelivers it to the new owner (POPinvisibleTime), so the client sees a real answer and a retry is safe.Tests
StaleOwnerFencingTest— new case: a delivery without partition scope is not fenced; the existing partition-scoped fencing cases still pass.PushServiceTest#failedAckKeepsTheDeliveryAckable— a throwing ACK leaves the delivery resolvable.UniHttpServerAckErrorTest(new) — ACK returns409for a fenced delivery,404for an unknown one,200on success../gradlew :eventmesh-runtime:test— 12 tests in the three classes, 0 failures; full:eventmesh-runtime:testgreen.Fixes #5432