From 336d6ff1817d2ad6400859fe3308df3a0bcd8639 Mon Sep 17 00:00:00 2001 From: Roman Gershman Date: Tue, 6 Feb 2024 11:57:26 +0200 Subject: [PATCH] Update helio (#2538) chore: UpdateHelio dependency Add support for asan/ubsan checkers in our dev environment. Remove more clang warnings. Once we fix all the problems we will enable them in our CI as well. Signed-off-by: Roman Gershman --- .github/workflows/ci.yml | 28 ++++++++++++++++---- CMakeLists.txt | 8 +++--- helio | 2 +- src/core/sorted_map.cc | 8 ------ src/server/CMakeLists.txt | 14 ++++------ src/server/acl/validator.cc | 2 ++ src/server/bitops_family.cc | 2 ++ src/server/cluster/cluster_slot_migration.cc | 2 +- src/server/conn_context.cc | 2 +- src/server/hset_family_test.cc | 4 +-- src/server/memory_cmd.cc | 11 ++++---- src/server/multi_test.cc | 4 +-- src/server/rdb_load.cc | 2 +- src/server/rdb_save.cc | 2 +- src/server/search/doc_accessors.cc | 6 ++--- src/server/search/doc_index.cc | 2 +- src/server/search/doc_index.h | 14 +++++++++- src/server/stream_family.cc | 2 +- src/server/string_family.cc | 2 +- src/server/test_utils.cc | 4 +-- src/server/zset_family.cc | 4 +-- 21 files changed, 74 insertions(+), 51 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d623963b8..f587f8bdc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,7 +14,7 @@ jobs: - uses: actions/checkout@v4 with: fetch-depth: 2 - - uses: actions/setup-python@v3 + - uses: actions/setup-python@v5 - name: Install dependencies run: | python -m pip install pre-commit @@ -53,20 +53,32 @@ jobs: container: image: ghcr.io/romange/${{ matrix.container }} + volumes: + - /:/hostroot + credentials: username: ${{ github.repository_owner }} password: ${{ secrets.GITHUB_TOKEN }} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - - name: Install dependencies + - name: Prepare Environment run: | uname -a cmake --version mkdir -p ${GITHUB_WORKSPACE}/build + echo "===================Before freeing up space ============================================" + df -h + rm -rf /hostroot/usr/share/dotnet + rm -rf /hostroot/usr/local/share/boost + rm -rf /hostroot/usr/local/lib/android + rm -rf /hostroot/opt/ghc + echo "===================After freeing up space ============================================" + df -h + - name: Run sccache-cache uses: mozilla-actions/sccache-action@v0.0.3 @@ -94,9 +106,15 @@ jobs: - name: Build run: | cd ${GITHUB_WORKSPACE}/build + ninja search_family_test + df -h + echo "-----------------------------" ninja src/all - ${SCCACHE_PATH} --show-stats | tee $GITHUB_STEP_SUMMARY - + - name: PostFail + if: failure() + run: | + echo "disk space is:" + df -h - name: C++ Unit Tests run: | cd ${GITHUB_WORKSPACE}/build diff --git a/CMakeLists.txt b/CMakeLists.txt index bbef74261..dd89dfe29 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -35,12 +35,12 @@ option(DF_USE_SSL "Provide support for SSL connections" ON) find_package(OpenSSL) -if (SUPPORT_ASAN) - # set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fsanitize=address") +if (SUPPORT_ASAN AND NOT DEFINED ENV{CI}) + set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fsanitize=address") endif() -if (SUPPORT_USAN) - # set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fsanitize=undefined") +if (SUPPORT_USAN AND NOT DEFINED ENV{CI}) + set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fsanitize=undefined") endif() include(third_party) diff --git a/helio b/helio index 6b3c2c5d2..d19d64699 160000 --- a/helio +++ b/helio @@ -1 +1 @@ -Subproject commit 6b3c2c5d249ccb0f64da80d168bc35c574724fb4 +Subproject commit d19d64699e7d23e6fa0a63614cda49f17eaa04d8 diff --git a/src/core/sorted_map.cc b/src/core/sorted_map.cc index 0b4564dcb..8e4b1cc55 100644 --- a/src/core/sorted_map.cc +++ b/src/core/sorted_map.cc @@ -35,14 +35,6 @@ constexpr uint64_t kInfTag = 1ULL << 63; constexpr uint64_t kIgnoreDoubleTag = 1ULL << 62; constexpr uint64_t kSdsMask = (1ULL << 60) - 1; -inline zskiplistNode* Next(bool reverse, zskiplistNode* ln) { - return reverse ? ln->backward : ln->level[0].forward; -} - -inline bool IsUnder(bool reverse, double score, const zrangespec& spec) { - return reverse ? zslValueGteMin(score, &spec) : zslValueLteMax(score, &spec); -} - double GetObjScore(const void* obj) { sds s = (sds)obj; char* ptr = s + sdslen(s) + 1; diff --git a/src/server/CMakeLists.txt b/src/server/CMakeLists.txt index 8ac1007c4..51a0c8fcc 100644 --- a/src/server/CMakeLists.txt +++ b/src/server/CMakeLists.txt @@ -1,11 +1,9 @@ +option(DF_ENABLE_MEMORY_TRACKING "Adds memory tracking debugging via MEMORY TRACK command" ON) +option(PRINT_STACKTRACES_ON_SIGNAL "Enables DF to print all fiber stacktraces on SIGUSR1" OFF) + add_executable(dragonfly dfly_main.cc version_monitor.cc) cxx_link(dragonfly base dragonfly_lib) -option(DF_ENABLE_MEMORY_TRACKING "Adds memory tracking debugging via MEMORY TRACK command" ON) -if (DF_ENABLE_MEMORY_TRACKING) - target_compile_definitions(dragonfly PRIVATE DFLY_ENABLE_MEMORY_TRACKING) -endif() - if (CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64" AND CMAKE_BUILD_TYPE STREQUAL "Release") # Add core2 only to this file, thus avoiding instructions in this object file that # can cause SIGILL. @@ -54,18 +52,16 @@ add_library(dragonfly_lib engine_shard_set.cc channel_store.cc if (DF_ENABLE_MEMORY_TRACKING) target_compile_definitions(dragonfly_lib PRIVATE DFLY_ENABLE_MEMORY_TRACKING) + target_compile_definitions(dragonfly PRIVATE DFLY_ENABLE_MEMORY_TRACKING) endif() -cxx_link(dfly_transaction dfly_core strings_lib TRDP::fast_float) - -option(PRINT_STACKTRACES_ON_SIGNAL "Enables DF to print all fiber stacktraces on SIGUSR1" OFF) - if (PRINT_STACKTRACES_ON_SIGNAL) target_compile_definitions(dragonfly_lib PRIVATE PRINT_STACKTRACES_ON_SIGNAL) endif() find_library(ZSTD_LIB NAMES libzstd.a libzstdstatic.a zstd NAMES_PER_DIR REQUIRED) +cxx_link(dfly_transaction dfly_core strings_lib TRDP::fast_float) cxx_link(dragonfly_lib dfly_transaction dfly_facade redis_lib awsv2_lib strings_lib html_lib http_client_lib absl::random_random TRDP::jsoncons ${ZSTD_LIB} TRDP::lz4 TRDP::croncpp) diff --git a/src/server/acl/validator.cc b/src/server/acl/validator.cc index b76d3918d..c75c87285 100644 --- a/src/server/acl/validator.cc +++ b/src/server/acl/validator.cc @@ -35,8 +35,10 @@ namespace dfly::acl { } // GCC yields a wrong warning about uninitialized optional use +#ifdef __GNUC__ #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wmaybe-uninitialized" +#endif [[nodiscard]] std::pair IsUserAllowedToInvokeCommandGeneric( uint32_t acl_cat, const std::vector& acl_commands, const AclKeys& keys, diff --git a/src/server/bitops_family.cc b/src/server/bitops_family.cc index 1714340cd..a17375cc6 100644 --- a/src/server/bitops_family.cc +++ b/src/server/bitops_family.cc @@ -594,8 +594,10 @@ void BitCount(CmdArgList args, ConnectionContext* cntx) { } // GCC yields a wrong warning about uninitialized optional use +#ifdef __GNUC__ #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wmaybe-uninitialized" +#endif enum class EncodingType { UINT, INT, NILL }; diff --git a/src/server/cluster/cluster_slot_migration.cc b/src/server/cluster/cluster_slot_migration.cc index 497794966..f08e33859 100644 --- a/src/server/cluster/cluster_slot_migration.cc +++ b/src/server/cluster/cluster_slot_migration.cc @@ -39,7 +39,7 @@ vector> Partition(unsigned num_flows) { ClusterSlotMigration::ClusterSlotMigration(string host_ip, uint16_t port, Service* se, std::vector slots) - : ProtocolClient(move(host_ip), port), service_(*se), slots_(std::move(slots)) { + : ProtocolClient(std::move(host_ip), port), service_(*se), slots_(std::move(slots)) { } ClusterSlotMigration::~ClusterSlotMigration() { diff --git a/src/server/conn_context.cc b/src/server/conn_context.cc index ce4a57a44..a6286fb1a 100644 --- a/src/server/conn_context.cc +++ b/src/server/conn_context.cc @@ -37,7 +37,7 @@ StoredCmd::StoredCmd(const CommandId* cid, CmdArgList args, facade::ReplyMode mo } StoredCmd::StoredCmd(string&& buffer, const CommandId* cid, CmdArgList args, facade::ReplyMode mode) - : cid_{cid}, buffer_{move(buffer)}, sizes_(args.size()), reply_mode_{mode} { + : cid_{cid}, buffer_{std::move(buffer)}, sizes_(args.size()), reply_mode_{mode} { for (unsigned i = 0; i < args.size(); i++) { // Assume tightly packed list. DCHECK(i + 1 == args.size() || args[i].data() + args[i].size() == args[i + 1].data()); diff --git a/src/server/hset_family_test.cc b/src/server/hset_family_test.cc index b49ec7993..b59007677 100644 --- a/src/server/hset_family_test.cc +++ b/src/server/hset_family_test.cc @@ -32,8 +32,8 @@ class HestFamilyTestProtocolVersioned : public HSetFamilyTest, protected: }; -INSTANTIATE_TEST_CASE_P(HestFamilyTestProtocolVersioned, HestFamilyTestProtocolVersioned, - ::testing::Values("2", "3")); +INSTANTIATE_TEST_SUITE_P(HestFamilyTestProtocolVersioned, HestFamilyTestProtocolVersioned, + ::testing::Values("2", "3")); TEST_F(HSetFamilyTest, Basic) { auto resp = Run({"hset", "x", "a"}); diff --git a/src/server/memory_cmd.cc b/src/server/memory_cmd.cc index bd08bb41f..7f266e0b3 100644 --- a/src/server/memory_cmd.cc +++ b/src/server/memory_cmd.cc @@ -332,15 +332,16 @@ void MemoryCmd::Track(CmdArgList args) { } if (sub_cmd == "ADD") { - auto [lower_bound, upper_bound, odds] = parser.Next(); + AllocationTracker::TrackingInfo tracking_info; + std::tie(tracking_info.lower_bound, tracking_info.upper_bound, tracking_info.sample_odds) = + parser.Next(); if (parser.HasError()) { return cntx_->SendError(parser.Error()->MakeReply()); } atomic_bool error{false}; shard_set->pool()->Await([&](unsigned index, auto*) { - if (!AllocationTracker::Get().Add( - {.lower_bound = lower_bound, .upper_bound = upper_bound, .sample_odds = odds})) { + if (!AllocationTracker::Get().Add(tracking_info)) { error.store(true); } }); @@ -359,8 +360,8 @@ void MemoryCmd::Track(CmdArgList args) { } atomic_bool error{false}; - shard_set->pool()->Await([&](unsigned index, auto*) { - if (!AllocationTracker::Get().Remove(lower_bound, upper_bound)) { + shard_set->pool()->Await([&, lo = lower_bound, hi = upper_bound](unsigned index, auto*) { + if (!AllocationTracker::Get().Remove(lo, hi)) { error.store(true); } }); diff --git a/src/server/multi_test.cc b/src/server/multi_test.cc index 4e734499f..c0ad7c99b 100644 --- a/src/server/multi_test.cc +++ b/src/server/multi_test.cc @@ -638,7 +638,7 @@ TEST_F(MultiTest, MultiCauseUnblocking) { const int kRounds = 10; vector keys = {kKeySid0, kKeySid1, kKeySid2}; - auto push = [this, keys, kRounds]() mutable { + auto push = [this, keys]() mutable { int i = 0; do { Run({"multi"}); @@ -648,7 +648,7 @@ TEST_F(MultiTest, MultiCauseUnblocking) { } while (next_permutation(keys.begin(), keys.end()) || i++ < kRounds); }; - auto pop = [this, keys, kRounds]() mutable { + auto pop = [this, keys]() mutable { int i = 0; do { for (int j = keys.size() - 1; j >= 0; j--) diff --git a/src/server/rdb_load.cc b/src/server/rdb_load.cc index 654c71b8d..ab2484652 100644 --- a/src/server/rdb_load.cc +++ b/src/server/rdb_load.cc @@ -2276,7 +2276,7 @@ error_code RdbLoader::HandleAux() { } else if (auxkey == "repl-offset") { // TODO } else if (auxkey == "lua") { - LoadScriptFromAux(move(auxval)); + LoadScriptFromAux(std::move(auxval)); } else if (auxkey == "redis-ver") { VLOG(1) << "Loading RDB produced by version " << auxval; } else if (auxkey == "ctime") { diff --git a/src/server/rdb_save.cc b/src/server/rdb_save.cc index 804ab7366..cb2a360ce 100644 --- a/src/server/rdb_save.cc +++ b/src/server/rdb_save.cc @@ -1286,7 +1286,7 @@ RdbSaver::GlobalData RdbSaver::GetGlobalData(const Service* service) { auto scripts = service->script_mgr()->GetAll(); script_bodies.reserve(scripts.size()); for (auto& [sha, data] : scripts) - script_bodies.push_back(move(data.body)); + script_bodies.push_back(std::move(data.body)); } #ifndef __APPLE__ diff --git a/src/server/search/doc_accessors.cc b/src/server/search/doc_accessors.cc index 8eb6b7536..b1c705a34 100644 --- a/src/server/search/doc_accessors.cc +++ b/src/server/search/doc_accessors.cc @@ -179,11 +179,11 @@ JsonAccessor::JsonPathContainer* JsonAccessor::GetPath(std::string_view field) c return nullptr; } - JsonPathContainer path_container{move(path_expr)}; - auto ptr = make_unique(move(path_container)); + JsonPathContainer path_container{std::move(path_expr)}; + auto ptr = make_unique(std::move(path_container)); JsonPathContainer* path = ptr.get(); - path_cache_[field] = move(ptr); + path_cache_[field] = std::move(ptr); return path; } diff --git a/src/server/search/doc_index.cc b/src/server/search/doc_index.cc index 01693cf5b..c0b4822cc 100644 --- a/src/server/search/doc_index.cc +++ b/src/server/search/doc_index.cc @@ -248,7 +248,7 @@ ShardDocIndex* ShardDocIndices::GetIndex(string_view name) { void ShardDocIndices::InitIndex(const OpArgs& op_args, std::string_view name, shared_ptr index_ptr) { auto shard_index = make_unique(index_ptr); - auto [it, _] = indices_.emplace(name, move(shard_index)); + auto [it, _] = indices_.emplace(name, std::move(shard_index)); // Don't build while loading, shutting down, etc. // After loading, indices are rebuilt separately diff --git a/src/server/search/doc_index.h b/src/server/search/doc_index.h index 8bf85d9be..60218828f 100644 --- a/src/server/search/doc_index.h +++ b/src/server/search/doc_index.h @@ -175,7 +175,11 @@ class ShardDocIndices { absl::flat_hash_map> indices_; }; -#ifdef __APPLE__ +#if defined(__APPLE__) + +inline ShardDocIndices::ShardDocIndices() : local_mr_{nullptr} { +} + inline ShardDocIndex* ShardDocIndices::GetIndex(std::string_view name) { return nullptr; } @@ -211,5 +215,13 @@ inline SearchStats ShardDocIndices::GetStats() const { return {}; } +inline DocIndexInfo ShardDocIndex::GetInfo() const { + return {}; +} + +inline std::string DocIndexInfo::BuildRestoreCommand() const { + return {}; +} + #endif // __APPLE__ } // namespace dfly diff --git a/src/server/stream_family.cc b/src/server/stream_family.cc index be37a2908..ace624c38 100644 --- a/src/server/stream_family.cc +++ b/src/server/stream_family.cc @@ -169,7 +169,7 @@ const uint32_t STREAM_LISTPACK_MAX_PRE_ALLOCATE = 4096; /* Every stream item inside the listpack, has a flags field that is used to * mark the entry as deleted, or having the same field as the "master" * entry at the start of the listpack. */ -const uint32_t STREAM_ITEM_FLAG_DELETED = (1 << 0); /* Entry is deleted. Skip it. */ +// const uint32_t STREAM_ITEM_FLAG_DELETED = (1 << 0); /* Entry is deleted. Skip it. */ const uint32_t STREAM_ITEM_FLAG_SAMEFIELDS = (1 << 1); /* Same fields as master entry. */ string StreamIdRepr(const streamID& id) { diff --git a/src/server/string_family.cc b/src/server/string_family.cc index d29fa432a..92a3943e1 100644 --- a/src/server/string_family.cc +++ b/src/server/string_family.cc @@ -41,7 +41,7 @@ using namespace facade; using CI = CommandId; constexpr uint32_t kMaxStrLen = 1 << 28; -constexpr size_t kMinTieredLen = TieredStorage::kMinBlobLen; +[[maybe_unused]] constexpr size_t kMinTieredLen = TieredStorage::kMinBlobLen; size_t CopyValueToBuffer(const PrimeValue& pv, char* dest) { DCHECK_EQ(pv.ObjType(), OBJ_STRING); diff --git a/src/server/test_utils.cc b/src/server/test_utils.cc index 5b3d200ad..c6953b1a2 100644 --- a/src/server/test_utils.cc +++ b/src/server/test_utils.cc @@ -74,7 +74,7 @@ void TestConnection::SendPubMessageAsync(PubMessage pmsg) { } void TestConnection::SendInvalidationMessageAsync(InvalidationMessage msg) { - invalidate_messages.push_back(move(msg)); + invalidate_messages.push_back(std::move(msg)); } std::string TestConnection::RemoteEndpointStr() const { @@ -228,7 +228,7 @@ void BaseFamilyTest::ResetService() { watchdog_fiber_ = pp_->GetNextProactor()->LaunchFiber([this] { ThisFiber::SetName("Watchdog"); - if (!watchdog_done_.WaitFor(60s)) { + if (!watchdog_done_.WaitFor(20s)) { LOG(ERROR) << "Deadlock detected!!!!"; absl::SetFlag(&FLAGS_alsologtostderr, true); fb2::Mutex m; diff --git a/src/server/zset_family.cc b/src/server/zset_family.cc index c07198b8f..d8eadbf90 100644 --- a/src/server/zset_family.cc +++ b/src/server/zset_family.cc @@ -3249,7 +3249,7 @@ void ZSetFamily::Register(CommandRegistry* registry) { *registry << CI{"ZADD", CO::FAST | CO::WRITE | CO::DENYOOM, -4, 1, 1, acl::kZAdd}.HFUNC(ZAdd) << CI{"BZPOPMIN", CO::WRITE | CO::NOSCRIPT | CO::BLOCKING | CO::NO_AUTOJOURNAL, -3, 1, -2, - acl::kBZPopMax} + acl::kBZPopMin} .HFUNC(BZPopMin) << CI{"BZPOPMAX", CO::WRITE | CO::NOSCRIPT | CO::BLOCKING | CO::NO_AUTOJOURNAL, -3, 1, -2, acl::kBZPopMax} @@ -3268,7 +3268,7 @@ void ZSetFamily::Register(CommandRegistry* registry) { << CI{"ZREM", CO::FAST | CO::WRITE, -3, 1, 1, acl::kZRem}.HFUNC(ZRem) << CI{"ZRANGE", CO::READONLY, -4, 1, 1, acl::kZRange}.HFUNC(ZRange) << CI{"ZRANDMEMBER", CO::READONLY, -2, 1, 1, acl::kZRandMember}.HFUNC(ZRandMember) - << CI{"ZRANK", CO::READONLY | CO::FAST, 3, 1, 1, acl::kZRange}.HFUNC(ZRank) + << CI{"ZRANK", CO::READONLY | CO::FAST, 3, 1, 1, acl::kZRank}.HFUNC(ZRank) << CI{"ZRANGEBYLEX", CO::READONLY, -4, 1, 1, acl::kZRangeByLex}.HFUNC(ZRangeByLex) << CI{"ZRANGEBYSCORE", CO::READONLY, -4, 1, 1, acl::kZRangeByScore}.HFUNC(ZRangeByScore) << CI{"ZSCORE", CO::READONLY | CO::FAST, 3, 1, 1, acl::kZScore}.HFUNC(ZScore)