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 <[email protected]>
canonical · Aug 23, 2024 · 8f012e7 · 8f012e7
1 parent 144fd9f
commit 8f012e7
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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;
+}