fix(core): fix crash on column type conversion under low disk space

[ideoma]

Summary

ALTER TABLE ... ALTER COLUMN ... TYPE crashed the whole JVM with a
native SIGBUS when disk space was low, instead of failing the
statement with a normal error. The destination column file was sized
with ftruncate(), which only sets the logical file length and does
not reserve real disk blocks; a later native write into an unbacked
page faults with SIGBUS, which cannot be caught from Java/JNI. The fix
switches to TableUtils.allocateDiskSpace() (posix_fallocate),
matching every other writable-mmap call site in the codebase, so a
disk-space shortfall now surfaces as a catchable CairoException.

Crash

#
# A fatal error has been detected by the Java Runtime Environment:
#
#  SIGBUS (0x7) at pc=0x00007ff7e2d08a72, pid=3778, tid=10278
#
# Problematic frame:
# C  [libquestdb....so+0x5ba72]  Java_io_questdb_griffin_ConvertersNative_fixedToFixed+0x3522

Current thread (0x00007ff6e5244ac0):  JavaThread "testing_0" [_thread_in_native, id=10278]

Native frames:
C  [libquestdb....so+0x5ba72]  Java_io_questdb_griffin_ConvertersNative_fixedToFixed+0x3522

Java frames:
J  io.questdb.griffin.ConvertersNative.fixedToFixed(JJJJJ)J
J  io.questdb.cairo.ColumnTypeConverter.convertFixedToFixed(...)Z
J  io.questdb.cairo.ColumnTypeConverter.convertColumn(...)Z
J  io.questdb.griffin.ConvertOperatorImpl.cthConvertPartitionHandler(...)V
J  io.questdb.griffin.ConvertOperatorImpl$$Lambda.run(...)V
J  io.questdb.cairo.ColumnTaskJob.doRun(...)Z
J  io.questdb.mp.AbstractQueueConsumerJob.run(...)Z
J  io.questdb.mp.Worker.loopBody(...)V
...
J  io.questdb.mp.Worker.run()V

siginfo: si_signo: 7 (SIGBUS), si_code: 2 (BUS_ADRERR)

Test plan

• New regression test AlterTableChangeColumnTypeTest#testConvertFixedToFixedReservesDiskSpaceBeforeMapping asserts the destination column file's disk space is reserved via ff.allocate() before it is mapped for writing
• Full AlterTableChangeColumnTypeTest suite passes (87 tests)
• QwpIngressProcessorStateTest#testAlterColumnTypeWithBufferedRowsAppliesOnNextLookup passes, confirming the WAL-buffered-rows call path through the same fixed method is unaffected

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d0231c75-df66-43b2-91ac-7000ff6cae09

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[ideoma]

[PR Coverage check]

😍 pass : 1 / 1 (100.00%)

file detail

	path	covered line	new line	coverage
🔵	io/questdb/cairo/ColumnTypeConverter.java	1	1	100.00%

[ideoma]

Review (level 3)

Submodule provenance: no submodule pointer changed in this PR's diff (java-questdb-client untouched) — N/A.

Critical

None.

Moderate

Problem: New regression test never exercises ff.allocate() actually failing.
Net impact: The PR's own "surfaces as a catchable CairoException" claim is untested.
Evidence: testConvertFixedToFixedReservesDiskSpaceBeforeMapping's allocate() override only records the size and unconditionally return super.allocate(fd, size) — no branch ever returns false.

AlterTableChangeColumnTypeTest.java:1745-1751. The test proves allocate() is called with the right size before mmap(MAP_RW) — and reverting the production hunk and re-running confirms it does catch a regression back to ff.truncate() (fails with expected:<16> but was:<-1>). But it never proves the actual claim in the PR title: that a real ff.allocate() failure produces a catchable CairoException instead of corrupting state. The sibling test testConvertFailsOnColumnFileOpen in the same file already has the idiom for this (a gated override returning false/-1 on demand) — it just wasn't applied to allocate(). A repo-wide grep for "No space left"/ENOSPC assertions found nothing that goes through ColumnTypeConverter, so this exception path has zero coverage anywhere.

