Skip to content

A journal that could not be applied was deleted, and nobody was told either way - #207

Merged
dmitrat merged 1 commit into
mainfrom
rollback-journal-speaks
Aug 15, 2026
Merged

A journal that could not be applied was deleted, and nobody was told either way#207
dmitrat merged 1 commit into
mainfrom
rollback-journal-speaks

Conversation

@dmitrat

@dmitrat dmitrat commented Aug 15, 2026

Copy link
Copy Markdown
Owner

Item 5 of Docs/FIX-PLAN-FROM-SITE-FINDINGS-2026-08-15.md, in the shape the plan recommended: the failure is reported, the file is kept, and opening still does not fail.

What was there

catch
{
    // Skip corrupted journals
}

…and the file was deleted on the successful path only. So a journal that threw half way left the database carrying half a transaction, took the only evidence with it, and told nobody.

The intent is sound and is kept — one unreadable file must not lock somebody out of the rest of their data. What changes is that the failure reaches ITransactionJournal.RecoveryFailures and TransactionalStore.RecoveryFailures, and that the file survives to be looked at. A journal that was applied is still removed.

Nothing logs it, and that is written into the type rather than left to be found out. OutWit.Database.Core has no ILogger anywhere, and taking a logging dependency for this is a decision of a different size than the fix. A caller who never reads the property is no better off than before. RecoveryFailures is a default interface member returning empty, so no existing implementer had to change.

The second half was found by a test, and it is the more interesting one

The entry reader's own catch { break; } stopped at the first unreadable entry and returned what it had — which the caller could not tell from a journal that ended there. So a torn tail was applied in part and then deleted as though it had been applied in full.

The first attempt to distinguish them compared the stream position against the length, and that was wrong: BinaryReader.ReadBytes on a truncated entry returns a short array and leaves the stream at its end, so a torn tail looks exactly like a clean end to anything counting bytes. The damaged-tail case went red and said so. Damage is now an explicit flag set at every exit that is not "the file ended", and the short reads are checked.

Checkpoint() claimed something it never did

Its comment said it "ensures no orphan journals exist". It never did — and must not: since recovery now keeps what it could not apply, deleting orphans would throw away exactly the evidence just preserved. The comment says that instead.

One part had an empty red set, which is what sabotaging part by part is for

Restoring the old empty catch left every case green. A file that is not a journal never throws — it fails the magic check and comes back as damage — so nothing in the fixture reached the exception path at all.

A journal held open by somebody else does reach it, which is what a backup agent or a scanner looks like from here. That case is now in the fixture, and with it the two parts have different red sets, one case each:

sabotage red
the empty catch restored AJournalThatCannotBeOpenedIsReportedTest
damage detection removed AJournalWithADamagedTailAppliesThePrefixAndReportsTheRestTest

Plus the control that keeps them honest: a journal that can be applied is applied, restores the value from before the change, reports nothing, and is deleted.

Core 2376.

🤖 Generated with Claude Code

…either way

Item 5 of Docs/FIX-PLAN-FROM-SITE-FINDINGS-2026-08-15, in the shape the plan recommended:
the failure is reported, the file is kept, and opening still does not fail.

RollbackJournal.Recover wrapped every journal in try { ... } catch { } under the comment
"Skip corrupted journals" and deleted the file on the successful path only - so a journal
that threw half way left the database carrying half a transaction, took the only evidence
with it, and told nobody. The intent is sound and is kept: one unreadable file must not
lock somebody out of the rest of their data. What changes is that the failure reaches
ITransactionJournal.RecoveryFailures and TransactionalStore.RecoveryFailures, and that the
file survives to be looked at. A journal that WAS applied is still removed.

NOTHING LOGS IT, and that is written into the type rather than left to be found out:
OutWit.Database.Core has no ILogger anywhere, and taking a logging dependency for this is a
decision of a different size than the fix. A caller who never reads the property is no
better off than before. RecoveryFailures is a default interface member returning empty, so
an implementation that recovers all or nothing needs no opinion about it and no existing
implementer had to change.

THE SECOND HALF WAS FOUND BY A TEST AND IS THE MORE INTERESTING ONE. The entry reader's own
catch { break; } stopped at the first unreadable entry and returned what it had, which the
caller could not tell from a journal that ended there - so a torn tail was applied in part
and then deleted as though it had been applied in full. The first attempt to distinguish
them compared the stream position against the length and was WRONG: BinaryReader.ReadBytes
on a truncated entry returns a SHORT array and leaves the stream at its end, so a torn tail
looks exactly like a clean end to anything counting bytes. The damage is now an explicit
flag set at every exit that is not "the file ended", and the short reads are checked.

Checkpoint() claimed in a comment to ensure no orphan journals exist. It never did, and it
must not: since recovery now keeps what it could not apply, deleting orphans would throw
away exactly the evidence just preserved. The comment says that instead.

AND ONE PART HAD AN EMPTY RED SET, which is what sabotaging part by part is for. Restoring
the old empty catch left every case green: a file that is not a journal never throws - it
fails the magic check and comes back as damage - so nothing reached the exception path at
all. A journal held open by somebody else does reach it, which is what a backup agent or a
scanner looks like from here, and that case is now in the fixture. With it, the two parts
have different red sets: one case each.

Core 2376.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dmitrat
dmitrat merged commit 8ab8bcf into main Aug 15, 2026
1 check passed
@dmitrat
dmitrat deleted the rollback-journal-speaks branch August 15, 2026 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant