C0urante commented on PR #11818: URL: https://github.com/apache/kafka/pull/11818#issuecomment-1368004001
Thanks @emilnkrastev, good to know we're on the same page! Thinking about this a little more, I wonder if a slightly more-involved approach is warranted. With the current proposal, it looks like we might still miss some cases that are predicated on the last-seen (not last-synced) upstream/downstream offsets. For example, if the downstream topic is deleted and then recreated, the `downstreamOffset < previousDownstreamOffset` part of the [condition in `shouldSyncOffsets`](https://github.com/emilnkrastev/kafka/blob/67f6f71eef29b474ae642449de73862bf65aef7e/connect/mirror/src/main/java/org/apache/kafka/connect/mirror/MirrorSourceTask.java#L302) will evaluate to `true`, and we'll try to send an offset sync as a result. If there are too many in-flight offset syncs already and we can't acquire access to the semaphore, we'll skip that sync--but since we'll also have already updated the `previousDownstreamOffset` field, the next call to `shouldSyncOffsets` won't (necessarily) return `true`, even though we should still attempt a sync in that case. For a short-term fix, I think the `PartitionState` class could be adapted to "remember" whether syncs are necessary, and allow external callers to clear that state whenever an offset sync has actually been performed. That way, we never run the risk of "dropping" offset syncs that were supposed to be performed but were blocked by access to the semaphore. How does that sound? -- 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: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
