Fix BC-10008678752: tame self-hosted import disk demands by jeremy · Pull Request #2985 · basecamp/fizzy

jeremy · 2026-07-20T02:26:09Z

Summary

Hardens the self-hosted import pipeline, prompted by the investigation on card 10008678752: a large import demanded ~4× the export size in free disk, and running out mid-import crashes the shared puma/Solid Queue container — with SQLite reporting ENOSPC as SQLITE_FULL: "database or disk is full", which misdirects operators at the database.

Scope note: per the customer's own Thruster access logs, their upload died at the proxy read timeout (30s → 502) and never reached Rails — so this PR is not claimed as that customer's unblock. It fixes the processing-leg failure mode that the same investigation surfaced, and it halves the disk headroom script/import-account (their likely path, which bypasses the browser upload) needs.

Changes

ZipFile.read_from_disk reads the stored blob in place. blob.open copies the entire blob to a tempfile first; zip reading only needs random access, which a direct File handle provides. This removes a full-export-size tempfile copy in each of the check and process passes. Falls back to blob.open for services without a disk path. S3/SaaS is unaffected (read_from_s3 already streams).
Account::Import#check and #process preflight free disk against 2× the export size, failing fast with a new insufficient_space failure reason — surfaced on the import status page and in failure emails as "There wasn't enough storage space to process your import." (the remedy — free up at least twice the export file's size — lives in the self-hosting docs) — instead of crashing the container mid-import. The job discards (no auto-retry) on this error like other terminal import failures. Unparseable df output counts as indeterminate and the import proceeds; the parse anchors on the capacity NN% column so filesystem names with spaces can't shift it. A mid-process resume skips the re-check, since the import's own writes have consumed part of the estimated demand.
ZipFile.path_on_disk(blob) is the single locus for "is this blob on local disk, and where" — both the preflight and the in-place read derive from it, and df measures the volume actually holding the blob.
Docs: new "Importing an existing Fizzy account" section in docs/docker-deployment.md covering the free-space requirement, the insufficient-space error and remedy, script/import-account for very large exports, and THRUSTER_HTTP_READ_TIMEOUT for large browser uploads.

Peak disk demand for a 24 GB import

	Before	After
Stored blob (attach copy)	24 GB	24 GB
`blob.open` tempfile (check)	24 GB	—
`blob.open` tempfile (process)	24 GB	—
Imported attachment blobs	~24 GB	~24 GB

