lucasbru commented on code in PR #23942:
URL: https://github.com/apache/kafka/pull/23942#discussion_r4377305792
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/streams/AssignmentRefinerImpl.java:
##########
@@ -983,9 +983,11 @@ private static PatchedTasks patchFor(
* own predicate, so that both ends agree on when a warm-up task is caught
up.
*
* <p>The lag is the distance between the reported end offset and the
reported offset, and a lag that is not known
- * is never within the threshold: an offset missing on either side, or
capped at {@link Long#MAX_VALUE} to say that
- * the restore has not started, counts as not caught up. A slightly
negative lag does count, because the offset is a
- * position while the end offset is the last offset, so a fully restored
task reports a lag of -1.
+ * is never within the threshold: an offset missing on either side, or
reported as {@link Long#MAX_VALUE}, counts as
+ * not caught up. The client reports an end offset of {@link
Long#MAX_VALUE} while it does not know a changelog's end
+ * offset yet, and either offset as {@link Long#MAX_VALUE} when its sum
over the changelogs overflows. A slightly
+ * negative lag does count, because the offset is a position while the end
offset is the last offset, so a fully
+ * restored task reports a lag of -1.
Review Comment:
nit: the end offset isn't always the last offset.
`StoreChangelogReader#logicalChangelogEndOffsets` returns `restoreEndOffset`
when it's set, which is exclusive (log end offset / committed offset), so a
fully restored task reports a lag of 0 there. Only the
`storeMetadata.endOffset()` fallback is a last offset and gives -1. Doesn't
change the result since both are within the threshold, but maybe reword to "a
lag of 0 or -1"?
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/streams/AssignmentRefinerImpl.java:
##########
@@ -1008,12 +1010,12 @@ static boolean isCaughtUp(
}
/**
- * Whether the member has restored any of the task: it reports a position
for it, and that position is not the
- * cap the client reports before a restore has begun.
+ * Whether the member has restored any of the task: it reports a position
above 0 for it. The client reports a
Review Comment:
nit: this reads as if there's one value per changelog, but it's the sum of
the positions over all of the task's changelogs. Maybe something like "it
reports a changelog position sum above 0 for it, which is 0 only while it has
restored nothing from any of its changelogs"?
--
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]
