From 21156061ddbb6c4ef778e1017eff0e764f02d7e6 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Fri, 23 Aug 2024 01:17:42 -0400 Subject: [PATCH] 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 96cd0d72f..e14e59cac 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; +}