Skip to content

Commit 0ba7b9f

Browse files
committed
Fix BC-10008678752: tame self-hosted import disk demands
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
1 parent 595ebf5 commit 0ba7b9f

7 files changed

Lines changed: 82 additions & 5 deletions

File tree

app/jobs/account/data_import_job.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ class Account::DataImportJob < ApplicationJob
22
include ActiveJob::Continuable
33

44
queue_as :backend
5-
discard_on Account::DataTransfer::RecordSet::IntegrityError, ZipFile::InvalidFileError
5+
discard_on Account::DataTransfer::RecordSet::IntegrityError, ZipFile::InvalidFileError,
6+
Account::Import::InsufficientDiskSpaceError
67

78
def perform(import)
89
step :check do |step|

app/models/account/import.rb

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
1+
require "shellwords"
2+
13
class Account::Import < ApplicationRecord
4+
class InsufficientDiskSpaceError < StandardError; end
5+
6+
# Peak disk demand beyond the stored zip itself: the imported attachment
7+
# blobs roughly mirror the zip's contents, plus database growth and margin.
8+
REQUIRED_DISK_FACTOR = 2
9+
210
broadcasts_refreshes
311

412
belongs_to :account
@@ -7,7 +15,7 @@ class Account::Import < ApplicationRecord
715
has_one_attached :file, dependent: :purge_later
816

917
enum :status, %w[ pending processing completed failed ].index_by(&:itself), default: :pending
10-
enum :failure_reason, %w[ conflict invalid_export ].index_by(&:itself), prefix: :failed_due_to, scopes: false
18+
enum :failure_reason, %w[ conflict invalid_export insufficient_disk ].index_by(&:itself), prefix: :failed_due_to, scopes: false
1119

1220
scope :expired, -> { where(completed_at: ...24.hours.ago).or(where(status: :failed, created_at: ...7.days.ago)) }
1321

@@ -21,6 +29,7 @@ def process_later
2129

2230
def check(start: nil, callback: nil)
2331
processing!
32+
ensure_sufficient_disk_space
2433

2534
ZipFile.read_from(file.blob) do |zip|
2635
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)
3342
rescue Account::DataTransfer::RecordSet::IntegrityError, ZipFile::InvalidFileError => e
3443
mark_as_failed(:invalid_export)
3544
raise e
45+
rescue InsufficientDiskSpaceError => e
46+
mark_as_failed(:insufficient_disk)
47+
raise e
3648
rescue => e
3749
mark_as_failed
3850
raise e
@@ -69,6 +81,26 @@ def cleanup
6981
end
7082

7183
private
84+
# Running out of disk mid-import is the worst self-hosted failure mode:
85+
# Solid Queue shares the web process, so once its own SQLite writes start
86+
# failing ("database or disk is full") the whole app goes down with the
87+
# import. Fail fast with a clear reason before consuming the space.
88+
def ensure_sufficient_disk_space
89+
return unless file.blob.service.respond_to?(:root)
90+
91+
required = file.blob.byte_size * REQUIRED_DISK_FACTOR
92+
available = available_disk_space
93+
94+
if available && available < required
95+
raise InsufficientDiskSpaceError, "import needs ~#{required / 1.gigabyte} GB free, found #{available / 1.gigabyte} GB"
96+
end
97+
end
98+
99+
def available_disk_space
100+
fields = `df -Pk #{Shellwords.escape(file.blob.service.root.to_s)} 2>/dev/null`.lines.last&.split
101+
fields && fields[3].to_i * 1024
102+
end
103+
72104
def mark_completed
73105
update!(status: :completed, completed_at: Time.current)
74106
ImportMailer.completed(identity, account).deliver_later

app/models/zip_file.rb

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,21 @@ def read_from_s3(blob)
9191
end
9292

9393
def read_from_disk(blob)
94-
blob.open do |file|
95-
reader = Reader.new(file)
96-
yield reader
94+
# Read the stored file in place when the service exposes its path.
95+
# blob.open copies the whole blob to a tempfile first — for large
96+
# self-hosted imports that's a multi-gigabyte extra disk demand paid
97+
# on every read pass, and zip reading only needs random access, which
98+
# a plain File handle provides.
99+
if blob.service.respond_to?(:path_for)
100+
File.open(blob.service.path_for(blob.key), "rb") do |file|
101+
reader = Reader.new(file)
102+
yield reader
103+
end
104+
else
105+
blob.open do |file|
106+
reader = Reader.new(file)
107+
yield reader
108+
end
97109
end
98110
end
99111
end

app/views/account/imports/show.html.erb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
<div>The account you’re trying to import already exists. Make sure you’re importing a <%= Fizzy.saas? ? "self-hosted" : "fizzy.do" %> account.</div>
2323
<% elsif @import.failed_due_to_invalid_export? %>
2424
<div>The .zip file you uploaded doesn’t look like a Fizzy account export.</div>
25+
<% elsif @import.failed_due_to_insufficient_disk? %>
26+
<div>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.</div>
2527
<% else %>
2628
<div>This may be due to corrupted export data or a conflict with existing data. Please try again with a fresh export.</div>
2729
<% end %>

app/views/mailers/import_mailer/failed.html.erb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
<p>It looks like the account you are trying to import already exists.</p>
55
<% elsif @import.failed_due_to_invalid_export? %>
66
<p>The ZIP file isn't a Fizzy account export.</p>
7+
<% elsif @import.failed_due_to_insufficient_disk? %>
8+
<p>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.</p>
79
<% else %>
810
<p>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.</p>
911
<% end %>

app/views/mailers/import_mailer/failed.text.erb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ Unfortunately, we couldn't import your Fizzy account.
44
It looks like the account you are trying to import already exists.
55
<% elsif @import.failed_due_to_invalid_export? -%>
66
The ZIP file isn't a Fizzy account export.
7+
<% elsif @import.failed_due_to_insufficient_disk? -%>
8+
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.
79
<% else -%>
810
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.
911
<% end -%>

test/models/account/import_test.rb

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,33 @@ class Account::ImportTest < ActiveSupport::TestCase
220220
export_tempfile&.unlink
221221
end
222222

223+
test "check fails fast with a clear reason when free disk space is insufficient" do
224+
import = import_with_attached_zip
225+
import.stubs(:available_disk_space).returns(import.file.blob.byte_size)
226+
227+
error = assert_raises(Account::Import::InsufficientDiskSpaceError) { import.check }
228+
assert_match(/GB free/, error.message)
229+
assert import.reload.failed_due_to_insufficient_disk?
230+
end
231+
232+
test "check proceeds when free disk space cannot be determined" do
233+
import = import_with_attached_zip
234+
import.stubs(:available_disk_space).returns(nil)
235+
236+
assert_raises(ZipFile::InvalidFileError) { import.check }
237+
assert import.reload.failed_due_to_invalid_export?
238+
end
239+
223240
private
241+
def import_with_attached_zip
242+
account = Account.create!(name: "Disk Check")
243+
import = Account::Import.create!(account: account, identity: identities(:david))
244+
Current.set(account: account) do
245+
import.file.attach(io: StringIO.new("not actually a zip"), filename: "export.zip", content_type: "application/zip")
246+
end
247+
import
248+
end
249+
224250
def account_digest(account)
225251
{
226252
name: account.name,

0 commit comments

Comments
 (0)