From dd89b734bf7cc793500d339bf8fb7f72a7bf32a2 Mon Sep 17 00:00:00 2001 From: Andrew Hundt Date: Sat, 4 Jul 2026 18:06:09 -0400 Subject: [PATCH] fix(mcp): reap finished overlay compaction workers Allow long-lived MCP servers to start a later overlay compaction pass after a prior background worker has finished but has not been explicitly joined yet. The start path now reaps one finished worker through the existing join/reset logic, preserves false while a worker is active, and keeps failed reaps on the existing log path only. Add a regression that uses the existing two-overlay fixture to prove a finished unjoined worker does not block a second compaction start. Validation: build/c/test-runner rebuild /private/tmp/cbm-build-overlay-reap-20260704T1818.log; CBM_ONLY_SUITE=mcp /private/tmp/cbm-test-mcp-overlay-reap-20260704T1825.log (171 passed); source safety /private/tmp/cbm-source-safety-overlay-reap-20260704T1826.log; diff check /private/tmp/cbm-diff-check-overlay-reap-20260704T1826.log; production build/sign /private/tmp/cbm-prod-build-overlay-reap-20260704T1828.log. Signed-off-by: Andrew Hundt --- src/mcp/mcp.c | 44 +++++++++++++++++++++++++++---------- tests/test_mcp.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 11 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index baa3d0412..b1abdad79 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -2076,19 +2076,41 @@ bool cbm_mcp_server_start_overlay_compaction(cbm_mcp_server_t *srv, const char * return false; } - cbm_mutex_lock(&srv->overlay_compaction_lock); - if (srv->overlay_compaction_started) { + bool reaped_finished_worker = false; + for (;;) { + cbm_mutex_lock(&srv->overlay_compaction_lock); + if (!srv->overlay_compaction_started) { + int n = snprintf(srv->overlay_compaction_project, + sizeof(srv->overlay_compaction_project), "%s", project); + if (n < 0 || (size_t)n >= sizeof(srv->overlay_compaction_project)) { + cbm_mutex_unlock(&srv->overlay_compaction_lock); + return false; + } + srv->overlay_compaction_max_generations = max_generations; + srv->overlay_compaction_rc = CBM_STORE_ERR; + srv->overlay_compaction_compacted = 0; + srv->overlay_compaction_finished = false; + srv->overlay_compaction_started = true; + cbm_mutex_unlock(&srv->overlay_compaction_lock); + break; + } + bool finished = srv->overlay_compaction_finished; cbm_mutex_unlock(&srv->overlay_compaction_lock); - return false; + if (!finished || reaped_finished_worker) { + return false; + } + int compacted = 0; + int join_rc = cbm_mcp_server_join_overlay_compaction(srv, &compacted); + if (join_rc != CBM_STORE_OK) { + char rc_buf[CBM_SZ_32]; + char compacted_buf[CBM_SZ_32]; + snprintf(rc_buf, sizeof(rc_buf), "%d", join_rc); + snprintf(compacted_buf, sizeof(compacted_buf), "%d", compacted); + cbm_log_warn("overlay_compaction.reap_failed", "rc", rc_buf, "compacted", + compacted_buf); + } + reaped_finished_worker = true; } - snprintf(srv->overlay_compaction_project, sizeof(srv->overlay_compaction_project), "%s", - project); - srv->overlay_compaction_max_generations = max_generations; - srv->overlay_compaction_rc = CBM_STORE_ERR; - srv->overlay_compaction_compacted = 0; - srv->overlay_compaction_finished = false; - srv->overlay_compaction_started = true; - cbm_mutex_unlock(&srv->overlay_compaction_lock); if (cbm_thread_create(&srv->overlay_compaction_tid, 0, overlay_compaction_thread, srv) != 0) { diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 430d2ed0c..e75d0f0d5 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -4524,6 +4524,62 @@ TEST(mcp_overlay_compaction_worker_uses_own_store_and_joins) { PASS(); } +TEST(mcp_overlay_compaction_worker_reaps_finished_before_next_start) { + enum { + COMPACT_ONE_GENERATION = 1, + WAIT_ATTEMPTS = CBM_SZ_1K, + WAIT_SLEEP_US = (int)(CBM_USEC_PER_SEC / CBM_MSEC_PER_SEC), + }; + const char *project = "overlay-reap-project"; + char *cache_tmp = th_mktempdir("cbm_mcp_overlay_reap_cache"); + ASSERT_NOT_NULL(cache_tmp); + char cache[CBM_PATH_MAX]; + int n = snprintf(cache, sizeof(cache), "%s", cache_tmp); + ASSERT(n >= 0 && (size_t)n < sizeof(cache)); + + const char *saved = getenv("CBM_CACHE_DIR"); + char *saved_copy = saved ? cbm_strdup(saved) : NULL; + cbm_setenv("CBM_CACHE_DIR", cache, 1); + + char db_path[CBM_PATH_MAX]; + ASSERT_EQ(mcp_create_overlay_compaction_fixture(cache, project, db_path, + sizeof(db_path)), + CBM_STORE_OK); + + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + ASSERT_TRUE(cbm_mcp_server_start_overlay_compaction(srv, project, + COMPACT_ONE_GENERATION)); + for (int attempt = 0; attempt < WAIT_ATTEMPTS; attempt++) { + if (!cbm_mcp_server_overlay_compaction_active(srv)) { + break; + } + cbm_usleep(WAIT_SLEEP_US); + } + ASSERT_FALSE(cbm_mcp_server_overlay_compaction_active(srv)); + + ASSERT_TRUE(cbm_mcp_server_start_overlay_compaction( + srv, project, CBM_STORE_COMPACT_ALL_GENERATIONS)); + int compacted = -1; + ASSERT_EQ(cbm_mcp_server_join_overlay_compaction(srv, &compacted), CBM_STORE_OK); + ASSERT_EQ(compacted, 1); + + cbm_store_t *store = cbm_store_open_path_query(db_path); + ASSERT_NOT_NULL(store); + ASSERT_EQ(mcp_store_node_qn_exists(store, project, "overlay-reap-project.main.Old"), + 0); + ASSERT_EQ(mcp_store_node_qn_exists(store, project, + "overlay-reap-project.helper.Helper"), + 0); + cbm_store_close(store); + + cbm_mcp_server_free(srv); + mcp_unlink_db_sidecars(db_path); + mcp_restore_cache_dir(saved_copy); + th_cleanup(cache); + PASS(); +} + TEST(mcp_overlay_compaction_worker_missing_db_does_not_create_store) { const char *project = "overlay-missing-project"; char *cache_tmp = th_mktempdir("cbm_mcp_overlay_missing_cache"); @@ -6127,6 +6183,7 @@ SUITE(mcp) { RUN_TEST(tool_ingest_traces_basic); RUN_TEST(tool_ingest_traces_empty); RUN_TEST(mcp_overlay_compaction_worker_uses_own_store_and_joins); + RUN_TEST(mcp_overlay_compaction_worker_reaps_finished_before_next_start); RUN_TEST(mcp_overlay_compaction_worker_missing_db_does_not_create_store); RUN_TEST(mcp_overlay_compaction_worker_rejects_invalid_inputs); RUN_TEST(mcp_overlay_compaction_worker_free_joins_pending_worker);