mjsax commented on code in PR #23942:
URL: https://github.com/apache/kafka/pull/23942#discussion_r4383851388
##########
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:
Yes, it's annoying that `storeMetadata.endOffset()` is the last actual
offset introducing an off-by-on issue... That's what `A slightly negative lag
does count` refers too -- because we sum it up, it's not necessarily 0 or -1,
but could also be -k if we have changelog partitions for a task. Let me reword.
--
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]