From 076b9a9c474c66d3607b5c3c4b369979826ca403 Mon Sep 17 00:00:00 2001 From: Nikita Bige Date: Tue, 7 Jul 2026 22:28:28 +0300 Subject: [PATCH] fix(sqlite_writer): publish rebuilt db atomically Signed-off-by: Nikita Bige --- internal/cbm/sqlite_writer.c | 111 ++++++++++++++---- tests/test_sqlite_writer.c | 215 +++++++++++++++++++++++++++++++++++ 2 files changed, 305 insertions(+), 21 deletions(-) diff --git a/internal/cbm/sqlite_writer.c b/internal/cbm/sqlite_writer.c index 3b102a07..d3efe904 100644 --- a/internal/cbm/sqlite_writer.c +++ b/internal/cbm/sqlite_writer.c @@ -26,6 +26,15 @@ #include #include +#ifdef _WIN32 +#include +#include +#define cbm_writer_getpid _getpid +#else +#include +#define cbm_writer_getpid getpid +#endif + #define CBM_PAGE_SIZE 65536 /* SQLite reserves the page containing the 1 GiB file offset (the "pending byte" @@ -1706,6 +1715,8 @@ static uint32_t build_node_index_sorted(FILE *fp, uint32_t *next_page, CBMDumpNo typedef struct { FILE *fp; uint32_t next_page; + char final_path[CBM_SZ_4K]; + char temp_path[CBM_SZ_4K]; const char *project; const char *root_path; const char *indexed_at; @@ -1719,6 +1730,65 @@ typedef struct { int token_vec_count; } write_db_ctx_t; +static int make_writer_temp_path(const char *path, const void *token, char *out, size_t out_size) { + int n = snprintf(out, out_size, "%s.tmp.%ld.%p", path, (long)cbm_writer_getpid(), token); + return (n >= 0 && (size_t)n < out_size) ? 0 : ERR_WRITE_FAILED; +} + +static void remove_sqlite_sidecars(const char *path) { + char sidecar[CBM_SZ_4K]; + int n = snprintf(sidecar, sizeof(sidecar), "%s-wal", path); + if (n >= 0 && (size_t)n < sizeof(sidecar)) { + (void)remove(sidecar); + } + n = snprintf(sidecar, sizeof(sidecar), "%s-shm", path); + if (n >= 0 && (size_t)n < sizeof(sidecar)) { + (void)remove(sidecar); + } +} + +static int replace_writer_output(const char *tmp, const char *path) { +#ifdef _WIN32 + return MoveFileExA(tmp, path, MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH) + ? 0 + : ERR_WRITE_FAILED; +#else + return rename(tmp, path) == 0 ? 0 : ERR_WRITE_FAILED; +#endif +} + +static int discard_writer_output(write_db_ctx_t *w, int rc) { + if (w->fp) { + (void)fclose(w->fp); + w->fp = NULL; + } + if (w->temp_path[0]) { + (void)remove(w->temp_path); + } + return rc; +} + +static int publish_writer_output(write_db_ctx_t *w) { + if (fclose(w->fp) != 0) { + w->fp = NULL; + if (w->temp_path[0]) { + (void)remove(w->temp_path); + } + return ERR_WRITE_FAILED; + } + w->fp = NULL; + if (!w->temp_path[0] || !w->final_path[0]) { + return 0; + } + remove_sqlite_sidecars(w->final_path); + if (replace_writer_output(w->temp_path, w->final_path) != 0) { + (void)remove(w->temp_path); + return ERR_WRITE_FAILED; + } + remove_sqlite_sidecars(w->final_path); + return 0; +} + /* Callback type for building a record from an item at index i. */ typedef uint8_t *(*build_record_fn)(const void *items, int i, int *out_len); typedef int64_t (*get_rowid_fn)(const void *items, int i); @@ -1991,20 +2061,17 @@ static int write_db_after_nodes(write_db_ctx_t *w, uint32_t nodes_root) { int rc = write_one_table(w, &edges_root, w->edges, w->edge_count, adapt_build_edge, adapt_edge_id); if (rc != 0) { - (void)fclose(fp); - return rc; + return discard_writer_output(w, rc); } rc = write_one_table(w, &vectors_root, w->vectors, w->vector_count, adapt_build_vector, adapt_vector_id); if (rc != 0) { - (void)fclose(fp); - return rc; + return discard_writer_output(w, rc); } rc = write_one_table(w, &token_vecs_root, w->token_vecs, w->token_vec_count, adapt_build_token_vec, adapt_token_vec_id); if (rc != 0) { - (void)fclose(fp); - return rc; + return discard_writer_output(w, rc); } CBM_PROF_END_N("write_db", "1_data_tables", t_data, node_count + edge_count); @@ -2057,8 +2124,7 @@ static int write_db_after_nodes(write_db_ctx_t *w, uint32_t nodes_root) { &idx_nodes_name_root, &idx_nodes_file_root, &autoindex_nodes_root); CBM_PROF_END_N("write_db", "4_node_indexes_seq", t_node_idx, node_count * NODE_SORT_THREADS); if (nrc != 0) { - (void)fclose(fp); - return nrc; + return discard_writer_output(w, nrc); } CBM_PROF_START(t_edge_idx); @@ -2075,8 +2141,7 @@ static int write_db_after_nodes(write_db_ctx_t *w, uint32_t nodes_root) { &idx_edges_url_path_root, &autoindex_edges_root); CBM_PROF_END_N("write_db", "5_edge_indexes_seq", t_edge_idx, edge_count * EDGE_SORT_THREADS); if (erc != 0) { - (void)fclose(fp); - return erc; + return discard_writer_output(w, erc); } // Autoindex for projects(name TEXT PK) — single text column @@ -2187,12 +2252,10 @@ static int write_db_after_nodes(write_db_ctx_t *w, uint32_t nodes_root) { int master_count = sizeof(master) / sizeof(master[0]); int rc2 = write_master_page1(fp, master, master_count, next_page); if (rc2 != 0) { - (void)fclose(fp); - return rc2; + return discard_writer_output(w, rc2); } pad_file_to_page_boundary(fp, next_page); - (void)fclose(fp); - return 0; + return publish_writer_output(w); } // --- Streaming writer (incremental bulk node-table append) --- @@ -2206,13 +2269,20 @@ struct cbm_db_writer { }; cbm_db_writer_t *cbm_writer_open(const char *path) { - FILE *fp = fopen(path, "wb"); - if (!fp) { - return NULL; - } cbm_db_writer_t *w = (cbm_db_writer_t *)calloc(CBM_ALLOC_ONE, sizeof(*w)); if (!w) { - (void)fclose(fp); + return NULL; + } + int n = snprintf(w->wc.final_path, sizeof(w->wc.final_path), "%s", path); + if (n < 0 || (size_t)n >= sizeof(w->wc.final_path) || + make_writer_temp_path(path, w, w->wc.temp_path, sizeof(w->wc.temp_path)) != 0) { + free(w); + return NULL; + } + FILE *fp = fopen(w->wc.temp_path, "wb"); + if (!fp) { + (void)remove(w->wc.temp_path); + free(w); return NULL; } w->wc.fp = fp; @@ -2278,8 +2348,7 @@ int cbm_writer_finalize(cbm_db_writer_t *w, const char *project, const char *roo write_db_ctx_t wc = w->wc; /* value copy survives free(w) */ free(w); if (err != 0) { - (void)fclose(wc.fp); /* wc is a value copy, valid after free(w) */ - return err; + return discard_writer_output(&wc, err); } return write_db_after_nodes(&wc, nodes_root); } diff --git a/tests/test_sqlite_writer.c b/tests/test_sqlite_writer.c index 8898eb04..a33dc57b 100644 --- a/tests/test_sqlite_writer.c +++ b/tests/test_sqlite_writer.c @@ -8,6 +8,7 @@ * bypassing the SQL parser entirely. These tests verify integrity. */ #include "../src/foundation/compat.h" +#include "foundation/compat_fs.h" #include "test_framework.h" /* sqlite_writer.h is at internal/cbm/ — Makefile adds -Iinternal/cbm */ #include "sqlite_writer.h" /* CBMDumpNode, CBMDumpEdge, cbm_write_db */ @@ -25,6 +26,58 @@ static int make_temp_db(char *path, size_t pathsz) { return 0; } +static int assert_node_name(sqlite3 *db, const char *expected) { + sqlite3_stmt *stmt = NULL; + int rc = sqlite3_prepare_v2(db, "SELECT name FROM nodes WHERE id=1", -1, &stmt, NULL); + ASSERT_EQ(rc, SQLITE_OK); + rc = sqlite3_step(stmt); + ASSERT_EQ(rc, SQLITE_ROW); + ASSERT_STR_EQ((const char *)sqlite3_column_text(stmt, 0), expected); + sqlite3_finalize(stmt); + return 0; +} + +static int count_temp_outputs_for(const char *path) { + char dir[256]; + char base[256]; + const char *slash = strrchr(path, '/'); +#ifdef _WIN32 + const char *backslash = strrchr(path, '\\'); + if (!slash || (backslash && backslash > slash)) { + slash = backslash; + } +#endif + if (slash) { + size_t dir_len = (size_t)(slash - path); + if (dir_len == 0 || dir_len >= sizeof(dir)) { + return -1; + } + memcpy(dir, path, dir_len); + dir[dir_len] = '\0'; + snprintf(base, sizeof(base), "%s", slash + 1); + } else { + snprintf(dir, sizeof(dir), "."); + snprintf(base, sizeof(base), "%s", path); + } + + cbm_dir_t *d = cbm_opendir(dir); + if (!d) { + return -1; + } + size_t base_len = strlen(base); + int count = 0; + cbm_dirent_t *ent; + while ((ent = cbm_readdir(d)) != NULL) { + size_t name_len = strlen(ent->name); + if (name_len > base_len + 5 && strncmp(ent->name, base, base_len) == 0 && + strncmp(ent->name + base_len, ".tmp.", 5) == 0) { + count++; + } + } + cbm_closedir(d); + return count; +} + /* ── Tests ─────────────────────────────────────────────────────── */ TEST(sw_minimal_data) { @@ -605,6 +658,164 @@ TEST(sw_oversized_node) { PASS(); } +TEST(sw_stream_open_does_not_truncate_destination) { + char path[256]; + ASSERT_EQ(make_temp_db(path, sizeof(path)), 0); + + const char sentinel[] = "old-db-visible-until-finalize"; + FILE *fp = fopen(path, "wb"); + ASSERT(fp != NULL); + ASSERT_EQ(fwrite(sentinel, 1, sizeof(sentinel) - 1, fp), sizeof(sentinel) - 1); + ASSERT_EQ(fclose(fp), 0); + + cbm_db_writer_t *w = cbm_writer_open(path); + ASSERT(w != NULL); + + char buf[64] = {0}; + fp = fopen(path, "rb"); + ASSERT(fp != NULL); + size_t n = fread(buf, 1, sizeof(sentinel) - 1, fp); + ASSERT_EQ(fclose(fp), 0); + ASSERT_EQ(n, sizeof(sentinel) - 1); + ASSERT_EQ(memcmp(buf, sentinel, sizeof(sentinel) - 1), 0); + + int rc = cbm_writer_finalize(w, "test", "/tmp/test", "2026-07-07T00:00:00Z", NULL, 0, NULL, 0, + NULL, 0, NULL, 0); + ASSERT_EQ(rc, 0); + + sqlite3 *db = NULL; + rc = sqlite3_open(path, &db); + ASSERT_EQ(rc, SQLITE_OK); + + sqlite3_stmt *stmt = NULL; + sqlite3_prepare_v2(db, "PRAGMA integrity_check", -1, &stmt, NULL); + rc = sqlite3_step(stmt); + ASSERT_EQ(rc, SQLITE_ROW); + ASSERT_STR_EQ((const char *)sqlite3_column_text(stmt, 0), "ok"); + sqlite3_finalize(stmt); + + sqlite3_close(db); + unlink(path); + PASS(); +} + +TEST(sw_publish_removes_destination_sidecars) { + char path[256]; + ASSERT_EQ(make_temp_db(path, sizeof(path)), 0); + + char wal[320]; + char shm[320]; + snprintf(wal, sizeof(wal), "%s-wal", path); + snprintf(shm, sizeof(shm), "%s-shm", path); + + FILE *fp = fopen(wal, "wb"); + ASSERT(fp != NULL); + ASSERT_EQ(fwrite("stale-wal", 1, 9, fp), 9); + ASSERT_EQ(fclose(fp), 0); + + fp = fopen(shm, "wb"); + ASSERT(fp != NULL); + ASSERT_EQ(fwrite("stale-shm", 1, 9, fp), 9); + ASSERT_EQ(fclose(fp), 0); + + int rc = cbm_write_db(path, "test", "/tmp/test", "2026-07-07T00:00:00Z", NULL, 0, NULL, 0, NULL, + 0, NULL, 0); + ASSERT_EQ(rc, 0); + ASSERT(access(wal, F_OK) != 0); + ASSERT(access(shm, F_OK) != 0); + + sqlite3 *db = NULL; + rc = sqlite3_open(path, &db); + ASSERT_EQ(rc, SQLITE_OK); + + sqlite3_stmt *stmt = NULL; + sqlite3_prepare_v2(db, "PRAGMA integrity_check", -1, &stmt, NULL); + rc = sqlite3_step(stmt); + ASSERT_EQ(rc, SQLITE_ROW); + ASSERT_STR_EQ((const char *)sqlite3_column_text(stmt, 0), "ok"); + sqlite3_finalize(stmt); + + sqlite3_close(db); + unlink(path); + PASS(); +} + +TEST(sw_publish_failure_removes_temp_output) { + char path[256]; + snprintf(path, sizeof(path), "/tmp/cbm_sw_dir_XXXXXX"); + ASSERT(cbm_mkdtemp(path) != NULL); + + int rc = cbm_write_db(path, "test", "/tmp/test", "2026-07-07T00:00:00Z", NULL, 0, NULL, 0, NULL, + 0, NULL, 0); + ASSERT(rc != 0); + + ASSERT_EQ(count_temp_outputs_for(path), 0); + + cbm_rmdir(path); + PASS(); +} + +TEST(sw_publish_preserves_live_reader) { +#ifdef _WIN32 + SKIP_PLATFORM("POSIX open-reader rename semantics"); +#endif + char path[256]; + ASSERT_EQ(make_temp_db(path, sizeof(path)), 0); + + CBMDumpNode old_nodes[1] = {{ + .id = 1, + .project = "test", + .label = "Function", + .name = "old_fn", + .qualified_name = "test.old_fn", + .file_path = "old.go", + .start_line = 1, + .end_line = 2, + .properties = "{}", + }}; + int rc = cbm_write_db(path, "test", "/tmp/test", "2026-07-07T00:00:00Z", old_nodes, 1, NULL, 0, + NULL, 0, NULL, 0); + ASSERT_EQ(rc, 0); + + sqlite3 *reader = NULL; + rc = sqlite3_open(path, &reader); + ASSERT_EQ(rc, SQLITE_OK); + rc = sqlite3_exec(reader, "PRAGMA mmap_size=268435456", NULL, NULL, NULL); + ASSERT_EQ(rc, SQLITE_OK); + rc = sqlite3_exec(reader, "BEGIN", NULL, NULL, NULL); + ASSERT_EQ(rc, SQLITE_OK); + ASSERT_EQ(assert_node_name(reader, "old_fn"), 0); + + CBMDumpNode new_nodes[1] = {{ + .id = 1, + .project = "test", + .label = "Function", + .name = "new_fn", + .qualified_name = "test.new_fn", + .file_path = "new.go", + .start_line = 1, + .end_line = 2, + .properties = "{}", + }}; + rc = cbm_write_db(path, "test", "/tmp/test", "2026-07-07T00:00:01Z", new_nodes, 1, NULL, 0, + NULL, 0, NULL, 0); + ASSERT_EQ(rc, 0); + + ASSERT_EQ(assert_node_name(reader, "old_fn"), 0); + rc = sqlite3_exec(reader, "COMMIT", NULL, NULL, NULL); + ASSERT_EQ(rc, SQLITE_OK); + sqlite3_close(reader); + + sqlite3 *db = NULL; + rc = sqlite3_open(path, &db); + ASSERT_EQ(rc, SQLITE_OK); + ASSERT_EQ(assert_node_name(db, "new_fn"), 0); + sqlite3_close(db); + + unlink(path); + PASS(); +} + /* ── Suite ─────────────────────────────────────────────────────── */ SUITE(sqlite_writer) { @@ -615,4 +826,8 @@ SUITE(sqlite_writer) { RUN_TEST(sw_empty); RUN_TEST(sw_multi_page); RUN_TEST(sw_oversized_node); + RUN_TEST(sw_stream_open_does_not_truncate_destination); + RUN_TEST(sw_publish_removes_destination_sidecars); + RUN_TEST(sw_publish_failure_removes_temp_output); + RUN_TEST(sw_publish_preserves_live_reader); }