MB-67283: Add utilities/fusion_support.h to reduce code complexity

trondn · mohammadzaeem · commit 75bcafaa5df4 · 2025-07-11T09:33:18.000Z
The memcached core gets compiled with (EE) and without (CE) magma available, and fusion is only supported on a subset of EE platforms. To avoid cluttering the code with ifdef's making the code harder to read we hide the difference in a single file and provide a stub for the magma functions so that we don't need a preprocessor macro. Change-Id: Ia101355befcd8c5428ed1a489598380af413b967 Reviewed-on: https://review.couchbase.org/c/kv_engine/+/229523 Tested-by: Build Bot <build@couchbase.com> Reviewed-by: Faizan Alam <faizan.alam@couchbase.com>
diff --git a/daemon/CMakeLists.txt b/daemon/CMakeLists.txt
@@ -117,15 +117,6 @@ if (NUMA_FOUND)
     target_link_libraries(memcached_daemon PRIVATE ${NUMA_LIBRARIES})
 endif()
 
-if (TARGET fusionfs)
-    target_compile_definitions(memcached_daemon
-            PRIVATE -DUSE_FUSION=1)
-    target_include_directories(memcached_daemon
-            SYSTEM PRIVATE ${MAGMA_INCLUDE_DIR})
-    target_link_libraries(memcached_daemon
-            PRIVATE magma)
-endif()
-
 target_include_directories(memcached_daemon PRIVATE ${Memcached_BINARY_DIR})
 cb_enable_unity_build(memcached_daemon)
 
