From 373ce3e493651fe5cbb8ada7e5e155889d514d8c Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Tue, 13 Aug 2024 22:58:16 -0400 Subject: [PATCH 01/10] Add raft and dqlite APIs for querying the last log entry This enables better automation of dqlite_node_recover_ext by exposing enough information to determine which of several nodes has the most up-to-date log. Signed-off-by: Cole Miller --- include/dqlite.h | 23 +++++++++++++++++++++++ src/raft.h | 21 +++++++++++++++++++++ src/raft/raft.c | 29 +++++++++++++++++++++++++++++ src/server.c | 19 +++++++++++++++++++ 4 files changed, 92 insertions(+) diff --git a/include/dqlite.h b/include/dqlite.h index f064f4836..d0dbe153f 100644 --- a/include/dqlite.h +++ b/include/dqlite.h @@ -586,6 +586,29 @@ DQLITE_API int dqlite_node_recover_ext(dqlite_node *n, dqlite_node_info_ext infos[], int n_info); +/** + * Retrieve information about the last persisted raft log entry. + * + * This is intended to be used in combination with dqlite_node_recover_ext, to + * determine which of the surviving nodes in a cluster is most up-to-date. The + * raft rules for this are: + * + * - If the two logs have last entries with different terms, the log with the + * higher term is more up-to-date. + * - Otherwise, the longer log is more up-to-date. + * + * Note that this function may result in physically modifying the raft-related + * files in the data directory. These modifications do not affect the logical + * state of the node. Deletion of invalid segment files can be disabled with + * dqlite_node_set_auto_recovery. + * + * This should be called after dqlite_node_init, but the node must not be + * running. + */ +DQLITE_API int dqlite_node_describe_last_entry(dqlite_node *n, + uint64_t *last_entry_index, + uint64_t *last_entry_term); + /** * Return a human-readable description of the last error occurred. */ diff --git a/src/raft.h b/src/raft.h index fce3f5518..2c9d1c265 100644 --- a/src/raft.h +++ b/src/raft.h @@ -1061,6 +1061,27 @@ RAFT_API int raft_bootstrap(struct raft *r, RAFT_API int raft_recover(struct raft *r, const struct raft_configuration *conf); +/** + * Read information about the last raft log entry that's stored on disk. + * + * "Last log entry" here should be understood as including snapshots, + * so if there is one snapshot on disk and no individual entries, the + * values returned in `index` and `term` are the index and term of the + * last entry included in the snapshot. + * + * This function is just a wrapper around the `load` method of raft_io. + * Note that the `load` method of the uv raft_io implementation is not + * read-only: as it walks the segment files on disk, it closes open + * segments that contain valid entries and deletes other open segments. + * + * This should be called after the raft_io instance is initialized (e.g. + * after calling raft_uv_init), but no active raft node should be using + * the instance. + */ +int raft_io_describe_last_entry(struct raft_io *io, + raft_index *index, + raft_term *term); + RAFT_API int raft_start(struct raft *r); /** diff --git a/src/raft/raft.c b/src/raft/raft.c index ce3996b90..29bf467e7 100644 --- a/src/raft/raft.c +++ b/src/raft/raft.c @@ -10,11 +10,13 @@ #include "configuration.h" #include "convert.h" #include "election.h" +#include "entry.h" #include "err.h" #include "flags.h" #include "heap.h" #include "log.h" #include "membership.h" +#include "snapshot.h" #define DEFAULT_ELECTION_TIMEOUT 1000 /* One second */ #define DEFAULT_HEARTBEAT_TIMEOUT 100 /* One tenth of a second */ @@ -303,3 +305,30 @@ static int ioFsmVersionCheck(struct raft *r, return 0; } + +int raft_io_describe_last_entry(struct raft_io *io, + raft_index *index, + raft_term *term) +{ + raft_term current_term; + raft_id voted_for; + struct raft_snapshot *snapshot; + raft_index start_index; + struct raft_entry *entries; + size_t n_entries; + int rv; + + rv = io->load(io, ¤t_term, &voted_for, &snapshot, + &start_index, &entries, &n_entries); + if (rv != 0) { + return rv; + } + POST(ERGO(n_entries == 0, snapshot != NULL)); + *index = start_index + n_entries - 1; + *term = n_entries > 0 ? entries[n_entries - 1].term : snapshot->term; + if (snapshot != NULL) { + snapshotDestroy(snapshot); + } + entryBatchesDestroy(entries, n_entries); + return 0; +} diff --git a/src/server.c b/src/server.c index 7e44b1c49..713e8e139 100644 --- a/src/server.c +++ b/src/server.c @@ -1065,6 +1065,25 @@ int dqlite_node_recover_ext(dqlite_node *n, return rv; } +int dqlite_node_describe_last_entry(dqlite_node *n, + uint64_t *index, + uint64_t *term) +{ + static_assert(sizeof(*index) == sizeof(raft_index), + "unexpected index type size"); + raft_index *i = (raft_index *)index; + static_assert(sizeof(*term) == sizeof(raft_term), + "unexpected term type size"); + raft_term *t = (raft_term *)term; + int rv; + + rv = raft_io_describe_last_entry(&n->raft_io, i, t); + if (rv != 0) { + return DQLITE_ERROR; + } + return 0; +} + dqlite_node_id dqlite_generate_node_id(const char *address) { tracef("generate node id"); From 8b65b3e77ed657342bfdaabb328a17324cbcc203 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Tue, 13 Aug 2024 23:26:28 -0400 Subject: [PATCH 02/10] Incorporate new dqlite API in an existing test Signed-off-by: Cole Miller --- test/integration/test_cluster.c | 24 +++++++++++++++++++++++- test/lib/server.c | 13 ++++++++++++- test/lib/server.h | 8 +++++++- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/test/integration/test_cluster.c b/test/integration/test_cluster.c index 57f605cff..bd507f17e 100644 --- a/test/integration/test_cluster.c +++ b/test/integration/test_cluster.c @@ -98,6 +98,7 @@ TEST(cluster, restart, setUp, tearDown, 0, cluster_params) long n_records = strtol(munit_parameters_get(params, "num_records"), NULL, 0); char sql[128]; + int rv; HANDSHAKE; OPEN; @@ -112,7 +113,28 @@ TEST(cluster, restart, setUp, tearDown, 0, cluster_params) struct test_server *server = &f->servers[0]; test_server_stop(server); - test_server_start(server, params); + + test_server_prepare(server, params); + uint64_t last_entry_index; + uint64_t last_entry_term; + rv = dqlite_node_describe_last_entry(server->dqlite, + &last_entry_index, + &last_entry_term); + munit_assert_int(rv, ==, 0); + /* When we've inserted 0 records, there are two entries: + * the initial configuration and the CREATE TABLE transaction. + * + * At 993 records, the leader has generated one legacy checkpoint + * command entry. + * + * At 2200 records, the leader has generated a second checkpoint + * command, plus two barrier entries, one for each snapshot. */ + size_t fudge = n_records >= 2200 ? 6 : + n_records >= 993 ? 3 : + 2; + munit_assert_ullong(last_entry_index, ==, n_records + fudge); + munit_assert_ullong(last_entry_term, ==, 1); + test_server_run(server); /* The table is visible after restart. */ HANDSHAKE; diff --git a/test/lib/server.c b/test/lib/server.c index 89d4060cb..1bfdb6b98 100644 --- a/test/lib/server.c +++ b/test/lib/server.c @@ -58,7 +58,7 @@ void test_server_tear_down(struct test_server *s) test_dir_tear_down(s->dir); } -void test_server_start(struct test_server *s, const MunitParameter params[]) +void test_server_prepare(struct test_server *s, const MunitParameter params[]) { int rv; @@ -128,6 +128,11 @@ void test_server_start(struct test_server *s, const MunitParameter params[]) munit_assert_int(rv, ==, 0); } } +} + +void test_server_run(struct test_server *s) +{ + int rv; rv = dqlite_node_start(s->dqlite); munit_assert_int(rv, ==, 0); @@ -135,6 +140,12 @@ void test_server_start(struct test_server *s, const MunitParameter params[]) test_server_client_connect(s, &s->client); } +void test_server_start(struct test_server *s, const MunitParameter params[]) +{ + test_server_prepare(s, params); + test_server_run(s); +} + struct client_proto *test_server_client(struct test_server *s) { return &s->client; diff --git a/test/lib/server.h b/test/lib/server.h index 7e0dc111b..8b559108e 100644 --- a/test/lib/server.h +++ b/test/lib/server.h @@ -35,7 +35,13 @@ void test_server_setup(struct test_server *s, /* Cleanup the test server. */ void test_server_tear_down(struct test_server *s); -/* Start the test server. */ +/* Set up the test server without running it. */ +void test_server_prepare(struct test_server *s, const MunitParameter params[]); + +/* Run the test server after setting it up. */ +void test_server_run(struct test_server *s); + +/* Start the test server. Equivalent to test_server_prepare + test_server_run. */ void test_server_start(struct test_server *s, const MunitParameter params[]); /* Stop the test server. */ From d7ad1aa171d4c53bfbdce4373a6d6bb1105fb7a4 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Sun, 18 Aug 2024 13:14:29 -0400 Subject: [PATCH 03/10] Update test to make the number of barriers more deterministic Signed-off-by: Cole Miller --- test/integration/test_cluster.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/test/integration/test_cluster.c b/test/integration/test_cluster.c index bd507f17e..0007dd563 100644 --- a/test/integration/test_cluster.c +++ b/test/integration/test_cluster.c @@ -97,7 +97,6 @@ TEST(cluster, restart, setUp, tearDown, 0, cluster_params) struct rows rows; long n_records = strtol(munit_parameters_get(params, "num_records"), NULL, 0); - char sql[128]; int rv; HANDSHAKE; @@ -105,10 +104,14 @@ TEST(cluster, restart, setUp, tearDown, 0, cluster_params) PREPARE("CREATE TABLE test (n INT)", &stmt_id); EXEC(stmt_id, &last_insert_id, &rows_affected); + PREPARE("INSERT INTO TEST(n) VALUES(?)", &stmt_id); for (int i = 0; i < n_records; ++i) { - sprintf(sql, "INSERT INTO test(n) VALUES(%d)", i + 1); - PREPARE(sql, &stmt_id); - EXEC(stmt_id, &last_insert_id, &rows_affected); + struct value val = {.type = SQLITE_INTEGER, .integer = i}; + rv = clientSendExec(f->client, stmt_id, &val, 1, NULL); + munit_assert_int(rv, ==, 0); + rv = clientRecvResult(f->client, &last_insert_id, + &rows_affected, NULL); + munit_assert_int(rv, ==, 0); } struct test_server *server = &f->servers[0]; @@ -128,7 +131,8 @@ TEST(cluster, restart, setUp, tearDown, 0, cluster_params) * command entry. * * At 2200 records, the leader has generated a second checkpoint - * command, plus two barrier entries, one for each snapshot. */ + * command, plus two barriers. (The number of barriers is nondeterministic; + * but we seem to get two of them pretty consistently in this test.) */ size_t fudge = n_records >= 2200 ? 6 : n_records >= 993 ? 3 : 2; From 7875f3ecc384359208a796a9f9d4b07d34944f14 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 22 Aug 2024 22:30:36 -0400 Subject: [PATCH 04/10] Address review comments Signed-off-by: Cole Miller --- src/server.c | 6 ++---- test/integration/test_cluster.c | 9 +++++---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/server.c b/src/server.c index 713e8e139..25383996e 100644 --- a/src/server.c +++ b/src/server.c @@ -1069,6 +1069,7 @@ int dqlite_node_describe_last_entry(dqlite_node *n, uint64_t *index, uint64_t *term) { + PRE(n->initialized && !n->running); static_assert(sizeof(*index) == sizeof(raft_index), "unexpected index type size"); raft_index *i = (raft_index *)index; @@ -1078,10 +1079,7 @@ int dqlite_node_describe_last_entry(dqlite_node *n, int rv; rv = raft_io_describe_last_entry(&n->raft_io, i, t); - if (rv != 0) { - return DQLITE_ERROR; - } - return 0; + return rv == 0 ? 0 : DQLITE_ERROR; } dqlite_node_id dqlite_generate_node_id(const char *address) diff --git a/test/integration/test_cluster.c b/test/integration/test_cluster.c index 0007dd563..214d77010 100644 --- a/test/integration/test_cluster.c +++ b/test/integration/test_cluster.c @@ -131,12 +131,13 @@ TEST(cluster, restart, setUp, tearDown, 0, cluster_params) * command entry. * * At 2200 records, the leader has generated a second checkpoint - * command, plus two barriers. (The number of barriers is nondeterministic; - * but we seem to get two of them pretty consistently in this test.) */ - size_t fudge = n_records >= 2200 ? 6 : + * command. */ + size_t extra = n_records >= 2200 ? 4 : n_records >= 993 ? 3 : 2; - munit_assert_ullong(last_entry_index, ==, n_records + fudge); + /* This assertion isn't exact because we expect to see some barrier + * log entries as well, and the number of these is not deterministic. */ + munit_assert_ullong(last_entry_index, >=, n_records + extra); munit_assert_ullong(last_entry_term, ==, 1); test_server_run(server); From 144fd9f18ac1f5c6389f4db94e712d4c80960f4f Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 22 Aug 2024 23:56:48 -0400 Subject: [PATCH 05/10] Update cluster test following review Signed-off-by: Cole Miller --- test/integration/test_cluster.c | 79 ++++++++++++++++++--------------- test/lib/client.h | 12 +++++ 2 files changed, 56 insertions(+), 35 deletions(-) diff --git a/test/integration/test_cluster.c b/test/integration/test_cluster.c index 214d77010..61878960a 100644 --- a/test/integration/test_cluster.c +++ b/test/integration/test_cluster.c @@ -97,7 +97,6 @@ TEST(cluster, restart, setUp, tearDown, 0, cluster_params) struct rows rows; long n_records = strtol(munit_parameters_get(params, "num_records"), NULL, 0); - int rv; HANDSHAKE; OPEN; @@ -106,40 +105,13 @@ TEST(cluster, restart, setUp, tearDown, 0, cluster_params) PREPARE("INSERT INTO TEST(n) VALUES(?)", &stmt_id); for (int i = 0; i < n_records; ++i) { - struct value val = {.type = SQLITE_INTEGER, .integer = i}; - rv = clientSendExec(f->client, stmt_id, &val, 1, NULL); - munit_assert_int(rv, ==, 0); - rv = clientRecvResult(f->client, &last_insert_id, - &rows_affected, NULL); - munit_assert_int(rv, ==, 0); + EXEC_PARAMS(stmt_id, &last_insert_id, &rows_affected, + {.type = SQLITE_INTEGER, .integer = i}); } struct test_server *server = &f->servers[0]; test_server_stop(server); - - test_server_prepare(server, params); - uint64_t last_entry_index; - uint64_t last_entry_term; - rv = dqlite_node_describe_last_entry(server->dqlite, - &last_entry_index, - &last_entry_term); - munit_assert_int(rv, ==, 0); - /* When we've inserted 0 records, there are two entries: - * the initial configuration and the CREATE TABLE transaction. - * - * At 993 records, the leader has generated one legacy checkpoint - * command entry. - * - * At 2200 records, the leader has generated a second checkpoint - * command. */ - size_t extra = n_records >= 2200 ? 4 : - n_records >= 993 ? 3 : - 2; - /* This assertion isn't exact because we expect to see some barrier - * log entries as well, and the number of these is not deterministic. */ - munit_assert_ullong(last_entry_index, >=, n_records + extra); - munit_assert_ullong(last_entry_term, ==, 1); - test_server_run(server); + test_server_start(server, params); /* The table is visible after restart. */ HANDSHAKE; @@ -160,19 +132,19 @@ TEST(cluster, dataOnNewNode, setUp, tearDown, 0, cluster_params) struct rows rows; long n_records = strtol(munit_parameters_get(params, "num_records"), NULL, 0); - char sql[128]; unsigned id = 2; const char *address = "@2"; + int rv; HANDSHAKE; OPEN; PREPARE("CREATE TABLE test (n INT)", &stmt_id); EXEC(stmt_id, &last_insert_id, &rows_affected); + PREPARE("INSERT INTO test(n) VALUES(?)", &stmt_id); for (int i = 0; i < n_records; ++i) { - sprintf(sql, "INSERT INTO test(n) VALUES(%d)", i + 1); - PREPARE(sql, &stmt_id); - EXEC(stmt_id, &last_insert_id, &rows_affected); + EXEC_PARAMS(stmt_id, &last_insert_id, &rows_affected, + {.type = SQLITE_INTEGER, .integer = i}); } /* Add a second voting server, this one will receive all data from the @@ -185,6 +157,27 @@ TEST(cluster, dataOnNewNode, setUp, tearDown, 0, cluster_params) REMOVE(1); sleep(1); + struct test_server *first = &f->servers[0]; + test_server_stop(first); + test_server_prepare(first, params); + /* One entry per INSERT, plus one for the initial configuration, plus + * one for the CREATE TABLE, plus one legacy checkpoint command entry + * after 993 records or two after 2200 records. */ + size_t extra = n_records >= 2200 ? 4 : + n_records >= 993 ? 3 : + 2; + uint64_t last_entry_index; + uint64_t last_entry_term; + rv = dqlite_node_describe_last_entry(first->dqlite, + &last_entry_index, + &last_entry_term); + munit_assert_int(rv, ==, 0); + /* This assertion is not tight because the the leader also generates + * a nondeterministic number of barrier entries. */ + munit_assert_ullong(last_entry_index, >=, n_records + extra); + munit_assert_ullong(last_entry_term, ==, 1); + test_server_run(first); + /* The full table is visible from the new node */ SELECT(2); HANDSHAKE; @@ -193,6 +186,22 @@ TEST(cluster, dataOnNewNode, setUp, tearDown, 0, cluster_params) QUERY(stmt_id, &rows); munit_assert_long(rows.next->values->integer, ==, n_records); clientCloseRows(&rows); + + /* One more entry on the new node. */ + PREPARE("INSERT INTO test(n) VALUES(?)", &stmt_id); + EXEC_PARAMS(stmt_id, &last_insert_id, &rows_affected, + {.type = SQLITE_INTEGER, .integer = 5000}); + + struct test_server *second = &f->servers[1]; + test_server_stop(second); + test_server_prepare(second, params); + rv = dqlite_node_describe_last_entry(second->dqlite, + &last_entry_index, + &last_entry_term); + munit_assert_int(rv, ==, 0); + munit_assert_ullong(last_entry_index, >=, n_records + extra + 1); + munit_assert_ullong(last_entry_term, ==, 1); + test_server_run(second); return MUNIT_OK; } diff --git a/test/lib/client.h b/test/lib/client.h index daab3affa..a42bd8ed6 100644 --- a/test/lib/client.h +++ b/test/lib/client.h @@ -140,6 +140,18 @@ munit_assert_int(rv_, ==, 0); \ } +#define EXEC_PARAMS(STMT_ID, LAST_INSERT_ID, ROWS_AFFECTED, ...) \ + { \ + int rv_; \ + struct value vals_[] = {__VA_ARGS__}; \ + size_t len_ = sizeof(vals_) / sizeof(vals_[0]); \ + rv_ = clientSendExec(f->client, STMT_ID, vals_, len_, NULL); \ + munit_assert_int(rv_, ==, 0); \ + rv_ = clientRecvResult(f->client, LAST_INSERT_ID, \ + ROWS_AFFECTED, NULL); \ + munit_assert_int(rv_, ==, 0); \ + } + #define EXEC_SQL(SQL, LAST_INSERT_ID, ROWS_AFFECTED) \ { \ int rv_; \ From 8f012e7547805555ef6637dc4176e04b05cf7e31 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Fri, 23 Aug 2024 01:17:42 -0400 Subject: [PATCH 06/10] Rectify, document, and test edge case of describe_last_entry We should gracefully handle the case where there are no snapshots and no entries on disk. Signed-off-by: Cole Miller --- src/raft.h | 3 ++- src/raft/raft.c | 5 +++-- test/integration/test_cluster.c | 35 +++++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/src/raft.h b/src/raft.h index 2c9d1c265..76bea3500 100644 --- a/src/raft.h +++ b/src/raft.h @@ -1067,7 +1067,8 @@ RAFT_API int raft_recover(struct raft *r, * "Last log entry" here should be understood as including snapshots, * so if there is one snapshot on disk and no individual entries, the * values returned in `index` and `term` are the index and term of the - * last entry included in the snapshot. + * last entry included in the snapshot. If there are no snapshot and no + * entries, then `index` and `term` are both set to 0. * * This function is just a wrapper around the `load` method of raft_io. * Note that the `load` method of the uv raft_io implementation is not diff --git a/src/raft/raft.c b/src/raft/raft.c index 29bf467e7..e98927b4e 100644 --- a/src/raft/raft.c +++ b/src/raft/raft.c @@ -323,9 +323,10 @@ int raft_io_describe_last_entry(struct raft_io *io, if (rv != 0) { return rv; } - POST(ERGO(n_entries == 0, snapshot != NULL)); *index = start_index + n_entries - 1; - *term = n_entries > 0 ? entries[n_entries - 1].term : snapshot->term; + *term = n_entries > 0 ? entries[n_entries - 1].term : + snapshot != NULL ? snapshot->term : + 0; if (snapshot != NULL) { snapshotDestroy(snapshot); } diff --git a/test/integration/test_cluster.c b/test/integration/test_cluster.c index 61878960a..4ba9198b8 100644 --- a/test/integration/test_cluster.c +++ b/test/integration/test_cluster.c @@ -324,3 +324,38 @@ TEST(cluster, modifyingQuerySql, setUp, tearDown, 0, cluster_params) clientCloseRows(&rows); return MUNIT_OK; } + +/* Edge cases for dqlite_node_describe_last_entry. */ +TEST(cluster, last_entry_edge_cases, setUp, tearDown, 0, NULL) +{ + struct fixture *f = data; + uint64_t index; + uint64_t term; + int rv; + + sleep(1); + + struct test_server *first = &f->servers[0]; + test_server_stop(first); + test_server_prepare(first, params); + rv = dqlite_node_describe_last_entry(first->dqlite, &index, &term); + munit_assert_int(rv, ==, 0); + /* The log contains only the bootstrap configuration. */ + munit_assert_ullong(index, ==, 1); + /* The bootstrap configuration is always tagged with term 1. */ + munit_assert_ullong(term, ==, 1); + test_server_run(first); + + struct test_server *second = &f->servers[1]; + test_server_stop(second); + test_server_prepare(second, params); + rv = dqlite_node_describe_last_entry(second->dqlite, &index, &term); + munit_assert_int(rv, ==, 0); + /* We didn't bootstrap and haven't joined the leader, so our log is + * empty. */ + munit_assert_ullong(index, ==, 0); + munit_assert_ullong(term, ==, 0); + test_server_run(second); + + return MUNIT_OK; +} From 0f6d5adf18dc9c0d038ecfc1a6bde832235aaabf Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Mon, 26 Aug 2024 11:09:08 -0400 Subject: [PATCH 07/10] Address review comment Signed-off-by: Cole Miller --- test/integration/test_cluster.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/test/integration/test_cluster.c b/test/integration/test_cluster.c index 4ba9198b8..245aa652c 100644 --- a/test/integration/test_cluster.c +++ b/test/integration/test_cluster.c @@ -163,19 +163,21 @@ TEST(cluster, dataOnNewNode, setUp, tearDown, 0, cluster_params) /* One entry per INSERT, plus one for the initial configuration, plus * one for the CREATE TABLE, plus one legacy checkpoint command entry * after 993 records or two after 2200 records. */ - size_t extra = n_records >= 2200 ? 4 : - n_records >= 993 ? 3 : - 2; + uint64_t expected_entries = n_records + (n_records >= 2200 ? 4 : + n_records >= 993 ? 3 : + 2); + /* We also expect a variable number of barrier entries. Just specify an + * upper bound since we don't know the exact count. */ + uint64_t max_barriers = 10; uint64_t last_entry_index; uint64_t last_entry_term; rv = dqlite_node_describe_last_entry(first->dqlite, &last_entry_index, &last_entry_term); munit_assert_int(rv, ==, 0); - /* This assertion is not tight because the the leader also generates - * a nondeterministic number of barrier entries. */ - munit_assert_ullong(last_entry_index, >=, n_records + extra); - munit_assert_ullong(last_entry_term, ==, 1); + munit_assert_uint64(expected_entries, <=, last_entry_index); + munit_assert_uint64(last_entry_index, <, expected_entries + max_barriers); + munit_assert_uint64(last_entry_term, ==, 1); test_server_run(first); /* The full table is visible from the new node */ @@ -199,8 +201,9 @@ TEST(cluster, dataOnNewNode, setUp, tearDown, 0, cluster_params) &last_entry_index, &last_entry_term); munit_assert_int(rv, ==, 0); - munit_assert_ullong(last_entry_index, >=, n_records + extra + 1); - munit_assert_ullong(last_entry_term, ==, 1); + munit_assert_uint64(expected_entries + 1, <=, last_entry_index); + munit_assert_uint64(last_entry_index, <, expected_entries + max_barriers + 1); + munit_assert_uint64(last_entry_term, ==, 1); test_server_run(second); return MUNIT_OK; } From 881bc4c098c016063eefc2e91eb89884d6a4069b Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Mon, 26 Aug 2024 11:11:47 -0400 Subject: [PATCH 08/10] Update new edge case test too Signed-off-by: Cole Miller --- test/integration/test_cluster.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/integration/test_cluster.c b/test/integration/test_cluster.c index 245aa652c..ea8f9d6bb 100644 --- a/test/integration/test_cluster.c +++ b/test/integration/test_cluster.c @@ -344,9 +344,9 @@ TEST(cluster, last_entry_edge_cases, setUp, tearDown, 0, NULL) rv = dqlite_node_describe_last_entry(first->dqlite, &index, &term); munit_assert_int(rv, ==, 0); /* The log contains only the bootstrap configuration. */ - munit_assert_ullong(index, ==, 1); + munit_assert_uint64(index, ==, 1); /* The bootstrap configuration is always tagged with term 1. */ - munit_assert_ullong(term, ==, 1); + munit_assert_uint64(term, ==, 1); test_server_run(first); struct test_server *second = &f->servers[1]; @@ -356,8 +356,8 @@ TEST(cluster, last_entry_edge_cases, setUp, tearDown, 0, NULL) munit_assert_int(rv, ==, 0); /* We didn't bootstrap and haven't joined the leader, so our log is * empty. */ - munit_assert_ullong(index, ==, 0); - munit_assert_ullong(term, ==, 0); + munit_assert_uint64(index, ==, 0); + munit_assert_uint64(term, ==, 0); test_server_run(second); return MUNIT_OK; From 642ad9a2bbd43d61e4db5ba497f62fcbba4906c8 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Mon, 26 Aug 2024 11:22:06 -0400 Subject: [PATCH 09/10] Add a shim for external raft build Signed-off-by: Cole Miller --- src/server.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/server.c b/src/server.c index 25383996e..6a29e80c3 100644 --- a/src/server.c +++ b/src/server.c @@ -1078,8 +1078,15 @@ int dqlite_node_describe_last_entry(dqlite_node *n, raft_term *t = (raft_term *)term; int rv; +#ifdef USE_SYSTEM_RAFT + (void)i; + (void)t; + (void)rv; + return DQLITE_ERROR; +#else rv = raft_io_describe_last_entry(&n->raft_io, i, t); return rv == 0 ? 0 : DQLITE_ERROR; +#endif } dqlite_node_id dqlite_generate_node_id(const char *address) From 057725e45fd7beb6c98b600609c87480efc47d69 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Mon, 26 Aug 2024 11:34:34 -0400 Subject: [PATCH 10/10] Guard tests that rely on new API Signed-off-by: Cole Miller --- test/integration/test_cluster.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/integration/test_cluster.c b/test/integration/test_cluster.c index ea8f9d6bb..131be5132 100644 --- a/test/integration/test_cluster.c +++ b/test/integration/test_cluster.c @@ -134,7 +134,6 @@ TEST(cluster, dataOnNewNode, setUp, tearDown, 0, cluster_params) strtol(munit_parameters_get(params, "num_records"), NULL, 0); unsigned id = 2; const char *address = "@2"; - int rv; HANDSHAKE; OPEN; @@ -160,6 +159,8 @@ TEST(cluster, dataOnNewNode, setUp, tearDown, 0, cluster_params) struct test_server *first = &f->servers[0]; test_server_stop(first); test_server_prepare(first, params); +#ifndef USE_SYSTEM_RAFT + int rv; /* One entry per INSERT, plus one for the initial configuration, plus * one for the CREATE TABLE, plus one legacy checkpoint command entry * after 993 records or two after 2200 records. */ @@ -178,6 +179,7 @@ TEST(cluster, dataOnNewNode, setUp, tearDown, 0, cluster_params) munit_assert_uint64(expected_entries, <=, last_entry_index); munit_assert_uint64(last_entry_index, <, expected_entries + max_barriers); munit_assert_uint64(last_entry_term, ==, 1); +#endif test_server_run(first); /* The full table is visible from the new node */ @@ -197,6 +199,7 @@ TEST(cluster, dataOnNewNode, setUp, tearDown, 0, cluster_params) struct test_server *second = &f->servers[1]; test_server_stop(second); test_server_prepare(second, params); +#ifndef USE_SYSTEM_RAFT rv = dqlite_node_describe_last_entry(second->dqlite, &last_entry_index, &last_entry_term); @@ -204,6 +207,7 @@ TEST(cluster, dataOnNewNode, setUp, tearDown, 0, cluster_params) munit_assert_uint64(expected_entries + 1, <=, last_entry_index); munit_assert_uint64(last_entry_index, <, expected_entries + max_barriers + 1); munit_assert_uint64(last_entry_term, ==, 1); +#endif test_server_run(second); return MUNIT_OK; } @@ -328,6 +332,8 @@ TEST(cluster, modifyingQuerySql, setUp, tearDown, 0, cluster_params) return MUNIT_OK; } +#ifndef USE_SYSTEM_RAFT + /* Edge cases for dqlite_node_describe_last_entry. */ TEST(cluster, last_entry_edge_cases, setUp, tearDown, 0, NULL) { @@ -362,3 +368,5 @@ TEST(cluster, last_entry_edge_cases, setUp, tearDown, 0, NULL) return MUNIT_OK; } + +#endif