MB-67265: Replace check for fusionLogstoreURI with ...

... isFusionSupportEnabled

We have recently allowed initialising MagmaKVStore before the
logstore URI is set. Therefore, it is possible that fusionStats
are updated before the URI is set. This patch replaces the check
for fusionLogstoreURI with isFusionSupportIsEnabled().

Along with this change, we can remove the fusion stat test from
testapp_fusion.cc and add it to ep_testsuite.cc, such that all
stats are in a single place.

Change-Id: I2e65d82e53c903e081a4dc8ef843ccbb44b21fed
Reviewed-on: https://review.couchbase.org/c/kv_engine/+/235270
Reviewed-by: Trond Norbye <[email protected]>
Tested-by: Build Bot <[email protected]>

Author: mohammadzaeem

engines/ep/src/ep_engine.cc

@@ -3320,7 +3320,7 @@ cb::engine_errc EventuallyPersistentEngine::doEngineStatsLowCardinality(
         doEngineStatsCouchDB(collector, epstats);
     } else if (configuration.getBackendString() == "magma") {
         doEngineStatsMagma(collector);
-        if (!configuration.getMagmaFusionLogstoreUri().empty()) {
+        if (isFusionSupportEnabled()) {
             doEngineStatsFusion(collector);
         }
     } else if (configuration.getBackendString() == "nexus") {

engines/ep/tests/ep_testsuite.cc

@@ -36,6 +36,7 @@
 #include <programs/engine_testapp/mock_cookie.h>
 #include <programs/engine_testapp/mock_engine.h>
 #include <programs/engine_testapp/mock_server.h>
+#include <utilities/fusion_support.h>
 #include <utilities/string_utilities.h>
 #include <xattr/blob.h>
 #include <chrono>
@@ -7194,6 +7195,41 @@ static enum test_result test_mb19687_fixed(EngineIface* h) {
         }
     }
 
+    if (isMagmaBucket(h) && isFusionSupportEnabled()) {
+        auto& eng_stats = statsKeys.at("");
+        eng_stats.insert(eng_stats.end(),
+                         {"ep_fusion_namespace",
+                          "ep_fusion_syncs",
+                          "ep_fusion_bytes_synced",
+                          "ep_fusion_logs_migrated",
+                          "ep_fusion_bytes_migrated",
+                          "ep_fusion_log_store_data_size",
+                          "ep_fusion_log_store_garbage_size",
+                          "ep_fusion_logs_cleaned",
+                          "ep_fusion_log_clean_bytes_read",
+                          "ep_fusion_extent_merger_reads",
+                          "ep_fusion_extent_merger_bytes_read",
+                          "ep_fusion_log_clean_reads",
+                          "ep_fusion_log_store_remote_puts",
+                          "ep_fusion_log_store_reads",
+                          "ep_fusion_log_store_remote_gets",
+                          "ep_fusion_log_store_remote_lists",
+                          "ep_fusion_log_store_remote_deletes",
+                          "ep_fusion_file_map_mem_used",
+                          "ep_fusion_sync_failures",
+                          "ep_fusion_migration_failures",
+                          "ep_fusion_num_logs_mounted",
+                          "ep_fusion_num_log_segments",
+                          "ep_fusion_num_file_extents",
+                          "ep_fusion_num_files",
+                          "ep_fusion_total_file_size",
+                          "ep_fusion_sync_session_total_bytes",
+                          "ep_fusion_sync_session_completed_bytes",
+                          "ep_fusion_migration_total_bytes",
+                          "ep_fusion_migration_completed_bytes",
+                          "ep_fusion_log_store_pending_delete_size"});
+    }
+
     if (isPersistentBucket(h)) {
         auto& vb_details = statsKeys.at("vbucket-details 0");
         vb_details.insert(vb_details.end(),

tests/testapp/testapp_fusion.cc

@@ -928,83 +928,6 @@ TEST_P(FusionTest, Stat_UploaderState_KVStoreInvalid) {
     }
 }
 
-TEST_P(FusionTest, GetPrometheusFusionStats) {
-    if (!isFusionSupportedInBucket()) {
-        GTEST_SKIP() << "Fusion is not supported in this bucket";
-    }
-    std::array<std::string_view, 30> statKeysExpected = {
-            "ep_fusion_namespace",
-            "ep_fusion_syncs",
-            "ep_fusion_bytes_synced",
-            "ep_fusion_logs_migrated",
-            "ep_fusion_bytes_migrated",
-            "ep_fusion_log_store_data_size",
-            "ep_fusion_log_store_garbage_size",
-            "ep_fusion_logs_cleaned",
-            "ep_fusion_log_clean_bytes_read",
-            "ep_fusion_extent_merger_reads",
-            "ep_fusion_extent_merger_bytes_read",
-            "ep_fusion_log_clean_reads",
-            "ep_fusion_log_store_remote_puts",
-            "ep_fusion_log_store_reads",
-            "ep_fusion_log_store_remote_gets",
-            "ep_fusion_log_store_remote_lists",
-            "ep_fusion_log_store_remote_deletes",
-            "ep_fusion_file_map_mem_used",
-            "ep_fusion_sync_failures",
-            "ep_fusion_migration_failures",
-            "ep_fusion_num_logs_mounted",
-            "ep_fusion_num_log_segments",
-            "ep_fusion_num_file_extents",
-            "ep_fusion_num_files",
-            "ep_fusion_total_file_size",
-            "ep_fusion_sync_session_total_bytes",
-            "ep_fusion_sync_session_completed_bytes",
-            "ep_fusion_migration_total_bytes",
-            "ep_fusion_migration_completed_bytes",
-            "ep_fusion_log_store_pending_delete_size",
-    };
-    std::vector<std::string> actualKeys;
-    connection->stats(
-            [&actualKeys](auto& k, auto& v) {
-                if (k.starts_with("ep_fusion")) {
-                    actualKeys.emplace_back(k);
-                }
-            },
-            ""); // we convert empty to null to get engine stats
-
-    // Sort both collections
-    std::ranges::sort(actualKeys);
-    std::ranges::sort(statKeysExpected);
-
-    // Find missing keys that are expected but not found in actual
-    std::vector<std::string_view> missingKeys;
-    std::ranges::set_difference(
-            statKeysExpected,
-            actualKeys,
-            std::inserter(missingKeys, missingKeys.begin()));
-
-    bool error = false;
-    for (const auto& key : missingKeys) {
-        error = true;
-        fmt::println(stderr, "Expected {} but not found in actual", key);
-    }
-
-    // Find any extra fusion stats - those that are found in actual but not
-    // expected
-    std::vector<std::string_view> extraKeys;
-    std::ranges::set_difference(actualKeys,
-                                statKeysExpected,
-                                std::inserter(extraKeys, extraKeys.begin()));
-
-    for (const auto& key : extraKeys) {
-        error = true;
-        fmt::println(stderr, "Found stat {} but was not expected", key);
-    }
-
-    EXPECT_EQ(false, error) << "Missing stats found";
-}
-
 TEST_P(FusionTest, DeleteInvalidFusionNamespace) {
     auto cmd = BinprotGenericCommand{
             cb::mcbp::ClientOpcode::DeleteFusionNamespace};