[zahariash]

This work was produced with the help of language models.

Fixes #1199.

Land with or after #1389: with this PR a checkpoint refused at max_db_size no longer fails its statement, so without #1389 the next statement segfaults (#1386).

In the issue's script, COMMIT raised Transaction committed successfully, but the post-commit checkpoint failed. The committed data is durable and will be recovered on restart: Buffer manager exception: Unable to allocate memory! ... while another connection held the buffer pool full, and the retry stored a second relationship. A checkpoint that timed out waiting for another connection's open write transaction failed the write the same way.

Fix. A failed automatic checkpoint no longer fails the statement that committed, since the commit is already in the WAL and the error only invited a retry that applied it twice. commit() drops the failure and prints one stderr warning with the WAL size and the reason; the core has no logger. A rolled-back checkpoint is retried by a later write commit. A checkpoint that poisoned the WAL, past its record or in cleanup, now fails the next write instead, which really did not commit. The forced checkpoint after COPY FROM or CREATE INDEX on a large ART still fails the statement, since that data is not in the WAL.

Failed rollback. A checkpoint whose rollback threw left the engine accepting writes on a half-undone checkpoint. Checkpointer::rollback now poisons the WAL, as a failed cleanup already does. A beforeRollback hook, added after the existing virtual hooks, lets the test make it fail.

Forced checkpoint message. It promised the data "is durable and will be recovered on restart". On 0.21.2, a COPY of 1,000 rels whose checkpoint failed this way left none after a kill and reopen. The message now says the changes "may be lost on restart unless a later CHECKPOINT succeeds". If that checkpoint poisoned the WAL instead, after its record may have reached storage or in a failed rollback, no later CHECKPOINT can run, so the message says the database refuses writes until restart and that the changes survive it only if the checkpoint's record reached storage.

Tests. PostCommitCheckpointFailureTest checkpoints after every commit. Five of its tests fail on main: with the checkpoint failing, a CREATE and a COMMIT succeed with the warning and survive a reopen; the next write's checkpoint empties the WAL; a checkpoint blocked by another open write transaction no longer fails the write; and a checkpoint that poisons the WAL, or fails in cleanup, leaves the write successful and fails the next one. Two more keep the forced path failing after COPY and CREATE INDEX, where a later CHECKPOINT keeps the rows and the index. ForcedCheckpointFailingAfterItsRecordRefusesWrites fails the forced checkpoint after its record: the error says writes are refused, not that a later CHECKPOINT helps, and a reopen has the rows. FailedRollbackPoisonsWAL has a rollback fail: reads continue, the next write is refused, and a reopen recovers all 100 rows. Each test fails when its guard is removed. The issue's script now prints attempt 1: ok with one relationship. Full ctest: 3145/3145.

[adsharma]

Low (non-blocking): when this guard is false the auto-checkpoint failure is swallowed with no log, so the statement succeeds silently and a persistently failing auto-checkpoint is invisible until WAL growth surfaces it. Consider a warn-level log here (reason + WAL size).

[zahariash]

The core has no logger (spdlog is test/benchmark only), so it's one stderr line with the WAL size and the reason, like the Warning: in ice_disk_utils.h. It prints once per failed checkpoint, so a persistent failure repeats it on each write commit. A poisoned WAL also reports its reason on the next write.

[adsharma]

Nit: 'the next write commit retries' is only true when canAutoCheckpoint triggers again (threshold-gated; always-true only under the test's checkpoint_threshold=0). Suggest 'a later write commit retries'.

[zahariash]

Done in b9c44b4: "a rolled-back checkpoint is retried by a later write commit".

[adsharma]

One open question from review (two inline nits posted separately):

Question — cleanup-path contract: postCheckpointCleanup exceptions bypass the CheckpointException wrap and still fail the statement via the generic catch, even when the checkpoint already retired the WAL. That re-exposes the #1199 double-apply hazard on the cleanup path (data durable, statement reports failure → client retries). Pre-existing, not introduced here — but FailedCheckpointCleanupFailsCommit now locks it in. Is that the intended permanent contract, or should post-retirement cleanup failures also be swallowed on the auto path?

[zahariash]

No, so the approach changed in two commits on top.

Before. The first version dropped a failed automatic checkpoint only when it was rolled back, and still failed the statement when the WAL was poisoned or cleanup failed. The argument was that a poisoned WAL refuses writes, so a retry can't apply twice. That holds only until a restart: both tests reopen and find the row, so a client that restarts and retries applies it twice.

Now. b9c44b4 drops every failure of the automatic checkpoint and prints one stderr warning with the WAL size and the reason. The commit is in the WAL in all of these cases. A poisoned WAL fails the next write instead, which really did not commit, so retrying it is correct. The forced checkpoint after COPY FROM or a large CREATE INDEX still fails the statement, since its data is not in the WAL. The code no longer depends on which exception type checkpointNoLock throws, and WAL::isPoisoned() is gone.

New guard. Dropping everything would have made one case silent: a rollback that throws midway left the engine accepting writes on a half-undone checkpoint. 82135e1 makes Checkpointer::rollback poison the WAL, as the cleanup path already does, tested through a new beforeRollback hook.

Stress. A loop through the C API with failing checkpoints, kills and the same seeds on both builds: retries applied a write twice 372 times on main and never here. Committed writes went missing after a reopen in 33 of 300 rounds on main and 37 here (paired p = 0.54), all through #1304's pinned-checkpoint truncation; the repro is in my comment there.

DuckDB. 2.0.0 keeps failing the statement. With a checkpoint after every commit and an I/O error during it, the INSERT raises FatalException: Transaction COMMIT succeeded and is durable, but the autocheckpoint failed, and every later query fails until reopen; the reopened database has the row. Its FatalException is a separate Python class from TransactionException, so retry loops can tell them apart. Ladybug's driver raises RuntimeError for both, which is why this PR drops the error instead.