Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions runtime/spinel/db.rb
Original file line number Diff line number Diff line change
Expand Up @@ -900,10 +900,10 @@ class DbPool
# compile-time default nobody had written down.
#
# Which is why this is SET rather than left to agree by accident.
# The sibling shims (db_cruby.rb, db_jruby.rb) still set nothing and
# happen to read NORMAL today; a gem rebuilt with a different
# default would move them silently, and that is the shape of bug
# this line exists to remove.
# The sibling shims (db_cruby.rb, db_jruby.rb) also state
# synchronous=NORMAL (and WAL) explicitly; leaving it to the gem's
# bundled default would move them silently on a rebuild, and that
# is the shape of bug this line exists to remove.
#
# The trade is the standard WAL one and worth stating: NORMAL is
# safe across a process crash and risks only the most recent commits
Expand Down
106 changes: 95 additions & 11 deletions runtime/spinel/db_cruby.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
# target compiles against.

require "sqlite3"
require "fileutils"

module Db
@pool = nil
Expand All @@ -63,6 +64,10 @@ module Db
# lease. See `start_checkpointer`.
@checkpoint_wanted = false
@checkpointer_pid = nil
# The process-shared flock File while this process holds it. Retained
# so `adopt_after_fork` can close the child's inherited copy without
# LOCK_UN (parent keeps the lock). Cleared on release.
@checkpoint_lock_file = nil
# Failed resources stay reachable, but never re-enter the free list.
@quarantined = []
@missing_connections = 0
Expand Down Expand Up @@ -121,6 +126,16 @@ def self.open_connection
# compile-time accident is not agreement, so each states it.
db.execute("PRAGMA journal_mode=WAL")
db.execute("PRAGMA synchronous=NORMAL")
# Page cache + mmap — same measured knobs as runtime/spinel/db.rb
# (roundhouse#17 CRuby half). SQLite's default cache is 2 MB; a
# long-lived serving process re-reads the working set from the OS
# on every visit. Spinel measured ~400 pread64s per /top visit at
# the default vs a large cache; tip CRuby still opened at
# cache_size=-2000 / mmap_size=0. Negative cache_size is a KiB
# budget (-65536 = 64 MiB); mmap_size maps the file so hits skip
# the read() copy. Harmless on :memory: (pages are already heap).
db.execute("PRAGMA cache_size=-65536")
db.execute("PRAGMA mmap_size=268435456")
# The gem's default is 0: a second writer fails at once with
# SQLITE_BUSY. Rails' database.yml says `timeout: 5000`, and so do
# the binary's PRAGMAS — the harness's file database (see
Expand Down Expand Up @@ -168,6 +183,19 @@ def self.adopt_after_fork
# Re-checked under the lock: every worker thread in a fresh child
# reaches this together on the first request.
if @owner_pid != Process.pid
# Drop the inherited checkpoint-lock FD without LOCK_UN. The
# parent may still hold the flock via its own descriptor; if we
# unlocked here we would release the parent's hold. Closing the
# child copy lets a surviving worker acquire after the parent
# exits (Puma preload / clustered fork after checkpointer start).
inherited = @checkpoint_lock_file
if inherited && !inherited.closed?
begin
inherited.close
rescue StandardError
end
end
@checkpoint_lock_file = nil
# The parent's handles are simply dropped. They are already
# discarded by the gem's fork safety, and closing a descriptor
# this process shares with its parent is not ours to do.
Expand Down Expand Up @@ -567,26 +595,82 @@ def self.start_checkpointer
end
end

# One checkpointer across WEB_CONCURRENCY / Resque siblings (the bit
# Campfire once-campfire#319 adds on top of the same PASSIVE loop).
# Without a flock every forked worker runs its own 250ms PASSIVE and
# they contend on the WAL; with it, losers skip the tick. Lock file
# sits next to the database so each file-backed DB has its own.
def self.checkpoint_lock_path(path)
File.join(File.dirname(path), ".#{File.basename(path)}.wal_checkpoint.lock")
end

# File on acquire, `:busy` when another process holds the flock, `nil`
# when the lock file cannot be used (mkdir/open/flock error). Callers
# skip only on `:busy`; `nil` still checkpoints so a broken lock path
# cannot disable WAL copy after `wal_autocheckpoint=0`.
def self.try_checkpoint_lock(lock_path)
file = nil
FileUtils.mkdir_p(File.dirname(lock_path))
file = File.open(lock_path, File::RDWR | File::CREAT, 0644)
if file.flock(File::LOCK_EX | File::LOCK_NB)
@checkpoint_lock_file = file
return file
end
file.close
:busy
rescue StandardError
begin
file.close if file && !file.closed?
rescue StandardError
end
nil
Comment thread
thomasklemm marked this conversation as resolved.
end

def self.release_checkpoint_lock(file)
return if file.nil? || !file.is_a?(File)
begin
file.flock(File::LOCK_UN)
ensure
file.close
@checkpoint_lock_file = nil if @checkpoint_lock_file.equal?(file)
end
rescue StandardError
end

