-
Notifications
You must be signed in to change notification settings - Fork 10
fix(replay): fail-stop at a corrupt transaction-log frame and discard the transaction it truncated #2087
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
kriszyp
wants to merge
15
commits into
main
Choose a base branch
from
kris/2063-midlog-resync
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+646
−27
Open
fix(replay): fail-stop at a corrupt transaction-log frame and discard the transaction it truncated #2087
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
8d2551f
fix(replay): resync past a mid-log corrupt frame and report it as dat…
kriszyp 1eca0f3
fix(replay): key corrupt-frame severity on the break, not on whether …
kriszyp f25159a
fix(replay): scope break reports per database and evict rather than r…
kriszyp 0510c89
fix(replay): scope break reports by store path, not databaseName
kriszyp 3714e5a
fix(replay): treat null corrupt-frame offsets as absent
kriszyp 57bdc44
fix(replay): reset corruption bounds after progress
kriszyp 115ceea
docs(replay): clarify corrupt-frame report scope
kriszyp 0369d6b
docs(replay): describe the consecutive resync bound
kriszyp 52f13bf
Clarify corrupt-frame restart recovery
kriszyp 259b077
Merge remote-tracking branch 'origin/main' into kris/2063-midlog-resync
kriszyp 58f9c26
Escalate repeated corrupt-frame stops
kriszyp ad90fca
fix(replay): fail-stop at a corrupt transaction-log frame instead of …
kriszyp 32d8aee
fix(replay): attribute a corrupt frame to the log that broke, not to …
kriszyp 71050b1
fix(replay): track the truncated version on a single-log range too
kriszyp 3b81edb
perf(replay): keep the last-yielded version on the iterator, not in a…
kriszyp File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
147 changes: 147 additions & 0 deletions
147
integrationTests/server/replay-transaction-atomicity.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| /** | ||
| * A corrupt transaction-log frame in the middle of a source transaction must not leave that | ||
| * transaction half-applied. | ||
| * | ||
| * Replay groups every equal-version entry into one transaction and commits it at the next version | ||
| * boundary, so a break inside such a group used to commit whatever part of it was still readable — | ||
| * a transaction that never committed that way at the source becoming durable here. This drives a | ||
| * real multi-record insert, tears the log inside it, and requires the replayed table to hold all of | ||
| * that insert or none of it. See HarperFast/harper#2016 and #2063. | ||
| */ | ||
| import { suite, test, before, after } from 'node:test'; | ||
| import { ok, strictEqual as equal } from 'node:assert'; | ||
| import { readdirSync, readFileSync, openSync, writeSync, closeSync } from 'node:fs'; | ||
| import { join } from 'node:path'; | ||
|
|
||
| import { | ||
| startHarper, | ||
| teardownHarper, | ||
| sendOperation, | ||
| type ContextWithHarper, | ||
| type HarperContext, | ||
| } from '@harperfast/integration-testing'; | ||
| import { constants } from '@harperfast/rocksdb-js'; | ||
|
|
||
| // Transaction-log framing (big-endian): a fixed-size file header, then entries shaped | ||
| // [float64 timestamp][uint32 length][flags byte][length bytes of data]. | ||
| const { TRANSACTION_LOG_FILE_HEADER_SIZE, TRANSACTION_LOG_ENTRY_HEADER_SIZE } = constants; | ||
|
|
||
| const DB = 'atomicity'; | ||
| const TABLE = 'orders'; | ||
| const EARLIER_IDS = 60; | ||
| const TORN_IDS = 60; | ||
| // Entries of the torn transaction left readable before the break, so the test proves the readable | ||
| // part is discarded rather than proving the whole transaction was unreachable anyway. | ||
| const READABLE_BEFORE_BREAK = 10; | ||
|
|
||
| async function op(ctx: HarperContext, body: any) { | ||
| return await sendOperation(ctx, { ...body, authorization: ctx.admin }); | ||
| } | ||
|
|
||
| function records(start: number, count: number) { | ||
| const out = []; | ||
| for (let i = 0; i < count; i++) out.push({ id: start + i, payload: 'x'.repeat(256), n: i }); | ||
| return out; | ||
| } | ||
|
|
||
| async function countInRange(ctx: HarperContext, start: number, count: number): Promise<number> { | ||
| const rows = await op(ctx, { | ||
| operation: 'sql', | ||
| sql: `select count(*) as c from ${DB}.${TABLE} where id >= ${start} and id < ${start + count}`, | ||
| }); | ||
| return rows[0]?.c ?? 0; | ||
| } | ||
|
|
||
| function userTxnLogFiles(dataRootDir: string): string[] { | ||
| const out: string[] = []; | ||
| const dbRoot = join(dataRootDir, 'database'); | ||
| for (const db of readdirSync(dbRoot)) { | ||
| if (db === 'system') continue; | ||
| const tlogRoot = join(dbRoot, db, 'transaction_logs'); | ||
| let nodes: string[]; | ||
| try { | ||
| nodes = readdirSync(tlogRoot); | ||
| } catch { | ||
| continue; | ||
| } | ||
| for (const node of nodes) { | ||
| for (const file of readdirSync(join(tlogRoot, node))) { | ||
| if (file.endsWith('.txnlog')) out.push(join(tlogRoot, node, file)); | ||
| } | ||
| } | ||
| } | ||
| return out; | ||
| } | ||
|
|
||
| /** | ||
| * Break the framing partway through the log's LAST transaction — the run of trailing entries that | ||
| * share the highest timestamp — leaving `readableBefore` of its entries intact ahead of the break. | ||
| * Returns how many entries that transaction has, or 0 if the log has no such run to tear. | ||
| */ | ||
| function tearLastTransaction(path: string, readableBefore: number): number { | ||
| const buf = readFileSync(path); | ||
| const entries: { lengthPos: number; timestamp: number }[] = []; | ||
| let pos = TRANSACTION_LOG_FILE_HEADER_SIZE; | ||
| while (pos + TRANSACTION_LOG_ENTRY_HEADER_SIZE <= buf.length) { | ||
| const timestamp = buf.readDoubleBE(pos); | ||
| if (timestamp === 0) break; // a zero timestamp marks end-of-log to the reader | ||
| const lengthPos = pos + 8; | ||
| const length = buf.readUInt32BE(lengthPos); | ||
| const next = pos + TRANSACTION_LOG_ENTRY_HEADER_SIZE + length; | ||
| if (length === 0 || next > buf.length) break; | ||
| entries.push({ lengthPos, timestamp }); | ||
| pos = next; | ||
| } | ||
| if (entries.length === 0) return 0; | ||
| const lastTimestamp = entries.at(-1).timestamp; | ||
| let first = entries.length - 1; | ||
| while (first > 0 && entries[first - 1].timestamp === lastTimestamp) first--; | ||
| const transactionEntries = entries.length - first; | ||
| if (transactionEntries <= readableBefore) return 0; | ||
| // Force this entry's declared length to overrun the log (top byte → 0xff, ≥ 4 GB): the reader | ||
| // throws a bounded RangeError there and cannot locate any entry after it. | ||
| const fd = openSync(path, 'r+'); | ||
| try { | ||
| writeSync(fd, Buffer.from([0xff]), 0, 1, entries[first + readableBefore].lengthPos); | ||
| } finally { | ||
| closeSync(fd); | ||
| } | ||
| return transactionEntries; | ||
| } | ||
|
|
||
| suite('Replay transaction atomicity across a corrupt frame', (ctx: ContextWithHarper) => { | ||
| before(async () => { | ||
| // Don't flush on exit: the crash must leave these writes recoverable only from the txn log. | ||
| await startHarper(ctx, { env: { HARPER_NO_FLUSH_ON_EXIT: true } }); | ||
| await op(ctx.harper, { operation: 'create_database', database: DB }); | ||
| await op(ctx.harper, { operation: 'create_table', database: DB, table: TABLE, primary_key: 'id' }); | ||
| }); | ||
| after(async () => teardownHarper(ctx)); | ||
|
|
||
| test('discards a transaction the corrupt frame truncated instead of applying part of it', async () => { | ||
| await op(ctx.harper, { operation: 'insert', database: DB, table: TABLE, records: records(1, EARLIER_IDS) }); | ||
| // The last insert is one source transaction, and is the one the tear lands inside. | ||
| await op(ctx.harper, { operation: 'insert', database: DB, table: TABLE, records: records(1001, TORN_IDS) }); | ||
|
|
||
| const dataRootDir = ctx.harper.dataRootDir; | ||
| await new Promise<void>((resolve) => { | ||
| ctx.harper.process.once('exit', () => resolve()); | ||
| ctx.harper.process.kill('SIGKILL'); | ||
| }); | ||
| let torn = 0; | ||
| for (const file of userTxnLogFiles(dataRootDir)) { | ||
| torn = Math.max(torn, tearLastTransaction(file, READABLE_BEFORE_BREAK)); | ||
| } | ||
| // Fail loudly, not vacuously, if the framing or the transaction grouping ever changes. | ||
| ok(torn > READABLE_BEFORE_BREAK, `expected to tear a multi-entry transaction, tore ${torn} entries`); | ||
|
|
||
| await startHarper(ctx); | ||
|
|
||
| // None of it: the break makes the rest of that insert unreadable, so the readable prefix is | ||
| // discarded rather than committed as a transaction the source never committed. | ||
| equal(await countInRange(ctx.harper, 1001, TORN_IDS), 0, 'the truncated transaction must not be applied in part'); | ||
| // The transactions that completed ahead of the break are unaffected: fail-stop costs the | ||
| // transaction the break landed in, not the log up to it. | ||
| equal(await countInRange(ctx.harper, 1, EARLIER_IDS), EARLIER_IDS); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This summary always logs at
errorwith "repair the transaction log or re-clone this node," even when the break was a benign torn tail (nothing readable follows — the expected outcome of an unflushed crash, the primary case this PR handles).CorruptFrameStoponly tracksbreaks/truncatedVersions, so there's no way here to tell that apart from a genuine mid-log break (real data loss).That distinction is already computed one file away:
createCorruptFrameReporterinreplayLogsGuards.tsderivesmidLogfromerror.resyncPosition != nullspecifically so severity "follows the break's own shape" rather than the pass's outcome (see its docstring and theif (midLog) error else warnsplit at replayLogsGuards.ts:310-319). This block re-derives the same "a break happened" signal but skips that discrimination, so a routine crash-restart now also emits a second, always-erroralarm here — on top of the correctly-warn-leveled per-log message — telling operators to repair/re-clone for what may be nothing more than the normal torn tail of an ungraceful shutdown. The "every entry after the break is quarantined" wording is also inaccurate for a torn tail, since there are no entries after it.Consider threading
midLogthroughCorruptFrameStop(set alongsidetruncatedVersionsintrackCorruptFrames) and gating this message's severity/wording on it, the same way the per-log reporter already does.