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 <roman@dragonflydb.io>

.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

CMakeLists.txt

@@ -35,12 +35,12 @@ option(DF_USE_SSL "Provide support for SSL connections" ON)
 
 find_package(OpenSSL)
 
-if (SUPPORT_ASAN)
+if (SUPPORT_ASAN AND NOT DEFINED ENV{CI})
-  # set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fsanitize=address")
+  set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fsanitize=address")
 endif()
 
-if (SUPPORT_USAN)
+if (SUPPORT_USAN AND NOT DEFINED ENV{CI})
-  # set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fsanitize=undefined")
+  set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fsanitize=undefined")
 endif()
 
 include(third_party)

helio

@@ -1 +1 @@
-Subproject commit 6b3c2c5d249ccb0f64da80d168bc35c574724fb4
+Subproject commit d19d64699e7d23e6fa0a63614cda49f17eaa04d8

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;

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)

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<bool, AclLog::Reason> IsUserAllowedToInvokeCommandGeneric(
     uint32_t acl_cat, const std::vector<uint64_t>& acl_commands, const AclKeys& keys,

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 };
 

src/server/cluster/cluster_slot_migration.cc

@@ -39,7 +39,7 @@ vector<vector<unsigned>> Partition(unsigned num_flows) {
 
 ClusterSlotMigration::ClusterSlotMigration(string host_ip, uint16_t port, Service* se,
                                            std::vector<ClusterConfig::SlotRange> 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() {

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());

src/server/hset_family_test.cc

@@ -32,8 +32,8 @@ class HestFamilyTestProtocolVersioned : public HSetFamilyTest,
  protected:
 };
 
-INSTANTIATE_TEST_CASE_P(HestFamilyTestProtocolVersioned, HestFamilyTestProtocolVersioned,
+INSTANTIATE_TEST_SUITE_P(HestFamilyTestProtocolVersioned, HestFamilyTestProtocolVersioned,
-                        ::testing::Values("2", "3"));
+                         ::testing::Values("2", "3"));
 
 TEST_F(HSetFamilyTest, Basic) {
   auto resp = Run({"hset", "x", "a"});

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<size_t, size_t, double>();
+    AllocationTracker::TrackingInfo tracking_info;
+    std::tie(tracking_info.lower_bound, tracking_info.upper_bound, tracking_info.sample_odds) =
+        parser.Next<size_t, size_t, double>();
     if (parser.HasError()) {
       return cntx_->SendError(parser.Error()->MakeReply());
     }
 
     atomic_bool error{false};
     shard_set->pool()->Await([&](unsigned index, auto*) {
-      if (!AllocationTracker::Get().Add(
+      if (!AllocationTracker::Get().Add(tracking_info)) {
-              {.lower_bound = lower_bound, .upper_bound = upper_bound, .sample_odds = odds})) {
         error.store(true);
       }
     });
@@ -359,8 +360,8 @@ void MemoryCmd::Track(CmdArgList args) {
     }
 
     atomic_bool error{false};
-    shard_set->pool()->Await([&](unsigned index, auto*) {
+    shard_set->pool()->Await([&, lo = lower_bound, hi = upper_bound](unsigned index, auto*) {
-      if (!AllocationTracker::Get().Remove(lower_bound, upper_bound)) {
+      if (!AllocationTracker::Get().Remove(lo, hi)) {
         error.store(true);
       }
     });

src/server/multi_test.cc

@@ -638,7 +638,7 @@ TEST_F(MultiTest, MultiCauseUnblocking) {
   const int kRounds = 10;
   vector<string> 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--)

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") {

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__

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)};
+  JsonPathContainer path_container{std::move(path_expr)};
-  auto ptr = make_unique<JsonPathContainer>(move(path_container));
+  auto ptr = make_unique<JsonPathContainer>(std::move(path_container));
 
   JsonPathContainer* path = ptr.get();
-  path_cache_[field] = move(ptr);
+  path_cache_[field] = std::move(ptr);
   return path;
 }
 

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<DocIndex> index_ptr) {
   auto shard_index = make_unique<ShardDocIndex>(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

src/server/search/doc_index.h

@@ -175,7 +175,11 @@ class ShardDocIndices {
   absl::flat_hash_map<std::string, std::unique_ptr<ShardDocIndex>> 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

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) {

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);

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;

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)