def self.checkpoint_loop(path)
conn = SQLite3::Database.new(path)
conn.busy_handler_timeout = 100
lock_path = checkpoint_lock_path(path)
loop do
sleep CHECKPOINT_INTERVAL
# Hold the flock for the whole inner loop (Campfire #319), not
# per tick: releasing every 250ms lets a sibling overlap a
# PASSIVE with ours. Losers sleep and retry; a dead winner
# drops the flock so another worker takes over.
Comment thread
thomasklemm marked this conversation as resolved.
lock = try_checkpoint_lock(lock_path)
if lock == :busy
sleep CHECKPOINT_INTERVAL
next
end
begin
row = conn.execute("PRAGMA wal_checkpoint(PASSIVE)")[0]
log_frames = row.nil? ? 0 : row[1].to_i
if log_frames >= CHECKPOINT_RESTART_FRAMES
if acquire_permit
begin
conn.execute("PRAGMA wal_checkpoint(RESTART)")
ensure
release_permit
loop do
sleep CHECKPOINT_INTERVAL
row = conn.execute("PRAGMA wal_checkpoint(PASSIVE)")[0]
log_frames = row.nil? ? 0 : row[1].to_i
if log_frames >= CHECKPOINT_RESTART_FRAMES
if acquire_permit
begin
conn.execute("PRAGMA wal_checkpoint(RESTART)")
ensure
release_permit
end
end
end
end
rescue StandardError
# A busy or failed checkpoint is retried on the next tick; the
# log only grows meanwhile.
# A busy or failed checkpoint is retried on the next outer
# pass; the log only grows meanwhile.
ensure
release_checkpoint_lock(lock)
end
end
end
Expand Down
87 changes: 87 additions & 0 deletions tests/db_sqlite_concurrency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,93 @@ check("the abandoned insert rolled back", count("t") == 1)
);
}

/// Serving connections pin the measured page-cache / mmap knobs
/// (roundhouse#17 CRuby half). Tip used to inherit SQLite defaults
/// (`cache_size=-2000`, `mmap_size=0`) while the Spinel shim already
/// set the 64 MiB / 256 MiB budget — same working-set win, every
/// RH CRuby app, not Campfire-specific.
#[test]
fn open_connection_sets_cache_and_mmap() {
run(
"pragmas",
r#"
Db.with_connection do
c = Db.current_dbh
check("journal_mode=WAL", c.execute("PRAGMA journal_mode")[0][0].to_s.downcase == "wal")
check("synchronous=NORMAL", c.execute("PRAGMA synchronous")[0][0] == 1)
check("cache_size=-65536", c.execute("PRAGMA cache_size")[0][0] == -65536)
check("mmap_size=256MiB", c.execute("PRAGMA mmap_size")[0][0] == 268435456)
end
"#,
);
}

/// Multi-worker checkpointing takes a non-blocking flock so only one
/// process copies the WAL (Campfire once-campfire#319's generalizable
/// bit). A second holder must fail while the first still holds it.
#[test]
fn checkpoint_flock_is_exclusive_across_processes() {
run(
"checkpoint_flock",
r#"
path = Db.instance_variable_get(:@path)
lock_path = Db.checkpoint_lock_path(path)
held = Db.try_checkpoint_lock(lock_path)
check("first holder acquired", held.is_a?(File))
pid = fork do
other = Db.try_checkpoint_lock(lock_path)
exit(other == :busy ? 0 : 1)
end
_pid, status = Process.wait2(pid)
check("sibling saw :busy", status.exitstatus == 0)
Db.release_checkpoint_lock(held)
again = Db.try_checkpoint_lock(lock_path)
check("lock free after release", again.is_a?(File))
Db.release_checkpoint_lock(again)
# mkdir_p fails when the parent is a file — that is setup failure, not
# contention; the loop must still checkpoint (nil, not :busy).
parent = File.join(File.dirname(path), "not-a-dir")
File.write(parent, "x")
got = Db.try_checkpoint_lock(File.join(parent, "x.lock"))
check("setup failure is nil so checkpoint still runs", got.nil?)
"#,
);
}

/// After fork, the child must close the inherited checkpoint-lock FD
/// without LOCK_UN. Otherwise a surviving worker keeps the OFD open
/// after the parent exits and its new checkpointer stays `:busy` forever
/// while `wal_autocheckpoint=0` (CodeRabbit on #548 / Puma preload).
#[test]
fn adopt_after_fork_closes_inherited_checkpoint_lock() {
run(
"checkpoint_flock_fork",
r#"
path = Db.instance_variable_get(:@path)
lock_path = Db.checkpoint_lock_path(path)
held = Db.try_checkpoint_lock(lock_path)
check("parent acquired", held.is_a?(File))
check("Db retained lock file", Db.instance_variable_get(:@checkpoint_lock_file).equal?(held))
r, w = IO.pipe
pid = fork do
w.close
r.read(1)
Db.adopt_after_fork
got = Db.try_checkpoint_lock(lock_path)
exit(got.is_a?(File) ? 0 : 1)
end
r.close
# Drop the parent's FD without unlocking — same as the parent exiting
# while the child still holds an inherited copy of the OFD.
held.close
w.write("x")
w.close
_pid, status = Process.wait2(pid)
check("child acquired after adopt closed inherited FD", status.exitstatus == 0)
"#,
);
}

/// Once the server asks, serving connections stop checkpointing inside
/// COMMIT and a background thread copies the log into the database
/// file instead: the file grows without any request checkpointing.
Expand Down
Loading