Skip to content

Commit 45b9a5c

Browse files
committed
fix(sqlite_writer): publish rebuilt db atomically
Keep the live database visible until a synced temporary generation can replace it. Preserve destination sidecars on replacement failure and use UTF-8-safe Windows file operations. Signed-off-by: Nikita Bige <wargloom@gmail.com>
1 parent 2469ecc commit 45b9a5c

4 files changed

Lines changed: 376 additions & 43 deletions

File tree

‎internal/cbm/sqlite_writer.c‎

Lines changed: 86 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,16 @@
2727
#include <stdint.h>
2828
#include <stdbool.h>
2929

30+
#ifdef _WIN32
31+
#include <io.h>
32+
#include <process.h>
33+
#include <windows.h>
34+
#define cbm_writer_getpid _getpid
35+
#else
36+
#include <unistd.h>
37+
#define cbm_writer_getpid getpid
38+
#endif
39+
3040
#define CBM_PAGE_SIZE 65536
3141

3242
/* SQLite reserves the page containing the 1 GiB file offset (the "pending byte"
@@ -1707,6 +1717,8 @@ static uint32_t build_node_index_sorted(FILE *fp, uint32_t *next_page, CBMDumpNo
17071717
typedef struct {
17081718
FILE *fp;
17091719
uint32_t next_page;
1720+
char final_path[CBM_SZ_4K];
1721+
char temp_path[CBM_SZ_4K];
17101722
const char *project;
17111723
const char *root_path;
17121724
const char *indexed_at;
@@ -1720,6 +1732,60 @@ typedef struct {
17201732
int token_vec_count;
17211733
} write_db_ctx_t;
17221734

1735+
static int make_writer_temp_path(const char *path, const void *token, char *out, size_t out_size) {
1736+
int n = snprintf(out, out_size, "%s.tmp.%ld.%p", path, (long)cbm_writer_getpid(), token);
1737+
return (n >= 0 && (size_t)n < out_size) ? 0 : ERR_WRITE_FAILED;
1738+
}
1739+
1740+
static int sync_writer_output(FILE *fp) {
1741+
if (fflush(fp) != 0) {
1742+
return ERR_WRITE_FAILED;
1743+
}
1744+
#ifdef _WIN32
1745+
return _commit(_fileno(fp)) == 0 ? 0 : ERR_WRITE_FAILED;
1746+
#else
1747+
return fsync(fileno(fp)) == 0 ? 0 : ERR_WRITE_FAILED;
1748+
#endif
1749+
}
1750+
1751+
static int discard_writer_output(write_db_ctx_t *w, int rc) {
1752+
if (w->fp) {
1753+
(void)fclose(w->fp);
1754+
w->fp = NULL;
1755+
}
1756+
if (w->temp_path[0]) {
1757+
(void)cbm_unlink(w->temp_path);
1758+
}
1759+
return rc;
1760+
}
1761+
1762+
static int publish_writer_output(write_db_ctx_t *w) {
1763+
if (sync_writer_output(w->fp) != 0) {
1764+
return discard_writer_output(w, ERR_WRITE_FAILED);
1765+
}
1766+
if (fclose(w->fp) != 0) {
1767+
w->fp = NULL;
1768+
if (w->temp_path[0]) {
1769+
(void)cbm_unlink(w->temp_path);
1770+
}
1771+
return ERR_WRITE_FAILED;
1772+
}
1773+
w->fp = NULL;
1774+
if (!w->temp_path[0] || !w->final_path[0]) {
1775+
return 0;
1776+
}
1777+
if (cbm_rename_replace(w->temp_path, w->final_path) != 0) {
1778+
(void)cbm_unlink(w->temp_path);
1779+
return ERR_WRITE_FAILED;
1780+
}
1781+
/* Sidecars are removed only after the replacement succeeds. On POSIX,
1782+
* readers of the old generation retain their unlinked handles. On
1783+
* Windows, an incompatible open handle makes MoveFileExW fail before
1784+
* this cleanup, preserving the old DB and its sidecars. */
1785+
cbm_remove_db_sidecars(w->final_path);
1786+
return 0;
1787+
}
1788+
17231789
/* Callback type for building a record from an item at index i. */
17241790
typedef uint8_t *(*build_record_fn)(const void *items, int i, int *out_len);
17251791
typedef int64_t (*get_rowid_fn)(const void *items, int i);
@@ -1992,20 +2058,17 @@ static int write_db_after_nodes(write_db_ctx_t *w, uint32_t nodes_root) {
19922058
int rc =
19932059
write_one_table(w, &edges_root, w->edges, w->edge_count, adapt_build_edge, adapt_edge_id);
19942060
if (rc != 0) {
1995-
(void)fclose(fp);
1996-
return rc;
2061+
return discard_writer_output(w, rc);
19972062
}
19982063
rc = write_one_table(w, &vectors_root, w->vectors, w->vector_count, adapt_build_vector,
19992064
adapt_vector_id);
20002065
if (rc != 0) {
2001-
(void)fclose(fp);
2002-
return rc;
2066+
return discard_writer_output(w, rc);
20032067
}
20042068
rc = write_one_table(w, &token_vecs_root, w->token_vecs, w->token_vec_count,
20052069
adapt_build_token_vec, adapt_token_vec_id);
20062070
if (rc != 0) {
2007-
(void)fclose(fp);
2008-
return rc;
2071+
return discard_writer_output(w, rc);
20092072
}
20102073
CBM_PROF_END_N("write_db", "1_data_tables", t_data, node_count + edge_count);
20112074

@@ -2058,8 +2121,7 @@ static int write_db_after_nodes(write_db_ctx_t *w, uint32_t nodes_root) {
20582121
&idx_nodes_name_root, &idx_nodes_file_root, &autoindex_nodes_root);
20592122
CBM_PROF_END_N("write_db", "4_node_indexes_seq", t_node_idx, node_count * NODE_SORT_THREADS);
20602123
if (nrc != 0) {
2061-
(void)fclose(fp);
2062-
return nrc;
2124+
return discard_writer_output(w, nrc);
20632125
}
20642126

20652127
CBM_PROF_START(t_edge_idx);
@@ -2076,8 +2138,7 @@ static int write_db_after_nodes(write_db_ctx_t *w, uint32_t nodes_root) {
20762138
&idx_edges_url_path_root, &autoindex_edges_root);
20772139
CBM_PROF_END_N("write_db", "5_edge_indexes_seq", t_edge_idx, edge_count * EDGE_SORT_THREADS);
20782140
if (erc != 0) {
2079-
(void)fclose(fp);
2080-
return erc;
2141+
return discard_writer_output(w, erc);
20812142
}
20822143

20832144
// Autoindex for projects(name TEXT PK) — single text column
@@ -2188,12 +2249,10 @@ static int write_db_after_nodes(write_db_ctx_t *w, uint32_t nodes_root) {
21882249
int master_count = sizeof(master) / sizeof(master[0]);
21892250
int rc2 = write_master_page1(fp, master, master_count, next_page);
21902251
if (rc2 != 0) {
2191-
(void)fclose(fp);
2192-
return rc2;
2252+
return discard_writer_output(w, rc2);
21932253
}
21942254
pad_file_to_page_boundary(fp, next_page);
2195-
(void)fclose(fp);
2196-
return 0;
2255+
return publish_writer_output(w);
21972256
}
21982257

21992258
// --- Streaming writer (incremental bulk node-table append) ---
@@ -2207,21 +2266,20 @@ struct cbm_db_writer {
22072266
};
22082267

22092268
cbm_db_writer_t *cbm_writer_open(const char *path) {
2210-
/* Installing a fresh DB generation: drop the destination's leftover
2211-
* -wal/-shm or a crashed session's WAL gets replayed on top of the
2212-
* new file at the next open (#897). */
2213-
cbm_remove_db_sidecars(path);
2214-
/* cbm_fopen, not raw fopen: the cache dir lives under the user profile,
2215-
* and an ANSI-CP fopen fails to create the DB on non-ASCII Windows
2216-
* profiles — the reported phase=dump failure (#996). Everything around
2217-
* this call (cbm_mkdir_p, sqlite3_open_v2 reopen) is already wide-safe. */
2218-
FILE *fp = cbm_fopen(path, "wb");
2219-
if (!fp) {
2220-
return NULL;
2221-
}
22222269
cbm_db_writer_t *w = (cbm_db_writer_t *)calloc(CBM_ALLOC_ONE, sizeof(*w));
22232270
if (!w) {
2224-
(void)fclose(fp);
2271+
return NULL;
2272+
}
2273+
int n = snprintf(w->wc.final_path, sizeof(w->wc.final_path), "%s", path);
2274+
if (n < 0 || (size_t)n >= sizeof(w->wc.final_path) ||
2275+
make_writer_temp_path(path, w, w->wc.temp_path, sizeof(w->wc.temp_path)) != 0) {
2276+
free(w);
2277+
return NULL;
2278+
}
2279+
FILE *fp = cbm_fopen(w->wc.temp_path, "wb");
2280+
if (!fp) {
2281+
(void)cbm_unlink(w->wc.temp_path);
2282+
free(w);
22252283
return NULL;
22262284
}
22272285
w->wc.fp = fp;
@@ -2287,8 +2345,7 @@ int cbm_writer_finalize(cbm_db_writer_t *w, const char *project, const char *roo
22872345
write_db_ctx_t wc = w->wc; /* value copy survives free(w) */
22882346
free(w);
22892347
if (err != 0) {
2290-
(void)fclose(wc.fp); /* wc is a value copy, valid after free(w) */
2291-
return err;
2348+
return discard_writer_output(&wc, err);
22922349
}
22932350
return write_db_after_nodes(&wc, nodes_root);
22942351
}

‎src/foundation/compat_fs.c‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -761,17 +761,19 @@ int cbm_canonical_path(const char *path, char *out, size_t out_sz) {
761761

762762
/* rename() with overwrite semantics on every platform: POSIX rename already
763763
* replaces atomically; Windows rename fails with EEXIST when the target
764-
* exists, so use MoveFileExW(MOVEFILE_REPLACE_EXISTING) there (wide paths —
765-
* raw MoveFileExA would re-mangle non-ASCII cache paths). */
764+
* exists, so use write-through MoveFileExW(MOVEFILE_REPLACE_EXISTING) there
765+
* (wide paths — raw MoveFileExA would re-mangle non-ASCII cache paths). */
766766
int cbm_rename_replace(const char *src, const char *dst) {
767767
#ifdef _WIN32
768768
wchar_t *wsrc = cbm_utf8_to_wide(src);
769769
wchar_t *wdst = cbm_utf8_to_wide(dst);
770770
int ret = CBM_NOT_FOUND;
771771
if (wsrc && wdst) {
772-
ret = MoveFileExW(wsrc, wdst, MOVEFILE_REPLACE_EXISTING | MOVEFILE_COPY_ALLOWED)
773-
? 0
774-
: CBM_NOT_FOUND;
772+
ret =
773+
MoveFileExW(wsrc, wdst,
774+
MOVEFILE_REPLACE_EXISTING | MOVEFILE_COPY_ALLOWED | MOVEFILE_WRITE_THROUGH)
775+
? 0
776+
: CBM_NOT_FOUND;
775777
}
776778
free(wsrc);
777779
free(wdst);
@@ -782,11 +784,10 @@ int cbm_rename_replace(const char *src, const char *dst) {
782784
}
783785

784786
/* Remove a SQLite database's -wal/-shm sidecars (both platforms). Any code
785-
* path that installs a FRESH database file at a path where a previous
786-
* generation lived must call this first: SQLite decides whether to replay a
787-
* WAL purely from the sidecar's own header/checksums, so a leftover WAL
788-
* from a crashed session is recovered ON TOP of the freshly installed file
789-
* at the next open, splicing old-generation pages into it (#897). */
787+
* path that installs a FRESH database file where a previous generation lived
788+
* must remove them before the new generation can be opened: SQLite decides
789+
* whether to replay a WAL purely from the sidecar's own header/checksums, so
790+
* a leftover WAL can splice old-generation pages into the new file (#897). */
790791
void cbm_remove_db_sidecars(const char *db_path) {
791792
if (!db_path || !db_path[0]) {
792793
return;

‎src/foundation/compat_fs.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,12 @@ bool cbm_mkdir_p(const char *path, int mode);
4646

4747
/* Delete a file. Returns 0 on success. */
4848
int cbm_unlink(const char *path);
49-
/* Remove <db_path>-wal/-shm. MUST be called by any path installing a fresh
50-
* DB file where a previous generation lived — a leftover WAL is otherwise
51-
* replayed on top of the new file at the next open (#897). */
49+
/* Remove <db_path>-wal/-shm. Any path installing a fresh DB generation must
50+
* do this before the new generation can be opened; a leftover WAL is
51+
* otherwise replayed on top of the new file (#897). */
5252
void cbm_remove_db_sidecars(const char *db_path);
5353
/* rename() that replaces an existing destination on every platform
54-
* (Windows rename fails with EEXIST; this uses MoveFileExW there). */
54+
* (Windows rename fails with EEXIST; this uses write-through MoveFileExW). */
5555
int cbm_rename_replace(const char *src, const char *dst);
5656
/* Canonicalize an EXISTING path (realpath / wide GetFullPathNameW). Locale-
5757
* independent on Windows — never routes UTF-8 through the ANSI CRT (#973).

0 commit comments

Comments
 (0)