mjsax commented on code in PR #22614:
URL: https://github.com/apache/kafka/pull/22614#discussion_r3632299051


##########
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:
   Thanks -- we usually use "position" to reason about a lag, which is 
"record.offset + 1". Everything in Kafka is based on this model. Eg, "seek to 
beginning" is setting position=0 (not "offset = -1"), and if we reach the end 
of the topic we get "position == endOffset == lastRecordInTheLog.offset + 1".
   
   Yes, using either "position" or "record offsets" here will lead to the same 
result; it's just "bending my mind" atm to change my mental model from 
"position" to "offset" and I expect other might get confused in a similar way.
   
   The variable here is `restoredFrom` which I read as a "position" ie, the 
first record offset which was/will-be restored.
   
   I would prefer to make this change. Thanks a lot.



-- 
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]

Reply via email to