nicktelford commented on code in PR #22614:
URL: https://github.com/apache/kafka/pull/22614#discussion_r3616069132
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/StoreChangelogReader.java:
##########
@@ -730,6 +739,23 @@ private int restoreChangelog(final Task task, final
ChangelogMetadata changelogM
return numRecords;
}
+ /**
+ * Record restoration progress: restore-total/restore-rate advance by the
records restored
+ * ({@code numRecords}), while the remaining-records metric is decremented
by the offset slots
+ * between {@code previousOffset} (or {@code restoreStartOffset} if null)
and {@code restoredToOffset}.
+ * Measuring the latter in offset slots accounts for offsets the restore
consumer never returns
+ * (transaction markers, compacted records) so it reaches exactly zero on
completion.
+ */
+ private void recordRestorationProgress(final Task task,
+ final ChangelogMetadata
changelogMetadata,
+ final long numRecords,
+ final Long previousOffset,
+ final long restoredToOffset) {
+ final long restoredFrom = previousOffset == null ?
changelogMetadata.restoreStartOffset - 1 : previousOffset;
Review Comment:
I'm going to answer all your offset related comments here, because they're
all the same. Ultimately, it comes down to the "coordinate space", or the thing
we're measuring. The current version of the PR is measuring the "last applied
offset", i.e. the difference between the offset of the state before restore
starts and the offset of the tail record in the changelog.
I can easily change this to instead measure the next changelog offset to
restore. This would remove the `-1` and instead replace it with a `+1` (as you
mentioned below).
Both strategies ultimately end up with the same, correct result, so it's a
matter of preference.
--
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]
