From 06230ec89345ab03b329e56cd30a59fe3551e93c Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Mon, 29 Apr 2024 10:39:18 +0100 Subject: [PATCH 1/7] Restore raft testing on assorted filesystems Signed-off-by: Cole Miller --- .github/workflows/build-and-test.yml | 7 ++++++- configure.ac | 8 ++++++++ test/raft/lib/fs.sh | 2 +- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index e3453858e..5dc7fd7e5 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -26,7 +26,9 @@ jobs: - name: Set up dependencies run: | sudo apt update - sudo apt install -y lcov libsqlite3-dev liblz4-dev libuv1-dev + sudo apt install -y libsqlite3-dev liblz4-dev libuv1-dev \ + linux-libc-dev btrfs-progs xfsprogs zfsutils-linux \ + lcov - name: Build dqlite env: @@ -47,7 +49,10 @@ jobs: CC: ${{ matrix.compiler }} LIBDQLITE_TRACE: 1 run: | + ./test/raft/lib/fs.sh setup + export $(test/raft/lib/fs.sh detect) make check || (cat ./test-suite.log && false) + ./test/raft/lib/fs.sh teardown - name: Coverage env: diff --git a/configure.ac b/configure.ac index 4ef34b5b0..af45a0a0b 100644 --- a/configure.ac +++ b/configure.ac @@ -50,6 +50,14 @@ AC_CHECK_HEADERS([linux/io_uring.h linux/aio_abi.h]) # Checks for library functions and definitions. AC_CHECK_DECLS(RWF_NOWAIT, [], [AC_MSG_ERROR(Linux kernel >= 4.14 required.)], [#include ]) +# Check if zfs >= 0.8.0 is available (for direct I/O support). +AC_CHECK_PROG(have_zfs, zfs, yes) +AS_IF([test x"$have_zfs" = x"yes"], + [AX_COMPARE_VERSION($(cat /sys/module/zfs/version | cut -f 1 -d -), [ge], [0.8.0], + [AC_DEFINE(RAFT_HAVE_ZFS_WITH_DIRECT_IO)], []) + ], + []) + # Enable large file support. This is mandatory in order to interoperate with # libuv, which enables large file support by default, making the size of 'off_t' # on 32-bit architecture be 8 bytes instead of the normal 4. diff --git a/test/raft/lib/fs.sh b/test/raft/lib/fs.sh index 638eeda91..c6b101527 100755 --- a/test/raft/lib/fs.sh +++ b/test/raft/lib/fs.sh @@ -43,7 +43,7 @@ fi if [ "${cmd}" = "detect" ]; then vars="" for type in $types; do - vars="${vars}RAFT_TMP_$(echo ${type} | tr [a-z] [A-Z])=./tmp/${type} " + vars="${vars}RAFT_TMP_$(echo ${type} | tr a-z A-Z)=./tmp/${type} " done echo $vars exit 0 From c104c0a485cc8b6aee7febc1f1bb90395cd9173d Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Mon, 29 Apr 2024 10:39:44 +0100 Subject: [PATCH 2/7] Enable tracing for raft tests Signed-off-by: Cole Miller --- Makefile.am | 1 + test/raft/lib/runner.h | 2 ++ 2 files changed, 3 insertions(+) diff --git a/Makefile.am b/Makefile.am index 6e3e5c345..c14e564a7 100644 --- a/Makefile.am +++ b/Makefile.am @@ -282,6 +282,7 @@ raft_core_fuzzy_test_LDFLAGS = -no-install raft_core_fuzzy_test_LDADD = libtest.la libraft.la raft_uv_unit_test_SOURCES = \ + src/tracing.c \ src/raft/err.c \ src/raft/heap.c \ src/raft/syscall.c \ diff --git a/test/raft/lib/runner.h b/test/raft/lib/runner.h index 13244a33a..3c4c22207 100644 --- a/test/raft/lib/runner.h +++ b/test/raft/lib/runner.h @@ -4,6 +4,7 @@ #define TEST_RUNNER_H_ #include "munit.h" +#include "../../../src/tracing.h" /* Top-level suites array declaration. * @@ -24,6 +25,7 @@ extern int _main_suites_n; int main(int argc, char *argv[MUNIT_ARRAY_PARAM(argc)]) \ { \ MunitSuite suite = {(char *)"", NULL, _main_suites, 1, 0}; \ + dqliteTracingMaybeEnable(true); \ return munit_suite_main(&suite, (void *)NAME, argc, argv); \ } From 042f8ad27b7f870f4f97416a59f5737cc43f363e Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Mon, 29 Apr 2024 11:20:39 +0100 Subject: [PATCH 3/7] Remove `#undef tracef` everywhere Signed-off-by: Cole Miller --- src/raft/client.c | 1 - src/raft/configuration.c | 1 - src/raft/convert.c | 1 - src/raft/election.c | 1 - src/raft/progress.c | 1 - src/raft/recv.c | 1 - src/raft/recv_append_entries.c | 1 - src/raft/recv_append_entries_result.c | 1 - src/raft/recv_install_snapshot.c | 1 - src/raft/recv_request_vote.c | 1 - src/raft/recv_request_vote_result.c | 1 - src/raft/recv_timeout_now.c | 1 - src/raft/replication.c | 1 - src/raft/snapshot.c | 1 - src/raft/start.c | 1 - src/raft/tick.c | 1 - src/raft/uv.c | 1 - src/raft/uv_finalize.c | 1 - src/raft/uv_list.c | 1 - src/raft/uv_prepare.c | 1 - src/raft/uv_recv.c | 1 - src/raft/uv_segment.c | 1 - src/raft/uv_send.c | 1 - src/raft/uv_snapshot.c | 1 - src/raft/uv_truncate.c | 1 - src/raft/uv_work.c | 1 - 26 files changed, 26 deletions(-) diff --git a/src/raft/client.c b/src/raft/client.c index 3a7173049..7f8e4bd7f 100644 --- a/src/raft/client.c +++ b/src/raft/client.c @@ -452,4 +452,3 @@ int raft_transfer(struct raft *r, return rv; } -#undef tracef diff --git a/src/raft/configuration.c b/src/raft/configuration.c index 04ca13764..7f104ea6a 100644 --- a/src/raft/configuration.c +++ b/src/raft/configuration.c @@ -398,4 +398,3 @@ void configurationTrace(const struct raft *r, } tracef("=== CONFIG END ==="); } -#undef tracef diff --git a/src/raft/convert.c b/src/raft/convert.c index e29adc5ee..28a0b0cea 100644 --- a/src/raft/convert.c +++ b/src/raft/convert.c @@ -268,4 +268,3 @@ void convertToUnavailable(struct raft *r) convertSetState(r, RAFT_UNAVAILABLE); } -#undef tracef diff --git a/src/raft/election.c b/src/raft/election.c index ecdcd20f0..62815b408 100644 --- a/src/raft/election.c +++ b/src/raft/election.c @@ -324,4 +324,3 @@ bool electionTally(struct raft *r, size_t voter_index) return votes >= half + 1; } -#undef tracef diff --git a/src/raft/progress.c b/src/raft/progress.c index 696134c70..fce6b40c8 100644 --- a/src/raft/progress.c +++ b/src/raft/progress.c @@ -322,4 +322,3 @@ bool progressSnapshotDone(struct raft *r, const unsigned i) return p->match_index >= p->snapshot_index; } -#undef tracef diff --git a/src/raft/recv.c b/src/raft/recv.c index 5f0da1723..de19bf858 100644 --- a/src/raft/recv.c +++ b/src/raft/recv.c @@ -222,4 +222,3 @@ int recvUpdateLeader(struct raft *r, const raft_id id, const char *address) return 0; } -#undef tracef diff --git a/src/raft/recv_append_entries.c b/src/raft/recv_append_entries.c index 7d4adbcc0..5291b2b37 100644 --- a/src/raft/recv_append_entries.c +++ b/src/raft/recv_append_entries.c @@ -164,4 +164,3 @@ int recvAppendEntries(struct raft *r, return 0; } -#undef tracef diff --git a/src/raft/recv_append_entries_result.c b/src/raft/recv_append_entries_result.c index ddef54f14..d044cf7c1 100644 --- a/src/raft/recv_append_entries_result.c +++ b/src/raft/recv_append_entries_result.c @@ -72,4 +72,3 @@ int recvAppendEntriesResult(struct raft *r, return 0; } -#undef tracef diff --git a/src/raft/recv_install_snapshot.c b/src/raft/recv_install_snapshot.c index d3e1493a2..96c8e6150 100644 --- a/src/raft/recv_install_snapshot.c +++ b/src/raft/recv_install_snapshot.c @@ -106,4 +106,3 @@ int recvInstallSnapshot(struct raft *r, return 0; } -#undef tracef diff --git a/src/raft/recv_request_vote.c b/src/raft/recv_request_vote.c index f51742869..7f16370a0 100644 --- a/src/raft/recv_request_vote.c +++ b/src/raft/recv_request_vote.c @@ -147,4 +147,3 @@ int recvRequestVote(struct raft *r, return 0; } -#undef tracef diff --git a/src/raft/recv_request_vote_result.c b/src/raft/recv_request_vote_result.c index ca7ece487..6acbdb004 100644 --- a/src/raft/recv_request_vote_result.c +++ b/src/raft/recv_request_vote_result.c @@ -151,4 +151,3 @@ int recvRequestVoteResult(struct raft *r, return 0; } -#undef tracef diff --git a/src/raft/recv_timeout_now.c b/src/raft/recv_timeout_now.c index c503c7600..c2097d4dc 100644 --- a/src/raft/recv_timeout_now.c +++ b/src/raft/recv_timeout_now.c @@ -78,4 +78,3 @@ int recvTimeoutNow(struct raft *r, return 0; } -#undef tracef diff --git a/src/raft/replication.c b/src/raft/replication.c index feecfd8f7..8ba6ad9d4 100644 --- a/src/raft/replication.c +++ b/src/raft/replication.c @@ -1834,4 +1834,3 @@ inline bool replicationInstallSnapshotBusy(struct raft *r) return r->last_stored == 0 && r->snapshot.put.data != NULL; } -#undef tracef diff --git a/src/raft/snapshot.c b/src/raft/snapshot.c index d05994fcb..ad330c0fb 100644 --- a/src/raft/snapshot.c +++ b/src/raft/snapshot.c @@ -111,4 +111,3 @@ int snapshotCopy(const struct raft_snapshot *src, struct raft_snapshot *dst) return 0; } -#undef tracef diff --git a/src/raft/start.c b/src/raft/start.c index 023d51e74..ba6fc2fbc 100644 --- a/src/raft/start.c +++ b/src/raft/start.c @@ -229,4 +229,3 @@ int raft_start(struct raft *r) return 0; } -#undef tracef diff --git a/src/raft/tick.c b/src/raft/tick.c index f6dd407c7..d6a196433 100644 --- a/src/raft/tick.c +++ b/src/raft/tick.c @@ -256,4 +256,3 @@ void tickCb(struct raft_io *io) } } -#undef tracef diff --git a/src/raft/uv.c b/src/raft/uv.c index f1450e742..377785554 100644 --- a/src/raft/uv.c +++ b/src/raft/uv.c @@ -812,4 +812,3 @@ void raft_uv_set_auto_recovery(struct raft_io *io, bool flag) uv->auto_recovery = flag; } -#undef tracef diff --git a/src/raft/uv_finalize.c b/src/raft/uv_finalize.c index beb9d4c00..22f5ad2c7 100644 --- a/src/raft/uv_finalize.c +++ b/src/raft/uv_finalize.c @@ -173,4 +173,3 @@ int UvFinalize(struct uv *uv, return 0; } -#undef tracef diff --git a/src/raft/uv_list.c b/src/raft/uv_list.c index 18639db36..ee7ef79ab 100644 --- a/src/raft/uv_list.c +++ b/src/raft/uv_list.c @@ -113,4 +113,3 @@ int UvList(struct uv *uv, return rv; } -#undef tracef diff --git a/src/raft/uv_prepare.c b/src/raft/uv_prepare.c index 199da6f29..4dfc46d40 100644 --- a/src/raft/uv_prepare.c +++ b/src/raft/uv_prepare.c @@ -336,4 +336,3 @@ void UvPrepareClose(struct uv *uv) } } -#undef tracef diff --git a/src/raft/uv_recv.c b/src/raft/uv_recv.c index fe93a21ef..61e13ae26 100644 --- a/src/raft/uv_recv.c +++ b/src/raft/uv_recv.c @@ -420,4 +420,3 @@ void UvRecvClose(struct uv *uv) } } -#undef tracef diff --git a/src/raft/uv_segment.c b/src/raft/uv_segment.c index ca178238d..023a4b722 100644 --- a/src/raft/uv_segment.c +++ b/src/raft/uv_segment.c @@ -1155,4 +1155,3 @@ int uvSegmentTruncate(struct uv *uv, return rv; } -#undef tracef diff --git a/src/raft/uv_send.c b/src/raft/uv_send.c index b4a4aad78..0f9b28a8c 100644 --- a/src/raft/uv_send.c +++ b/src/raft/uv_send.c @@ -516,4 +516,3 @@ void UvSendClose(struct uv *uv) } } -#undef tracef diff --git a/src/raft/uv_snapshot.c b/src/raft/uv_snapshot.c index 343d1ce07..c3b24ee58 100644 --- a/src/raft/uv_snapshot.c +++ b/src/raft/uv_snapshot.c @@ -805,4 +805,3 @@ int UvSnapshotGet(struct raft_io *io, return rv; } -#undef tracef diff --git a/src/raft/uv_truncate.c b/src/raft/uv_truncate.c index 4365a1b96..834d6d98c 100644 --- a/src/raft/uv_truncate.c +++ b/src/raft/uv_truncate.c @@ -197,4 +197,3 @@ int UvTruncate(struct raft_io *io, raft_index index) return rv; } -#undef tracef diff --git a/src/raft/uv_work.c b/src/raft/uv_work.c index 42777462f..fa479cbe8 100644 --- a/src/raft/uv_work.c +++ b/src/raft/uv_work.c @@ -75,4 +75,3 @@ int UvAsyncWork(struct raft_io *io, return rv; } -#undef tracef From 8cf993b07a9515945e2480dd7cc3bb46895cbfe0 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Mon, 29 Apr 2024 11:44:19 +0100 Subject: [PATCH 4/7] tracing: Demacroize somewhat Signed-off-by: Cole Miller --- src/tracing.c | 12 ++++++++++++ src/tracing.h | 12 ++---------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/tracing.c b/src/tracing.c index 0314a54fe..99b9ea58e 100644 --- a/src/tracing.c +++ b/src/tracing.c @@ -96,3 +96,15 @@ void stderrTracerEmit(const char *file, if (level >= tracer__level) tracerEmit(file, line, func, level, message); } + +void tracef0(int level, const char *file, int line, const char *func, const char *fmt, ...) +{ + if (UNLIKELY(_dqliteTracingEnabled)) { + va_list a; + va_start(a, fmt); + char msg[1024]; + vsnprintf(msg, sizeof(msg), fmt, a); + stderrTracerEmit(file, line, func, level, msg); + va_end(a); + } +} diff --git a/src/tracing.h b/src/tracing.h index 31bc0e7a3..0c6eac281 100644 --- a/src/tracing.h +++ b/src/tracing.h @@ -21,15 +21,7 @@ DQLITE_VISIBLE_TO_TESTS void stderrTracerEmit(const char *file, unsigned int level, const char *message); -#define tracef0(LEVEL, ...) \ - do { \ - if (UNLIKELY(_dqliteTracingEnabled)) { \ - char _msg[1024]; \ - snprintf(_msg, sizeof _msg, __VA_ARGS__); \ - stderrTracerEmit(__FILE__, __LINE__, __func__, \ - (LEVEL), _msg); \ - } \ - } while (0) +void tracef0(int level, const char *file, int line, const char *func, const char *fmt, ...); enum dqlite_trace_level { /** Represents an invalid trace level */ @@ -50,7 +42,7 @@ enum dqlite_trace_level { TRACE_NR, }; -#define tracef(...) tracef0(TRACE_DEBUG, __VA_ARGS__) +#define tracef(fmt, ...) tracef0(TRACE_DEBUG, __FILE__, __LINE__, __func__, (fmt), ##__VA_ARGS__) /* Enable tracing if the appropriate env variable is set, or disable tracing. */ DQLITE_VISIBLE_TO_TESTS void dqliteTracingMaybeEnable(bool enabled); From e3abef42143a68ac544896dbdb0e5083de1237f9 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Mon, 29 Apr 2024 12:17:59 +0100 Subject: [PATCH 5/7] Fixes; try to pin down GHA hangs Signed-off-by: Cole Miller --- .github/workflows/build-and-test.yml | 8 +++++++- src/tracing.c | 16 ++++++++-------- src/tracing.h | 6 +++--- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 5dc7fd7e5..9bce7e339 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -51,7 +51,13 @@ jobs: run: | ./test/raft/lib/fs.sh setup export $(test/raft/lib/fs.sh detect) - make check || (cat ./test-suite.log && false) + ./unit-test + ./integration-test + ./raft-core-fuzzy-test + ./raft-core-integration-test + ./raft-core-unit-test + ./raft-uv-integration-test + ./raft-uv-unit-test ./test/raft/lib/fs.sh teardown - name: Coverage diff --git a/src/tracing.c b/src/tracing.c index 99b9ea58e..d1510adc4 100644 --- a/src/tracing.c +++ b/src/tracing.c @@ -10,7 +10,7 @@ #define LIBDQLITE_TRACE "LIBDQLITE_TRACE" bool _dqliteTracingEnabled = false; -static unsigned tracer__level; +static int tracer__level; static pid_t tracerPidCached; void dqliteTracingMaybeEnable(bool enable) @@ -21,7 +21,7 @@ void dqliteTracingMaybeEnable(bool enable) tracerPidCached = getpid(); _dqliteTracingEnabled = enable; - tracer__level = (unsigned)atoi(trace_level); + tracer__level = atoi(trace_level); tracer__level = tracer__level < TRACE_NR ? tracer__level : TRACE_NONE; } @@ -36,13 +36,13 @@ static inline const char *tracerShortFileName(const char *fname) return p != NULL ? p + strlen(top_src_dir) : fname; } -static inline const char *tracerTraceLevelName(unsigned int level) +static inline const char *tracerTraceLevelName(int level) { static const char *levels[] = { "NONE", "DEBUG", "INFO", "WARN", "ERROR", "FATAL", }; - return level < ARRAY_SIZE(levels) ? levels[level] : levels[0]; + return level < (int)ARRAY_SIZE(levels) ? levels[level] : levels[0]; } static pid_t tracerPidCached; @@ -56,9 +56,9 @@ static inline pid_t gettidImpl(void) } static inline void tracerEmit(const char *file, - unsigned int line, + int line, const char *func, - unsigned int level, + int level, const char *message) { struct timespec ts = {0}; @@ -86,9 +86,9 @@ static inline void tracerEmit(const char *file, } void stderrTracerEmit(const char *file, - unsigned int line, + int line, const char *func, - unsigned int level, + int level, const char *message) { assert(tracer__level < TRACE_NR); diff --git a/src/tracing.h b/src/tracing.h index 0c6eac281..aec41f414 100644 --- a/src/tracing.h +++ b/src/tracing.h @@ -16,12 +16,12 @@ * from there on. Users should not manipulate the value of this variable. */ DQLITE_VISIBLE_TO_TESTS extern bool _dqliteTracingEnabled; DQLITE_VISIBLE_TO_TESTS void stderrTracerEmit(const char *file, - unsigned int line, + int line, const char *func, - unsigned int level, + int level, const char *message); -void tracef0(int level, const char *file, int line, const char *func, const char *fmt, ...); +DQLITE_VISIBLE_TO_TESTS void tracef0(int level, const char *file, int line, const char *func, const char *fmt, ...); enum dqlite_trace_level { /** Represents an invalid trace level */ From 1f3675dc46549548992d2a5088ca563483e64054 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Mon, 29 Apr 2024 12:23:04 +0100 Subject: [PATCH 6/7] Remove -Wformat-nonliteral In practice it seems we want to use vsnprintf all over the place, so I don't know what the point of enabling this is. Signed-off-by: Cole Miller --- configure.ac | 1 + src/logger.c | 3 --- test/lib/logger.c | 3 --- 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/configure.ac b/configure.ac index af45a0a0b..1a699c6f9 100644 --- a/configure.ac +++ b/configure.ac @@ -124,6 +124,7 @@ CC_CHECK_FLAGS_APPEND([AM_CFLAGS],[CFLAGS],[ \ -Wdate-time \ -Wnested-externs \ -Wconversion \ + -Wno-format-nonliteral \ -Werror \ ]) # To enable: diff --git a/src/logger.c b/src/logger.c index 92bdf919e..78614249d 100644 --- a/src/logger.c +++ b/src/logger.c @@ -36,10 +36,7 @@ void loggerDefaultEmit(void *data, int level, const char *fmt, va_list args) /* Then render the message, possibly truncating it. */ n = EMIT_BUF_LEN - strlen(buf) - 1; -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wformat-nonliteral" vsnprintf(cursor, n, fmt, args); -#pragma GCC diagnostic pop fprintf(stderr, "%s\n", buf); } diff --git a/test/lib/logger.c b/test/lib/logger.c index 2970254fa..20d2588c4 100644 --- a/test/lib/logger.c +++ b/test/lib/logger.c @@ -34,10 +34,7 @@ void test_logger_emit(void *data, int level, const char *format, va_list args) sprintf(buf + strlen(buf), "%2d -> [%s] ", t->id, level_name); -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wformat-nonliteral" vsnprintf(buf + strlen(buf), 1024 - strlen(buf), format, args); -#pragma GCC diagnostic pop munit_log(MUNIT_LOG_INFO, buf); return; From 301ca02aaae7c3b4cd7d65f8d14ceffb5c06ab09 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Mon, 29 Apr 2024 13:08:44 +0100 Subject: [PATCH 7/7] Debug Signed-off-by: Cole Miller --- .github/workflows/build-and-test.yml | 2 +- test/unit/test_conn.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 9bce7e339..e4172692b 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -51,7 +51,7 @@ jobs: run: | ./test/raft/lib/fs.sh setup export $(test/raft/lib/fs.sh detect) - ./unit-test + ./unit-test --show-stderr ./integration-test ./raft-core-fuzzy-test ./raft-core-integration-test diff --git a/test/unit/test_conn.c b/test/unit/test_conn.c index f801b839d..45968d26b 100644 --- a/test/unit/test_conn.c +++ b/test/unit/test_conn.c @@ -15,6 +15,7 @@ #include "../../src/lib/transport.h" #include "../../src/raft.h" #include "../../src/transport.h" +#include "../../src/tracing.h" TEST_MODULE(conn); @@ -75,6 +76,7 @@ static void connCloseCb(struct conn *conn) pool_fini(pool_ut_fallback()); \ conn__stop(&f->conn_test.conn); \ while (!f->conn_test.closed) { \ + tracef("conn_test not yet closed..."); \ test_uv_run(&f->loop, 1); \ }; \ TEAR_DOWN_RAFT; \