From 0ba7b9f24f821b27771ae68e907e745cccff5760 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Sun, 19 Jul 2026 19:26:04 -0700 Subject: [PATCH 1/9] Fix BC-10008678752: tame self-hosted import disk demands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- app/jobs/account/data_import_job.rb | 3 +- app/models/account/import.rb | 34 ++++++++++++++++++- app/models/zip_file.rb | 18 ++++++++-- app/views/account/imports/show.html.erb | 2 ++ .../mailers/import_mailer/failed.html.erb | 2 ++ .../mailers/import_mailer/failed.text.erb | 2 ++ test/models/account/import_test.rb | 26 ++++++++++++++ 7 files changed, 82 insertions(+), 5 deletions(-) diff --git a/app/jobs/account/data_import_job.rb b/app/jobs/account/data_import_job.rb index 67862d4c71..f9425289e9 100644 --- a/app/jobs/account/data_import_job.rb +++ b/app/jobs/account/data_import_job.rb @@ -2,7 +2,8 @@ class Account::DataImportJob < ApplicationJob include ActiveJob::Continuable queue_as :backend - discard_on Account::DataTransfer::RecordSet::IntegrityError, ZipFile::InvalidFileError + discard_on Account::DataTransfer::RecordSet::IntegrityError, ZipFile::InvalidFileError, + Account::Import::InsufficientDiskSpaceError def perform(import) step :check do |step| diff --git a/app/models/account/import.rb b/app/models/account/import.rb index 2bf1b88e7b..cae1b0ff62 100644 --- a/app/models/account/import.rb +++ b/app/models/account/import.rb @@ -1,4 +1,12 @@ +require "shellwords" + class Account::Import < ApplicationRecord + class InsufficientDiskSpaceError < StandardError; end + + # Peak disk demand beyond the stored zip itself: the imported attachment + # blobs roughly mirror the zip's contents, plus database growth and margin. + REQUIRED_DISK_FACTOR = 2 + broadcasts_refreshes belongs_to :account @@ -7,7 +15,7 @@ class Account::Import < ApplicationRecord has_one_attached :file, dependent: :purge_later enum :status, %w[ pending processing completed failed ].index_by(&:itself), default: :pending - enum :failure_reason, %w[ conflict invalid_export ].index_by(&:itself), prefix: :failed_due_to, scopes: false + enum :failure_reason, %w[ conflict invalid_export insufficient_disk ].index_by(&:itself), prefix: :failed_due_to, scopes: false scope :expired, -> { where(completed_at: ...24.hours.ago).or(where(status: :failed, created_at: ...7.days.ago)) } @@ -21,6 +29,7 @@ def process_later def check(start: nil, callback: nil) processing! + ensure_sufficient_disk_space ZipFile.read_from(file.blob) do |zip| Account::DataTransfer::Manifest.new(account).each_record_set(start: start) do |record_set, last_id| @@ -33,6 +42,9 @@ def check(start: nil, callback: nil) rescue Account::DataTransfer::RecordSet::IntegrityError, ZipFile::InvalidFileError => e mark_as_failed(:invalid_export) raise e + rescue InsufficientDiskSpaceError => e + mark_as_failed(:insufficient_disk) + raise e rescue => e mark_as_failed raise e @@ -69,6 +81,26 @@ def cleanup end private + # Running out of disk mid-import is the worst self-hosted failure mode: + # Solid Queue shares the web process, so once its own SQLite writes start + # failing ("database or disk is full") the whole app goes down with the + # import. Fail fast with a clear reason before consuming the space. + def ensure_sufficient_disk_space + return unless file.blob.service.respond_to?(:root) + + required = file.blob.byte_size * REQUIRED_DISK_FACTOR + available = available_disk_space + + if available && available < required + raise InsufficientDiskSpaceError, "import needs ~#{required / 1.gigabyte} GB free, found #{available / 1.gigabyte} GB" + end + end + + def available_disk_space + fields = `df -Pk #{Shellwords.escape(file.blob.service.root.to_s)} 2>/dev/null`.lines.last&.split + fields && fields[3].to_i * 1024 + end + def mark_completed update!(status: :completed, completed_at: Time.current) ImportMailer.completed(identity, account).deliver_later diff --git a/app/models/zip_file.rb b/app/models/zip_file.rb index 3415f5879e..b2b09f1e85 100644 --- a/app/models/zip_file.rb +++ b/app/models/zip_file.rb @@ -91,9 +91,21 @@ def read_from_s3(blob) end def read_from_disk(blob) - blob.open do |file| - reader = Reader.new(file) - yield reader + # Read the stored file in place when the service exposes its path. + # blob.open copies the whole blob to a tempfile first — for large + # self-hosted imports that's a multi-gigabyte extra disk demand paid + # on every read pass, and zip reading only needs random access, which + # a plain File handle provides. + if blob.service.respond_to?(:path_for) + File.open(blob.service.path_for(blob.key), "rb") do |file| + reader = Reader.new(file) + yield reader + end + else + blob.open do |file| + reader = Reader.new(file) + yield reader + end end end end diff --git a/app/views/account/imports/show.html.erb b/app/views/account/imports/show.html.erb index 745eb7bae3..55e2d0d4b2 100644 --- a/app/views/account/imports/show.html.erb +++ b/app/views/account/imports/show.html.erb @@ -22,6 +22,8 @@
The account you’re trying to import already exists. Make sure you’re importing a <%= Fizzy.saas? ? "self-hosted" : "fizzy.do" %> account.
<% elsif @import.failed_due_to_invalid_export? %>
The .zip file you uploaded doesn’t look like a Fizzy account export.
+ <% elsif @import.failed_due_to_insufficient_disk? %> +
Your server doesn’t have enough free disk space for this import. Free up at least twice the export file’s size and try again.
<% else %>
This may be due to corrupted export data or a conflict with existing data. Please try again with a fresh export.
<% end %> diff --git a/app/views/mailers/import_mailer/failed.html.erb b/app/views/mailers/import_mailer/failed.html.erb index 6aa9fab0ac..99098df21b 100644 --- a/app/views/mailers/import_mailer/failed.html.erb +++ b/app/views/mailers/import_mailer/failed.html.erb @@ -4,6 +4,8 @@

