Skip to content

Commit e678b2b

Browse files
authored
Merge pull request #1137 from DeusData/fix/1083-wal-journal-size-limit
fix(store): bound the WAL (journal_size_limit) + warn on checkpoint starvation (#1083 part 1)
2 parents 7235a4d + 9dfe52f commit e678b2b

3 files changed

Lines changed: 87 additions & 1 deletion

File tree

src/store/store.c

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ enum {
2626
ST_FOUND = -1,
2727
ST_BUF_16 = 16,
2828
ST_BUF_64 = 64,
29+
/* #1083: warn when a PASSIVE checkpoint sees a WAL this large (frames) but
30+
* can't reset it — ~1 GiB at a 4 KiB page, far past the healthy ~1000-frame
31+
* autocheckpoint, so it only fires under genuine starvation. */
32+
ST_WAL_STARVE_WARN_FRAMES = 262144,
2933
/* file: URI for the immutable read-only fallback. A path is at most
3034
* CBM_SZ_1K; percent-encoding can triple it, plus the "file://" prefix,
3135
* the "?immutable=1" suffix and a leading '/'. */
@@ -432,6 +436,20 @@ static int configure_pragmas(cbm_store_t *s, bool in_memory, bool read_only) {
432436
if (rc != CBM_STORE_OK) {
433437
return rc;
434438
}
439+
/* #1083: bound the WAL file so a checkpoint-starved log is physically
440+
* reclaimed the next time a checkpoint can reset it. Our checkpoints are
441+
* all PASSIVE (they never ftruncate — see cbm_store_checkpoint's SIGBUS
442+
* note), so without a size limit the -wal file only ever grows; a
443+
* journal_size_limit truncates it back to N bytes on the next successful
444+
* reset. N is far above the healthy WAL (~4 MiB under the default
445+
* 1000-page autocheckpoint), so normal indexing never triggers
446+
* truncate/regrow churn — it only fires after abnormal growth. We do NOT
447+
* use a TRUNCATE checkpoint: its ftruncate(fd,0) can raise SIGBUS in a
448+
* sibling process that has the DB mmap'd on macOS. */
449+
rc = exec_sql(s, "PRAGMA journal_size_limit = 268435456;"); /* 256 MiB */
450+
if (rc != CBM_STORE_OK) {
451+
return rc;
452+
}
435453
char mmap_sql[ST_BUF_64];
436454
snprintf(mmap_sql, sizeof(mmap_sql), "PRAGMA mmap_size = %lld;",
437455
(long long)cbm_store_resolve_mmap_size());
@@ -1073,14 +1091,50 @@ int cbm_store_checkpoint(cbm_store_t *s) {
10731091
* SIGBUS in a sibling process that has the DB mmap'd through SQLite
10741092
* when it next faults a page in the now-shorter region.
10751093
* See https://www.sqlite.org/c3ref/c_checkpoint_full.html */
1076-
int rc = sqlite3_wal_checkpoint_v2(s->db, NULL, SQLITE_CHECKPOINT_PASSIVE, NULL, NULL);
1094+
int wal_frames = 0;
1095+
int checkpointed = 0;
1096+
int rc = sqlite3_wal_checkpoint_v2(s->db, NULL, SQLITE_CHECKPOINT_PASSIVE, &wal_frames,
1097+
&checkpointed);
10771098
if (rc != SQLITE_OK) {
10781099
store_set_error_sqlite(s, "checkpoint");
10791100
return CBM_STORE_ERR;
10801101
}
1102+
/* #1083: a large WAL that a PASSIVE checkpoint can't fully reset — because
1103+
* concurrent readers hold marks — is the checkpoint-starvation signal. Warn
1104+
* so an operator can see it (and the driving processes) before the -wal file
1105+
* fills the disk; journal_size_limit only reclaims once a reset succeeds.
1106+
* wal_frames = frames in the WAL, checkpointed = frames reclaimed this pass. */
1107+
if (wal_frames >= ST_WAL_STARVE_WARN_FRAMES && checkpointed < wal_frames) {
1108+
char frames_buf[ST_BUF_16];
1109+
char ckpt_buf[ST_BUF_16];
1110+
snprintf(frames_buf, sizeof(frames_buf), "%d", wal_frames);
1111+
snprintf(ckpt_buf, sizeof(ckpt_buf), "%d", checkpointed);
1112+
cbm_log_warn("store.wal.starving", "wal_frames", frames_buf, "checkpointed", ckpt_buf,
1113+
"hint",
1114+
"concurrent readers block the WAL reset — the -wal file keeps growing");
1115+
}
10811116
return exec_sql(s, "PRAGMA optimize;");
10821117
}
10831118

1119+
/* #1083: the WAL size limit configured on this (write) connection, in bytes.
1120+
* -1 means unlimited (SQLite's default — the pre-fix behavior). Per-connection
1121+
* and not persisted, so it can only be read on the connection that set it. */
1122+
int64_t cbm_store_journal_size_limit(cbm_store_t *s) {
1123+
if (!s || !s->db) {
1124+
return -1;
1125+
}
1126+
sqlite3_stmt *stmt = NULL;
1127+
if (sqlite3_prepare_v2(s->db, "PRAGMA journal_size_limit;", -1, &stmt, NULL) != SQLITE_OK) {
1128+
return -1;
1129+
}
1130+
int64_t limit = -1;
1131+
if (sqlite3_step(stmt) == SQLITE_ROW) {
1132+
limit = sqlite3_column_int64(stmt, 0);
1133+
}
1134+
sqlite3_finalize(stmt);
1135+
return limit;
1136+
}
1137+
10841138
/* ── Dump ───────────────────────────────────────────────────────── */
10851139

10861140
/* Dump entire in-memory database to a file via sqlite3_backup.

src/store/store.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,10 @@ int cbm_store_create_indexes(cbm_store_t *s);
262262
/* Force WAL checkpoint + PRAGMA optimize. */
263263
int cbm_store_checkpoint(cbm_store_t *s);
264264

265+
/* #1083: the WAL size limit (journal_size_limit) applied to this write
266+
* connection, in bytes; -1 = unlimited (SQLite default / pre-fix). */
267+
int64_t cbm_store_journal_size_limit(cbm_store_t *s);
268+
265269
/* Resolve the mmap_size pragma value applied to on-disk stores from the
266270
* CBM_SQLITE_MMAP_SIZE environment variable. Defaults to 67108864 (64 MB)
267271
* when the variable is unset, malformed, or partially numeric. Negative

tests/test_store_pragmas.c

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,33 @@ TEST(store_open_with_mmap_disabled) {
8686
PASS();
8787
}
8888

89+
/* #1083: on-disk write connections must bound the WAL via journal_size_limit
90+
* so a checkpoint-starved log is physically reclaimed once a checkpoint can
91+
* reset it. On main this is UNSET (-1 = unlimited), so the -wal file only ever
92+
* grows (all our checkpoints are PASSIVE and never ftruncate). Read the pragma
93+
* back on the SAME connection — it's per-connection and not persisted. */
94+
TEST(journal_size_limit_bounds_wal_issue1083) {
95+
char tmp_path[256];
96+
snprintf(tmp_path, sizeof(tmp_path), "%s/cbm_test_jsl_%d.db", cbm_tmpdir(), (int)getpid());
97+
unlink(tmp_path);
98+
99+
cbm_store_t *s = cbm_store_open_path(tmp_path);
100+
ASSERT(s != NULL);
101+
/* 256 MiB — far above the healthy WAL (~4 MiB), so no truncate/regrow churn
102+
* in normal operation; it only fires after abnormal (starved) growth. */
103+
ASSERT(cbm_store_journal_size_limit(s) == (int64_t)268435456);
104+
cbm_store_close(s);
105+
106+
unlink(tmp_path);
107+
char tmp_wal[300];
108+
char tmp_shm[300];
109+
snprintf(tmp_wal, sizeof(tmp_wal), "%s-wal", tmp_path);
110+
snprintf(tmp_shm, sizeof(tmp_shm), "%s-shm", tmp_path);
111+
unlink(tmp_wal);
112+
unlink(tmp_shm);
113+
PASS();
114+
}
115+
89116

90117
/* #896: a row-scan that dies mid-stream (SQLITE_CORRUPT) must surface a
91118
* loud store error, not masquerade as a clean end of results. Counts are
@@ -179,6 +206,7 @@ TEST(corrupt_page_scan_returns_error_not_truncation) {
179206
}
180207

181208
SUITE(store_pragmas) {
209+
RUN_TEST(journal_size_limit_bounds_wal_issue1083);
182210
RUN_TEST(corrupt_page_scan_returns_error_not_truncation);
183211
RUN_TEST(mmap_size_default_when_unset);
184212
RUN_TEST(mmap_size_zero_disables_mmap);

0 commit comments

Comments
 (0)