Retrieve provider options before provider is initialized

sciascid · sciascid · commit db41d67c9cea · 2025-04-04T14:39:11.000+02:00
Change provider_options so that it does not depend on provider.
Call into config_service when provider is created, i.e. after
library is loaded, before it is initialized, so that we can
inject additional options at startup.
Change the signature of the call provider options callback to
take provider_options object as parameter.
diff --git a/include/wsrep/provider.hpp b/include/wsrep/provider.hpp
@@ -51,6 +51,7 @@ namespace wsrep
     class event_service;
     class client_service;
     class connection_monitor_service;
+    class provider_options;
     class stid
     {
     public:
@@ -520,7 +521,7 @@ namespace wsrep
         static std::unique_ptr<provider> make_provider(
             wsrep::server_state&,
             const std::string& provider_spec,
-            const std::function<std::string()>& provider_options_cb,
+            const std::function<std::string(provider_options&)>& provider_options_cb,
             const wsrep::provider::services& services
             = wsrep::provider::services());
     protected:
diff --git a/include/wsrep/provider_options.hpp b/include/wsrep/provider_options.hpp
@@ -214,7 +214,7 @@ namespace wsrep
             int flags_;
         };
 
-        provider_options(wsrep::provider&);
+        provider_options();
         provider_options(const provider_options&) = delete;
         provider_options& operator=(const provider_options&) = delete;
 
@@ -225,7 +225,7 @@ namespace wsrep
          *
          * @return Provider status code.
          */
-        enum wsrep::provider::status initial_options();
+        enum wsrep::provider::status initial_options(wsrep::provider& provider);
 
         /**
          * Get the option with the given name
@@ -241,7 +241,8 @@ namespace wsrep
          * @return wsrep::provider::error_size_exceeded if memory could
          *         not be allocated for the new value.
          */
-        enum wsrep::provider::status set(const std::string& name,
+        enum wsrep::provider::status set(wsrep::provider& provider,
+                                         const std::string& name,
                                          std::unique_ptr<option_value> value);
 
         /**
@@ -255,7 +256,6 @@ namespace wsrep
         void for_each(const std::function<void(option*)>& fn);
 
     private:
-        provider& provider_;
         using options_map = std::map<std::string, std::unique_ptr<option>>;
         options_map options_;
     };
diff --git a/include/wsrep/server_state.hpp b/include/wsrep/server_state.hpp
@@ -117,6 +117,7 @@ namespace wsrep
     class server_service;
     class client_service;
     class encryption_service;
+    class provider_options;
 
     /** @class Server Context
      *
@@ -296,16 +297,15 @@ namespace wsrep
          * @return Zero on success, non-zero on error.
          */
         int load_provider(const std::string& provider,
-                          const std::function<std::string()>& provider_options_cb,
+                          const std::function<std::string(provider_options&)>&,
                           const wsrep::provider::services& services
                           = wsrep::provider::services());
 
-
         /**
          * Load WSRep provider.
          *
          * @param provider WSRep provider library to be loaded.
-         * @param provider_options Provider specific options string
+         * @param options Provider specific options string
          *        to be passed for provider during initialization.
          * @param services Application defined services passed to
          *                 the provider.
@@ -315,13 +315,13 @@ namespace wsrep
          * @note Provided for backward compatibility.
          */
         int load_provider(const std::string& provider,
-                          const std::string& provider_options,
+                          const std::string& options,
                           const wsrep::provider::services& services
                           = wsrep::provider::services())
         {
-            return load_provider(provider,
-                                [provider_options]() { return provider_options; },
-                                services);
+            return load_provider(
+                provider, [options](provider_options&) { return options; },
+                services);
         }
 
         using provider_factory_func =
diff --git a/src/config_service_v1.cpp b/src/config_service_v1.cpp
@@ -19,6 +19,7 @@
 
 #include "config_service_v1.hpp"
 #include "service_helpers.hpp"
+#include "v26/wsrep_api.h"
 #include "v26/wsrep_config_service.h"
 #include "wsrep/logger.hpp"
 #include "wsrep/provider_options.hpp"
@@ -147,10 +148,9 @@ static void config_service_v1_deinit(void* dlh)
         dlh, WSREP_CONFIG_SERVICE_DEINIT_FUNC_V1, "config service v1");
 }
 
