Skip to content

Commit 803a85f

Browse files
nicktelfordclaude
andcommitted
KAFKA-20711: Measure restore progress in offset slots
The restore-remaining-records metric never reached zero when a changelog contained offsets the restore consumer does not return as records, such as transaction markers or compacted-away records. The metric is initialized from the offset range (restoreEndOffset minus startOffset), which counts every offset slot, but it was decremented by the number of records actually restored. Those two quantities diverge whenever the changelog has gaps, leaving the metric stuck at a positive value after restoration completed. The decrements are now expressed in offset slots rather than record counts. Each batch advances the metric by the change in store offset, and on completion any trailing slots between the last restored record and the end offset are accounted for, so the metric lands at exactly zero. The store offset before each batch is captured to measure the slots covered, and the consumer position at which restoration began is recorded so progress can be tracked from the first record onward. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 5b70498 commit 803a85f

2 files changed

Lines changed: 118 additions & 2 deletions

File tree

streams/src/main/java/org/apache/kafka/streams/processor/internals/StoreChangelogReader.java

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,12 @@ static class ChangelogMetadata {
141141

142142
private long restoreStartTimeNs;
143143

144+
// the consumer position at which restoration started (set once for active tasks). together
145+
// with the store offset it lets restoration progress be measured in offset slots rather than
146+
// record counts, so the remaining-records metric stays consistent with its offset-based
147+
// initial value even when the consumer skips transaction markers or compacted-away offsets
148+
private long restoreStartOffset;
149+
144150
private ChangelogMetadata(final StateStoreMetadata storeMetadata, final ProcessorStateManager stateManager) {
145151
this.changelogState = ChangelogState.REGISTERED;
146152
this.storeMetadata = storeMetadata;
@@ -651,6 +657,9 @@ private int restoreChangelog(final Task task, final ChangelogMetadata changelogM
651657

652658
if (numRecords != 0) {
653659
final List<ConsumerRecord<byte[], byte[]>> records = changelogMetadata.bufferedRecords.subList(0, numRecords);
660+
// the store offset before this batch marks where restoration had previously reached; a null
661+
// value means nothing has been restored yet, so progress is measured from restoreStartOffset
662+
final Long offsetBeforeRestore = storeMetadata.offset();
654663
final OptionalLong optionalLag = restoreConsumer.currentLag(partition);
655664
stateManager.restore(storeMetadata, records, optionalLag);
656665

@@ -663,9 +672,12 @@ private int restoreChangelog(final Task task, final ChangelogMetadata changelogM
663672
changelogMetadata.bufferedRecords.clear();
664673
}
665674

666-
task.recordRestoration(time, numRecords, false);
667-
675+
// restoring a batch advances the store offset to the last restored record's offset
668676
final Long currentOffset = storeMetadata.offset();
677+
678+
// advance the restoration metrics by the changelog offset slots this batch covered
679+
recordRestorationProgress(task, changelogMetadata, offsetBeforeRestore, currentOffset);
680+
669681
log.trace("Restored {} records from changelog {} to store {}, end offset is {}, current offset is {}",
670682
numRecords, partition, storeName, recordEndOffset(changelogMetadata.restoreEndOffset), currentOffset);
671683

@@ -693,6 +705,11 @@ private int restoreChangelog(final Task task, final ChangelogMetadata changelogM
693705
log.info("Finished restoring changelog {} to store {} with a total number of {} records",
694706
partition, storeName, changelogMetadata.totalRestored);
695707

708+
// any offset slots between the last restored record and the end offset (e.g. trailing
709+
// transaction markers) were never seen as records; account for them now so the
710+
// remaining-records metric reaches exactly zero on completion
711+
recordRestorationProgress(task, changelogMetadata, storeMetadata.offset(), changelogMetadata.restoreEndOffset - 1);
712+
696713
changelogMetadata.transitTo(ChangelogState.COMPLETED);
697714
pauseChangelogsFromRestoreConsumer(Collections.singleton(partition));
698715
if (storeMetadata.store() instanceof MeteredStateStore) {
@@ -713,6 +730,28 @@ private int restoreChangelog(final Task task, final ChangelogMetadata changelogM
713730
return numRecords;
714731
}
715732

733+
/**
734+
* Advance the restore/update total and rate metrics (and, for active tasks, the remaining-records
735+
* metric) by the number of changelog offset slots covered between {@code previousOffset} and
736+
* {@code restoredToOffset}, rather than by a record count. Measuring in offset slots accounts for
737+
* offsets occupied by transaction markers or compacted-away records that the restore consumer never
738+
* returns, keeping the metrics consistent with the offset-based remaining-records value so it reaches
739+
* exactly zero on completion.
740+
*
741+
* @param previousOffset the store offset before this progress (null if nothing has been restored yet,
742+
* in which case restoration is measured from {@code restoreStartOffset})
743+
* @param restoredToOffset the offset restoration has now reached
744+
*/
745+
private void recordRestorationProgress(final Task task,
746+
final ChangelogMetadata changelogMetadata,
747+
final Long previousOffset,
748+
final long restoredToOffset) {
749+
final long restoredFrom = previousOffset == null ? changelogMetadata.restoreStartOffset - 1 : previousOffset;
750+
if (restoredToOffset > restoredFrom) {
751+
task.recordRestoration(time, restoredToOffset - restoredFrom, false);
752+
}
753+
}
754+
716755
private Set<Task> getTasksFromPartitions(final Map<TaskId, Task> tasks,
717756
final Set<TopicPartition> partitions) {
718757
final Set<Task> result = new HashSet<>();
@@ -1016,6 +1055,9 @@ private void prepareChangelogs(final Map<TaskId, Task> tasks,
10161055
throw new StreamsException("Restore consumer get unexpected error trying to get the position " +
10171056
" of " + partition, e);
10181057
}
1058+
// remember where restoration began so progress can be measured in offset slots; for an
1059+
// active task this also seeds the remaining-records metric below
1060+
changelogMetadata.restoreStartOffset = startOffset;
10191061
if (changelogMetadata.stateManager.taskType() == Task.TaskType.ACTIVE) {
10201062
try {
10211063
stateRestoreListener.onRestoreStart(partition, storeName, startOffset, changelogMetadata.restoreEndOffset);

streams/src/test/java/org/apache/kafka/streams/processor/internals/StoreChangelogReaderTest.java

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
import org.junit.jupiter.api.extension.ExtendWith;
5555
import org.junit.jupiter.params.ParameterizedTest;
5656
import org.junit.jupiter.params.provider.EnumSource;
57+
import org.mockito.ArgumentCaptor;
5758
import org.mockito.Mock;
5859
import org.mockito.Mockito;
5960
import org.mockito.junit.jupiter.MockitoExtension;
@@ -94,6 +95,7 @@
9495
import static org.mockito.ArgumentMatchers.any;
9596
import static org.mockito.ArgumentMatchers.anyBoolean;
9697
import static org.mockito.ArgumentMatchers.anyLong;
98+
import static org.mockito.Mockito.atLeastOnce;
9799
import static org.mockito.Mockito.mock;
98100
import static org.mockito.Mockito.never;
99101
import static org.mockito.Mockito.times;
@@ -663,6 +665,78 @@ public void shouldCheckCompletionIfPositionLargerThanEndOffset() {
663665
assertNull(callback.storeNameCalledStates.get(RESTORE_BATCH));
664666
}
665667

668+
@Test
669+
public void shouldDecrementRemainingRecordsToZeroWithOffsetGaps() {
670+
// The restore-remaining-records metric is initialized from the offset range
671+
// (restoreEndOffset - startOffset), which counts every offset slot including those
672+
// occupied by transaction markers / compacted-away records that the restore consumer
673+
// never returns. The per-batch decrements must therefore also be expressed in offset
674+
// slots, so that the metric reaches exactly zero once restoration completes.
675+
setupActiveStateManager();
676+
setupStoreMetadata();
677+
setupStore();
678+
final TaskId taskId = new TaskId(0, 0);
679+
680+
// offset is null while preparing (forcing a seek-to-beginning so startOffset == 0, the
681+
// from-the-beginning edge case) and still null immediately before the first batch is
682+
// restored; once a batch is applied it reflects the last restored record's offset (7)
683+
when(storeMetadata.offset()).thenReturn(null).thenReturn(null).thenReturn(7L);
684+
when(activeStateManager.taskId()).thenReturn(taskId);
685+
686+
consumer.updateBeginningOffsets(Collections.singletonMap(tp, 0L));
687+
// end offset is 10, but the changelog only contains 8 data records (offsets 0..7);
688+
// offsets 8 and 9 are occupied by transaction markers, so the metric is initialized to 10
689+
adminClient.updateEndOffsets(Collections.singletonMap(tp, 10L));
690+
691+
final Task mockTask = mock(Task.class);
692+
693+
changelogReader.register(tp, activeStateManager);
694+
695+
// first restore initializes the changelog and records the initial remaining (= 10 slots)
696+
changelogReader.restore(Collections.singletonMap(taskId, mockTask));
697+
assertEquals(0L, consumer.position(tp));
698+
assertEquals(StoreChangelogReader.ChangelogState.RESTORING, changelogReader.changelogMetadata(tp).state());
699+
700+
// 8 contiguous data records at offsets 0..7
701+
for (int offset = 0; offset < 8; offset++) {
702+
consumer.addRecord(new ConsumerRecord<>(topicName, 0, offset, "key".getBytes(), "value".getBytes()));
703+
}
704+
705+
// second restore applies the 8 data records
706+
changelogReader.restore(Collections.singletonMap(taskId, mockTask));
707+
assertEquals(8L, changelogReader.changelogMetadata(tp).totalRestored());
708+
assertEquals(StoreChangelogReader.ChangelogState.RESTORING, changelogReader.changelogMetadata(tp).state());
709+
710+
// bypass the trailing transaction-marker offsets (8, 9) so the consumer reaches the end
711+
consumer.seek(tp, 10L);
712+
713+
// third restore observes position >= endOffset and completes restoration
714+
changelogReader.restore(Collections.singletonMap(taskId, mockTask));
715+
assertEquals(StoreChangelogReader.ChangelogState.COMPLETED, changelogReader.changelogMetadata(tp).state());
716+
// totalRestored continues to count actual records, not offset slots
717+
assertEquals(8L, changelogReader.changelogMetadata(tp).totalRestored());
718+
719+
// capture every recordRestoration call: the single initRemaining=true call provides the
720+
// initial value, and the initRemaining=false calls are decrements applied to the metric
721+
final ArgumentCaptor<Long> numCaptor = ArgumentCaptor.forClass(Long.class);
722+
final ArgumentCaptor<Boolean> initCaptor = ArgumentCaptor.forClass(Boolean.class);
723+
verify(mockTask, atLeastOnce()).recordRestoration(any(), numCaptor.capture(), initCaptor.capture());
724+
725+
long initial = 0L;
726+
long decremented = 0L;
727+
for (int i = 0; i < numCaptor.getAllValues().size(); i++) {
728+
if (initCaptor.getAllValues().get(i)) {
729+
initial += numCaptor.getAllValues().get(i);
730+
} else {
731+
decremented += numCaptor.getAllValues().get(i);
732+
}
733+
}
734+
735+
assertEquals(10L, initial, "metric should be initialized from the offset range");
736+
// the decrements must net the metric back to exactly zero
737+
assertEquals(initial, decremented, "remaining-records metric should decrement to exactly zero");
738+
}
739+
666740
@Test
667741
public void shouldRequestPositionAndHandleTimeoutException() {
668742
setupActiveStateManager();

0 commit comments

Comments
 (0)