[Store] Structure ClientNumaConfig environment settings

mooncake-common/include/environment_variables.h

@@ -50,6 +50,12 @@ struct ClientAutoPortEnvironmentVariables {
     MC_DEFINE_ENV_VAR(int, MC_STORE_CLIENT_MAX_PORT);
 };
 
+struct ClientNumaEnvironmentVariables {
+    // Keep the raw string to preserve the legacy strtol syntax and warning
+    // behavior.
+    MC_DEFINE_ENV_VAR(std::string, MC_STORE_NUMA_SOCKET_ID);
+};
+
 struct RegisteredPinnedMemoryEnvironmentVariables {
     // Keep the raw string because the legacy parser rejects a leading '+',
     // unlike the shared typed integer parser.

mooncake-store/src/CMakeLists.txt

@@ -100,6 +100,7 @@ set(MOONCAKE_STORE_CLIENT_SOURCES
     config/file_storage_config.cpp
     config/file_per_key_config.cpp
     config/client_auto_port_config.cpp
+    config/client_numa_config.cpp
     config/local_hot_cache_config.cpp
     config/offset_allocator_backend_config.cpp
     config/replica_selection_config.cpp

mooncake-store/src/client_service.cpp

@@ -47,6 +47,7 @@
 #endif
 #include "crc_checksum.h"
 #include "environ.h"
+#include "config/client_numa_config.h"
 #include "storage/distributed/distributed_storage_backend.h"
 
 namespace mooncake {
@@ -81,26 +82,6 @@ class ScopedObjectChecksumBuffer {
 };
 
 #ifdef USE_NOF
-std::optional<int> GetConfiguredNumaSocketId() {
-    const char* raw_value = std::getenv("MC_STORE_NUMA_SOCKET_ID");
-    if (!raw_value || raw_value[0] == '\0') {
-        return std::nullopt;
-    }
-
-    char* end_ptr = nullptr;
-    errno = 0;
-    long parsed = std::strtol(raw_value, &end_ptr, 10);
-    if (errno != 0 || end_ptr == raw_value ||
-        (end_ptr != nullptr && *end_ptr != '\0') || parsed < 0 ||
-        parsed > std::numeric_limits<int>::max()) {
-        LOG(WARNING) << "Invalid MC_STORE_NUMA_SOCKET_ID=" << raw_value
-                     << ", falling back to auto-detect";
-        return std::nullopt;
-    }
-
-    return static_cast<int>(parsed);
-}
-
 int GetCurrentNumaSocketId() {
     if (numa_available() < 0) {
         return 0;
@@ -1010,8 +991,9 @@ void Client::InitTransferSubmitter() {
     // Keep using logical local_hostname for name-based behaviors; endpoint is
     // used separately where needed.
 #ifdef USE_NOF
-    int numa_socket_id =
-        GetConfiguredNumaSocketId().value_or(GetCurrentNumaSocketId());
+    const int numa_socket_id =
+        ClientNumaConfig::FromEnvironment().socket_id.value_or(
+            GetCurrentNumaSocketId());
     transfer_submitter_ = std::make_unique<TransferSubmitter>(
         *transfer_engine_, storage_backend_, local_hostname_,
         metrics_ ? &metrics_->transfer_metric : nullptr, numa_socket_id);

mooncake-store/src/config/client_numa_config.cpp

@@ -0,0 +1,38 @@
+#include "client_numa_config.h"
+
+#include <cerrno>
+#include <cstdlib>
+#include <limits>
+
+#include <glog/logging.h>
+
+#include "environ.h"
+#include "environment_variables.h"
+
+namespace mooncake {
+
+ClientNumaConfig ClientNumaConfig::FromEnvironment() {
+    ClientNumaConfig config;
+    using Variables = ClientNumaEnvironmentVariables;
+
+    const auto raw_value = Environ::Read(Variables::MC_STORE_NUMA_SOCKET_ID);
+    if (!raw_value.has_value() || raw_value->empty()) {
+        return config;
+    }
+
+    char* end_ptr = nullptr;
+    errno = 0;
+    const long parsed = std::strtol(raw_value->c_str(), &end_ptr, 10);
+    if (errno != 0 || end_ptr == raw_value->c_str() ||
+        (end_ptr != nullptr && *end_ptr != '\0') || parsed < 0 ||
+        parsed > std::numeric_limits<int>::max()) {
+        LOG(WARNING) << "Invalid MC_STORE_NUMA_SOCKET_ID=" << *raw_value
+                     << ", falling back to auto-detect";
+        return config;
+    }
+
+    config.socket_id = static_cast<int>(parsed);
+    return config;
+}
+
+}  // namespace mooncake

mooncake-store/src/config/client_numa_config.h

@@ -0,0 +1,13 @@
+#pragma once
+
+#include <optional>
+
+namespace mooncake {
+
+struct ClientNumaConfig {
+    std::optional<int> socket_id;
+
+    static ClientNumaConfig FromEnvironment();
+};
+
+}  // namespace mooncake

mooncake-store/tests/CMakeLists.txt

@@ -189,6 +189,7 @@ add_store_test(hugepage_size_env_test hugepage_size_env_test.cpp)
 add_store_test(random_test random_test.cpp)
 add_store_test(client_buffer_test client_buffer_test.cpp)
 add_store_test(client_auto_port_config_test client_auto_port_config_test.cpp)
+add_store_test(client_numa_config_test client_numa_config_test.cpp)
 add_store_test(local_hot_cache_config_test local_hot_cache_config_test.cpp)
 add_store_test(client_local_hot_cache_test client_local_hot_cache_test.cpp)
 add_store_test(client_tcp_local_memcpy_test client_tcp_local_memcpy_test.cpp)

mooncake-store/tests/client_numa_config_test.cpp

@@ -0,0 +1,97 @@
+#include <gtest/gtest.h>
+
+#include <cstdlib>
+#include <limits>
+#include <optional>
+#include <string>
+
+#include "../src/config/client_numa_config.h"
+
+namespace mooncake {
+namespace {
+
+class ClientNumaConfigTest : public ::testing::Test {
+   protected:
+    void SetUp() override {
+        if (const char* value = std::getenv(kEnvironmentVariable)) {
+            original_ = value;
+        }
+        ASSERT_EQ(unsetenv(kEnvironmentVariable), 0);
+    }
+
+    void TearDown() override {
+        if (original_.has_value()) {
+            EXPECT_EQ(setenv(kEnvironmentVariable, original_->c_str(), 1), 0);
+        } else {
+            EXPECT_EQ(unsetenv(kEnvironmentVariable), 0);
+        }
+    }
+
+    void Set(const char* value) {
+        ASSERT_EQ(setenv(kEnvironmentVariable, value, 1), 0);
+    }
+
+    static constexpr const char* kEnvironmentVariable =
+        "MC_STORE_NUMA_SOCKET_ID";
+
+   private:
+    std::optional<std::string> original_;
+};
+
+TEST_F(ClientNumaConfigTest, UnsetAndEmptyUseAutoDetectionWithoutWarning) {
+    ::testing::internal::CaptureStderr();
+    EXPECT_FALSE(ClientNumaConfig::FromEnvironment().socket_id.has_value());
+    EXPECT_TRUE(::testing::internal::GetCapturedStderr().empty());
+
+    Set("");
+    ::testing::internal::CaptureStderr();
+    EXPECT_FALSE(ClientNumaConfig::FromEnvironment().socket_id.has_value());
+    EXPECT_TRUE(::testing::internal::GetCapturedStderr().empty());
+}
+
+TEST_F(ClientNumaConfigTest, PreservesAcceptedDecimalSyntax) {
+    struct Case {
+        const char* value;
+        int expected;
+    };
+    const Case cases[] = {{"0", 0},
+                          {"7", 7},
+                          {"+7", 7},
+                          {" \t7", 7},
+                          {"2147483647", std::numeric_limits<int>::max()}};
+
+    for (const auto& entry : cases) {
+        SCOPED_TRACE(entry.value);
+        Set(entry.value);
+        EXPECT_EQ(ClientNumaConfig::FromEnvironment().socket_id,
+                  entry.expected);
+    }
+}
+
+TEST_F(ClientNumaConfigTest, InvalidValuesWarnAndUseAutoDetection) {
+    for (const char* value :
+         {"-1", "7suffix", "7 ", "2147483648", "999999999999999999999999"}) {
+        SCOPED_TRACE(value);
+        Set(value);
+        ::testing::internal::CaptureStderr();
+        EXPECT_FALSE(ClientNumaConfig::FromEnvironment().socket_id.has_value());
+        const std::string logs = ::testing::internal::GetCapturedStderr();
+        EXPECT_NE(logs.find(std::string("Invalid MC_STORE_NUMA_SOCKET_ID=") +
+                            value + ", falling back to auto-detect"),
+                  std::string::npos);
+    }
+}
+
+TEST_F(ClientNumaConfigTest, EachConfigReadsCurrentEnvironment) {
+    Set("1");
+    const auto first = ClientNumaConfig::FromEnvironment();
+
+    Set("2");
+    const auto second = ClientNumaConfig::FromEnvironment();
+
+    EXPECT_EQ(first.socket_id, 1);
+    EXPECT_EQ(second.socket_id, 2);
+}
+
+}  // namespace
+}  // namespace mooncake