hcq/MaxScale
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Clean up server config parameter handling

Browse Source 
Removes helper classes which are no longer required.
This commit is contained in:
Esa Korhonen
2019-04-30 18:32:16 +03:00
parent d3a1f5c51f
commit 4e6ffc0381
6 changed files with 137 additions and 163 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
include/maxscale/config.hh
View File

@ -451,6 +451,7 @@ public:
void remove(const std::string& key); void remove(const std::string& key);
void clear(); void clear();
bool empty() const;
ContainerType::const_iterator begin() const; ContainerType::const_iterator begin() const;
ContainerType::const_iterator end() const; ContainerType::const_iterator end() const;

29
server/core/config.cc
View File

@ -2212,6 +2212,11 @@ void MXS_CONFIG_PARAMETER::clear()
m_contents.clear(); m_contents.clear();
} }
bool MXS_CONFIG_PARAMETER::empty() const
{
return m_contents.empty();
}
MXS_CONFIG_PARAMETER::ContainerType::const_iterator MXS_CONFIG_PARAMETER::begin() const MXS_CONFIG_PARAMETER::ContainerType::const_iterator MXS_CONFIG_PARAMETER::begin() const
{ {
return m_contents.begin(); return m_contents.begin();
@ -3817,34 +3822,24 @@ void config_add_module_params_json(const MXS_CONFIG_PARAMETER* parameters,
const MXS_MODULE_PARAM* module_params, const MXS_MODULE_PARAM* module_params,
json_t* output) json_t* output)
{ {
// Create a map of the config values to ease their extraction
std::unordered_map<std::string, std::string> params;
for (const auto& elem : *parameters)
{
params[elem.first] = elem.second;
}
for (const auto* param_info : {basic_params, module_params}) for (const auto* param_info : {basic_params, module_params})
{ {
for (int i = 0; param_info[i].name; i++) for (int i = 0; param_info[i].name; i++)
{ {
if (ignored_params.count(param_info[i].name) == 0 const string param_name = param_info[i].name;
&& !json_object_get(output, param_info[i].name)) if (ignored_params.count(param_name) == 0 && !json_object_get(output, param_name.c_str()))
{ {
if (params.count(param_info[i].name) > 0) if (parameters->contains(param_name))
{ {
const string name = param_info[i].name; const string value = parameters->get_string(param_name);
const string value = params[name]; json_object_set_new(output, param_name.c_str(),
json_object_set_new(output, name.c_str(), param_value_to_json(&param_info[i], param_name, value));
param_value_to_json(&param_info[i], name,
value));
} }
else else
{ {
// The parameter was not set in config and does not have a default value. // The parameter was not set in config and does not have a default value.
// Print a null value. // Print a null value.
json_object_set_new(output, param_info[i].name, json_null()); json_object_set_new(output, param_name.c_str(), json_null());
} }
} }
} }

65
server/core/config_runtime.cc
View File

@ -576,57 +576,60 @@ bool runtime_alter_server(Server* server, const char* key, const char* value)
return false; return false;
} }
const MXS_MODULE* mod = get_module(server->protocol().c_str(), MODULE_PROTOCOL); bool is_normal_parameter = !server->is_custom_parameter(key);
// As servers allow unknown parameters, we must only validate known parameters // Only validate known parameters as custom parameters cannot be validated.
if (param_is_known(config_server_params, mod->parameters, key) if (is_normal_parameter)
&& !param_is_valid(config_server_params, mod->parameters, key, value)) {
const MXS_MODULE* mod = get_module(server->protocol().c_str(), MODULE_PROTOCOL);
if (!param_is_valid(config_server_params, mod->parameters, key, value))
{ {
config_runtime_error("Invalid value for parameter '%s': %s", key, value); config_runtime_error("Invalid value for parameter '%s': %s", key, value);
return false; return false;
} }
if (strcmp(key, CN_DISK_SPACE_THRESHOLD) == 0)
{
// This cannot be safely modified during runtime since the monitor thread could be reading the
// value. TODO: To enable this, the threshold value needs to be moved to the private Server-class
// so that locking can be enforced on any access.
config_runtime_error("The server parameter '%s' cannot be modified during runtime.",
CN_DISK_SPACE_THRESHOLD);
return false;
} }
std::lock_guard<std::mutex> guard(crt_lock); std::lock_guard<std::mutex> guard(crt_lock);
server->set_parameter(key, value);
bool setting_changed = false;
if (is_normal_parameter)
{
// Only some normal parameters can be changed runtime. The key/value-combination has already
// been checked to be valid.
if (strcmp(key, CN_ADDRESS) == 0 || strcmp(key, CN_SOCKET) == 0) if (strcmp(key, CN_ADDRESS) == 0 || strcmp(key, CN_SOCKET) == 0)
{ {
server->server_update_address(value); server->server_update_address(value);
setting_changed = true;
} }
else if (strcmp(key, CN_PORT) == 0) else if (strcmp(key, CN_PORT) == 0)
{ {
if (int ival = get_positive_int(value)) if (int ival = get_positive_int(value))
{ {
server->update_port(ival); server->update_port(ival);
setting_changed = true;
} }
} }
else if (strcmp(key, CN_EXTRA_PORT) == 0) else if (strcmp(key, CN_EXTRA_PORT) == 0)
{ {
server->update_extra_port(atoi(value)); server->update_extra_port(atoi(value));
setting_changed = true;
} }
else if (strcmp(key, CN_MONITORUSER) == 0) else if (strcmp(key, CN_MONITORUSER) == 0)
{ {
server->set_monitor_user(value); server->set_monitor_user(value);
setting_changed = true;
} }
else if (strcmp(key, CN_MONITORPW) == 0) else if (strcmp(key, CN_MONITORPW) == 0)
{ {
server->set_monitor_password(value); server->set_monitor_password(value);
setting_changed = true;
} }
else if (strcmp(key, CN_PERSISTPOOLMAX) == 0) else if (strcmp(key, CN_PERSISTPOOLMAX) == 0)
{ {
if (is_valid_integer(value)) if (is_valid_integer(value))
{ {
server->set_persistpoolmax(atoi(value)); server->set_persistpoolmax(atoi(value));
setting_changed = true;
} }
} }
else if (strcmp(key, CN_PERSISTMAXTIME) == 0) else if (strcmp(key, CN_PERSISTMAXTIME) == 0)
@ -634,15 +637,16 @@ bool runtime_alter_server(Server* server, const char* key, const char* value)
if (is_valid_integer(value)) if (is_valid_integer(value))
{ {
server->set_persistmaxtime(atoi(value)); server->set_persistmaxtime(atoi(value));
setting_changed = true;
} }
} }
if (strcmp(key, CN_RANK) == 0) else if (strcmp(key, CN_RANK) == 0)
{ {
auto v = config_enum_to_value(value, rank_values); auto v = config_enum_to_value(value, rank_values);
if (v != MXS_UNKNOWN_ENUM_VALUE) if (v != MXS_UNKNOWN_ENUM_VALUE)
{ {
server->set_rank(v); server->set_rank(v);
setting_changed = true;
} }
else else
{ {
@ -651,17 +655,32 @@ bool runtime_alter_server(Server* server, const char* key, const char* value)
} }
else else
{ {
/** // Was a recognized parameter but runtime modification is not supported.
* It's likely that this parameter is used as a weighting parameter. config_runtime_error("Server parameter '%s' cannot be modified during runtime. A similar "
* We need to update the weights of services that use this. "effect can be produced by destroying the server and recreating it "
*/ "with the new settings.", key);
service_update_weights();
} }
if (setting_changed)
{
// Successful modification of a normal parameter, write to text storage.
server->set_normal_parameter(key, value);
}
}
else
{
// This is a custom parameter and may be used for weighting. Update the weights of services.
server->set_custom_parameter(key, value);
service_update_weights();
setting_changed = true;
}
if (setting_changed)
{
server->serialize(); server->serialize();
MXS_NOTICE("Updated server '%s': %s=%s", server->name(), key, value); MXS_NOTICE("Updated server '%s': %s=%s", server->name(), key, value);
}
return true; return setting_changed;
} }
static bool undefined_mandatory_parameter(const MXS_MODULE_PARAM* mod_params, static bool undefined_mandatory_parameter(const MXS_MODULE_PARAM* mod_params,

23
server/core/internal/server.hh
View File

@ -36,12 +36,6 @@ public:
m_settings.authenticator = authenticator; m_settings.authenticator = authenticator;
} }
struct ConfigParameter
{
std::string name;
std::string value;
};
long persistpoolmax() const long persistpoolmax() const
{ {
return m_settings.persistpoolmax; return m_settings.persistpoolmax;
@ -186,10 +180,10 @@ public:
static Server* find_by_unique_name(const std::string& name); static Server* find_by_unique_name(const std::string& name);
/** /**
* Test if name is a normal server setting name. * Test if name is not a normal server setting name.
* *
* @param name Name to check * @param name Name to check
* @return True if name is a standard parameter * @return True if name is not a standard parameter
*/ */
bool is_custom_parameter(const std::string& name) const; bool is_custom_parameter(const std::string& name) const;
@ -241,16 +235,17 @@ public:
static json_t* server_json_attributes(const Server* server); static json_t* server_json_attributes(const Server* server);
/** /**
* @brief Set server parameter * Set server custom parameter.
* *
* @param server Server to update
* @param name Parameter to set * @param name Parameter to set
* @param value Value of parameter * @param value Value of parameter
*/ */
void set_parameter(const std::string& name, const std::string& value); void set_custom_parameter(const std::string& name, const std::string& value);
std::string get_custom_parameter(const std::string& name) const override; std::string get_custom_parameter(const std::string& name) const override;
void set_normal_parameter(const std::string& name, const std::string& value);
/** /**
* @brief Serialize a server to a file * @brief Serialize a server to a file
* *
@ -330,7 +325,7 @@ private:
/** All config settings in text form. This is only read and written from the admin thread /** All config settings in text form. This is only read and written from the admin thread
* so no need for locking. */ * so no need for locking. */
std::vector<ConfigParameter> all_parameters; MXS_CONFIG_PARAMETER all_parameters;
std::string protocol; /**< Backend protocol module name. Does not change so needs no locking. */ std::string protocol; /**< Backend protocol module name. Does not change so needs no locking. */
std::string authenticator; /**< Authenticator module name. Does not change so needs no locking. */ std::string authenticator; /**< Authenticator module name. Does not change so needs no locking. */
@ -347,9 +342,9 @@ private:
* by mutex. */ * by mutex. */
DiskSpaceLimits disk_space_limits; DiskSpaceLimits disk_space_limits;
/** Additional custom parameters which may affect routing decisions or the monitor module. /** Additional custom parameters which may affect routing decisions or a monitor.
* Can be queried from modules at any time so access must be protected by mutex. */ * Can be queried from modules at any time so access must be protected by mutex. */
std::map<std::string, std::string> custom_parameters; MXS_CONFIG_PARAMETER custom_parameters;
}; };
/** /**

91
server/core/server.cc
View File

@ -122,34 +122,6 @@ ThisUnit this_unit;
const char ERR_TOO_LONG_CONFIG_VALUE[] = "The new value for %s is too long. Maximum length is %i characters."; const char ERR_TOO_LONG_CONFIG_VALUE[] = "The new value for %s is too long. Maximum length is %i characters.";
// Converts Server::ConfigParam to MXS_CONFIG_PARAM and keeps them in the same order. Required for some
// functions taking MXS_CONFIG_PARAMs as arguments.
class ParamAdaptor
{
public:
ParamAdaptor(const std::vector<Server::ConfigParameter>& parameters)
{
m_params = new MXS_CONFIG_PARAMETER;
for (const auto& elem : parameters)
{
m_params->set(elem.name, elem.value);
}
}
~ParamAdaptor()
{
delete m_params;
}
operator MXS_CONFIG_PARAMETER*()
{
return m_params;
}
private:
MXS_CONFIG_PARAMETER* m_params = nullptr;
};
/** /**
* Write to char array by first zeroing any extra space. This reduces effects of concurrent reading. * Write to char array by first zeroing any extra space. This reduces effects of concurrent reading.
* Concurrent writing should be prevented by the caller. * Concurrent writing should be prevented by the caller.
@ -273,14 +245,14 @@ Server* Server::server_alloc(const char* name, MXS_CONFIG_PARAMETER* params)
server->set_monitor_password(monpw); server->set_monitor_password(monpw);
} }
server->m_settings.all_parameters = *params;
for (auto p : *params) for (auto p : *params)
{ {
const string name = p.first; const string& param_name = p.first;
const string value = p.second; const string& param_value = p.second;
server->m_settings.all_parameters.push_back({name, value}); if (server->is_custom_parameter(param_name))
if (server->is_custom_parameter(name))
{ {
server->set_parameter(name, value); server->set_custom_parameter(param_name, param_value);
} }
} }
@ -522,7 +494,7 @@ void Server::print_to_dcb(DCB* dcb) const
for (const auto& elem : server->m_settings.all_parameters) for (const auto& elem : server->m_settings.all_parameters)
{ {
dcb_printf(dcb, "\t %s\t%s\n", dcb_printf(dcb, "\t %s\t%s\n",
elem.name.c_str(), elem.value.c_str()); elem.first.c_str(), elem.second.c_str());
} }
} }
dcb_printf(dcb, "\tNumber of connections: %d\n", server->stats.n_connections); dcb_printf(dcb, "\tNumber of connections: %d\n", server->stats.n_connections);
@ -757,37 +729,23 @@ string Server::monitor_password() const
return m_settings.monpw; return m_settings.monpw;
} }
void Server::set_parameter(const string& name, const string& value) void Server::set_custom_parameter(const string& name, const string& value)
{ {
// Set/add the parameter in both containers // Set/add the parameter in both containers.
bool found = false; m_settings.all_parameters.set(name, value);
for (auto& elem : m_settings.all_parameters) Guard guard(m_settings.lock);
{ m_settings.custom_parameters.set(name, value);
if (name == elem.name)
{
found = true;
elem.value = value; // Update
break;
}
}
if (!found)
{
m_settings.all_parameters.push_back({name, value});
}
std::lock_guard<std::mutex> guard(m_settings.lock);
m_settings.custom_parameters[name] = value;
} }
string Server::get_custom_parameter(const string& name) const string Server::get_custom_parameter(const string& name) const
{ {
string rval; Guard guard(m_settings.lock);
std::lock_guard<std::mutex> guard(m_settings.lock); return m_settings.custom_parameters.get_string(name);
auto iter = m_settings.custom_parameters.find(name); }
if (iter != m_settings.custom_parameters.end())
{ void Server::set_normal_parameter(const std::string& name, const std::string& value)
rval = iter->second; {
} m_settings.all_parameters.set(name, value);
return rval;
} }
/** /**
@ -898,15 +856,19 @@ bool Server::create_server_config(const Server* server, const char* filename)
// TODO: Check for return values on all of the dprintf calls // TODO: Check for return values on all of the dprintf calls
const MXS_MODULE* mod = get_module(server->m_settings.protocol.c_str(), MODULE_PROTOCOL); const MXS_MODULE* mod = get_module(server->m_settings.protocol.c_str(), MODULE_PROTOCOL);
string config = generate_config_string(server->name(), string config = generate_config_string(server->name(),
*ParamAdaptor(server->m_settings.all_parameters), server->m_settings.all_parameters,
config_server_params, config_server_params,
mod->parameters); mod->parameters);
// Print custom parameters // Print custom parameters. The generate_config_string()-call doesn't print them.
{
Guard guard(server->m_settings.lock);
for (const auto& elem : server->m_settings.custom_parameters) for (const auto& elem : server->m_settings.custom_parameters)
{ {
config += elem.first + "=" + elem.second + "\n"; config += elem.first + "=" + elem.second + "\n";
} }
}
if (dprintf(file, "%s", config.c_str()) == -1) if (dprintf(file, "%s", config.c_str()) == -1)
{ {
@ -1029,13 +991,15 @@ json_t* Server::server_json_attributes(const Server* server)
json_t* params = json_object(); json_t* params = json_object();
const MXS_MODULE* mod = get_module(server->m_settings.protocol.c_str(), MODULE_PROTOCOL); const MXS_MODULE* mod = get_module(server->m_settings.protocol.c_str(), MODULE_PROTOCOL);
config_add_module_params_json(ParamAdaptor(server->m_settings.all_parameters), config_add_module_params_json(&server->m_settings.all_parameters,
{CN_TYPE}, {CN_TYPE},
config_server_params, config_server_params,
mod->parameters, mod->parameters,
params); params);
// Add weighting parameters that weren't added by config_add_module_params_json // Add weighting parameters that weren't added by config_add_module_params_json
{
Guard guard(server->m_settings.lock);
for (const auto& elem : server->m_settings.custom_parameters) for (const auto& elem : server->m_settings.custom_parameters)
{ {
if (!json_object_get(params, elem.first.c_str())) if (!json_object_get(params, elem.first.c_str()))
@ -1043,6 +1007,7 @@ json_t* Server::server_json_attributes(const Server* server)
json_object_set_new(params, elem.first.c_str(), json_string(elem.second.c_str())); json_object_set_new(params, elem.first.c_str(), json_string(elem.second.c_str()));
} }
} }
}
json_object_set_new(attr, CN_PARAMETERS, params); json_object_set_new(attr, CN_PARAMETERS, params);

3
server/core/test/test_server.cc
View File

@ -51,7 +51,6 @@ static MXS_CONFIG_PARAMETER* params = new MXS_CONFIG_PARAMETER;
*/ */
static int test1() static int test1()
{ {
int result;
std::string status; std::string status;
/* Server tests */ /* Server tests */
@ -62,7 +61,7 @@ static int test1()
fprintf(stderr, "\t..done\nTest Parameter for Server."); fprintf(stderr, "\t..done\nTest Parameter for Server.");
mxb_assert_message(server->get_custom_parameter("name").empty(), mxb_assert_message(server->get_custom_parameter("name").empty(),
"Parameter should be empty when not set"); "Parameter should be empty when not set");
server->set_parameter("name", "value"); server->set_custom_parameter("name", "value");
std::string buf = server->get_custom_parameter("name"); std::string buf = server->get_custom_parameter("name");
mxb_assert_message(buf == "value", "Parameter should be returned correctly"); mxb_assert_message(buf == "value", "Parameter should be returned correctly");
fprintf(stderr, "\t..done\nTesting Unique Name for Server."); fprintf(stderr, "\t..done\nTesting Unique Name for Server.");