Skip to content

Commit 4b87a04

Browse files
author
Gergely Békési
committed
fix: heartbeat bug
1 parent 8d7757c commit 4b87a04

2 files changed

Lines changed: 30 additions & 13 deletions

File tree

src/modules/rollingFeed.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,10 @@ export class RollingFeedWriter {
133133
}
134134

135135
/**
136-
* Backfills every period strictly between the last populated one and `periodIdx`
137-
* (default: current) with that period's last known payload/reference. Never writes
138-
* `periodIdx` itself - that period is the caller's to write with fresh data.
136+
* Backfills every period from the last populated one (exclusive) up to `periodIdx`
137+
* (default: current) with that period's last known payload/reference. Skips `periodIdx`
138+
* itself if it's already populated, so it never clobbers fresh data the caller may have
139+
* already written there via `uploadPayload`/`uploadReference`.
139140
*/
140141
async catchUp(postageBatchId: string | BatchId, periodIdx?: number): Promise<void> {
141142
const requestOptions = this.context.getRequestOptionsForCall()
@@ -163,7 +164,10 @@ export class RollingFeedWriter {
163164
const { feedIndex: sourceIndex } = await probeFeed(requestOptions, owner, sourceTopic)
164165
const sourceChunk = await downloadFeedUpdateAsCAC(requestOptions, owner, sourceTopic, sourceIndex)
165166

166-
for (let period = lastGoodPeriod + 1; period < targetPeriod; period++) {
167+
const targetAlreadyWritten = await isPeriodPopulated(requestOptions, owner, topicFor(this.baseTopic, targetPeriod))
168+
const backfillEnd = targetAlreadyWritten ? targetPeriod : targetPeriod + 1
169+
170+
for (let period = lastGoodPeriod + 1; period < backfillEnd; period++) {
167171
const identifier = makeFeedIdentifier(topicFor(this.baseTopic, period), 0)
168172
await uploadSingleOwnerChunkWithWrappedChunk(requestOptions, this.signer, stamp, identifier, sourceChunk)
169173
}

test/integration/rolling-feed.spec.ts

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -98,15 +98,30 @@ test('catchUp backfills a gap so the reader resolves it again', async () => {
9898

9999
expect(await writer.isCaughtUp(1003)).toBe(false)
100100
await writer.catchUp(batch(), 1003)
101-
// catchUp never writes its target period itself (1003) - only the gap strictly before it
102-
await waitUntil(async () => writer.isCaughtUp(1002))
103-
expect(await writer.isCaughtUp(1003)).toBe(false)
101+
// 1003 wasn't written yet, so catchUp fills it too, not just the gap strictly before it
102+
await waitUntil(async () => writer.isCaughtUp(1003))
104103

105104
setPeriod(1003)
106-
// reader falls back one period, from the still-empty 1003 to the now-backfilled 1002
107105
await waitUntil(async () => (await reader.downloadPayload()).payload.toUtf8() === 'Backfilled payload')
108106
})
109107

108+
test('heartbeat idiom (isCaughtUp then catchUp with no args) converges after one silent period', async () => {
109+
const { writer, reader } = makeWriterAndReader()
110+
111+
setPeriod(1000)
112+
await writer.uploadPayload(batch(), 'Heartbeat payload', { deferred: false })
113+
await waitUntil(async () => writer.isCaughtUp(1000))
114+
115+
// one whole period of silence: 1001 is mirrored, 1002 is not
116+
setPeriod(1002)
117+
expect(await writer.isCaughtUp()).toBe(false)
118+
await writer.catchUp(batch())
119+
await waitUntil(async () => writer.isCaughtUp())
120+
121+
const result = await reader.downloadPayload()
122+
expect(result.payload.toUtf8()).toBe('Heartbeat payload')
123+
})
124+
110125
test('catchUp does not clobber fresh data already written to its target period', async () => {
111126
const { writer, reader } = makeWriterAndReader()
112127

@@ -141,13 +156,11 @@ test('catchUp is not fooled by an older buried gap', async () => {
141156

142157
expect(await writer.isCaughtUp(6)).toBe(false)
143158
await writer.catchUp(batch(), 6)
144-
// catchUp never writes period 6 itself - only fills the gap up to period 5
145-
await waitUntil(async () => writer.isCaughtUp(5))
146-
expect(await writer.isCaughtUp(6)).toBe(false)
159+
// 6 wasn't written yet, so catchUp fills it too, not just the gap strictly before it
160+
await waitUntil(async () => writer.isCaughtUp(6))
147161

148162
setPeriod(6)
149-
// reader falls back from the still-empty 6 to the now-backfilled 5; must be the recent
150-
// (period 4) content, not the one from before the buried gap
163+
// must resume from the recent (period 4) content, not the one from before the buried gap
151164
await waitUntil(async () => (await reader.downloadPayload()).payload.toUtf8() === 'Recent payload')
152165
})
153166

0 commit comments

Comments
 (0)