C0urante commented on code in PR #13367:
URL: https://github.com/apache/kafka/pull/13367#discussion_r1135661761
##########
connect/mirror/src/main/java/org/apache/kafka/connect/mirror/MirrorSourceTask.java:
##########
@@ -315,18 +314,16 @@ static class PartitionState {
// true if we should emit an offset sync
boolean update(long upstreamOffset, long downstreamOffset) {
- // 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.
+ // Emit an offset sync if any of the following conditions are true
+ boolean noPreviousSyncThisLifetime = lastSyncDownstreamOffset ==
-1L;
+ // the OffsetSync::translateDownstream method will translate this
offset 1 past the last sync, so add 1.
// TODO: share common implementation to enforce this relationship
- long downstreamTargetOffset = lastSyncDownstreamOffset + 1;
- if (lastSyncDownstreamOffset == -1L
- || downstreamOffset - downstreamTargetOffset >=
maxOffsetLag
- || upstreamOffset - previousUpstreamOffset != 1L
Review Comment:
Why are we removing this condition?
##########
connect/mirror/src/main/java/org/apache/kafka/connect/mirror/MirrorSourceTask.java:
##########
@@ -315,18 +314,16 @@ static class PartitionState {
// true if we should emit an offset sync
boolean update(long upstreamOffset, long downstreamOffset) {
- // 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.
+ // Emit an offset sync if any of the following conditions are true
+ boolean noPreviousSyncThisLifetime = lastSyncDownstreamOffset ==
-1L;
+ // the OffsetSync::translateDownstream method will translate this
offset 1 past the last sync, so add 1.
// TODO: share common implementation to enforce this relationship
- long downstreamTargetOffset = lastSyncDownstreamOffset + 1;
- if (lastSyncDownstreamOffset == -1L
- || downstreamOffset - downstreamTargetOffset >=
maxOffsetLag
- || upstreamOffset - previousUpstreamOffset != 1L
Review Comment:
Why are we removing this condition? Is that necessary for this fix?
--
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