Suggested fix: add a gated allocate() override (mirroring testConvertFailsOnColumnFileOpen's pattern) that returns false for the destination fd, and assert the ALTER throws a CairoException containing "No space left" rather than crashing.

Minor

New test doesn't guard with assumeNonWal()/drainWalQueue() like its neighbor testConvertFailsOnColumnFileOpen does, relying implicitly on DefaultCairoConfiguration.getWalEnabledDefault() being false. Not a live bug today (the default is stable and unoverridden), just a latent robustness gap.

Adjacent findings (not blocking — pre-existing, not introduced by this PR)

1. convertToDecimal() and convertDecimalToBinaryFloat() in the same file have the identical bug this PR fixes.

• Location: ColumnTypeConverter.java:889 (convertToDecimal) and :1213 (convertDecimalToBinaryFloat), both unchanged by this diff.
• Under low disk space, these two conversions defeat TableUtils.allocateDiskSpace's guard the same way the pre-fix convertFixedToFixed did: ff.truncate() sets the file's logical length first, so the ff.length(fd) < size check inside allocateDiskSpace (invoked later via MemoryCMARWImpl's mapRW) sees the length already satisfied and never calls ff.allocate()/posix_fallocate. ALTER ... TYPE DECIMAL(p,s) or DECIMAL→DOUBLE/FLOAT can still crash the JVM with the same uncatchable SIGBUS.
• Reachable via the same ConvertOperatorImpl/CopyWalSegmentUtils dispatch as the fixed method, just routed by decimal-involving type pairs.
• Suggested fix: same one-line swap (ff.truncate → TableUtils.allocateDiskSpace) at both sites.
• Severity if filed standalone: Critical (identical defect class to the one this PR's title claims to have fixed).

Same file, same root cause, same one-line fix pattern — worth considering for this PR rather than a follow-up, given it's the exact crash this PR's title claims to close.

2. The destination-file mmap allocation isn't wired into the per-query MemoryTracker.

• Location: ColumnTypeConverter.java:193-194 and the two sibling methods above.
• rowCount * dstColumnTypeSize allocation bypasses a configured cairo.query.memory.limit.bytes — confirmed live and bound: AlterOperation.apply() registers a MemoryTrackerWorkload.QUERY tracker via queryRegistry.register() around every ALTER, but ConvertOperatorImpl/ColumnTypeConverter never reference it.
• Severity if filed standalone: Moderate (architecture gap, unrelated to this PR's actual diff).

Coverage map

One admitted gap (the ff.allocate()-failure branch, Moderate, above). The happy path, base-comparison, and the direct-ALTER call path are covered — independently re-verified: test is green at PR head and red at base (git show 1f9265b8925863db473a1f291705d6eb4f2a8c7c reverted) with the exact predicted assertion failure (expected:<16> but was:<-1>).

Summary

Verdict: approve with comments. No open Critical findings; test gate passes (no admitted Critical coverage gap).

The production fix itself (ff.truncate → TableUtils.allocateDiskSpace) is correct, minimal, and matches the established pattern used at 5+ other writable-mmap sites in the codebase (MemoryCMARWImpl, MemoryM, Mig607). I independently verified the regression test both statically and dynamically (built and ran it at head and against the reverted hunk in a scratch worktree).

The Moderate item (testing the actual allocate()-failure path) and the adjacent sibling-method bug are worth a look but don't block this PR.

[ideoma]

/azp run macwin

[ideoma]

Review (level 3)

Submodule provenance: no submodule pointer changed in this PR's diff (java-questdb-client untouched) — N/A.

Critical

None.

Moderate

Problem: New regression test never exercises ff.allocate() actually failing.
Net impact: The PR's own "surfaces as a catchable CairoException" claim is untested for this codepath.
Evidence: AlterTableChangeColumnTypeTest.java:1746-1751 — the allocate() override only records the fd/size and unconditionally delegates to super.allocate(); grep -rln "No space left" core/src/test/ finds no test going through ColumnTypeConverter/ConvertOperatorImpl.

The test proves allocate() is called with the right size, before the RW mmap — reverting the production hunk does make it fail with the exact predicted message (verified dynamically: green at head, red at base, expected:<16> but was:<-1>). But nothing asserts that a real ff.allocate() failure produces a catchable CairoException rather than corrupting state. This is tempered by the fact that TableUtils.allocateDiskSpace's failure branch is already well-exercised generically elsewhere (TableWriterTest, O3FailureTest, O3FailureFuzzTest, CairoEngineTest, WalWriterTest, PostingIndexCriticalIssuesTest all fake allocate() returning false and assert a clean CairoException), so the underlying mechanism is proven — only this specific integration point is unverified. Suggested fix: add a gated allocate() override (mirroring the file's own testConvertFailsOnColumnFileOpen idiom) returning false for the destination fd, and assert the ALTER throws CairoException with "No space left" instead of crashing.

Minor

New test doesn't guard with assumeNonWal()/drainWalQueue() like neighboring tests in the same file, relying implicitly on DefaultCairoConfiguration.getWalEnabledDefault() being false. Not a live bug today, just a latent robustness/dimension gap — the class randomizes walEnabled but this test never exercises the WAL-queued conversion path.

Adjacent findings (not blocking — pre-existing, not introduced by this PR)

convertToDecimal() and convertDecimalToBinaryFloat() retain the identical defect this PR fixes.

• Location: ColumnTypeConverter.java:889 and :1213, both unchanged by this diff.
• Mechanism (verified by direct source trace): both methods call ff.truncate(dstFixFd, dstMapBytes) first, which sets ff.length(fd) to exactly dstMapBytes. They then call dstFixMem.of(ff, dstFixFd, true, null, Files.PAGE_SIZE, dstMapBytes, memoryTag), which reaches TableUtils.mapRW() → allocateDiskSpace(ff, fd, dstMapBytes). That function's guard (TableUtils.java:281) is ff.length(fd) < size — since the preceding truncate already made length equal to size, the guard is false and ff.allocate() is skipped entirely. So real block reservation never happens, and the same uncatchable SIGBUS-on-write risk this PR fixes for convertFixedToFixed remains live for ALTER ... TYPE DECIMAL(p,s) and DECIMAL→DOUBLE/FLOAT conversions. Confirmed exhaustive: grep -n "ff.truncate(dstFixFd" finds exactly these two remaining sites in the file.
• Reachable via the same ConvertOperatorImpl/CopyWalSegmentUtils dispatch as the fixed method (ColumnTypeConverter.java:114-117), for any fixed→DECIMAL or DECIMAL→DOUBLE/FLOAT ALTER TABLE ... ALTER COLUMN ... TYPE.
• Suggested fix: same one-line swap (ff.truncate → TableUtils.allocateDiskSpace) at both sites.
• Severity if filed standalone: Critical (identical defect class to the one this PR's title claims to have fixed).

Worth folding into this PR rather than a follow-up, given it's the exact crash class this PR's title claims to close, in the same file, with the same one-line fix.

Coverage map

One admitted gap (the ff.allocate()-failure branch, Moderate, above). Happy path, base-comparison, and both call paths (direct ALTER and WAL-lazy-conversion) are covered by the shared production method; the WAL-lazy-conversion path's own test (QwpIngressProcessorStateTest) exercises correctness but doesn't separately assert the allocate-before-mmap ordering — low-risk since it's the same shared method already covered by the new test.

Summary

Verdict: approve with comments. No open Critical findings; test gate passes (no admitted Critical coverage gap).

The production fix (ff.truncate → TableUtils.allocateDiskSpace) is correct, minimal, and independently confirmed as a genuine regression test — dynamically ran it green at head, then red (with the exact predicted assertion failure) after reverting just the production hunk, then restored the tree.

The Moderate item (untested allocate()-failure path) and the adjacent sibling-method bug (worth fixing in the same PR, same file, same fix pattern) don't block merge.

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).