@@ -149,6 +140,7 @@ target_link_libraries(memcached_daemon
                       ep
                       ewouldblock_engine
                       folly_io_callbacks
+                      fusion_support
                       mcd_dek
                       mcd_executor
                       mcd_tracing
diff --git a/daemon/mcbp_executors.cc b/daemon/mcbp_executors.cc
@@ -70,6 +70,7 @@
 #include <nlohmann/json.hpp>
 #include <serverless/config.h>
 #include <utilities/engine_errc_2_mcbp.h>
+#include <utilities/fusion_support.h>
 #include <utilities/throttle_utilities.h>
 
 /**
@@ -705,11 +706,20 @@ static void set_chronicle_auth_token_executor(Cookie& cookie) {
 }
 
 static void delete_fusion_namespace_executor(Cookie& cookie) {
-    cookie.obtainContext<DeleteFusionNamespaceCommandContext>(cookie).drive();
+    if (isFusionSupportEnabled()) {
+        cookie.obtainContext<DeleteFusionNamespaceCommandContext>(cookie)
+                .drive();
+    } else {
+        cookie.sendResponse(cb::mcbp::Status::NotSupported);
+    }
 }
 
 static void get_fusion_namespaces_executor(Cookie& cookie) {
-    cookie.obtainContext<GetFusionNamespacesCommandContext>(cookie).drive();
+    if (isFusionSupportEnabled()) {
+        cookie.obtainContext<GetFusionNamespacesCommandContext>(cookie).drive();
+    } else {
+        cookie.sendResponse(cb::mcbp::Status::NotSupported);
+    }
 }
 
 static void process_bin_noop_response(Cookie&) {
diff --git a/daemon/memcached.cc b/daemon/memcached.cc
@@ -43,9 +43,6 @@
 #include <folly/io/async/EventBase.h>
 #include <folly/portability/Unistd.h>
 #include <gsl/gsl-lite.hpp>
-#ifdef USE_FUSION
-#include <libmagma/magma.h>
-#endif
 #include <mcbp/mcbp.h>
 #include <memcached/rbac.h>
 #include <nlohmann/json.hpp>
@@ -62,6 +59,7 @@
 #include <serverless/config.h>
 #include <statistics/prometheus.h>
 #include <utilities/breakpad.h>
+#include <utilities/fusion_support.h>
 #include <chrono>
 #include <csignal>
 #include <cstdlib>
@@ -492,7 +490,6 @@ static void settings_init() {
                         s.getMaxConcurrentAuthentications());
             });
 
-#ifdef USE_FUSION
     settings.addChangeListener("fusion_migration_rate_limit",
                                [](const auto&, auto& s) {
                                    magma::Magma::SetFusionMigrationRateLimit(
@@ -503,7 +500,6 @@ static void settings_init() {
                                    magma::Magma::SetFusionSyncRateLimit(
                                            s.getFusionSyncRateLimit());
                                });
-#endif
 }
 
 /**
diff --git a/daemon/protocol/mcbp/CMakeLists.txt b/daemon/protocol/mcbp/CMakeLists.txt
@@ -135,6 +135,7 @@ target_link_libraries(memcached_daemon_mcbp
         PRIVATE
         memcached_daemon_protocol_utils
         Folly::folly
+        fusion_support
         hdrhistogram
         memcached_logger
         nlohmann_json::nlohmann_json
@@ -143,12 +144,3 @@ target_link_libraries(memcached_daemon_mcbp
         sigar_cpp
         statistics
         subjson)
-
-if (TARGET fusionfs)
-    target_compile_definitions(memcached_daemon_mcbp
-            PRIVATE -DUSE_FUSION=1)
-    target_include_directories(memcached_daemon_mcbp
-            SYSTEM PRIVATE ${MAGMA_INCLUDE_DIR})
-    target_link_libraries(memcached_daemon_mcbp
-            PRIVATE magma)
-endif()
diff --git a/daemon/protocol/mcbp/delete_fusion_namespace_command_context.cc b/daemon/protocol/mcbp/delete_fusion_namespace_command_context.cc
@@ -9,14 +9,11 @@
  */
 
 #include "delete_fusion_namespace_command_context.h"
-
 #include <daemon/concurrency_semaphores.h>
 #include <daemon/cookie.h>
 #include <executor/globaltask.h>
-#ifdef USE_FUSION
-#include <libmagma/magma.h>
-#endif
 #include <logger/logger.h>
+#include <utilities/fusion_support.h>
 
 DeleteFusionNamespaceCommandContext::DeleteFusionNamespaceCommandContext(
         Cookie& cookie)
@@ -29,7 +26,6 @@ DeleteFusionNamespaceCommandContext::DeleteFusionNamespaceCommandContext(
 }
 
 cb::engine_errc DeleteFusionNamespaceCommandContext::execute() {
-#ifdef USE_FUSION
     const auto& req = cookie.getRequest();
     const auto request = nlohmann::json::parse(req.getValueString());
     const auto logstore = request["logstore_uri"];
@@ -38,16 +34,13 @@ cb::engine_errc DeleteFusionNamespaceCommandContext::execute() {
     const auto ns = request["namespace"];
     const auto status = magma::Magma::DeleteFusionNamespace(
             logstore, metadatastore, token, ns);
-    if (status.ErrorCode() != magma::Status::Code::Ok) {
-        LOG_WARNING_CTX("DeleteFusionNamespace: ",
-                        {"logstore_uri", logstore},
-                        {"metadatastore_uri", metadatastore},
-                        {"namespace", ns},
-                        {"error", status.String()});
-        return cb::engine_errc::failed;
+    if (status.IsOK()) {
+        return cb::engine_errc::success;
     }
-    return cb::engine_errc::success;
-#else
-    return cb::engine_errc::not_supported;
-#endif
+    LOG_WARNING_CTX("DeleteFusionNamespace",
+                    {"logstore_uri", logstore},
+                    {"metadatastore_uri", metadatastore},
+                    {"namespace", ns},
+                    {"error", status.String()});
+    return cb::engine_errc::failed;
 }
diff --git a/daemon/protocol/mcbp/get_fusion_namespaces_command_context.cc b/daemon/protocol/mcbp/get_fusion_namespaces_command_context.cc
@@ -9,15 +9,11 @@
  */
 
 #include "get_fusion_namespaces_command_context.h"
-
 #include <daemon/concurrency_semaphores.h>
 #include <daemon/cookie.h>
 #include <executor/globaltask.h>
-#ifdef USE_FUSION
-#include <libmagma/magma.h>
-#endif
-
 #include <logger/logger.h>
+#include <utilities/fusion_support.h>
 #include <utilities/fusion_utilities.h>
 
 GetFusionNamespacesCommandContext::GetFusionNamespacesCommandContext(
@@ -31,7 +27,6 @@ GetFusionNamespacesCommandContext::GetFusionNamespacesCommandContext(
 }
 
 cb::engine_errc GetFusionNamespacesCommandContext::execute() {
-#ifdef USE_FUSION
     const auto& req = cookie.getRequest();
     const auto request = nlohmann::json::parse(req.getValueString());
     const auto metadatastore = request["metadatastore_uri"];
@@ -44,19 +39,15 @@ cb::engine_errc GetFusionNamespacesCommandContext::execute() {
                                               token,
                                               magma_fusion_namespace_prefix,
                                               namespaceDepth);
-    if (status.ErrorCode() != magma::Status::Code::Ok) {
-        LOG_WARNING_CTX("GetFusionNamespaces: ",
-                        {"metadatastore_uri", metadatastore},
-                        {"error", status.String()});
-        cookie.setErrorContext(
-                fmt::format("Failed with error: {}", status.String()));
-        return cb::engine_errc::failed;
+    if (status.IsOK()) {
+        response = json.dump();
+        datatype = cb::mcbp::Datatype::JSON;
+        return cb::engine_errc::success;
     }
-    response = json.dump();
-    datatype = cb::mcbp::Datatype::JSON;
-
-    return cb::engine_errc::success;
-#else
-    return cb::engine_errc::not_supported;
-#endif
-}
+    LOG_WARNING_CTX("GetFusionNamespaces",
+                    {"metadatastore_uri", metadatastore},
+                    {"error", status.String()});
+    cookie.setErrorContext(
+            fmt::format("Failed with error: {}", status.String()));
+    return cb::engine_errc::failed;
+}
diff --git a/daemon/protocol/mcbp/stats_context.cc b/daemon/protocol/mcbp/stats_context.cc
@@ -39,6 +39,7 @@
 #include <statistics/cbstat_collector.h>
 #include <statistics/labelled_collector.h>
 #include <utilities/engine_errc_2_mcbp.h>
+#include <utilities/fusion_support.h>
 #include <utilities/string_utilities.h>
 #include <chrono>
 #include <cinttypes>
@@ -690,10 +691,13 @@ static cb::engine_errc stat_scheduler_executor(const std::string& arg,
 
 static cb::engine_errc stat_fusion_executor(const std::string& arg,
                                             Cookie& cookie) {
-    return bucket_get_stats(cookie,
-                            cookie.getRequest().getKeyString(),
-                            cookie.getRequest().getValueString(),
-                            appendStatsFn);
+    if (isFusionSupportEnabled()) {
+        return bucket_get_stats(cookie,
+                                cookie.getRequest().getKeyString(),
+                                cookie.getRequest().getValueString(),
+                                appendStatsFn);
+    }
+    return cb::engine_errc::not_supported;
 }
 
 /***************************** STAT HANDLERS *****************************/
diff --git a/daemon/settings.cc b/daemon/settings.cc
@@ -20,6 +20,7 @@
 #include <platform/dirutils.h>
 #include <platform/json_log_conversions.h>
 #include <platform/timeutils.h>
+#include <utilities/fusion_support.h>
 #include <utilities/json_utilities.h>
 #include <utilities/logtags.h>
 #include <algorithm>
@@ -396,9 +397,21 @@ void Settings::reconfigure(const nlohmann::json& json) {
         } else if (key == "max_concurrent_commands_per_connection"sv) {
             setMaxConcurrentCommandsPerConnection(value.get<size_t>());
         } else if (key == "fusion_migration_rate_limit"sv) {
-            setFusionMigrationRateLimit(value.get<size_t>());
+            if (isFusionSupportEnabled()) {
+                setFusionMigrationRateLimit(value.get<size_t>());
+            } else {
+                LOG_WARNING_CTX("Ignore fusion_migration_rate_limit",
+                                {"value", value.dump()},
+                                {"reason", "Fusion support is not enabled"});
+            }
         } else if (key == "fusion_sync_rate_limit") {
-            setFusionSyncRateLimit(value.get<size_t>());
+            if (isFusionSupportEnabled()) {
+                setFusionSyncRateLimit(value.get<size_t>());
+            } else {
+                LOG_WARNING_CTX("Ignore fusion_sync_rate_limit",
+                                {"value", value.dump()},
+                                {"reason", "Fusion support is not enabled"});
+            }
         } else if (key == "phosphor_config"sv) {
             auto config = value.get<std::string>();
             // throw an exception if the config is invalid
diff --git a/daemon/stats.cc b/daemon/stats.cc
@@ -20,9 +20,6 @@
 #include "settings.h"
 #include <fmt/chrono.h>
 #include <folly/Chrono.h>
-#ifdef USE_FUSION
-#include <libmagma/magma.h>
-#endif
 #include <logger/logger.h>
 #include <platform/cb_arena_malloc.h>
 #include <platform/timeutils.h>
@@ -32,6 +29,7 @@
 #include <statistics/labelled_collector.h>
 #include <statistics/prometheus.h>
 #include <statistics/prometheus_collector.h>
+#include <utilities/fusion_support.h>
 #include <utilities/string_utilities.h>
 #include <string_view>
 
@@ -67,12 +65,13 @@ static void server_global_stats(const StatCollector& collector) {
         collector.addStat(Key::connection_structures, stats.conn_structs);
         collector.addStat(Key::curr_connections_closing,
                           stats.curr_conn_closing);
-#ifdef USE_FUSION
-        collector.addStat(Key::fusion_migration_rate_limit,
-                          magma::Magma::GetFusionMigrationRateLimit());
-        collector.addStat(Key::fusion_sync_rate_limit,
-                          magma::Magma::GetFusionSyncRateLimit());
-#endif
+        if (isFusionSupportEnabled()) {
+            collector.addStat(Key::fusion_migration_rate_limit,
+                              magma::Magma::GetFusionMigrationRateLimit());
+            collector.addStat(Key::fusion_sync_rate_limit,
+                              magma::Magma::GetFusionSyncRateLimit());
+        }
+
         auto sdks = SdkConnectionManager::instance().getConnectedSdks();
         for (const auto& [key, value] : sdks) {
             collector.withLabels({{"sdk", key}})
diff --git a/engines/ep/CMakeLists.txt b/engines/ep/CMakeLists.txt
@@ -426,6 +426,7 @@ target_link_libraries(ep
         cbcompress
         engine_utilities
         ep-engine_collections
+        fusion_support
         mcbp
         mc_client_connection
         mcd_executor
diff --git a/engines/ep/src/ep_engine.cc b/engines/ep/src/ep_engine.cc
@@ -85,6 +85,7 @@
 #include <statistics/labelled_collector.h>
 #include <statistics/prometheus.h>
 #include <utilities/engine_errc_2_mcbp.h>
+#include <utilities/fusion_support.h>
 #include <utilities/fusion_utilities.h>
 #include <utilities/logtags.h>
 #include <utilities/math_utilities.h>
@@ -1985,13 +1986,12 @@ cb::engine::FeatureSet EventuallyPersistentEngine::getFeatures() {
     if (configuration.getBucketTypeString() == "ephemeral") {
         return {cb::engine::Feature::Collections};
     }
-#ifdef USE_FUSION
-    return {cb::engine::Feature::Collections,
-            cb::engine::Feature::Persistence,
-            cb::engine::Feature::Fusion};
-#else
+    if (isFusionSupportEnabled()) {
+        return {cb::engine::Feature::Collections,
+                cb::engine::Feature::Persistence,
+                cb::engine::Feature::Fusion};
+    }
     return {cb::engine::Feature::Collections, cb::engine::Feature::Persistence};
-#endif
 }
 
 bool EventuallyPersistentEngine::isXattrEnabled() {
diff --git a/engines/ep/tests/CMakeLists.txt b/engines/ep/tests/CMakeLists.txt
@@ -152,6 +152,7 @@ cb_add_test_executable(ep-engine_ep_unit_tests
         ${Memcached_SOURCE_DIR}/logger/logger_test_fixture.cc)
 kv_enable_pch(ep-engine_ep_unit_tests)
 IF(EP_USE_MAGMA)
+    target_compile_definitions(ep-engine_ep_unit_tests PUBLIC HAVE_MAGMA_SUPPORT=1)
     target_sources(ep-engine_ep_unit_tests PUBLIC
             module_tests/continuous_backup_test.cc
             module_tests/history_dcp_test.cc
diff --git a/engines/ep/tests/module_tests/magma_bucket_tests.cc b/engines/ep/tests/module_tests/magma_bucket_tests.cc
@@ -24,6 +24,7 @@
 #include "vbucket.h"
 
 #include <platform/cb_arena_malloc.h>
+#include <utilities/fusion_support.h>
 #include <utilities/test_manifest.h>
 
 #include <folly/synchronization/Baton.h>
@@ -1158,8 +1159,6 @@ INSTANTIATE_TEST_SUITE_P(STParamMagmaBucketTest,
                          STParameterizedBucketTest::magmaConfigValues(),
                          STParameterizedBucketTest::PrintToStringParamName);
 
-#ifdef USE_FUSION
-
 class STMagmaFusionTest : public STParamMagmaBucketTest {
 public:
     void SetUp() override {
@@ -1174,6 +1173,9 @@ class STMagmaFusionTest : public STParamMagmaBucketTest {
         config_string += ";magma_fusion_logstore_fragmentation_threshold=" +
                          std::to_string(fusionLogstoreFragmentationThreshold);
         STParameterizedBucketTest::SetUp();
+        if (!isFusionSupportEnabled()) {
+            GTEST_SKIP() << "Fusion support is not enabled";
+        }
     }
 
 protected:
@@ -1473,5 +1475,3 @@ INSTANTIATE_TEST_SUITE_P(STMagmaFusionTest,
                          STMagmaFusionTest,
                          STParameterizedBucketTest::magmaConfigValues(),
                          STParameterizedBucketTest::PrintToStringParamName);
-
-#endif // USE_FUSION
diff --git a/tests/testapp/testapp_fusion.cc b/tests/testapp/testapp_fusion.cc
diff --git a/utilities/CMakeLists.txt b/utilities/CMakeLists.txt
diff --git a/utilities/fusion_support.cc b/utilities/fusion_support.cc
diff --git a/utilities/fusion_support.h b/utilities/fusion_support.h
