[Store] Report why Client::Create failed

mooncake-store/include/client_service.h

@@ -126,10 +126,10 @@ class Client {
      * @param master_server_entry The entry of master server (IP:Port of master
      *        address for non-HA mode, or <backend>://connstring for HA mode,
      *        e.g. etcd://IP:Port;IP:Port;...;IP:Port)
-     * @return std::optional containing a shared_ptr to Client if successful,
-     * std::nullopt otherwise
+     * @return A shared_ptr to Client if successful, otherwise the error of
+     * the step that failed
      */
-    static std::optional<std::shared_ptr<Client>> Create(
+    static tl::expected<std::shared_ptr<Client>, ErrorCode> Create(
         const std::string& local_hostname,
         const std::string& metadata_connstring, const std::string& protocol,
         const std::optional<std::string>& device_names = std::nullopt,

mooncake-store/src/client_service.cpp

@@ -1177,7 +1177,7 @@ std::shared_ptr<NVMeoFInitiator> Client::GetNofInitiatorForTesting() const {
     return nof_initiator_;
 }
 
-std::optional<std::shared_ptr<Client>> Client::Create(
+tl::expected<std::shared_ptr<Client>, ErrorCode> Client::Create(
     const std::string& local_hostname, const std::string& metadata_connstring,
     const std::string& protocol, const std::optional<std::string>& device_names,
     const std::string& master_server_entry,
@@ -1203,7 +1203,7 @@ std::optional<std::shared_ptr<Client>> Client::Create(
 
     ErrorCode err = client->ConnectToMaster(master_server_entry);
     if (err != ErrorCode::OK) {
-        return std::nullopt;
+        return tl::unexpected(err);
     }
 
     // Initialize storage backend if storage_root_dir is valid
@@ -1234,11 +1234,11 @@ std::optional<std::shared_ptr<Client>> Client::Create(
                         << "Failed to initialize storage backend: " << prep_err
                         << ". Persistence was requested via fsdir but is "
                            "unavailable.";
-                    return std::nullopt;
+                    return tl::unexpected(prep_err);
                 }
             } else {
                 LOG(ERROR) << "Invalid fsdir format: " << dir_string;
-                return std::nullopt;
+                return tl::unexpected(ErrorCode::INVALID_PARAMS);
             }
         }
     } else {
@@ -1266,11 +1266,11 @@ std::optional<std::shared_ptr<Client>> Client::Create(
                         << "Failed to initialize storage backend: " << prep_err
                         << ". Persistence was requested via storage config "
                            "but is unavailable.";
-                    return std::nullopt;
+                    return tl::unexpected(prep_err);
                 }
             } else {
                 LOG(ERROR) << "Invalid fsdir format: " << config.fsdir;
-                return std::nullopt;
+                return tl::unexpected(ErrorCode::INVALID_PARAMS);
             }
         }
     }
@@ -1290,7 +1290,7 @@ std::optional<std::shared_ptr<Client>> Client::Create(
                                          protocol, device_names);
         if (err != ErrorCode::OK) {
             LOG(ERROR) << "Failed to initialize transfer engine";
-            return std::nullopt;
+            return tl::unexpected(err);
         }
     } else {
         client->transfer_engine_ = transfer_engine;

mooncake-store/src/real_client.cpp

@@ -1131,14 +1131,16 @@ tl::expected<void, ErrorCode> RealClient::setup_internal(
             device_name, resolved_master_server_addr, transfer_engine,
             {{"client_mode", "real"}}, tenant_id, nof_runtime_.initiator);
         if (!client_opt) {
-            LOG(ERROR) << "Failed to create client";
-            return tl::unexpected(ErrorCode::INVALID_PARAMS);
+            LOG(ERROR) << "Failed to create client: "
+                       << toString(client_opt.error());
+            return tl::unexpected(client_opt.error());
         }
         client_ = *client_opt;
     } else {
-        // Auto port binding with retry on metadata registration failure
+        // Auto port binding with retry
         const auto auto_port_config = ClientAutoPortConfig::FromEnvironment();
         bool success = false;
+        ErrorCode last_error = ErrorCode::INTERNAL_ERROR;
 
         for (int retry = 0; retry < auto_port_config.max_retries; ++retry) {
             // Create port binder to hold a port
@@ -1169,19 +1171,19 @@ tl::expected<void, ErrorCode> RealClient::setup_internal(
                 break;
             }
 
-            // Failed to create client (possibly due to metadata registration
-            // conflict), release port and retry with a different port
-            LOG(WARNING) << "Failed to create client on port " << port
-                         << ", retry " << (retry + 1) << "/"
-                         << auto_port_config.max_retries;
+            last_error = client_opt.error();
+            LOG(WARNING) << "Failed to create client on port " << port << ": "
+                         << toString(last_error) << ", retry " << (retry + 1)
+                         << "/" << auto_port_config.max_retries;
             port_binder_.reset();
             std::this_thread::sleep_for(std::chrono::milliseconds(100));
         }
 
         if (!success) {
             LOG(ERROR) << "Failed to create client after "
-                       << auto_port_config.max_retries << " retries";
-            return tl::unexpected(ErrorCode::INTERNAL_ERROR);
+                       << auto_port_config.max_retries
+                       << " retries, last error: " << toString(last_error);
+            return tl::unexpected(last_error);
         }
     }
 

mooncake-store/tests/client_local_hot_cache_test.cpp

@@ -176,7 +176,7 @@ class LocalHotCacheTest : public ::testing::Test {
     };
 
     // Helper to create client with common parameters
-    std::optional<std::shared_ptr<Client>> CreateTestClient(
+    tl::expected<std::shared_ptr<Client>, ErrorCode> CreateTestClient(
         const std::string& hostname) {
         return Client::Create(hostname,
                               "P2PHANDSHAKE",  // use in-proc metadata server

mooncake-store/tests/pybind_client_test.cpp

@@ -2572,7 +2572,7 @@ TEST_F(RealClientTest, EgmConfigCannotFallbackWithoutMnnvl) {
 
     auto result = py_client_->setup_internal(config);
     ASSERT_FALSE(result.has_value());
-    EXPECT_EQ(result.error(), ErrorCode::INVALID_PARAMS);
+    EXPECT_EQ(result.error(), ErrorCode::INTERNAL_ERROR);
     EXPECT_EQ(py_client_->client_buffer_allocator_, nullptr);
     EXPECT_TRUE(py_client_->segment_ptrs_.empty());
     EXPECT_TRUE(py_client_->hugepage_segment_ptrs_.empty());

mooncake-store/tests/real_client_hugepage_register_test.cpp

@@ -206,7 +206,7 @@ TEST_F(RealClientHugepageRegisterTest, HotCacheShmRegistersAlignedSegment) {
     EXPECT_TRUE(client_opt.value()->IsHotCacheEnabled())
         << "hot cache registration must use the aligned segment size";
 
-    client_opt.reset();  // tear down before restoring the environment
+    client_opt.value().reset();  // tear down before restoring the environment
     if (orig_size) {
         setenv("MC_STORE_LOCAL_HOT_CACHE_SIZE", orig_size, 1);
     } else {

mooncake-store/tests/real_client_uninitialized_test.cpp

@@ -14,6 +14,7 @@
 
 #include <gtest/gtest.h>
 
+#include <cstdlib>
 #include <memory>
 #include <string>
 #include <vector>
@@ -101,5 +102,21 @@ TEST_F(RealClientUninitializedTest, GetIntoRangesDoesNotCrash) {
     ASSERT_EQ(results.size(), 1u);
 }
 
+TEST_F(RealClientUninitializedTest, FailedSetupReturnsTheCreateError) {
+    setenv("MC_STORE_CLIENT_SETUP_RETRIES", "1", 1);
+    unsetenv("MOONCAKE_ENABLE_EMBEDDED_MASTER");
+    const ConfigDict config = {
+        {CONFIG_KEY_LOCAL_HOSTNAME, "127.0.0.1"},
+        {CONFIG_KEY_METADATA_SERVER, "P2PHANDSHAKE"},
+        {CONFIG_KEY_MASTER_SERVER_ADDR, "127.0.0.1:1"},
+        {CONFIG_KEY_GLOBAL_SEGMENT_SIZE, "0"},
+        {CONFIG_KEY_LOCAL_BUFFER_SIZE, "0"},
+    };
+    const auto result = client_->setup_internal(config);
+    unsetenv("MC_STORE_CLIENT_SETUP_RETRIES");
+    ASSERT_FALSE(result.has_value());
+    EXPECT_EQ(result.error(), ErrorCode::RPC_FAIL);
+}
+
 }  // namespace testing
 }  // namespace mooncake