From 8753f138d7bb1a5c853fa652ef2d124e2475085e Mon Sep 17 00:00:00 2001 From: Daniele Sciascia Date: Fri, 28 Feb 2025 15:53:45 +0100 Subject: [PATCH] Fixes for wsrep provider options plugin The plugin didn't work with options given on the command line or config file. --- .../r/wsrep_provider_plugin_config.result | 12 ++ .../wsrep/t/wsrep_provider_plugin_config.cnf | 10 ++ .../wsrep/t/wsrep_provider_plugin_config.test | 9 ++ sql/wsrep_mysqld.cc | 11 +- sql/wsrep_plugin.cc | 4 +- sql/wsrep_server_state.cc | 145 ++++++++++++++++-- sql/wsrep_server_state.h | 6 +- wsrep-lib | 2 +- 8 files changed, 174 insertions(+), 25 deletions(-) create mode 100644 mysql-test/suite/wsrep/r/wsrep_provider_plugin_config.result create mode 100644 mysql-test/suite/wsrep/t/wsrep_provider_plugin_config.cnf create mode 100644 mysql-test/suite/wsrep/t/wsrep_provider_plugin_config.test diff --git a/mysql-test/suite/wsrep/r/wsrep_provider_plugin_config.result b/mysql-test/suite/wsrep/r/wsrep_provider_plugin_config.result new file mode 100644 index 0000000000000..cec6d82a3aedc --- /dev/null +++ b/mysql-test/suite/wsrep/r/wsrep_provider_plugin_config.result @@ -0,0 +1,12 @@ +SHOW VARIABLES LIKE 'wsrep_provider_pc_npvo'; +Variable_name Value +wsrep_provider_pc_npvo ON +SELECT REGEXP_SUBSTR(@@wsrep_provider_options, 'pc.npvo = [^;]+') AS provider_option; +provider_option +pc.npvo = true +SHOW VARIABLES LIKE 'wsrep_provider_repl_max_ws_size'; +Variable_name Value +wsrep_provider_repl_max_ws_size 1024 +SELECT REGEXP_SUBSTR(@@wsrep_provider_options, 'repl.max_ws_size = [^;]+') AS provider_option; +provider_option +repl.max_ws_size = 1024 diff --git a/mysql-test/suite/wsrep/t/wsrep_provider_plugin_config.cnf b/mysql-test/suite/wsrep/t/wsrep_provider_plugin_config.cnf new file mode 100644 index 0000000000000..660815462842b --- /dev/null +++ b/mysql-test/suite/wsrep/t/wsrep_provider_plugin_config.cnf @@ -0,0 +1,10 @@ +!include ../my.cnf + +[mysqld.1] +wsrep-on=ON +wsrep-cluster-address=gcomm:// +wsrep-provider=@ENV.WSREP_PROVIDER +binlog-format=ROW +plugin-wsrep-provider=ON +wsrep-provider-pc-npvo=ON +wsrep-provider-repl-max-ws-size=1024 diff --git a/mysql-test/suite/wsrep/t/wsrep_provider_plugin_config.test b/mysql-test/suite/wsrep/t/wsrep_provider_plugin_config.test new file mode 100644 index 0000000000000..7865dcb58fb0b --- /dev/null +++ b/mysql-test/suite/wsrep/t/wsrep_provider_plugin_config.test @@ -0,0 +1,9 @@ +--source include/have_wsrep.inc +--source include/have_innodb.inc + +SHOW VARIABLES LIKE 'wsrep_provider_pc_npvo'; +SELECT REGEXP_SUBSTR(@@wsrep_provider_options, 'pc.npvo = [^;]+') AS provider_option; + +SHOW VARIABLES LIKE 'wsrep_provider_repl_max_ws_size'; +SELECT REGEXP_SUBSTR(@@wsrep_provider_options, 'repl.max_ws_size = [^;]+') AS provider_option; + diff --git a/sql/wsrep_mysqld.cc b/sql/wsrep_mysqld.cc index 4085a17c92b9b..7dedd2b2a1cdb 100644 --- a/sql/wsrep_mysqld.cc +++ b/sql/wsrep_mysqld.cc @@ -901,7 +901,11 @@ int wsrep_init() Wsrep_server_state::init_provider_services(); if (Wsrep_server_state::instance().load_provider( wsrep_provider, - wsrep_provider_options, + [&]() { + std::string options(wsrep_provider_options); + Wsrep_server_state::init_options(options); + return options; + }, Wsrep_server_state::instance().provider_services())) { WSREP_ERROR("Failed to load provider"); @@ -1009,11 +1013,6 @@ void wsrep_init_startup (bool sst_first) wsrep_create_rollbacker(); wsrep_create_appliers(1); - if (Wsrep_server_state::init_options()) - { - WSREP_WARN("Failed to initialize provider options"); - } - Wsrep_server_state& server_state= Wsrep_server_state::instance(); /* If the SST happens before server initialization, wait until the server diff --git a/sql/wsrep_plugin.cc b/sql/wsrep_plugin.cc index 644dc921f7092..5c42cead57b28 100644 --- a/sql/wsrep_plugin.cc +++ b/sql/wsrep_plugin.cc @@ -131,7 +131,8 @@ static void wsrep_provider_sysvar_update(THD *thd, T new_value= *((T *) save); auto options= Wsrep_server_state::get_options(); - if (options->set(opt->name(), make_option_value(new_value))) + if (options->set(Wsrep_server_state::get_provider(), opt->name(), + make_option_value(new_value))) { my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), opt->name(), make_option_value(new_value)->as_string()); @@ -286,6 +287,7 @@ static int wsrep_provider_plugin_init(void *p) } provider_plugin_enabled= true; + wsrep_refresh_provider_options(); // When plugin-wsrep-provider is enabled we set // wsrep_provider_options parameter as READ_ONLY diff --git a/sql/wsrep_server_state.cc b/sql/wsrep_server_state.cc index f80320fe21668..70bcd4037b709 100644 --- a/sql/wsrep_server_state.cc +++ b/sql/wsrep_server_state.cc @@ -20,6 +20,8 @@ #include "wsrep_event_service.h" #include "wsrep_binlog.h" /* init/deinit group commit */ #include "wsrep_plugin.h" /* make/destroy sysvar helpers */ +#include "my_getopt.h" /* handle_options() */ +#include mysql_mutex_t LOCK_wsrep_server_state; mysql_cond_t COND_wsrep_server_state; @@ -32,9 +34,11 @@ PSI_cond_key key_COND_wsrep_server_state; wsrep::provider::services Wsrep_server_state::m_provider_services; Wsrep_server_state* Wsrep_server_state::m_instance; -std::unique_ptr Wsrep_server_state::m_options; std::vector Wsrep_server_state::m_sysvars; +static const char* provider_options_prefix="wsrep-provider-"; +static int provider_options_prefix_length=15; + Wsrep_server_state::Wsrep_server_state(const std::string& name, const std::string& incoming_address, const std::string& address, @@ -94,31 +98,146 @@ int Wsrep_server_state::init_provider(const std::string& provider, return 0; } -int Wsrep_server_state::init_options() +struct st_mysql_sys_var { - if (!m_instance) return 1; - m_options= std::unique_ptr( - new wsrep::provider_options(m_instance->provider())); - int ret= m_options->initial_options(); - if (ret) + MYSQL_PLUGIN_VAR_HEADER; +}; + +static void my_option_init(struct my_option &my_opt, + void* app_type, + struct st_mysql_sys_var *var) +{ + std::string option_name(provider_options_prefix); + option_name.append(var->name); + for (size_t i= 0; i < option_name.size(); ++i) + if (option_name[i] == '_') + option_name[i]= '-'; + my_opt.name= my_strdup(PSI_INSTRUMENT_ME, option_name.c_str(), MYF(0)); + my_opt.id= 0; + plugin_opt_set_limits(&my_opt, var); + my_opt.value= my_opt.u_max_value= *(uchar ***) (var + 1); + my_opt.deprecation_substitute= NULL; + my_opt.block_size= 0; + my_opt.app_type= app_type; +} + +static void my_option_deinit(struct my_option &my_opt) +{ + if (my_opt.name) + my_free((void*)my_opt.name); +} + +static void make_my_options(std::vector &my_options, + std::vector sysvars, + void* app_type) +{ + for (struct st_mysql_sys_var* var : sysvars) { + if (var) { + struct my_option my_opt; + my_option_init(my_opt, app_type, var); + my_options.push_back(my_opt); + } + } + struct my_option null_opt; + null_opt.name= NULL; + my_options.push_back(null_opt); +} + +// Convert plugin option name to provider option name. +// For example, given 'wsrep-provider-repl-max-ws-size' +// return 'repl.max_ws_size' +static std::string option_name_to_provider_name(const char *name) +{ + if (strncmp(name, provider_options_prefix, + provider_options_prefix_length) != 0) { - WSREP_ERROR("Failed to initialize provider options"); - m_options = nullptr; - m_instance->unload_provider(); - return ret; + assert(0); + return ""; + } + + std::string option(name + provider_options_prefix_length); + size_t pos= option.find('-'); + if (pos == std::string::npos) + { + assert(0); + return ""; + } + + // Replace the first '-' with a '.' + option[pos]= '.'; + + // Replace the remaining '-' with '_' + for (size_t i= pos + 1; i < option.length(); ++i) + if (option[i] == '-') + option[i]= '_'; + + return option; +} + +std::string make_provider_option_string(const char* name, const char* value) +{ + std::string option_string(option_name_to_provider_name(name)); + option_string.append("="); + option_string.append(value); + return option_string; +} + +my_bool option_changed(const struct my_option *opt, const char *value, + const char *filename) +{ + std::string* extra_options= (std::string*)opt->app_type; + std::string option_string(make_provider_option_string(opt->name, value)); + extra_options->append(";"); + extra_options->append(option_string); + return 0; +} + +static void parse_config_params(std::vector sysvars, + std::string &extra_options) +{ + int argc=orig_argc; + char **argv=orig_argv; + + if (load_defaults(MYSQL_CONFIG_NAME, load_default_groups, &argc, &argv)) + { + assert(0); + return; + } + char** defaults_argv= argv; + + std::vector my_options; + make_my_options(my_options, sysvars, &extra_options); + my_getopt_skip_unknown= 1; // reset to original value + int error= handle_options(&argc, &argv, &my_options[0], option_changed); + if (error) + { + WSREP_ERROR("handle_options failed"); + assert(0); } - m_options->for_each([](wsrep::provider_options::option *opt) { + + for (struct my_option &opt : my_options) + my_option_deinit(opt); + if (argv) + free_defaults(defaults_argv); +} + +int Wsrep_server_state::init_options(std::string& extra_options) +{ + if (!m_instance) + return 1; + auto options= m_instance->provider_options(); + options->for_each([](wsrep::provider_options::option *opt) { struct st_mysql_sys_var *var= wsrep_make_sysvar_for_option(opt); m_sysvars.push_back(var); }); m_sysvars.push_back(nullptr); wsrep_provider_plugin_set_sysvars(&m_sysvars[0]); + parse_config_params(m_sysvars, extra_options); return 0; } void Wsrep_server_state::deinit_provider() { - m_options = nullptr; m_instance->unload_provider(); } diff --git a/sql/wsrep_server_state.h b/sql/wsrep_server_state.h index d169e5b219ddb..3066eef8e9d40 100644 --- a/sql/wsrep_server_state.h +++ b/sql/wsrep_server_state.h @@ -19,7 +19,6 @@ /* wsrep-lib */ #include "wsrep/server_state.hpp" #include "wsrep/provider.hpp" -#include "wsrep/provider_options.hpp" /* implementation */ #include "wsrep_server_service.h" @@ -37,7 +36,7 @@ class Wsrep_server_state : public wsrep::server_state int max_protocol_version); static int init_provider(const std::string& provider, const std::string& options); - static int init_options(); + static int init_options(std::string& extra_options); static void deinit_provider(); static void destroy(); @@ -58,7 +57,7 @@ class Wsrep_server_state : public wsrep::server_state static wsrep::provider_options* get_options() { - return m_options.get(); + return instance().provider_options(); } static bool has_capability(int capability) @@ -87,7 +86,6 @@ class Wsrep_server_state : public wsrep::server_state Wsrep_server_service m_service; static wsrep::provider::services m_provider_services; static Wsrep_server_state* m_instance; - static std::unique_ptr m_options; // Sysvars for provider plugin. We keep these here because // they are allocated dynamically and must be freed at some // point during shutdown (after the plugin is deinitialized). diff --git a/wsrep-lib b/wsrep-lib index 31db8476768ba..9349f854c0b23 160000 --- a/wsrep-lib +++ b/wsrep-lib @@ -1 +1 @@ -Subproject commit 31db8476768ba68296ad91b6785bb06a6a9abf71 +Subproject commit 9349f854c0b231a08a46766643d4db341ef0e38c