(The upload leg's multipart buffering and in-request attach are a separate improvement — direct_upload: true — noted on the card.)

Testing

New model tests: insufficient disk space fails fast with the insufficient_space reason in both check and fresh process; resumed process skips the preflight; indeterminate free space proceeds; unparseable df output returns nil rather than 0; df lines with spaces in the filesystem name parse correctly
New mailer test: insufficient_space failure email renders the remedy copy (text + HTML parts)
Existing export→import round-trip test exercises the in-place read path end to end (Disk service)
zip_file, account/import, import_mailer, account/export, data_transfer, and blob-key-traversal suites green (64 tests)

Bug Card

https://app.basecamp.com/2914079/buckets/27/card_tables/cards/10008678752

A self-hosted import paid ~4x the export size in peak disk: multipart buffer + attach copy + a blob.open tempfile copy per read pass + the imported attachment blobs. ENOSPC mid-import crashes the shared puma/Solid Queue container, and SQLite reports it as 'database or disk is full' — misdirecting operators at the database. - ZipFile.read_from_disk now opens the stored file in place via service.path_for (zip reading only needs random access), eliminating a full-size tempfile copy in each of check and process - Account::Import#check preflights free disk (df -Pk on the service root) and fails fast with a clear insufficient_disk reason instead of crashing the container mid-import Card: https://app.basecamp.com/2914079/buckets/27/card_tables/cards/10008678752 Claude-Session: https://claude.ai/code/session_01CSpm1M4ZQwmurqqNb5uQx8

Copilot

Pull request overview

Improves reliability of self-hosted account imports by reducing peak disk usage during ZIP reads and by failing early with a clear error when the server doesn’t have enough free disk space to safely complete an import (avoiding mid-import ENOSPC crashes that can take down the shared web/Solid Queue process).

Changes:

Read the stored ZIP blob in place on disk services that expose a filesystem path, avoiding blob.open’s tempfile copy during import passes.
Preflight available disk space during Account::Import#check, marking imports failed with a new insufficient_disk failure reason and discarding the job without retries.
Surface the new failure reason in the import status page and failure emails, and add model tests for the new behavior.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
test/models/account/import_test.rb	Adds model coverage for insufficient-disk fast-fail and “indeterminate disk space proceeds” behavior.
app/views/mailers/import_mailer/failed.text.erb	Adds user-facing copy for the new `insufficient_disk` failure reason (text email).
app/views/mailers/import_mailer/failed.html.erb	Adds user-facing copy for the new `insufficient_disk` failure reason (HTML email).
app/views/account/imports/show.html.erb	Shows the new insufficient-disk failure message on the import status page.
app/models/zip_file.rb	Avoids double-tempfile disk demand by reading disk-backed blobs in place when possible.
app/models/account/import.rb	Adds disk-space preflight, new failure reason, and terminal error class for insufficient disk.
app/jobs/account/data_import_job.rb	Discards import jobs on insufficient disk to prevent retries of terminal failures.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…arse, docs - ZipFile.path_on_disk is now the one place that knows how to find a blob on disk; both the import preflight and in-place zip reads derive from it, and df now measures the volume actually holding the blob. - Rename failure reason to insufficient_disk_space and the constant to REQUIRED_DISK_SPACE_FACTOR (it's a multiplier, not a size). - Unparseable df output now yields nil (indeterminate -> proceed) instead of 0 (false failure), with a regression test. - Mailer test for the insufficient_disk_space reason; trimmed comments. - Document self-hosted import disk requirements, the insufficient-space error, script/import-account, and THRUSTER_HTTP_READ_TIMEOUT.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

- Anchor the df parse on the capacity NN% field instead of a fixed column index, so filesystem names containing spaces can't shift the read into the wrong column (false insufficient-disk-space failures). - Run the disk preflight at the start of process as well as check: a restarted job resumes at the process step, where the writes actually happen, and free space can have changed since check. Skipped on mid-process resume — the import's own writes have consumed part of the estimated demand, so re-requiring the full factor would terminally fail a nearly-done import.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Integer division truncated the GB figures (1.9 GB read as 1 GB), understating the requirement. Use number_to_human_size instead.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

The fail-fast rationale (mid-import ENOSPC crashes the shared web/Solid Queue process) lives in the PR and prior commit message.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

monorkin

Better, but there are still very descriptive comments that describe things obvious to any reader - you can drop them completely.

"disk_storage" can be misleading depending on the storage service used, better to use just "storage"

And the error message in the UI and emails is overly descriptive to regular users. I understand what you are trying to do, but this is the wrong place for it - that UI is used by admins and regular people alike, you can't surface errors about the instance like that to people that can't take action on it. The better way to do it is to print a generic, friendly error and update the self-hosting docs to explain what the error means. Also, you are already printing the detailed explanation of the error to the logs.

Rename insufficient_disk_space -> insufficient_space throughout (error class, enum, methods, constant) and adopt the suggested failure copy, keeping the remedy sentence. Depending on the storage backing the server, there might not be a disk.

jeremy · 2026-07-20T08:57:08Z

🤖 Done — took your direction across the board: renamed the family to insufficient_space (error class, enum, methods, constant — no more disk), adopted your failure copy with the remedy sentence kept after it, and dropped the remaining explanatory comments completely. 8176eb8 + 4535723.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Use the kilobytes size helper instead of a bare 1024, and document that the preflight is skipped when free space can't be determined.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

monorkin

I think we should tweak the error message. As I said earlier, the current version over exposes the solution to people that can't act on it. Better to keep it simple.

Keep the user-facing message simple; the remedy lives in the self-hosting docs where the operator can act on it.

Copilot