From eaf1801d6d122a87cbbe7c5c3729887bd8b82ca5 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Thu, 7 May 2026 11:07:41 +0000 Subject: [PATCH] feat: implement kvmgr_capture_keys for lock-free checkpoint serialization (P0-14a) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add kvtab_foreach walk primitive (P0-14) to kv.c/kv.h, then implement kvmgr_capture_keys in kvmgr.c that deep-copies all key→index mappings into a malloc'd slot_key_t[] array safe to read after the write lock is released during checkpoint serialization (Part 3.8). Key design points: - Both kvtab_foreach and kvmgr_capture_keys take var (RTTI data pointer) and use RTTI_DATA_TO_OBJ internally — the RTTI framework's makeInstance returns the data region pointer, not the meta_obj_t header pointer - String keys are copied via strncpy into slot_key_t.key[XCE_MAXNAME_LEN] - Integer keys are formatted as decimal strings via snprintf - Two-pass approach: count live entries first, allocate, then fill - Tests cover string kvmgr (3 entries), empty kvmgr, and deep-copy safety --- src/include/kv.h | 18 +++ src/include/kvmgr.h | 19 +++ src/lib/kv.c | 29 ++++ src/lib/kvmgr.c | 108 +++++++++++++++ src/lib/meson.build | 1 + src/lib/testsuite/test.c | 2 + src/lib/testsuite/test_kvmgr_capture.c | 184 +++++++++++++++++++++++++ 7 files changed, 361 insertions(+) create mode 100644 src/lib/testsuite/test_kvmgr_capture.c diff --git a/src/include/kv.h b/src/include/kv.h index a132ea4e..1137b54c 100644 --- a/src/include/kv.h +++ b/src/include/kv.h @@ -30,4 +30,22 @@ extern kv_key_ops_t int_key_ops; extern kv_key_ops_t string_key_ops; extern int key_cmp_default(void*, void*, size_t); + +/* + * kvtab_foreach — iterate every live entry in the kvtab (P0-14). + * + * For each entry, cb(key, index, arg) is called. + * - key : the raw key pointer stored in the entry (char* for + * string-keyed tables, (void*)(long)intval for int-keyed). + * - index : the slot index stored as the entry value. + * - arg : caller-supplied context. + * + * Iteration order is unspecified. + * Returns the number of entries visited, or -EINVAL if kvtab is NULL. + */ +typedef void (*kvtab_foreach_fn_t)(void* key, int index, void* arg); +extern int kvtab_foreach(var kvtab, + kvtab_foreach_fn_t cb, + void* arg); + #endif diff --git a/src/include/kvmgr.h b/src/include/kvmgr.h index 4981870d..9e21adf7 100644 --- a/src/include/kvmgr.h +++ b/src/include/kvmgr.h @@ -7,6 +7,7 @@ #include "xce/types.h" #include "xlist.h" #include "zarray.h" +#include "persist.h" typedef enum { KVMGR_KEY_INT = 0, @@ -22,4 +23,22 @@ typedef struct { extern typeinfo_t xkvmgr_type; +/* + * kvmgr_capture_keys — capture key→index mappings for checkpoint + * serialization (Part 3.8). + * + * Caller MUST hold the kvmgr write lock for the full duration of + * this call. The function allocates *keys_out and populates it with + * *n_keys_out entries. Each entry's key[] field is an owned deep + * copy that remains valid after the write lock is released. + * + * Caller must free(*keys_out) when done. + * + * Returns 0 on success. + * Returns -ENOMEM if allocation fails. + */ +int kvmgr_capture_keys(var kvmgr, + slot_key_t** keys_out, + size_t* n_keys_out); + #endif diff --git a/src/lib/kv.c b/src/lib/kv.c index 0f349a7e..54970d59 100644 --- a/src/lib/kv.c +++ b/src/lib/kv.c @@ -248,6 +248,35 @@ static int kvtab_find(meta_obj_t* obj, void* key, void** value) return 1; } +/* + * kvtab_foreach — walk every live entry and invoke cb(key, index, arg). + * index is recovered by casting the stored void* value back to int. + * Caller must hold any necessary external lock before calling. + */ +int kvtab_foreach(var kvtab_var, kvtab_foreach_fn_t cb, void* arg) +{ + if (kvtab_var == NULL || cb == NULL) { + return -EINVAL; + } + + meta_obj_t* obj = RTTI_DATA_TO_OBJ(kvtab_var); + kvtab_t* t = RTTI_OBJ_TO_DATA(kvtab_t, obj); + int count = 0; + uint32_t i; + + for (i = 0; i < t->nslots; i++) { + kvent_t* e = t->slots[i]; + while (e != NULL) { + int idx = (int)(long)e->value; + cb(e->key, idx, arg); + count++; + e = e->next; + } + } + + return count; +} + typeinfo_t kvtab_type = { .name = "kvtab", .privateSize = 0, diff --git a/src/lib/kvmgr.c b/src/lib/kvmgr.c index 69c76437..5891035e 100644 --- a/src/lib/kvmgr.c +++ b/src/lib/kvmgr.c @@ -1,12 +1,15 @@ #include +#include #include #include #include +#include #include "htab.h" #include "kv.h" #include "kvmgr.h" #include "mylock.h" +#include "persist.h" #include "utils.h" #include "xce/types.h" #include "zarray.h" @@ -302,3 +305,108 @@ typeinfo_t xkvmgr_type = { .lock = &(intf_lock_t) { .lock = (lock_fn_t)xkvmgr_lock, .unlock = (unlock_fn_t)xkvmgr_unlock } } }; + +/* ------------------------------------------------------------------ */ +/* kvmgr_capture_keys — P0-14a */ +/* ------------------------------------------------------------------ */ + +typedef struct { + slot_key_t* keys; + size_t capacity; + size_t filled; + kvmgr_key_type_t key_type; + int oom; +} capture_ctx_t; + +static void capture_one(void* raw_key, int index, void* arg) +{ + capture_ctx_t* ctx = (capture_ctx_t*)arg; + + if (ctx->oom || ctx->filled >= ctx->capacity) { + return; + } + + slot_key_t* sk = &ctx->keys[ctx->filled]; + sk->index = index; + + if (ctx->key_type == KVMGR_KEY_STRING) { + const char* src = (const char*)raw_key; + strncpy(sk->key, src, XCE_MAXNAME_LEN - 1); + sk->key[XCE_MAXNAME_LEN - 1] = '\0'; + sk->key_len = strlen(sk->key) + 1; + } else { + /* Integer key — format as decimal string */ + int64_t intval = (int64_t)(long)raw_key; + int n = snprintf(sk->key, XCE_MAXNAME_LEN, + "%" PRId64, intval); + if (n < 0 || n >= XCE_MAXNAME_LEN) { + /* Truncated — mark as oom so caller can detect */ + ctx->oom = 1; + return; + } + sk->key_len = (size_t)n + 1; + } + + ctx->filled++; +} + +/* + * Count live entries by iterating; kvtab_foreach returns the count. + * We use a NULL callback to count first, then allocate, then fill. + */ +static void count_one(void* key, int index, void* arg) +{ + (void)key; + (void)index; + size_t* n = (size_t*)arg; + (*n)++; +} + +int kvmgr_capture_keys(var kvmgr_var, + slot_key_t** keys_out, + size_t* n_keys_out) +{ + *keys_out = NULL; + *n_keys_out = 0; + + if (kvmgr_var == NULL) { + return -EINVAL; + } + + /* The public API pointer (var) points to the data region; + * convert to the meta_obj_t header, then extract xkvmgr_t. */ + meta_obj_t* obj = RTTI_DATA_TO_OBJ(kvmgr_var); + xkvmgr_t* kvm = RTTI_OBJ_TO_DATA(xkvmgr_t, obj); + + /* Count live entries */ + size_t n = 0; + kvtab_foreach(kvm->kvtab, count_one, &n); + + if (n == 0) { + return 0; + } + + slot_key_t* keys = calloc(n, sizeof(slot_key_t)); + if (keys == NULL) { + return -ENOMEM; + } + + capture_ctx_t ctx = { + .keys = keys, + .capacity = n, + .filled = 0, + .key_type = kvm->key_type, + .oom = 0, + }; + + kvtab_foreach(kvm->kvtab, capture_one, &ctx); + + if (ctx.oom) { + free(keys); + return -ENOMEM; + } + + *keys_out = keys; + *n_keys_out = ctx.filled; + return 0; +} diff --git a/src/lib/meson.build b/src/lib/meson.build index 1da3de05..486853da 100644 --- a/src/lib/meson.build +++ b/src/lib/meson.build @@ -164,6 +164,7 @@ libxce_test_src = ['testsuite/test.c', 'testsuite/test_qobj.c', 'testsuite/test_request.c', 'testsuite/test_zarray_cow.c', + 'testsuite/test_kvmgr_capture.c', ] libxce_test_deps = [libxce_deps, librtti_dep, unity_dep, libxce_dep] diff --git a/src/lib/testsuite/test.c b/src/lib/testsuite/test.c index 333ce3e0..4ef9aff1 100644 --- a/src/lib/testsuite/test.c +++ b/src/lib/testsuite/test.c @@ -30,6 +30,7 @@ extern void test_qobj(void); extern void test_wobj(void); extern void test_request(void); extern void test_zarray_cow(void); +extern void test_kvmgr_capture(void); void setUp(void) { return; } @@ -129,6 +130,7 @@ int main(int argc, char* argv[]) TEST(test_qobj); TEST(test_request); TEST(test_zarray_cow); + TEST(test_kvmgr_capture); return UNITY_END(); } diff --git a/src/lib/testsuite/test_kvmgr_capture.c b/src/lib/testsuite/test_kvmgr_capture.c new file mode 100644 index 00000000..cbeafc3b --- /dev/null +++ b/src/lib/testsuite/test_kvmgr_capture.c @@ -0,0 +1,184 @@ +/* + * Test suite for kvmgr_capture_keys() — P0-14a. + * + * Tests follow TDD: written BEFORE implementation. + * All tests exercise the public API declared in kvmgr.h. + */ + +#include +#include +#include +#include + +#include + +#include "kv.h" +#include "kvmgr.h" +#include "persist.h" +#include "utils.h" + +/* ------------------------------------------------------------------ */ +/* helpers */ +/* ------------------------------------------------------------------ */ + +static int find_key_in_captured(slot_key_t* keys, size_t n, + const char* target_key, int* index_out) +{ + size_t i; + for (i = 0; i < n; i++) { + if (strcmp(keys[i].key, target_key) == 0) { + if (index_out != NULL) { + *index_out = keys[i].index; + } + return 1; + } + } + return 0; +} + +/* ------------------------------------------------------------------ */ +/* test 1: capture from a string-keyed kvmgr with 3 entries */ +/* ------------------------------------------------------------------ */ + +static void test_capture_keys_string_kvmgr(void) +{ + kvmgr_params_t params = { .key_type = KVMGR_KEY_STRING, + .nslots = 8, + .delta = 4, + .datakiller = NULL }; + var kvmgr = makeInstance(&xkvmgr_type, ¶ms); + TEST_ASSERT_NOT_NULL(kvmgr); + + /* Add 3 string-keyed entries whose values are arbitrary pointers; + the index returned by key_add is the slot index we capture. */ + int idx_a = key_add(kvmgr, (void*)"alpha", (void*)"data_alpha"); + int idx_b = key_add(kvmgr, (void*)"beta", (void*)"data_beta"); + int idx_c = key_add(kvmgr, (void*)"gamma", (void*)"data_gamma"); + TEST_ASSERT_TRUE(idx_a >= 0); + TEST_ASSERT_TRUE(idx_b >= 0); + TEST_ASSERT_TRUE(idx_c >= 0); + + slot_key_t* keys = NULL; + size_t n_keys = 0; + int rc = kvmgr_capture_keys(kvmgr, &keys, &n_keys); + + TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_EQUAL_size_t(3, n_keys); + TEST_ASSERT_NOT_NULL(keys); + + /* Every inserted key must appear exactly once with the correct index */ + int found_idx = -1; + TEST_ASSERT_TRUE(find_key_in_captured(keys, n_keys, "alpha", &found_idx)); + TEST_ASSERT_EQUAL_INT(idx_a, found_idx); + + TEST_ASSERT_TRUE(find_key_in_captured(keys, n_keys, "beta", &found_idx)); + TEST_ASSERT_EQUAL_INT(idx_b, found_idx); + + TEST_ASSERT_TRUE(find_key_in_captured(keys, n_keys, "gamma", &found_idx)); + TEST_ASSERT_EQUAL_INT(idx_c, found_idx); + + /* key_len must equal strlen(key)+1 */ + size_t i; + for (i = 0; i < n_keys; i++) { + TEST_ASSERT_EQUAL_size_t(strlen(keys[i].key) + 1, keys[i].key_len); + } + + free(keys); + destroy(kvmgr); +} + +/* ------------------------------------------------------------------ */ +/* test 2: empty kvmgr returns 0 entries and NULL keys_out */ +/* ------------------------------------------------------------------ */ + +static void test_capture_keys_empty_kvmgr(void) +{ + kvmgr_params_t params = { .key_type = KVMGR_KEY_STRING, + .nslots = 4, + .delta = 4, + .datakiller = NULL }; + var kvmgr = makeInstance(&xkvmgr_type, ¶ms); + TEST_ASSERT_NOT_NULL(kvmgr); + + slot_key_t* keys = (slot_key_t*)(uintptr_t)0xdeadbeef; /* sentinel */ + size_t n_keys = 99; + + int rc = kvmgr_capture_keys(kvmgr, &keys, &n_keys); + + TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_EQUAL_size_t(0, n_keys); + TEST_ASSERT_NULL(keys); + + destroy(kvmgr); +} + +/* ------------------------------------------------------------------ */ +/* test 3: captured keys are deep copies — safe after kvmgr destroyed */ +/* ------------------------------------------------------------------ */ + +static void test_capture_keys_are_deep_copies(void) +{ + kvmgr_params_t params = { .key_type = KVMGR_KEY_STRING, + .nslots = 4, + .delta = 4, + .datakiller = NULL }; + var kvmgr = makeInstance(&xkvmgr_type, ¶ms); + TEST_ASSERT_NOT_NULL(kvmgr); + + key_add(kvmgr, (void*)"keyA", (void*)"valA"); + key_add(kvmgr, (void*)"keyB", (void*)"valB"); + + slot_key_t* keys = NULL; + size_t n_keys = 0; + int rc = kvmgr_capture_keys(kvmgr, &keys, &n_keys); + TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_EQUAL_size_t(2, n_keys); + + /* Stash manual copies of the captured key strings */ + char copy_a[256], copy_b[256]; + int found_a = 0, found_b = 0; + size_t i; + for (i = 0; i < n_keys; i++) { + if (strcmp(keys[i].key, "keyA") == 0) { + strncpy(copy_a, keys[i].key, sizeof(copy_a) - 1); + copy_a[sizeof(copy_a) - 1] = '\0'; + found_a = 1; + } else if (strcmp(keys[i].key, "keyB") == 0) { + strncpy(copy_b, keys[i].key, sizeof(copy_b) - 1); + copy_b[sizeof(copy_b) - 1] = '\0'; + found_b = 1; + } + } + TEST_ASSERT_TRUE(found_a); + TEST_ASSERT_TRUE(found_b); + + /* Destroy the kvmgr — this frees all kvtab entries including the + original key strings. Captured keys[] must remain valid. */ + destroy(kvmgr); + + TEST_ASSERT_EQUAL_STRING("keyA", copy_a); + TEST_ASSERT_EQUAL_STRING("keyB", copy_b); + + /* Directly verify the captured keys array is still readable */ + int still_a = 0, still_b = 0; + for (i = 0; i < n_keys; i++) { + if (strcmp(keys[i].key, "keyA") == 0) still_a = 1; + if (strcmp(keys[i].key, "keyB") == 0) still_b = 1; + } + TEST_ASSERT_TRUE(still_a); + TEST_ASSERT_TRUE(still_b); + + free(keys); +} + +/* ------------------------------------------------------------------ */ +/* entry point called from test.c */ +/* ------------------------------------------------------------------ */ + +void test_kvmgr_capture(void) +{ + printf("\n>> kvmgr_capture_keys test\n"); + RUN_TEST(test_capture_keys_string_kvmgr); + RUN_TEST(test_capture_keys_empty_kvmgr); + RUN_TEST(test_capture_keys_are_deep_copies); +}