From 6fce3fca9fd3b29e7422c9cc4c1a8164496e44ad Mon Sep 17 00:00:00 2001 From: Roman Gershman Date: Thu, 22 Feb 2024 12:02:20 +0200 Subject: [PATCH] chore: fix build for MacOs (#2635) Also update actions versions to Node 20. This change allows dragonfly to be built on MacOs. However, we still have multiple failing tests on MacOS. Signed-off-by: Roman Gershman --- .github/actions/lint-test-chart/action.yml | 2 +- .github/workflows/bullmq-tests.yml | 2 +- .github/workflows/cov.yml | 6 ++--- .github/workflows/daily-builds.yml | 4 +-- .github/workflows/docker-release.yml | 4 +-- .../reusable-container-workflow.yaml | 12 ++++----- src/core/uring.h | 3 +++ src/facade/cmd_arg_parser.cc | 3 ++- src/facade/cmd_arg_parser_test.cc | 2 -- src/facade/dragonfly_connection.cc | 5 ++++ src/server/CMakeLists.txt | 10 +++---- src/server/detail/save_stages_controller.cc | 2 ++ src/server/detail/snapshot_storage.cc | 2 ++ src/server/detail/snapshot_storage.h | 2 ++ src/server/list_family.cc | 18 ++++++------- src/server/tiered_storage.h | 26 ++++++++++++++++--- 16 files changed, 67 insertions(+), 36 deletions(-) diff --git a/.github/actions/lint-test-chart/action.yml b/.github/actions/lint-test-chart/action.yml index 45393a895..5fd8e34d3 100644 --- a/.github/actions/lint-test-chart/action.yml +++ b/.github/actions/lint-test-chart/action.yml @@ -5,7 +5,7 @@ runs: using: "composite" steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: fetch-depth: 0 diff --git a/.github/workflows/bullmq-tests.yml b/.github/workflows/bullmq-tests.yml index de5c10d2b..868e8361b 100644 --- a/.github/workflows/bullmq-tests.yml +++ b/.github/workflows/bullmq-tests.yml @@ -16,7 +16,7 @@ jobs: password: ${{ secrets.GITHUB_TOKEN }} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Install NodeJs diff --git a/.github/workflows/cov.yml b/.github/workflows/cov.yml index 6f925a259..73ae80f1e 100644 --- a/.github/workflows/cov.yml +++ b/.github/workflows/cov.yml @@ -27,7 +27,7 @@ jobs: password: ${{ secrets.GITHUB_TOKEN }} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Install dependencies @@ -38,7 +38,7 @@ jobs: apt update && apt install -y lcov pip - name: Cache build deps id: cache-deps - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: | ~/.ccache @@ -78,7 +78,7 @@ jobs: echo ls covout ls covout/ - name: Upload coverage - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: coverage-report path: build/covout/ diff --git a/.github/workflows/daily-builds.yml b/.github/workflows/daily-builds.yml index 7cf369284..55d5a2b45 100644 --- a/.github/workflows/daily-builds.yml +++ b/.github/workflows/daily-builds.yml @@ -33,7 +33,7 @@ jobs: password: ${{ secrets.GITHUB_TOKEN }} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Run sccache-cache @@ -70,7 +70,7 @@ jobs: runs-on: macos-latest timeout-minutes: 45 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Install dependencies diff --git a/.github/workflows/docker-release.yml b/.github/workflows/docker-release.yml index 2ff5434d2..dab5b6342 100644 --- a/.github/workflows/docker-release.yml +++ b/.github/workflows/docker-release.yml @@ -63,7 +63,7 @@ jobs: run: env - name: checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: fetch-depth: 0 @@ -71,7 +71,7 @@ jobs: uses: azure/setup-helm@v3 - name: Setup Go - uses: actions/setup-go@v3 + uses: actions/setup-go@v4 - name: Configure Git if: env.IS_PRERELEASE != 'true' diff --git a/.github/workflows/reusable-container-workflow.yaml b/.github/workflows/reusable-container-workflow.yaml index 6af52d8a2..e526b19e8 100644 --- a/.github/workflows/reusable-container-workflow.yaml +++ b/.github/workflows/reusable-container-workflow.yaml @@ -87,14 +87,14 @@ jobs: - name: Set up QEMU id: qemu - uses: docker/setup-qemu-action@v2 + uses: docker/setup-qemu-action@v3 with: platforms: arm64,amd64 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v2 + uses: docker/setup-buildx-action@v3 - name: Login to GitHub Container Registry - uses: docker/login-action@v2 + uses: docker/login-action@v3 with: registry: ${{ inputs.registry }} username: ${{ inputs.registry_username }} @@ -102,7 +102,7 @@ jobs: - name: Docker meta id: metadata - uses: docker/metadata-action@v4 + uses: docker/metadata-action@v5 with: images: | ${{ inputs.image }} @@ -132,7 +132,7 @@ jobs: # build is broken based on platforms as load: true is not supported with multi-platform builds - if: ${{ hashFiles(format('{0}-{1}', matrix.dockerfile, inputs.build_type)) }} name: Build release image for amd64 - uses: docker/build-push-action@v3 + uses: docker/build-push-action@v5 with: context: . platforms: linux/amd64 @@ -190,7 +190,7 @@ jobs: - if: ${{ hashFiles(format('{0}-{1}', matrix.dockerfile, inputs.build_type)) }} name: Push release image - uses: docker/build-push-action@v3 + uses: docker/build-push-action@v5 with: context: . platforms: linux/amd64,linux/arm64 diff --git a/src/core/uring.h b/src/core/uring.h index 0585b455b..6580cb3ee 100644 --- a/src/core/uring.h +++ b/src/core/uring.h @@ -4,6 +4,7 @@ #pragma once +#ifdef __linux__ #include "util/fibers/uring_file.h" #include "util/fibers/uring_proactor.h" namespace dfly { @@ -14,3 +15,5 @@ using util::fb2::OpenLinux; using util::fb2::OpenRead; } // namespace dfly + +#endif diff --git a/src/facade/cmd_arg_parser.cc b/src/facade/cmd_arg_parser.cc index 42cc95b2d..68225b1e4 100644 --- a/src/facade/cmd_arg_parser.cc +++ b/src/facade/cmd_arg_parser.cc @@ -74,7 +74,8 @@ template T CmdArgParser::Num(size_t idx) { template float CmdArgParser::Num(size_t); template double CmdArgParser::Num(size_t); -template uint64_t CmdArgParser::Num(size_t); +template unsigned long CmdArgParser::Num(size_t); +template unsigned long long CmdArgParser::Num(size_t); template int64_t CmdArgParser::Num(size_t); template uint32_t CmdArgParser::Num(size_t); template int32_t CmdArgParser::Num(size_t); diff --git a/src/facade/cmd_arg_parser_test.cc b/src/facade/cmd_arg_parser_test.cc index 6bc038745..45113e9ae 100644 --- a/src/facade/cmd_arg_parser_test.cc +++ b/src/facade/cmd_arg_parser_test.cc @@ -37,13 +37,11 @@ TEST_F(CmdArgParserTest, BasicTypes) { EXPECT_EQ(parser.Next(), "STRING"s); EXPECT_EQ(parser.Next(), "VIEW"sv); -#ifndef __APPLE__ EXPECT_EQ(parser.Next(), 11u); EXPECT_EQ(parser.Next(), 22u); auto [a, b] = parser.Next(); EXPECT_EQ(a, 33u); EXPECT_EQ(b, 44u); -#endif EXPECT_FALSE(parser.HasNext()); EXPECT_FALSE(parser.Error()); diff --git a/src/facade/dragonfly_connection.cc b/src/facade/dragonfly_connection.cc index ce80188a0..7e3f418e0 100644 --- a/src/facade/dragonfly_connection.cc +++ b/src/facade/dragonfly_connection.cc @@ -22,6 +22,7 @@ #include "facade/memcache_parser.h" #include "facade/redis_parser.h" #include "facade/service_interface.h" +#include "io/file.h" #include "util/fibers/proactor_base.h" #ifdef DFLY_USE_SSL @@ -149,6 +150,7 @@ void OpenTrafficLogger(string_view base_path) { if (tl_traffic_logger.log_file) return; +#ifdef __linux__ // Open file with append mode, without it concurrent fiber writes seem to conflict string path = absl::StrCat( base_path, "-", absl::Dec(ProactorBase::me()->GetPoolIndex(), absl::kZeroPad3), ".bin"); @@ -158,6 +160,9 @@ void OpenTrafficLogger(string_view base_path) { return; } tl_traffic_logger.log_file = unique_ptr{file.value()}; +#else + LOG(WARNING) << "Traffic logger is only supported on Linux"; +#endif } void LogTraffic(uint32_t id, bool has_more, absl::Span resp) { diff --git a/src/server/CMakeLists.txt b/src/server/CMakeLists.txt index 98f1e34d8..159368855 100644 --- a/src/server/CMakeLists.txt +++ b/src/server/CMakeLists.txt @@ -13,8 +13,12 @@ endif() set_property(SOURCE dfly_main.cc APPEND PROPERTY COMPILE_DEFINITIONS SOURCE_PATH_FROM_BUILD_ENV=${CMAKE_SOURCE_DIR}) -if("${CMAKE_SYSTEM_NAME}" STREQUAL "Linux") +if ("${CMAKE_SYSTEM_NAME}" STREQUAL "Linux") SET(TX_LINUX_SRCS io_mgr.cc tiered_storage.cc) + + add_executable(dfly_bench dfly_bench.cc) + cxx_link(dfly_bench dfly_facade fibers2 absl::random_random) + cxx_test(tiered_storage_test dfly_test_lib LABELS DFLY) endif() @@ -76,9 +80,6 @@ add_library(dfly_test_lib test_utils.cc) cxx_link(dfly_test_lib dragonfly_lib facade_test gtest_main_ext) -add_executable(dfly_bench dfly_bench.cc) -cxx_link(dfly_bench dfly_facade fibers2 absl::random_random) - cxx_test(dragonfly_test dfly_test_lib LABELS DFLY) cxx_test(multi_test dfly_test_lib LABELS DFLY) cxx_test(generic_family_test dfly_test_lib LABELS DFLY) @@ -95,7 +96,6 @@ cxx_test(zset_family_test dfly_test_lib LABELS DFLY) cxx_test(blocking_controller_test dfly_test_lib LABELS DFLY) cxx_test(json_family_test dfly_test_lib LABELS DFLY) cxx_test(journal/journal_test dfly_test_lib LABELS DFLY) -cxx_test(tiered_storage_test dfly_test_lib LABELS DFLY) cxx_test(top_keys_test dfly_test_lib LABELS DFLY) cxx_test(hll_family_test dfly_test_lib LABELS DFLY) cxx_test(cluster/cluster_config_test dfly_test_lib LABELS DFLY) diff --git a/src/server/detail/save_stages_controller.cc b/src/server/detail/save_stages_controller.cc index b6d5a5728..4892e9c8e 100644 --- a/src/server/detail/save_stages_controller.cc +++ b/src/server/detail/save_stages_controller.cc @@ -130,9 +130,11 @@ size_t RdbSnapshot::GetSaveBuffersSize() { } error_code RdbSnapshot::Close() { +#ifdef __linux__ if (is_linux_file_) { return static_cast(io_sink_.get())->Close(); } +#endif return static_cast(io_sink_.get())->Close(); } diff --git a/src/server/detail/snapshot_storage.cc b/src/server/detail/snapshot_storage.cc index f2f17111b..20055174d 100644 --- a/src/server/detail/snapshot_storage.cc +++ b/src/server/detail/snapshot_storage.cc @@ -376,6 +376,7 @@ AwsS3SnapshotStorage::ListObjects(std::string_view bucket_name, std::string_view return keys; } +#ifdef __linux__ io::Result LinuxWriteWrapper::WriteSome(const iovec* v, uint32_t len) { io::Result res = lf_->WriteSome(v, len, offset_, 0); if (res) { @@ -384,6 +385,7 @@ io::Result LinuxWriteWrapper::WriteSome(const iovec* v, uint32_t len) { return res; } +#endif void SubstituteFilenamePlaceholders(fs::path* filename, const FilenameSubstitutions& fns) { *filename = absl::StrReplaceAll( diff --git a/src/server/detail/snapshot_storage.h b/src/server/detail/snapshot_storage.h index 31d74b17d..daf20b369 100644 --- a/src/server/detail/snapshot_storage.h +++ b/src/server/detail/snapshot_storage.h @@ -105,6 +105,7 @@ class AwsS3SnapshotStorage : public SnapshotStorage { // Returns bucket_name, obj_path for an s3 path. std::optional> GetBucketPath(std::string_view path); +#ifdef __linux__ // takes ownership over the file. class LinuxWriteWrapper : public io::Sink { public: @@ -121,6 +122,7 @@ class LinuxWriteWrapper : public io::Sink { std::unique_ptr lf_; off_t offset_ = 0; }; +#endif struct FilenameSubstitutions { std::string_view ts; diff --git a/src/server/list_family.cc b/src/server/list_family.cc index 1cde047dd..99e20b338 100644 --- a/src/server/list_family.cc +++ b/src/server/list_family.cc @@ -22,10 +22,6 @@ extern "C" { #include "server/server_state.h" #include "server/transaction.h" -/* List related stuff */ -#define LIST_HEAD 0 -#define LIST_TAIL 1 - /** * The number of entries allowed per internal list node can be specified * as a fixed maximum size or a maximum number of elements. @@ -80,6 +76,8 @@ void* listPopSaver(unsigned char* data, size_t sz) { return new string((char*)data, sz); } +enum InsertParam { INSERT_BEFORE, INSERT_AFTER }; + string ListPop(ListDir dir, quicklist* ql) { long long vlong; string* pop_str = nullptr; @@ -544,7 +542,7 @@ OpResult> OpPos(const OpArgs& op_args, std::string_view key, } OpResult OpInsert(const OpArgs& op_args, string_view key, string_view pivot, string_view elem, - int insert_param) { + InsertParam insert_param) { auto& db_slice = op_args.shard->db_slice(); auto it_res = db_slice.FindMutable(op_args.db_cntx, key, OBJ_LIST); if (!it_res) @@ -564,10 +562,10 @@ OpResult OpInsert(const OpArgs& op_args, string_view key, string_view pivot int res = -1; if (found) { - if (insert_param == LIST_TAIL) { + if (insert_param == INSERT_AFTER) { quicklistInsertAfter(qiter, &entry, elem.data(), elem.size()); } else { - DCHECK_EQ(LIST_HEAD, insert_param); + DCHECK_EQ(INSERT_BEFORE, insert_param); quicklistInsertBefore(qiter, &entry, elem.data(), elem.size()); } res = quicklistCount(ql); @@ -1042,13 +1040,13 @@ void ListFamily::LInsert(CmdArgList args, ConnectionContext* cntx) { string_view param = ArgS(args, 1); string_view pivot = ArgS(args, 2); string_view elem = ArgS(args, 3); - int where; + InsertParam where; ToUpper(&args[1]); if (param == "AFTER") { - where = LIST_TAIL; + where = INSERT_AFTER; } else if (param == "BEFORE") { - where = LIST_HEAD; + where = INSERT_BEFORE; } else { return cntx->SendError(kSyntaxErr); } diff --git a/src/server/tiered_storage.h b/src/server/tiered_storage.h index 062b43287..ba8f19be6 100644 --- a/src/server/tiered_storage.h +++ b/src/server/tiered_storage.h @@ -114,11 +114,15 @@ class TieredStorage { public: static constexpr size_t kMinBlobLen = size_t(-1); // infinity. - explicit TieredStorage(DbSlice* db_slice) { + TieredStorage(DbSlice* db_slice, size_t max_file_size) { } ~TieredStorage() { } + static bool CanExternalizeEntry(PrimeIterator it) { + return false; + } + std::error_code Open(const std::string& path) { return {}; } @@ -127,19 +131,35 @@ class TieredStorage { return {}; } + PrimeIterator Load(DbIndex db_index, PrimeIterator it, std::string_view key) { + return {}; + } + // Schedules unloading of the item, pointed by the iterator. std::error_code ScheduleOffload(DbIndex db_index, PrimeIterator it) { return {}; } + IoMgrStats GetDiskStats() const { + return IoMgrStats{}; + } + + void CancelAllIos(DbIndex db_index) { + } + void CancelIo(DbIndex db_index, PrimeIterator it) { } - static bool EligibleForOffload(std::string_view val) { + static bool EligibleForOffload(size_t) { return false; } - void Free(size_t offset, size_t len) { + std::error_code ScheduleOffloadWithThrottle(DbIndex db_index, PrimeIterator it, + std::string_view key) { + return {}; + } + + void Free(PrimeIterator it, DbTableStats* stats) { } void Shutdown() {