It looks like the account you are trying to import already exists.

<% elsif @import.failed_due_to_invalid_export? %>

The ZIP file isn't a Fizzy account export.

+<% elsif @import.failed_due_to_insufficient_disk? %> +

Your server doesn't have enough free disk space for this import. Free up at least twice the export file's size and try again.

<% else %>

This may be due to corrupted export data or a conflict with existing data. Please try again with a fresh export, or reach out for help if the problem persists.

<% end %> diff --git a/app/views/mailers/import_mailer/failed.text.erb b/app/views/mailers/import_mailer/failed.text.erb index 1895f0d5db..feeb1cf9f2 100644 --- a/app/views/mailers/import_mailer/failed.text.erb +++ b/app/views/mailers/import_mailer/failed.text.erb @@ -4,6 +4,8 @@ Unfortunately, we couldn't import your Fizzy account. It looks like the account you are trying to import already exists. <% elsif @import.failed_due_to_invalid_export? -%> The ZIP file isn't a Fizzy account export. +<% elsif @import.failed_due_to_insufficient_disk? -%> +Your server doesn't have enough free disk space for this import. Free up at least twice the export file's size and try again. <% else -%> This may be due to corrupted export data or a conflict with existing data. Please try again with a fresh export, or reach out for help if the problem persists. <% end -%> diff --git a/test/models/account/import_test.rb b/test/models/account/import_test.rb index c1e9c2f7cb..219a079617 100644 --- a/test/models/account/import_test.rb +++ b/test/models/account/import_test.rb @@ -220,7 +220,33 @@ class Account::ImportTest < ActiveSupport::TestCase export_tempfile&.unlink end + test "check fails fast with a clear reason when free disk space is insufficient" do + import = import_with_attached_zip + import.stubs(:available_disk_space).returns(import.file.blob.byte_size) + + error = assert_raises(Account::Import::InsufficientDiskSpaceError) { import.check } + assert_match(/GB free/, error.message) + assert import.reload.failed_due_to_insufficient_disk? + end + + test "check proceeds when free disk space cannot be determined" do + import = import_with_attached_zip + import.stubs(:available_disk_space).returns(nil) + + assert_raises(ZipFile::InvalidFileError) { import.check } + assert import.reload.failed_due_to_invalid_export? + end + private + def import_with_attached_zip + account = Account.create!(name: "Disk Check") + import = Account::Import.create!(account: account, identity: identities(:david)) + Current.set(account: account) do + import.file.attach(io: StringIO.new("not actually a zip"), filename: "export.zip", content_type: "application/zip") + end + import + end + def account_digest(account) { name: account.name, From 400d2798370a5060c180346bfd8eadf775c763ba Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 20 Jul 2026 01:19:35 -0700 Subject: [PATCH 2/9] Address review: single disk-path locus, clearer naming, hardened df parse, 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. --- app/models/account/import.rb | 28 +++++++++---------- app/models/zip_file.rb | 15 +++++----- app/views/account/imports/show.html.erb | 2 +- .../mailers/import_mailer/failed.html.erb | 2 +- .../mailers/import_mailer/failed.text.erb | 2 +- docs/docker-deployment.md | 11 ++++++++ test/mailers/import_mailer_test.rb | 11 ++++++++ test/models/account/import_test.rb | 9 +++++- 8 files changed, 54 insertions(+), 26 deletions(-) diff --git a/app/models/account/import.rb b/app/models/account/import.rb index cae1b0ff62..08ef97a93e 100644 --- a/app/models/account/import.rb +++ b/app/models/account/import.rb @@ -3,9 +3,8 @@ class Account::Import < ApplicationRecord class InsufficientDiskSpaceError < StandardError; end - # Peak disk demand beyond the stored zip itself: the imported attachment - # blobs roughly mirror the zip's contents, plus database growth and margin. - REQUIRED_DISK_FACTOR = 2 + # Imported blobs roughly mirror the zip's contents, plus database growth and margin. + REQUIRED_DISK_SPACE_FACTOR = 2 broadcasts_refreshes @@ -15,7 +14,7 @@ class InsufficientDiskSpaceError < StandardError; end has_one_attached :file, dependent: :purge_later enum :status, %w[ pending processing completed failed ].index_by(&:itself), default: :pending - enum :failure_reason, %w[ conflict invalid_export insufficient_disk ].index_by(&:itself), prefix: :failed_due_to, scopes: false + enum :failure_reason, %w[ conflict invalid_export insufficient_disk_space ].index_by(&:itself), prefix: :failed_due_to, scopes: false scope :expired, -> { where(completed_at: ...24.hours.ago).or(where(status: :failed, created_at: ...7.days.ago)) } @@ -43,7 +42,7 @@ def check(start: nil, callback: nil) mark_as_failed(:invalid_export) raise e rescue InsufficientDiskSpaceError => e - mark_as_failed(:insufficient_disk) + mark_as_failed(:insufficient_disk_space) raise e rescue => e mark_as_failed @@ -81,24 +80,23 @@ def cleanup end private - # Running out of disk mid-import is the worst self-hosted failure mode: - # Solid Queue shares the web process, so once its own SQLite writes start - # failing ("database or disk is full") the whole app goes down with the - # import. Fail fast with a clear reason before consuming the space. + # Fail fast before consuming the space: self-hosted Solid Queue shares the web + # process, so mid-import ENOSPC ("database or disk is full") takes the app down. def ensure_sufficient_disk_space - return unless file.blob.service.respond_to?(:root) + return unless path = ZipFile.path_on_disk(file.blob) - required = file.blob.byte_size * REQUIRED_DISK_FACTOR - available = available_disk_space + required = file.blob.byte_size * REQUIRED_DISK_SPACE_FACTOR + available = available_disk_space(path) if available && available < required raise InsufficientDiskSpaceError, "import needs ~#{required / 1.gigabyte} GB free, found #{available / 1.gigabyte} GB" end end - def available_disk_space - fields = `df -Pk #{Shellwords.escape(file.blob.service.root.to_s)} 2>/dev/null`.lines.last&.split - fields && fields[3].to_i * 1024 + def available_disk_space(path) + fields = `df -Pk #{Shellwords.escape(path)} 2>/dev/null`.lines.last&.split + kilobytes = fields && Integer(fields[3] || "", exception: false) + kilobytes && kilobytes * 1024 end def mark_completed diff --git a/app/models/zip_file.rb b/app/models/zip_file.rb index b2b09f1e85..faf9842fd8 100644 --- a/app/models/zip_file.rb +++ b/app/models/zip_file.rb @@ -26,6 +26,10 @@ def read_from(blob) end end + def path_on_disk(blob) + blob.service.path_for(blob.key) if blob.service.respond_to?(:path_for) + end + private def s3_service?(service) # The S3 service doesn't get loaded in development unless it's used @@ -91,13 +95,10 @@ def read_from_s3(blob) end def read_from_disk(blob) - # Read the stored file in place when the service exposes its path. - # blob.open copies the whole blob to a tempfile first — for large - # self-hosted imports that's a multi-gigabyte extra disk demand paid - # on every read pass, and zip reading only needs random access, which - # a plain File handle provides. - if blob.service.respond_to?(:path_for) - File.open(blob.service.path_for(blob.key), "rb") do |file| + # blob.open copies the whole blob to a tempfile; zip reading only needs + # random access, so read the stored file in place when we can. + if path = path_on_disk(blob) + File.open(path, "rb") do |file| reader = Reader.new(file) yield reader end diff --git a/app/views/account/imports/show.html.erb b/app/views/account/imports/show.html.erb index 55e2d0d4b2..272e68ef36 100644 --- a/app/views/account/imports/show.html.erb +++ b/app/views/account/imports/show.html.erb @@ -22,7 +22,7 @@
The account you’re trying to import already exists. Make sure you’re importing a <%= Fizzy.saas? ? "self-hosted" : "fizzy.do" %> account.
<% elsif @import.failed_due_to_invalid_export? %>
The .zip file you uploaded doesn’t look like a Fizzy account export.
- <% elsif @import.failed_due_to_insufficient_disk? %> + <% elsif @import.failed_due_to_insufficient_disk_space? %>
Your server doesn’t have enough free disk space for this import. Free up at least twice the export file’s size and try again.
<% else %>
This may be due to corrupted export data or a conflict with existing data. Please try again with a fresh export.
diff --git a/app/views/mailers/import_mailer/failed.html.erb b/app/views/mailers/import_mailer/failed.html.erb index 99098df21b..b1a290c278 100644 --- a/app/views/mailers/import_mailer/failed.html.erb +++ b/app/views/mailers/import_mailer/failed.html.erb @@ -4,7 +4,7 @@

It looks like the account you are trying to import already exists.

<% elsif @import.failed_due_to_invalid_export? %>

The ZIP file isn't a Fizzy account export.

-<% elsif @import.failed_due_to_insufficient_disk? %> +<% elsif @import.failed_due_to_insufficient_disk_space? %>

Your server doesn't have enough free disk space for this import. Free up at least twice the export file's size and try again.

<% else %>

This may be due to corrupted export data or a conflict with existing data. Please try again with a fresh export, or reach out for help if the problem persists.

diff --git a/app/views/mailers/import_mailer/failed.text.erb b/app/views/mailers/import_mailer/failed.text.erb index feeb1cf9f2..83ec1fb9eb 100644 --- a/app/views/mailers/import_mailer/failed.text.erb +++ b/app/views/mailers/import_mailer/failed.text.erb @@ -4,7 +4,7 @@ Unfortunately, we couldn't import your Fizzy account. It looks like the account you are trying to import already exists. <% elsif @import.failed_due_to_invalid_export? -%> The ZIP file isn't a Fizzy account export. -<% elsif @import.failed_due_to_insufficient_disk? -%> +<% elsif @import.failed_due_to_insufficient_disk_space? -%> Your server doesn't have enough free disk space for this import. Free up at least twice the export file's size and try again. <% else -%> This may be due to corrupted export data or a conflict with existing data. Please try again with a fresh export, or reach out for help if the problem persists. diff --git a/docs/docker-deployment.md b/docs/docker-deployment.md index 219bcab1da..c318079c9c 100644 --- a/docs/docker-deployment.md +++ b/docs/docker-deployment.md @@ -153,6 +153,17 @@ This is for convenience: typically when you self-host you'll be running a single If you do want to allow multiple accounts to be created in your instance, set `MULTI_TENANT=true` +## Importing an existing Fizzy account + +You can move an account between Fizzy instances by exporting it on the old instance and uploading the export zip to the new one during signup. + +Imports need free disk space: at least twice the export file's size, beyond the export itself, since the imported attachments roughly mirror the zip's contents. If there isn't enough, the import fails fast with "Your server doesn't have enough free disk space for this import" — free up space (or grow the volume) and try again. + +For very large exports: + +- Browser uploads pass through Thruster, which drops slow uploads after its read timeout (a 502 before the import ever starts). Raise `THRUSTER_HTTP_READ_TIMEOUT` (seconds) and recreate the container so the setting takes effect. +- `script/import-account` runs the import directly on the server from a zip already on disk, bypassing the browser upload entirely — handy for multi-gigabyte exports. + ## Example Here's an example of a `docker-compose.yml` that you could use to run Fizzy via `docker compose up` diff --git a/test/mailers/import_mailer_test.rb b/test/mailers/import_mailer_test.rb index 7407933539..83644f61ed 100644 --- a/test/mailers/import_mailer_test.rb +++ b/test/mailers/import_mailer_test.rb @@ -47,4 +47,15 @@ class ImportMailerTest < ActionMailer::TestCase assert_match "isn't a Fizzy account export", email.body.encoded end + + test "failed with insufficient_disk_space reason" do + import = Account::Import.create!(account: Current.account, identity: identities(:david), status: :failed, failure_reason: :insufficient_disk_space) + email = ImportMailer.failed(import) + + assert_emails 1 do + email.deliver_now + end + + assert_match "twice the export file", email.body.encoded + end end diff --git a/test/models/account/import_test.rb b/test/models/account/import_test.rb index 219a079617..e057338184 100644 --- a/test/models/account/import_test.rb +++ b/test/models/account/import_test.rb @@ -226,7 +226,7 @@ class Account::ImportTest < ActiveSupport::TestCase error = assert_raises(Account::Import::InsufficientDiskSpaceError) { import.check } assert_match(/GB free/, error.message) - assert import.reload.failed_due_to_insufficient_disk? + assert import.reload.failed_due_to_insufficient_disk_space? end test "check proceeds when free disk space cannot be determined" do @@ -237,6 +237,13 @@ class Account::ImportTest < ActiveSupport::TestCase assert import.reload.failed_due_to_invalid_export? end + test "available_disk_space is indeterminate when df output is unparseable" do + import = import_with_attached_zip + import.stubs(:`).returns("Filesystem 1024-blocks Used Available Capacity Mounted on\n") + + assert_nil import.send(:available_disk_space, "/tmp") + end + private def import_with_attached_zip account = Account.create!(name: "Disk Check") From f942c93b770419ecfe88d8b38dd3b1988bd64a26 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 20 Jul 2026 01:27:05 -0700 Subject: [PATCH 3/9] Address review round 2: robust df column parse, preflight process too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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. --- app/models/account/import.rb | 14 ++++++++++++-- test/models/account/import_test.rb | 26 ++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/app/models/account/import.rb b/app/models/account/import.rb index 08ef97a93e..bc0e0fc066 100644 --- a/app/models/account/import.rb +++ b/app/models/account/import.rb @@ -52,6 +52,10 @@ def check(start: nil, callback: nil) def process(start: nil, callback: nil) processing! + # Free space can change between check and a later process run after a restart. + # Skip on resume: the import's own writes have consumed part of the estimate. + ensure_sufficient_disk_space if start.nil? + ZipFile.read_from(file.blob) do |zip| Account::DataTransfer::Manifest.new(account).each_record_set(start: start) do |record_set, last_id| record_set.import(from: zip, start: last_id, callback: callback) @@ -69,6 +73,9 @@ def process(start: nil, callback: nil) rescue Account::DataTransfer::RecordSet::IntegrityError, ZipFile::InvalidFileError => e mark_as_failed(:invalid_export) raise e + rescue InsufficientDiskSpaceError => e + mark_as_failed(:insufficient_disk_space) + raise e rescue => e mark_as_failed raise e @@ -94,8 +101,11 @@ def ensure_sufficient_disk_space end def available_disk_space(path) - fields = `df -Pk #{Shellwords.escape(path)} 2>/dev/null`.lines.last&.split - kilobytes = fields && Integer(fields[3] || "", exception: false) + fields = `df -Pk #{Shellwords.escape(path)} 2>/dev/null`.lines.last.to_s.split + # Anchor on the capacity percentage — the only NN% field — since spaces in + # the filesystem name would shift a fixed column index. + capacity_index = fields.index { |field| field.match?(/\A\d+%\z/) } + kilobytes = capacity_index && Integer(fields[capacity_index - 1].to_s, exception: false) kilobytes && kilobytes * 1024 end diff --git a/test/models/account/import_test.rb b/test/models/account/import_test.rb index e057338184..e16e91a4ed 100644 --- a/test/models/account/import_test.rb +++ b/test/models/account/import_test.rb @@ -237,6 +237,22 @@ class Account::ImportTest < ActiveSupport::TestCase assert import.reload.failed_due_to_invalid_export? end + test "process fails fast with a clear reason when free disk space is insufficient" do + import = import_with_attached_zip + import.stubs(:available_disk_space).returns(import.file.blob.byte_size) + + assert_raises(Account::Import::InsufficientDiskSpaceError) { import.process } + assert import.reload.failed_due_to_insufficient_disk_space? + end + + test "resumed process skips the disk space preflight" do + import = import_with_attached_zip + import.stubs(:available_disk_space).returns(import.file.blob.byte_size) + + assert_raises(ZipFile::InvalidFileError) { import.process(start: [ "Board", nil ]) } + assert import.reload.failed_due_to_invalid_export? + end + test "available_disk_space is indeterminate when df output is unparseable" do import = import_with_attached_zip import.stubs(:`).returns("Filesystem 1024-blocks Used Available Capacity Mounted on\n") @@ -244,6 +260,16 @@ class Account::ImportTest < ActiveSupport::TestCase assert_nil import.send(:available_disk_space, "/tmp") end + test "available_disk_space parses df output whose filesystem name contains spaces" do + import = import_with_attached_zip + import.stubs(:`).returns(<<~DF) + Filesystem 1024-blocks Used Available Capacity Mounted on + map auto home 1000000 250000 750000 25% /System/Volumes/Data/home + DF + + assert_equal 750_000 * 1024, import.send(:available_disk_space, "/tmp") + end + private def import_with_attached_zip account = Account.create!(name: "Disk Check") From b2fb194124c3dd76ea807c3ddedcef5ed1688675 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 20 Jul 2026 01:31:07 -0700 Subject: [PATCH 4/9] Report exact sizes in the insufficient-disk-space error Integer division truncated the GB figures (1.9 GB read as 1 GB), understating the requirement. Use number_to_human_size instead. --- app/models/account/import.rb | 6 +++++- test/models/account/import_test.rb | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/models/account/import.rb b/app/models/account/import.rb index bc0e0fc066..319e85fe8a 100644 --- a/app/models/account/import.rb +++ b/app/models/account/import.rb @@ -96,10 +96,14 @@ def ensure_sufficient_disk_space available = available_disk_space(path) if available && available < required - raise InsufficientDiskSpaceError, "import needs ~#{required / 1.gigabyte} GB free, found #{available / 1.gigabyte} GB" + raise InsufficientDiskSpaceError, "import needs ~#{human_size(required)} free, found #{human_size(available)}" end end + def human_size(bytes) + ActiveSupport::NumberHelper.number_to_human_size(bytes) + end + def available_disk_space(path) fields = `df -Pk #{Shellwords.escape(path)} 2>/dev/null`.lines.last.to_s.split # Anchor on the capacity percentage — the only NN% field — since spaces in diff --git a/test/models/account/import_test.rb b/test/models/account/import_test.rb index e16e91a4ed..d7bc8f22e4 100644 --- a/test/models/account/import_test.rb +++ b/test/models/account/import_test.rb @@ -225,7 +225,7 @@ class Account::ImportTest < ActiveSupport::TestCase import.stubs(:available_disk_space).returns(import.file.blob.byte_size) error = assert_raises(Account::Import::InsufficientDiskSpaceError) { import.check } - assert_match(/GB free/, error.message) + assert_match(/import needs ~.+ free, found/, error.message) assert import.reload.failed_due_to_insufficient_disk_space? end From 9f5b6a197d78ed116101598cca3e441726df0ff8 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 20 Jul 2026 01:38:00 -0700 Subject: [PATCH 5/9] Drop the disk-preflight comment per review The fail-fast rationale (mid-import ENOSPC crashes the shared web/Solid Queue process) lives in the PR and prior commit message. --- app/models/account/import.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/models/account/import.rb b/app/models/account/import.rb index 319e85fe8a..4a1af0373a 100644 --- a/app/models/account/import.rb +++ b/app/models/account/import.rb @@ -87,8 +87,6 @@ def cleanup end private - # Fail fast before consuming the space: self-hosted Solid Queue shares the web - # process, so mid-import ENOSPC ("database or disk is full") takes the app down. def ensure_sufficient_disk_space return unless path = ZipFile.path_on_disk(file.blob) From 8176eb8a1970cc7069415380a8614438250ceda6 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 20 Jul 2026 01:55:05 -0700 Subject: [PATCH 6/9] Take review direction on naming and copy: the space family 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. --- app/jobs/account/data_import_job.rb | 2 +- app/models/account/import.rb | 28 +++++++++---------- app/views/account/imports/show.html.erb | 4 +-- .../mailers/import_mailer/failed.html.erb | 4 +-- .../mailers/import_mailer/failed.text.erb | 4 +-- docs/docker-deployment.md | 2 +- test/mailers/import_mailer_test.rb | 4 +-- test/models/account/import_test.rb | 24 ++++++++-------- 8 files changed, 36 insertions(+), 36 deletions(-) diff --git a/app/jobs/account/data_import_job.rb b/app/jobs/account/data_import_job.rb index f9425289e9..f5428adcc3 100644 --- a/app/jobs/account/data_import_job.rb +++ b/app/jobs/account/data_import_job.rb @@ -3,7 +3,7 @@ class Account::DataImportJob < ApplicationJob queue_as :backend discard_on Account::DataTransfer::RecordSet::IntegrityError, ZipFile::InvalidFileError, - Account::Import::InsufficientDiskSpaceError + Account::Import::InsufficientSpaceError def perform(import) step :check do |step| diff --git a/app/models/account/import.rb b/app/models/account/import.rb index 4a1af0373a..231e16019e 100644 --- a/app/models/account/import.rb +++ b/app/models/account/import.rb @@ -1,10 +1,10 @@ require "shellwords" class Account::Import < ApplicationRecord - class InsufficientDiskSpaceError < StandardError; end + class InsufficientSpaceError < StandardError; end # Imported blobs roughly mirror the zip's contents, plus database growth and margin. - REQUIRED_DISK_SPACE_FACTOR = 2 + REQUIRED_SPACE_FACTOR = 2 broadcasts_refreshes @@ -14,7 +14,7 @@ class InsufficientDiskSpaceError < StandardError; end has_one_attached :file, dependent: :purge_later enum :status, %w[ pending processing completed failed ].index_by(&:itself), default: :pending - enum :failure_reason, %w[ conflict invalid_export insufficient_disk_space ].index_by(&:itself), prefix: :failed_due_to, scopes: false + enum :failure_reason, %w[ conflict invalid_export insufficient_space ].index_by(&:itself), prefix: :failed_due_to, scopes: false scope :expired, -> { where(completed_at: ...24.hours.ago).or(where(status: :failed, created_at: ...7.days.ago)) } @@ -28,7 +28,7 @@ def process_later def check(start: nil, callback: nil) processing! - ensure_sufficient_disk_space + ensure_sufficient_space ZipFile.read_from(file.blob) do |zip| Account::DataTransfer::Manifest.new(account).each_record_set(start: start) do |record_set, last_id| @@ -41,8 +41,8 @@ def check(start: nil, callback: nil) rescue Account::DataTransfer::RecordSet::IntegrityError, ZipFile::InvalidFileError => e mark_as_failed(:invalid_export) raise e - rescue InsufficientDiskSpaceError => e - mark_as_failed(:insufficient_disk_space) + rescue InsufficientSpaceError => e + mark_as_failed(:insufficient_space) raise e rescue => e mark_as_failed @@ -54,7 +54,7 @@ def process(start: nil, callback: nil) # Free space can change between check and a later process run after a restart. # Skip on resume: the import's own writes have consumed part of the estimate. - ensure_sufficient_disk_space if start.nil? + ensure_sufficient_space if start.nil? ZipFile.read_from(file.blob) do |zip| Account::DataTransfer::Manifest.new(account).each_record_set(start: start) do |record_set, last_id| @@ -73,8 +73,8 @@ def process(start: nil, callback: nil) rescue Account::DataTransfer::RecordSet::IntegrityError, ZipFile::InvalidFileError => e mark_as_failed(:invalid_export) raise e - rescue InsufficientDiskSpaceError => e - mark_as_failed(:insufficient_disk_space) + rescue InsufficientSpaceError => e + mark_as_failed(:insufficient_space) raise e rescue => e mark_as_failed @@ -87,14 +87,14 @@ def cleanup end private - def ensure_sufficient_disk_space + def ensure_sufficient_space return unless path = ZipFile.path_on_disk(file.blob) - required = file.blob.byte_size * REQUIRED_DISK_SPACE_FACTOR - available = available_disk_space(path) + required = file.blob.byte_size * REQUIRED_SPACE_FACTOR + available = available_space(path) if available && available < required - raise InsufficientDiskSpaceError, "import needs ~#{human_size(required)} free, found #{human_size(available)}" + raise InsufficientSpaceError, "import needs ~#{human_size(required)} free, found #{human_size(available)}" end end @@ -102,7 +102,7 @@ def human_size(bytes) ActiveSupport::NumberHelper.number_to_human_size(bytes) end - def available_disk_space(path) + def available_space(path) fields = `df -Pk #{Shellwords.escape(path)} 2>/dev/null`.lines.last.to_s.split # Anchor on the capacity percentage — the only NN% field — since spaces in # the filesystem name would shift a fixed column index. diff --git a/app/views/account/imports/show.html.erb b/app/views/account/imports/show.html.erb index 272e68ef36..4d0ac035bb 100644 --- a/app/views/account/imports/show.html.erb +++ b/app/views/account/imports/show.html.erb @@ -22,8 +22,8 @@
The account you’re trying to import already exists. Make sure you’re importing a <%= Fizzy.saas? ? "self-hosted" : "fizzy.do" %> account.
<% elsif @import.failed_due_to_invalid_export? %>
The .zip file you uploaded doesn’t look like a Fizzy account export.
- <% elsif @import.failed_due_to_insufficient_disk_space? %> -
Your server doesn’t have enough free disk space for this import. Free up at least twice the export file’s size and try again.
+ <% elsif @import.failed_due_to_insufficient_space? %> +
There wasn’t enough space to process your import. Free up at least twice the export file’s size and try again.
<% else %>
This may be due to corrupted export data or a conflict with existing data. Please try again with a fresh export.
<% end %> diff --git a/app/views/mailers/import_mailer/failed.html.erb b/app/views/mailers/import_mailer/failed.html.erb index b1a290c278..0706d2bfaf 100644 --- a/app/views/mailers/import_mailer/failed.html.erb +++ b/app/views/mailers/import_mailer/failed.html.erb @@ -4,8 +4,8 @@

It looks like the account you are trying to import already exists.

<% elsif @import.failed_due_to_invalid_export? %>

The ZIP file isn't a Fizzy account export.

-<% elsif @import.failed_due_to_insufficient_disk_space? %> -

Your server doesn't have enough free disk space for this import. Free up at least twice the export file's size and try again.

+<% elsif @import.failed_due_to_insufficient_space? %> +

There wasn't enough space to process your import. Free up at least twice the export file's size and try again.

<% else %>

This may be due to corrupted export data or a conflict with existing data. Please try again with a fresh export, or reach out for help if the problem persists.

<% end %> diff --git a/app/views/mailers/import_mailer/failed.text.erb b/app/views/mailers/import_mailer/failed.text.erb index 83ec1fb9eb..7d54f5c96f 100644 --- a/app/views/mailers/import_mailer/failed.text.erb +++ b/app/views/mailers/import_mailer/failed.text.erb @@ -4,8 +4,8 @@ Unfortunately, we couldn't import your Fizzy account. It looks like the account you are trying to import already exists. <% elsif @import.failed_due_to_invalid_export? -%> The ZIP file isn't a Fizzy account export. -<% elsif @import.failed_due_to_insufficient_disk_space? -%> -Your server doesn't have enough free disk space for this import. Free up at least twice the export file's size and try again. +<% elsif @import.failed_due_to_insufficient_space? -%> +There wasn't enough space to process your import. Free up at least twice the export file's size and try again. <% else -%> This may be due to corrupted export data or a conflict with existing data. Please try again with a fresh export, or reach out for help if the problem persists. <% end -%> diff --git a/docs/docker-deployment.md b/docs/docker-deployment.md index c318079c9c..ae4d05367b 100644 --- a/docs/docker-deployment.md +++ b/docs/docker-deployment.md @@ -157,7 +157,7 @@ If you do want to allow multiple accounts to be created in your instance, set `M You can move an account between Fizzy instances by exporting it on the old instance and uploading the export zip to the new one during signup. -Imports need free disk space: at least twice the export file's size, beyond the export itself, since the imported attachments roughly mirror the zip's contents. If there isn't enough, the import fails fast with "Your server doesn't have enough free disk space for this import" — free up space (or grow the volume) and try again. +Imports need free space: at least twice the export file's size, beyond the export itself, since the imported attachments roughly mirror the zip's contents. If there isn't enough, the import fails fast with "There wasn't enough space to process your import." — free up space (or grow the volume) and try again. For very large exports: diff --git a/test/mailers/import_mailer_test.rb b/test/mailers/import_mailer_test.rb index 83644f61ed..09fc6eaaf6 100644 --- a/test/mailers/import_mailer_test.rb +++ b/test/mailers/import_mailer_test.rb @@ -48,8 +48,8 @@ class ImportMailerTest < ActionMailer::TestCase assert_match "isn't a Fizzy account export", email.body.encoded end - test "failed with insufficient_disk_space reason" do - import = Account::Import.create!(account: Current.account, identity: identities(:david), status: :failed, failure_reason: :insufficient_disk_space) + test "failed with insufficient_space reason" do + import = Account::Import.create!(account: Current.account, identity: identities(:david), status: :failed, failure_reason: :insufficient_space) email = ImportMailer.failed(import) assert_emails 1 do diff --git a/test/models/account/import_test.rb b/test/models/account/import_test.rb index d7bc8f22e4..ebba9ae7ca 100644 --- a/test/models/account/import_test.rb +++ b/test/models/account/import_test.rb @@ -222,16 +222,16 @@ class Account::ImportTest < ActiveSupport::TestCase test "check fails fast with a clear reason when free disk space is insufficient" do import = import_with_attached_zip - import.stubs(:available_disk_space).returns(import.file.blob.byte_size) + import.stubs(:available_space).returns(import.file.blob.byte_size) - error = assert_raises(Account::Import::InsufficientDiskSpaceError) { import.check } + error = assert_raises(Account::Import::InsufficientSpaceError) { import.check } assert_match(/import needs ~.+ free, found/, error.message) - assert import.reload.failed_due_to_insufficient_disk_space? + assert import.reload.failed_due_to_insufficient_space? end test "check proceeds when free disk space cannot be determined" do import = import_with_attached_zip - import.stubs(:available_disk_space).returns(nil) + import.stubs(:available_space).returns(nil) assert_raises(ZipFile::InvalidFileError) { import.check } assert import.reload.failed_due_to_invalid_export? @@ -239,35 +239,35 @@ class Account::ImportTest < ActiveSupport::TestCase test "process fails fast with a clear reason when free disk space is insufficient" do import = import_with_attached_zip - import.stubs(:available_disk_space).returns(import.file.blob.byte_size) + import.stubs(:available_space).returns(import.file.blob.byte_size) - assert_raises(Account::Import::InsufficientDiskSpaceError) { import.process } - assert import.reload.failed_due_to_insufficient_disk_space? + assert_raises(Account::Import::InsufficientSpaceError) { import.process } + assert import.reload.failed_due_to_insufficient_space? end test "resumed process skips the disk space preflight" do import = import_with_attached_zip - import.stubs(:available_disk_space).returns(import.file.blob.byte_size) + import.stubs(:available_space).returns(import.file.blob.byte_size) assert_raises(ZipFile::InvalidFileError) { import.process(start: [ "Board", nil ]) } assert import.reload.failed_due_to_invalid_export? end - test "available_disk_space is indeterminate when df output is unparseable" do + test "available_space is indeterminate when df output is unparseable" do import = import_with_attached_zip import.stubs(:`).returns("Filesystem 1024-blocks Used Available Capacity Mounted on\n") - assert_nil import.send(:available_disk_space, "/tmp") + assert_nil import.send(:available_space, "/tmp") end - test "available_disk_space parses df output whose filesystem name contains spaces" do + test "available_space parses df output whose filesystem name contains spaces" do import = import_with_attached_zip import.stubs(:`).returns(<<~DF) Filesystem 1024-blocks Used Available Capacity Mounted on map auto home 1000000 250000 750000 25% /System/Volumes/Data/home DF - assert_equal 750_000 * 1024, import.send(:available_disk_space, "/tmp") + assert_equal 750_000 * 1024, import.send(:available_space, "/tmp") end private From 45357234fa17763c544078334292c9945e53c318 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 20 Jul 2026 01:56:59 -0700 Subject: [PATCH 7/9] Drop remaining explanatory comments per review --- app/models/account/import.rb | 5 ----- app/models/zip_file.rb | 2 -- 2 files changed, 7 deletions(-) diff --git a/app/models/account/import.rb b/app/models/account/import.rb index 231e16019e..7fa3a0468d 100644 --- a/app/models/account/import.rb +++ b/app/models/account/import.rb @@ -3,7 +3,6 @@ class Account::Import < ApplicationRecord class InsufficientSpaceError < StandardError; end - # Imported blobs roughly mirror the zip's contents, plus database growth and margin. REQUIRED_SPACE_FACTOR = 2 broadcasts_refreshes @@ -52,8 +51,6 @@ def check(start: nil, callback: nil) def process(start: nil, callback: nil) processing! - # Free space can change between check and a later process run after a restart. - # Skip on resume: the import's own writes have consumed part of the estimate. ensure_sufficient_space if start.nil? ZipFile.read_from(file.blob) do |zip| @@ -104,8 +101,6 @@ def human_size(bytes) def available_space(path) fields = `df -Pk #{Shellwords.escape(path)} 2>/dev/null`.lines.last.to_s.split - # Anchor on the capacity percentage — the only NN% field — since spaces in - # the filesystem name would shift a fixed column index. capacity_index = fields.index { |field| field.match?(/\A\d+%\z/) } kilobytes = capacity_index && Integer(fields[capacity_index - 1].to_s, exception: false) kilobytes && kilobytes * 1024 diff --git a/app/models/zip_file.rb b/app/models/zip_file.rb index faf9842fd8..370a59a928 100644 --- a/app/models/zip_file.rb +++ b/app/models/zip_file.rb @@ -95,8 +95,6 @@ def read_from_s3(blob) end def read_from_disk(blob) - # blob.open copies the whole blob to a tempfile; zip reading only needs - # random access, so read the stored file in place when we can. if path = path_on_disk(blob) File.open(path, "rb") do |file| reader = Reader.new(file) From 55228987e9d47af29f40abafff9857ec335352ad Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 20 Jul 2026 02:05:52 -0700 Subject: [PATCH 8/9] Spell out available_space plainly, note best-effort check in docs Use the kilobytes size helper instead of a bare 1024, and document that the preflight is skipped when free space can't be determined. --- app/models/account/import.rb | 7 +++++-- docs/docker-deployment.md | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/models/account/import.rb b/app/models/account/import.rb index 7fa3a0468d..47e08d30b1 100644 --- a/app/models/account/import.rb +++ b/app/models/account/import.rb @@ -102,8 +102,11 @@ def human_size(bytes) def available_space(path) fields = `df -Pk #{Shellwords.escape(path)} 2>/dev/null`.lines.last.to_s.split capacity_index = fields.index { |field| field.match?(/\A\d+%\z/) } - kilobytes = capacity_index && Integer(fields[capacity_index - 1].to_s, exception: false) - kilobytes && kilobytes * 1024 + + if capacity_index + available_kilobytes = Integer(fields[capacity_index - 1], exception: false) + available_kilobytes&.kilobytes + end end def mark_completed diff --git a/docs/docker-deployment.md b/docs/docker-deployment.md index ae4d05367b..68be0f8db8 100644 --- a/docs/docker-deployment.md +++ b/docs/docker-deployment.md @@ -157,7 +157,7 @@ If you do want to allow multiple accounts to be created in your instance, set `M You can move an account between Fizzy instances by exporting it on the old instance and uploading the export zip to the new one during signup. -Imports need free space: at least twice the export file's size, beyond the export itself, since the imported attachments roughly mirror the zip's contents. If there isn't enough, the import fails fast with "There wasn't enough space to process your import." — free up space (or grow the volume) and try again. +Imports need free space: at least twice the export file's size, beyond the export itself, since the imported attachments roughly mirror the zip's contents. If there isn't enough, the import fails fast with "There wasn't enough space to process your import." — free up space (or grow the volume) and try again. If free space can't be determined, the check is skipped and the import proceeds. For very large exports: From 44edf2edcaac39abd5a873ea3ec1d835d41cc743 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 20 Jul 2026 02:17:20 -0700 Subject: [PATCH 9/9] Simplify the insufficient-space copy per review Keep the user-facing message simple; the remedy lives in the self-hosting docs where the operator can act on it. --- app/views/account/imports/show.html.erb | 2 +- app/views/mailers/import_mailer/failed.html.erb | 2 +- app/views/mailers/import_mailer/failed.text.erb | 2 +- docs/docker-deployment.md | 2 +- test/mailers/import_mailer_test.rb | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/views/account/imports/show.html.erb b/app/views/account/imports/show.html.erb index 4d0ac035bb..469b8022fc 100644 --- a/app/views/account/imports/show.html.erb +++ b/app/views/account/imports/show.html.erb @@ -23,7 +23,7 @@ <% elsif @import.failed_due_to_invalid_export? %>
The .zip file you uploaded doesn’t look like a Fizzy account export.
<% elsif @import.failed_due_to_insufficient_space? %> -
There wasn’t enough space to process your import. Free up at least twice the export file’s size and try again.
+
There wasn’t enough storage space to process your import.
<% else %>
This may be due to corrupted export data or a conflict with existing data. Please try again with a fresh export.
<% end %> diff --git a/app/views/mailers/import_mailer/failed.html.erb b/app/views/mailers/import_mailer/failed.html.erb index 0706d2bfaf..b61720a22a 100644 --- a/app/views/mailers/import_mailer/failed.html.erb +++ b/app/views/mailers/import_mailer/failed.html.erb @@ -5,7 +5,7 @@ <% elsif @import.failed_due_to_invalid_export? %>

The ZIP file isn't a Fizzy account export.

<% elsif @import.failed_due_to_insufficient_space? %> -

There wasn't enough space to process your import. Free up at least twice the export file's size and try again.

+

There wasn't enough storage space to process your import.

<% else %>

This may be due to corrupted export data or a conflict with existing data. Please try again with a fresh export, or reach out for help if the problem persists.

<% end %> diff --git a/app/views/mailers/import_mailer/failed.text.erb b/app/views/mailers/import_mailer/failed.text.erb index 7d54f5c96f..ae050abf9f 100644 --- a/app/views/mailers/import_mailer/failed.text.erb +++ b/app/views/mailers/import_mailer/failed.text.erb @@ -5,7 +5,7 @@ It looks like the account you are trying to import already exists. <% elsif @import.failed_due_to_invalid_export? -%> The ZIP file isn't a Fizzy account export. <% elsif @import.failed_due_to_insufficient_space? -%> -There wasn't enough space to process your import. Free up at least twice the export file's size and try again. +There wasn't enough storage space to process your import. <% else -%> This may be due to corrupted export data or a conflict with existing data. Please try again with a fresh export, or reach out for help if the problem persists. <% end -%> diff --git a/docs/docker-deployment.md b/docs/docker-deployment.md index 68be0f8db8..4352e369e5 100644 --- a/docs/docker-deployment.md +++ b/docs/docker-deployment.md @@ -157,7 +157,7 @@ If you do want to allow multiple accounts to be created in your instance, set `M You can move an account between Fizzy instances by exporting it on the old instance and uploading the export zip to the new one during signup. -Imports need free space: at least twice the export file's size, beyond the export itself, since the imported attachments roughly mirror the zip's contents. If there isn't enough, the import fails fast with "There wasn't enough space to process your import." — free up space (or grow the volume) and try again. If free space can't be determined, the check is skipped and the import proceeds. +Imports need free space: at least twice the export file's size, beyond the export itself, since the imported attachments roughly mirror the zip's contents. If there isn't enough, the import fails fast with "There wasn't enough storage space to process your import." — free up space (or grow the volume) and try again. If free space can't be determined, the check is skipped and the import proceeds. For very large exports: diff --git a/test/mailers/import_mailer_test.rb b/test/mailers/import_mailer_test.rb index 09fc6eaaf6..555ceb828c 100644 --- a/test/mailers/import_mailer_test.rb +++ b/test/mailers/import_mailer_test.rb @@ -56,6 +56,6 @@ class ImportMailerTest < ActionMailer::TestCase email.deliver_now end - assert_match "twice the export file", email.body.encoded + assert_match "enough storage space", email.body.encoded end end