-int wsrep::config_service_v1_fetch(wsrep::provider& provider,
+int wsrep::config_service_v1_fetch(struct wsrep_st* wsrep,
                                    wsrep::provider_options* options)
 {
-    struct wsrep_st* wsrep = (struct wsrep_st*)provider.native();
     if (wsrep == nullptr)
     {
         // Not a provider which was loaded via wsrep-API
@@ -179,3 +179,10 @@ int wsrep::config_service_v1_fetch(wsrep::provider& provider,
 
     return 0;
 }
+
+int wsrep::config_service_v1_fetch(wsrep::provider& provider,
+                                   wsrep::provider_options* options)
+{
+  struct wsrep_st* wsrep = (struct wsrep_st*)provider.native();
+  return config_service_v1_fetch(wsrep, options);
+}
diff --git a/src/config_service_v1.hpp b/src/config_service_v1.hpp
@@ -20,11 +20,14 @@
 #ifndef WSREP_CONFIG_SERVICE_V1_HPP
 #define WSREP_CONFIG_SERVICE_V1_HPP
 
+struct wsrep_st;
+
 namespace wsrep
 {
     class provider;
     class provider_options;
     int config_service_v1_fetch(provider& provider, provider_options* opts);
+    int config_service_v1_fetch(struct wsrep_st* wsrep, provider_options* opts);
 } // namespace wsrep
 
 #endif // WSREP_CONFIG_SERVICE_V1_HPP
diff --git a/src/provider.cpp b/src/provider.cpp
@@ -19,6 +19,7 @@
 
 #include "wsrep/provider.hpp"
 #include "wsrep/logger.hpp"
+#include "wsrep/provider_options.hpp"
 
 #include "wsrep_provider_v26.hpp"
 
@@ -29,27 +30,30 @@
 std::unique_ptr<wsrep::provider> wsrep::provider::make_provider(
     wsrep::server_state& server_state,
     const std::string& provider_spec,
-    const std::function<std::string()>& provider_options_cb,
+    const std::function<std::string(provider_options&)>& provider_options_cb,
     const wsrep::provider::services& services)
 {
     try
     {
         return std::unique_ptr<wsrep::provider>(new wsrep::wsrep_provider_v26(
-            server_state, provider_spec, provider_options_cb, services));
+            server_state, provider_spec, provider_options_cb,
+            services));
     }
     catch (const wsrep::runtime_error& e)
     {
+        provider_options opts;
         wsrep::log_error() << "Failed to create a new provider '"
                            << provider_spec << "'"
-                           << " with options '" << provider_options_cb()
+                           << " with options '" << provider_options_cb(opts)
                            << "': " << e.what();
     }
     catch (...)
     {
+        provider_options opts;
         wsrep::log_error() << "Caught unknown exception when trying to "
                            << "create a new provider '"
                            << provider_spec << "'"
-                           << " with options '" << provider_options_cb();
+                           << " with options '" << provider_options_cb(opts);
     }
     return 0;
 }
diff --git a/src/provider_options.cpp b/src/provider_options.cpp
@@ -18,6 +18,7 @@
  */
 
 #include "wsrep/provider_options.hpp"
+#include "wsrep/provider.hpp"
 #include "config_service_v1.hpp"
 #include "wsrep/logger.hpp"
 
@@ -92,16 +93,16 @@ void wsrep::provider_options::option::update_value(
 
 wsrep::provider_options::option::~option() {}
 
-wsrep::provider_options::provider_options(wsrep::provider& provider)
-    : provider_(provider)
-    , options_()
+wsrep::provider_options::provider_options()
+    : options_()
 {
 }
 
-enum wsrep::provider::status wsrep::provider_options::initial_options()
+enum wsrep::provider::status
+wsrep::provider_options::initial_options(wsrep::provider& provider)
 {
     options_.clear();
-    if (config_service_v1_fetch(provider_, this))
+    if (config_service_v1_fetch(provider, this))
     {
         return wsrep::provider::error_not_implemented;
     }
@@ -123,7 +124,7 @@ wsrep::provider_options::get_option(const std::string& name) const
 }
 
 enum wsrep::provider::status wsrep::provider_options::set(
-    const std::string& name,
+    wsrep::provider& provider, const std::string& name,
     std::unique_ptr<wsrep::provider_options::option_value> value)
 {
     auto option(options_.find(name));
@@ -132,7 +133,7 @@ enum wsrep::provider::status wsrep::provider_options::set(
         return not_found_error;
     }
     provider_options_sep sep;
-    auto ret(provider_.options(std::string(option->second->real_name())
+    auto ret(provider.options(std::string(option->second->real_name())
                                + sep.key_value + value->as_string()
                                + sep.param));
     if (ret == provider::success)
diff --git a/src/server_state.cpp b/src/server_state.cpp
@@ -500,13 +500,13 @@ static int apply_toi(wsrep::provider& provider,
 
 int wsrep::server_state::load_provider(
     const std::string& provider_spec,
-    const std::function<std::string()>& provider_options_cb,
+    const std::function<std::string(provider_options&)>& provider_options_cb,
     const wsrep::provider::services& services)
 {
     wsrep::log_info() << "Loading provider " << provider_spec
                       << " initial position: " << initial_position_;
-    provider_
-        = provider_factory_(*this, provider_spec, provider_options_cb, services);
+    provider_ = provider_factory_(*this, provider_spec, provider_options_cb,
+                                  services);
     return (provider_ ? 0 : 1);
 }
 
diff --git a/src/wsrep_provider_v26.cpp b/src/wsrep_provider_v26.cpp
@@ -29,12 +29,14 @@
 #include "wsrep/tls_service.hpp"
 #include "wsrep/allowlist_service.hpp"
 #include "wsrep/connection_monitor_service.hpp"
+#include "wsrep/provider_options.hpp"
 
 #include "service_helpers.hpp"
 #include "thread_service_v1.hpp"
 #include "tls_service_v1.hpp"
 #include "allowlist_service_v1.hpp"
 #include "event_service_v1.hpp"
+#include "config_service_v1.hpp"
 #include "v26/wsrep_api.h"
 #include "v26/wsrep_node_isolation.h"
 #include "connection_monitor_service_v1.hpp"
@@ -772,7 +774,7 @@ void wsrep::wsrep_provider_v26::deinit_services()
 wsrep::wsrep_provider_v26::wsrep_provider_v26(
     wsrep::server_state& server_state,
     const std::string& provider_spec,
-    const std::function<std::string()>& provider_options_cb,
+    const std::function<std::string(provider_options&)>& provider_options_cb,
     const wsrep::provider::services& services)
     : provider(server_state)
     , wsrep_()
@@ -785,14 +787,24 @@ wsrep::wsrep_provider_v26::wsrep_provider_v26(
                 server_state.initial_position().id().data(),
                 sizeof(state_id.uuid.data));
     state_id.seqno = server_state.initial_position().seqno().get();
+
+    if (wsrep_load(provider_spec.c_str(), &wsrep_, logger_cb))
+    {
+        throw wsrep::runtime_error("Failed to load wsrep library");
+    }
+
+    init_services(services);
+    provider_options options;
+    config_service_v1_fetch(wsrep_, &options);
+
     struct wsrep_init_args init_args;
     memset(&init_args, 0, sizeof(init_args));
     init_args.app_ctx = &server_state;
     init_args.node_name = server_state_.name().c_str();
     init_args.node_address = server_state_.address().c_str();
     init_args.node_incoming = server_state_.incoming_address().c_str();
     init_args.data_dir = server_state_.working_dir().c_str();
-    const auto& provider_options = provider_options_cb();
+    const auto& provider_options = provider_options_cb(options);
     init_args.options = provider_options.c_str();
     init_args.proto_ver = server_state.max_protocol_version();
     init_args.state_id = &state_id;
@@ -807,13 +819,6 @@ wsrep::wsrep_provider_v26::wsrep_provider_v26(
     init_args.sst_donate_cb = &sst_donate_cb;
     init_args.synced_cb = &synced_cb;
 
-    if (wsrep_load(provider_spec.c_str(), &wsrep_, logger_cb))
-    {
-        throw wsrep::runtime_error("Failed to load wsrep library");
-    }
-
-    init_services(services);
-
     if (wsrep_->init(wsrep_, &init_args) != WSREP_OK)
     {
         throw wsrep::runtime_error("Failed to initialize wsrep provider");
diff --git a/src/wsrep_provider_v26.hpp b/src/wsrep_provider_v26.hpp
@@ -26,14 +26,15 @@ struct wsrep_st;
 
 namespace wsrep
 {
-    class thread_service;
+    class provider_options;
+
     class wsrep_provider_v26 : public wsrep::provider
     {
     public:
         void init_services(const wsrep::provider::services& services);
         void deinit_services();
         wsrep_provider_v26(wsrep::server_state&, const std::string&,
-                           const std::function<std::string()>&,
+                           const std::function<std::string(provider_options&)>&,
                            const wsrep::provider::services& services);
         ~wsrep_provider_v26() WSREP_OVERRIDE;
         enum wsrep::provider::status
diff --git a/test/mock_server_state.hpp b/test/mock_server_state.hpp
@@ -22,6 +22,7 @@
 
 #include "wsrep/server_state.hpp"
 #include "wsrep/server_service.hpp"
+#include "wsrep/provider_options.hpp"
 #include "mock_client_state.hpp"
 #include "mock_high_priority_service.hpp"
 #include "mock_storage_service.hpp"
@@ -260,16 +261,16 @@ namespace wsrep
             , cond_()
             , provider_()
         {
-            set_provider_factory([&](wsrep::server_state&,
-                                     const std::string&,
-                                     const std::function<std::string()>&,
-                                     const wsrep::provider::services&)
-            {
-                // The provider object is destroyed upon server state
-                // destruction, so using a raw pointer is safe.
-                provider_ = new wsrep::mock_provider(*this);
-                return std::unique_ptr<wsrep::provider>(provider_);
-            });
+            set_provider_factory(
+                [&](wsrep::server_state&, const std::string&,
+                    const std::function<std::string(wsrep::provider_options&)>&,
+                    const wsrep::provider::services&)
+                {
+                    // The provider object is destroyed upon server state
+                    // destruction, so using a raw pointer is safe.
+                    provider_ = new wsrep::mock_provider(*this);
+                    return std::unique_ptr<wsrep::provider>(provider_);
+                });
 
             const int ret WSREP_UNUSED = load_provider("mock", "");
             assert(ret == 0);

Original file line number	Diff line number	Diff line change
`@@ -19,6 +19,7 @@`
`19`	`19`
`20`	`20`	`#include "wsrep/provider.hpp"`
`21`	`21`	`#include "wsrep/logger.hpp"`
	`22`	`+#include "wsrep/provider_options.hpp"`
`22`	`23`
`23`	`24`	`#include "wsrep_provider_v26.hpp"`
`24`	`25`
`@@ -29,27 +30,30 @@`
`29`	`30`	`std::unique_ptr<wsrep::provider> wsrep::provider::make_provider(`
`30`	`31`	`wsrep::server_state& server_state,`
`31`	`32`	`const std::string& provider_spec,`
`32`		`- const std::function<std::string()>& provider_options_cb,`
	`33`	`+ const std::function<std::string(provider_options&)>& provider_options_cb,`
`33`	`34`	`const wsrep::provider::services& services)`
`34`	`35`	`{`
`35`	`36`	`try`
`36`	`37`	`{`
`37`	`38`	`return std::unique_ptr<wsrep::provider>(new wsrep::wsrep_provider_v26(`
`38`		`- server_state, provider_spec, provider_options_cb, services));`
	`39`	`+ server_state, provider_spec, provider_options_cb,`
	`40`	`+ services));`
`39`	`41`	`}`
`40`	`42`	`catch (const wsrep::runtime_error& e)`
`41`	`43`	`{`
	`44`	`+ provider_options opts;`
`42`	`45`	`wsrep::log_error() << "Failed to create a new provider '"`
`43`	`46`	`<< provider_spec << "'"`
`44`		`- << " with options '" << provider_options_cb()`
	`47`	`+ << " with options '" << provider_options_cb(opts)`
`45`	`48`	`<< "': " << e.what();`
`46`	`49`	`}`
`47`	`50`	`catch (...)`
`48`	`51`	`{`
	`52`	`+ provider_options opts;`
`49`	`53`	`wsrep::log_error() << "Caught unknown exception when trying to "`
`50`	`54`	`<< "create a new provider '"`
`51`	`55`	`<< provider_spec << "'"`
`52`		`- << " with options '" << provider_options_cb();`
	`56`	`+ << " with options '" << provider_options_cb(opts);`
`53`	`57`	`}`
`54`	`58`	`return 0;`
`55`	`59`	`}`
Original file line number	Diff line number	Diff line change
`@@ -18,6 +18,7 @@`
`18`	`18`	`*/`
`19`	`19`
`20`	`20`	`#include "wsrep/provider_options.hpp"`
	`21`	`+#include "wsrep/provider.hpp"`
`21`	`22`	`#include "config_service_v1.hpp"`
`22`	`23`	`#include "wsrep/logger.hpp"`
`23`	`24`
`@@ -92,16 +93,16 @@ void wsrep::provider_options::option::update_value(`
`92`	`93`
`93`	`94`	`wsrep::provider_options::option::~option() {}`
`94`	`95`
`95`		`-wsrep::provider_options::provider_options(wsrep::provider& provider)`
`96`		`- : provider_(provider)`
`97`		`- , options_()`
	`96`	`+wsrep::provider_options::provider_options()`
	`97`	`+ : options_()`
`98`	`98`	`{`
`99`	`99`	`}`
`100`	`100`
`101`		`-enum wsrep::provider::status wsrep::provider_options::initial_options()`
	`101`	`+enum wsrep::provider::status`
	`102`	`+wsrep::provider_options::initial_options(wsrep::provider& provider)`
`102`	`103`	`{`
`103`	`104`	`options_.clear();`
`104`		`- if (config_service_v1_fetch(provider_, this))`
	`105`	`+ if (config_service_v1_fetch(provider, this))`
`105`	`106`	`{`
`106`	`107`	`return wsrep::provider::error_not_implemented;`
`107`	`108`	`}`
`@@ -123,7 +124,7 @@ wsrep::provider_options::get_option(const std::string& name) const`
`123`	`124`	`}`
`124`	`125`
`125`	`126`	`enum wsrep::provider::status wsrep::provider_options::set(`
`126`		`- const std::string& name,`
	`127`	`+ wsrep::provider& provider, const std::string& name,`
`127`	`128`	`std::unique_ptr<wsrep::provider_options::option_value> value)`
`128`	`129`	`{`
`129`	`130`	`auto option(options_.find(name));`
`@@ -132,7 +133,7 @@ enum wsrep::provider::status wsrep::provider_options::set(`
`132`	`133`	`return not_found_error;`
`133`	`134`	`}`
`134`	`135`	`provider_options_sep sep;`
`135`		`- auto ret(provider_.options(std::string(option->second->real_name())`
	`136`	`+ auto ret(provider.options(std::string(option->second->real_name())`
`136`	`137`	`+ sep.key_value + value->as_string()`
`137`	`138`	`+ sep.param));`
`138`	`139`	`if (ret == provider::success)`