C0urante commented on code in PR #13367:
URL: https://github.com/apache/kafka/pull/13367#discussion_r1134035661
##########
connect/mirror/src/main/java/org/apache/kafka/connect/mirror/MirrorSourceTask.java:
##########
@@ -316,13 +315,14 @@ static class PartitionState {
// true if we should emit an offset sync
boolean update(long upstreamOffset, long downstreamOffset) {
- long upstreamStep = upstreamOffset - lastSyncUpstreamOffset;
- long downstreamTargetOffset = lastSyncDownstreamOffset +
upstreamStep;
+ // This value is what OffsetSyncStore::translateOffsets would
compute for this offset given the last sync.
+ // Because this method is called at most once for each upstream
offset, simplify upstreamStep to 1.
+ // TODO: share common implementation to enforce this relationship
+ long downstreamTargetOffset = lastSyncDownstreamOffset + 1;
Review Comment:
I've started to like the name `downstreamTargetOffset` less and less, and
the need for 2-3 lines of comments to explain what it's supposed to represent
might hint that a better name could help more here.
We can remove this variable entirely and just inline the value
`lastSyncDownstreamOffset + 1` in its place, with a comment explaining the `+
1` part. Or we could rename to something like `translatedLastDownstreamOffset`.
--
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
