MXS-1929: Take SFilterDef into use

The service now uses a std::vector<SFilterDef> to store the filters it
uses. Most internal parts deal with the SFilterDef but debugcmd.cc still
moves raw pointers around (needs to be changed).
This commit is contained in:
Markus Mäkelä 2018-08-01 19:20:01 +03:00
parent 00ab890b19
commit 4d3dbb2040
No known key found for this signature in database
GPG Key ID: 72D48FCE664F7B19
12 changed files with 96 additions and 144 deletions

View File

@ -225,16 +225,6 @@ typedef struct mxs_filter_def
{
} MXS_FILTER_DEF;
/**
* Lookup a filter definition using the unique section name in
* the configuration file.
*
* @param name The name of a filter.
*
* @return A filter definition or NULL if not found.
*/
MXS_FILTER_DEF *filter_def_find(const char *name);
/**
* Get the name of a filter definition. This corresponds to
* to a filter section in the configuration file.

View File

@ -146,8 +146,6 @@ typedef struct service
* when querying them from the server. MySQL Workbench seems
* to escape at least the underscore character. */
SERVICE_REFRESH_RATE *rate_limits; /**< The refresh rate limits for users of each thread */
MXS_FILTER_DEF **filters; /**< Ordered list of filters */
int n_filters; /**< Number of filters */
int64_t conn_idle_timeout; /**< Session timeout in seconds */
char weightby[MAX_SERVICE_WEIGHTBY_LEN]; /**< Service weighting parameter name */
bool retry_start; /**< If starting of the service should be retried later */

View File

@ -1035,9 +1035,9 @@ bool runtime_create_filter(const char *name, const char *module, MXS_CONFIG_PARA
mxs::SpinLockGuard guard(crt_lock);
bool rval = false;
if (filter_def_find(name) == NULL)
if (!filter_find(name))
{
FilterDef* filter = NULL;
SFilterDef filter;
CONFIG_CONTEXT ctx{(char*)""};
ctx.parameters = load_defaults(module, MODULE_FILTER, CN_FILTER);
@ -1048,7 +1048,7 @@ bool runtime_create_filter(const char *name, const char *module, MXS_CONFIG_PARA
config_replace_param(&ctx, p->name, p->value);
}
if ((filter = filter_alloc(name, module, ctx.parameters)) == NULL)
if (!(filter = filter_alloc(name, module, ctx.parameters)))
{
runtime_error("Could not create filter '%s' with module '%s'", name, module);
}
@ -1077,7 +1077,7 @@ bool runtime_create_filter(const char *name, const char *module, MXS_CONFIG_PARA
return rval;
}
bool runtime_destroy_filter(FilterDef* filter)
bool runtime_destroy_filter(const SFilterDef& filter)
{
ss_dassert(filter);
bool rval = false;
@ -1091,7 +1091,7 @@ bool runtime_destroy_filter(FilterDef* filter)
else
{
runtime_error("Filter '%s' cannot be destroyed: Remove it from all services "
"first", filter->name);
"first", filter->name.c_str());
}
return rval;
@ -1438,7 +1438,7 @@ static bool filter_to_service_relation_is_valid(const std::string& type, const s
static bool service_to_filter_relation_is_valid(const std::string& type, const std::string& value)
{
return type == CN_FILTERS && filter_def_find(value.c_str());
return type == CN_FILTERS && filter_find(value.c_str());
}
static bool unlink_server_from_objects(SERVER* server, StringSet& relations)
@ -1898,9 +1898,9 @@ MXS_MONITOR* runtime_create_monitor_from_json(json_t* json)
return rval;
}
FilterDef* runtime_create_filter_from_json(json_t* json)
bool runtime_create_filter_from_json(json_t* json)
{
FilterDef* rval = NULL;
bool rval = false;
if (validate_object_json(json, {MXS_JSON_PTR_MODULE}, {filter_to_service}))
{
@ -1908,11 +1908,7 @@ FilterDef* runtime_create_filter_from_json(json_t* json)
const char* module = json_string_value(mxs_json_pointer(json, MXS_JSON_PTR_MODULE));
MXS_CONFIG_PARAMETER* params = extract_parameters_from_json(json);
if (runtime_create_filter(name, module, params))
{
rval = filter_find(name);
ss_dassert(rval);
}
rval = runtime_create_filter(name, module, params);
config_parameter_free(params);
}

View File

@ -139,6 +139,7 @@ FilterDef::~FilterDef()
*/
void filter_free(const SFilterDef& filter)
{
ss_dassert(filter);
// Removing the filter from the list will trigger deletion once it's no longer in use
Guard guard(filter_lock);
auto it = std::remove(all_filters.begin(), all_filters.end(), filter);
@ -162,11 +163,13 @@ SFilterDef filter_find(const char *name)
bool filter_can_be_destroyed(const SFilterDef& filter)
{
ss_dassert(filter);
return !service_filter_in_use(filter);
}
void filter_destroy(const SFilterDef& filter)
{
ss_dassert(filter);
ss_dassert(filter_can_be_destroyed(filter));
ss_info_dassert(!true, "Not yet implemented");
}
@ -188,18 +191,21 @@ void filter_destroy_instances()
const char* filter_def_get_name(const MXS_FILTER_DEF* filter_def)
{
const FilterDef* filter = static_cast<const FilterDef*>(filter_def);
ss_dassert(filter);
return filter->name.c_str();
}
const char* filter_def_get_module_name(const MXS_FILTER_DEF* filter_def)
{
const FilterDef* filter = static_cast<const FilterDef*>(filter_def);
ss_dassert(filter);
return filter->module.c_str();
}
MXS_FILTER* filter_def_get_instance(const MXS_FILTER_DEF* filter_def)
{
const FilterDef* filter = static_cast<const FilterDef*>(filter_def);
ss_dassert(filter);
return filter->filter;
}
@ -252,6 +258,7 @@ dprintAllFilters(DCB *dcb)
*/
void dprintFilter(DCB *dcb, const SFilterDef& filter)
{
ss_dassert(filter);
dcb_printf(dcb, "FilterDef %p (%s)\n", filter.get(), filter->name.c_str());
dcb_printf(dcb, "\tModule: %s\n", filter->module.c_str());
if (filter->obj && filter->filter)
@ -299,6 +306,7 @@ dListFilters(DCB *dcb)
void
filter_add_parameter(SFilterDef& filter, const char *name, const char *value)
{
ss_dassert(filter);
CONFIG_CONTEXT ctx = {};
ctx.object = (char*)"";
@ -321,6 +329,7 @@ filter_add_parameter(SFilterDef& filter, const char *name, const char *value)
*/
MXS_DOWNSTREAM* filter_apply(const SFilterDef& filter, MXS_SESSION *session, MXS_DOWNSTREAM *downstream)
{
ss_dassert(filter);
MXS_DOWNSTREAM *me;
if ((me = (MXS_DOWNSTREAM *)MXS_CALLOC(1, sizeof(MXS_DOWNSTREAM))) == NULL)
@ -355,6 +364,7 @@ MXS_DOWNSTREAM* filter_apply(const SFilterDef& filter, MXS_SESSION *session, MXS
*/
MXS_UPSTREAM* filter_upstream(const SFilterDef& filter, MXS_FILTER_SESSION *fsession, MXS_UPSTREAM *upstream)
{
ss_dassert(filter);
MXS_UPSTREAM *me = NULL;
/*
@ -382,6 +392,7 @@ MXS_UPSTREAM* filter_upstream(const SFilterDef& filter, MXS_FILTER_SESSION *fses
json_t* filter_parameters_to_json(const SFilterDef& filter)
{
ss_dassert(filter);
json_t* rval = json_object();
/** Add custom module parameters */
@ -393,6 +404,7 @@ json_t* filter_parameters_to_json(const SFilterDef& filter)
json_t* filter_json_data(const SFilterDef& filter, const char* host)
{
ss_dassert(filter);
json_t* rval = json_object();
json_object_set_new(rval, CN_ID, json_string(filter->name.c_str()));
@ -426,6 +438,7 @@ json_t* filter_json_data(const SFilterDef& filter, const char* host)
json_t* filter_to_json(const SFilterDef& filter, const char* host)
{
ss_dassert(filter);
string self = MXS_JSON_API_FILTERS;
self += filter->name;
return mxs_json_resource(host, self.c_str(), filter_json_data(filter, host));
@ -503,6 +516,7 @@ json_t* FilterSession::diagnostics_json() const
static bool create_filter_config(const SFilterDef& filter, const char *filename)
{
ss_dassert(filter);
int file = open(filename, O_EXCL | O_CREAT | O_WRONLY, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
if (file == -1)
@ -535,6 +549,7 @@ static bool create_filter_config(const SFilterDef& filter, const char *filename)
bool filter_serialize(const SFilterDef& filter)
{
ss_dassert(filter);
bool rval = false;
char filename[PATH_MAX];
snprintf(filename, sizeof(filename), "%s/%s.cnf.tmp", get_config_persistdir(),

View File

@ -15,6 +15,10 @@
#include <maxscale/service.h>
#include <maxscale/resultset.hh>
#include <vector>
#include "filter.hh"
/**
* @file service.h - MaxScale internal service functions
*/
@ -25,11 +29,13 @@
* These functions should only be called by the MaxScale core.
*/
// The internal service representation. Currently it only inherits the SERVICE struct.
// The internal service representation
class Service: public SERVICE
{
public:
~Service();
std::vector<SFilterDef> filters; /**< Ordered list of filters */
};
/**
@ -161,7 +167,7 @@ bool service_server_in_use(const SERVER *server);
*
* @return True if at least one service uses the filter
*/
bool service_filter_in_use(const MXS_FILTER_DEF *filter);
bool service_filter_in_use(const SFilterDef& filter);
/** Update the server weights used by services */
void service_update_weights();
@ -353,7 +359,7 @@ json_t* service_relations_to_server(const SERVER* server, const char* host);
*
* @return Array of service links
*/
json_t* service_relations_to_filter(const MXS_FILTER_DEF* filter, const char* host);
json_t* service_relations_to_filter(const SFilterDef& filter, const char* host);
std::unique_ptr<ResultSet> serviceGetList(void);
std::unique_ptr<ResultSet> serviceGetListenerList(void);

View File

@ -340,8 +340,9 @@ static bool process_argument(const MODULECMD *cmd, modulecmd_arg_type_t *type, c
break;
case MODULECMD_ARG_FILTER:
if ((arg->value.filter = filter_def_find((char*)value)))
if (auto f = filter_find((char*)value))
{
arg->value.filter = f.get();
const char* orig_name = filter_def_get_module_name(arg->value.filter);
const char* eff_name = mxs_module_get_effective_name(orig_name);
if (MODULECMD_ALLOW_NAME_MISMATCH(type) || strcasecmp(cmd->domain, eff_name) == 0)

View File

@ -104,7 +104,7 @@ bool Resource::matching_variable_path(const string& path, const string& target)
{
if ((path == ":service" && service_find(target.c_str())) ||
(path == ":server" && server_find_by_unique_name(target.c_str())) ||
(path == ":filter" && filter_def_find(target.c_str())) ||
(path == ":filter" && filter_find(target.c_str())) ||
(path == ":monitor" && monitor_find(target.c_str())) ||
(path == ":module" && get_module(target.c_str(), NULL)) ||
(path == ":inetuser" && admin_inet_user_exists(target.c_str())) ||
@ -486,7 +486,7 @@ HttpResponse cb_delete_service(const HttpRequest& request)
HttpResponse cb_delete_filter(const HttpRequest& request)
{
FilterDef* filter = filter_find(request.uri_part(1).c_str());
auto filter = filter_find(request.uri_part(1).c_str());
ss_dassert(filter);
if (runtime_destroy_filter(filter))
@ -549,7 +549,7 @@ HttpResponse cb_all_filters(const HttpRequest& request)
HttpResponse cb_get_filter(const HttpRequest& request)
{
FilterDef* filter = filter_find(request.uri_part(1).c_str());
auto filter = filter_find(request.uri_part(1).c_str());
ss_dassert(filter);
return HttpResponse(MHD_HTTP_OK, filter_to_json(filter, request.host()));
}

View File

@ -118,8 +118,6 @@ Service* service_alloc(const char *name, const char *router, MXS_CONFIG_PARAMETE
service->ports = NULL;
service->dbref = NULL;
service->n_dbref = 0;
service->filters = NULL;
service->n_filters = 0;
service->weightby[0] = '\0';
spinlock_init(&service->spin);
@ -211,7 +209,6 @@ Service::~Service()
config_parameter_free(svc_config_param);
MXS_FREE(name);
MXS_FREE(routerModule);
MXS_FREE(filters);
MXS_FREE(rate_limits);
}
@ -1234,18 +1231,18 @@ void service_set_retry_interval(Service *service, int value)
bool service_set_filters(Service* service, const char* filters)
{
bool rval = true;
std::vector<MXS_FILTER_DEF*> flist;
std::vector<SFilterDef> flist;
uint64_t capabilities = 0;
for (auto& f: mxs::strtok(filters, "|"))
{
fix_object_name(f);
if (FilterDef* def = filter_find(f.c_str()))
if (auto def = filter_find(f.c_str()))
{
flist.push_back(def);
const MXS_MODULE* module = get_module(def->module, MODULE_FILTER);
const MXS_MODULE* module = get_module(def->module.c_str(), MODULE_FILTER);
ss_dassert(module);
capabilities |= module->module_capabilities;
@ -1263,10 +1260,7 @@ bool service_set_filters(Service* service, const char* filters)
if (rval)
{
service->filters = (MXS_FILTER_DEF**)MXS_MALLOC((flist.size() + 1) * sizeof(MXS_FILTER_DEF*));
std::copy(flist.begin(), flist.end(), service->filters);
service->n_filters = flist.size();
service->filters[service->n_filters] = NULL;
service->filters = flist;
service->capabilities |= capabilities;
}
@ -1362,13 +1356,14 @@ void dprintService(DCB *dcb, SERVICE *svc)
asctime_r(localtime_r(&service->stats.started, &result), timebuf));
dcb_printf(dcb, "\tRoot user access: %s\n",
service->enable_root ? "Enabled" : "Disabled");
if (service->n_filters)
if (!service->filters.empty())
{
dcb_printf(dcb, "\tFilter chain: ");
for (i = 0; i < service->n_filters; i++)
const char* sep = "";
for (const auto& f : service->filters)
{
dcb_printf(dcb, "%s %s ", filter_def_get_name(service->filters[i]),
i + 1 < service->n_filters ? "|" : "");
dcb_printf(dcb, "%s %s ", f->name.c_str(), sep);
sep = "|";
}
dcb_printf(dcb, "\n");
}
@ -1930,23 +1925,21 @@ bool service_server_in_use(const SERVER *server)
return rval;
}
bool service_filter_in_use(const MXS_FILTER_DEF *filter)
bool service_filter_in_use(const SFilterDef& filter)
{
ss_dassert(filter);
bool rval = false;
spinlock_acquire(&service_spin);
for (Service *service: allServices)
for (Service *service : allServices)
{
spinlock_acquire(&service->spin);
for (int i = 0; i < service->n_filters; i++)
if (std::find(service->filters.begin(),
service->filters.end(), filter) != service->filters.end())
{
if (service->filters[i] == filter)
{
rval = true;
break;
}
rval = true;
}
spinlock_release(&service->spin);
@ -2315,18 +2308,20 @@ json_t* service_attributes(const SERVICE* service)
return attr;
}
json_t* service_relationships(const SERVICE* service, const char* host)
json_t* service_relationships(const SERVICE* svc, const char* host)
{
const Service* service = static_cast<const Service*>(svc);
/** Store relationships to other objects */
json_t* rel = json_object();
if (service->n_filters)
if (!service->filters.empty())
{
json_t* filters = mxs_json_relationship(host, MXS_JSON_API_FILTERS);
for (int i = 0; i < service->n_filters; i++)
for (const auto& f : service->filters)
{
mxs_json_add_relation(filters, filter_def_get_name(service->filters[i]), CN_FILTERS);
mxs_json_add_relation(filters, f->name.c_str(), CN_FILTERS);
}
json_object_set_new(rel, CN_FILTERS, filters);
@ -2418,7 +2413,7 @@ json_t* service_list_to_json(const char* host)
return mxs_json_resource(host, MXS_JSON_API_SERVICES, arr);
}
json_t* service_relations_to_filter(const MXS_FILTER_DEF* filter, const char* host)
json_t* service_relations_to_filter(const SFilterDef& filter, const char* host)
{
json_t* rel = mxs_json_relationship(host, MXS_JSON_API_SERVICES);
@ -2428,9 +2423,9 @@ json_t* service_relations_to_filter(const MXS_FILTER_DEF* filter, const char* ho
{
spinlock_acquire(&service->spin);
for (int i = 0; i < service->n_filters; i++)
for (const auto& f : service->filters)
{
if (service->filters[i] == filter)
if (f == filter)
{
mxs_json_add_relation(rel, service->name, CN_SERVICES);
}

View File

@ -199,7 +199,6 @@ static MXS_SESSION* session_alloc_body(SERVICE* service, DCB* client_dcb,
session->tail.clientReply = session_reply;
if (SESSION_STATE_TO_BE_FREED != session->state
&& service->n_filters > 0
&& !session_setup_filters(session))
{
session->state = SESSION_STATE_TO_BE_FREED;
@ -561,7 +560,7 @@ dprintSession(DCB *dcb, MXS_SESSION *print_session)
for (i = 0; i < print_session->n_filters; i++)
{
FilterDef* filter = static_cast<FilterDef*>(print_session->filters[i].filter);
dcb_printf(dcb, "\tFilter: %s\n", filter->name);
dcb_printf(dcb, "\tFilter: %s\n", filter->name.c_str());
filter->obj->diagnostics(print_session->filters[i].instance,
print_session->filters[i].session, dcb);
}
@ -653,52 +652,52 @@ session_state(mxs_session_state_t state)
static int
session_setup_filters(MXS_SESSION *session)
{
SERVICE *service = session->service;
Service* service = static_cast<Service*>(session->service);
MXS_DOWNSTREAM *head;
MXS_UPSTREAM *tail;
int i;
int i = 0;
if ((session->filters = (SESSION_FILTER*)MXS_CALLOC(service->n_filters,
if (service->filters.empty())
{
return 1;
}
if ((session->filters = (SESSION_FILTER*)MXS_CALLOC(service->filters.size(),
sizeof(SESSION_FILTER))) == NULL)
{
return 0;
}
session->n_filters = service->n_filters;
for (i = service->n_filters - 1; i >= 0; i--)
session->n_filters = service->filters.size();
for (auto r = service->filters.rbegin(); r != service->filters.rend(); r++)
{
if (service->filters[i] == NULL)
if ((head = filter_apply(*r, session, &session->head)) == NULL)
{
MXS_ERROR("Service '%s' contians an unresolved filter.", service->name);
MXS_ERROR("Failed to create filter '%s' for service '%s'.\n",
filter_def_get_name(r->get()), service->name);
return 0;
}
if ((head = filter_apply((FilterDef*)service->filters[i], session,
&session->head)) == NULL)
{
MXS_ERROR("Failed to create filter '%s' for "
"service '%s'.\n",
filter_def_get_name(service->filters[i]),
service->name);
return 0;
}
session->filters[i].filter = service->filters[i];
session->filters[i].filter = r->get();
session->filters[i].session = head->session;
session->filters[i].instance = head->instance;
i++;
session->head = *head;
MXS_FREE(head);
}
for (i = 0; i < service->n_filters; i++)
for (auto r = service->filters.begin(); r != service->filters.end(); r++)
{
if ((tail = filter_upstream((FilterDef*)service->filters[i],
session->filters[i].session,
&session->tail)) == NULL)
if ((tail = filter_upstream(*r, session->filters[i].session, &session->tail)) == NULL)
{
MXS_ERROR("Failed to create filter '%s' for service '%s'.",
filter_def_get_name(service->filters[i]),
filter_def_get_name(r->get()),
service->name);
return 0;
}
i--;
/*
* filter_upstream may simply return the 3 parameter if
* the filter has no upstream entry point. So no need

View File

@ -46,7 +46,7 @@
static int
test1()
{
FilterDef *f1, *f2;
SFilterDef f1, f2;
if ((f1 = filter_alloc("test1", "qlafilter", NULL)) == NULL)
{
@ -79,16 +79,13 @@ test1()
static int
test2()
{
FilterDef *f1;
SFilterDef f1;
if ((f1 = filter_alloc("test1", "qlafilter", NULL)) == NULL)
{
fprintf(stderr, "filter_alloc: test 1 failed.\n");
return 1;
}
filter_add_parameter(f1, "name1", "value1");
filter_add_parameter(f1, "name2", "value2");
filter_add_parameter(f1, "name3", "value3");
filter_free(f1);
return 0;
@ -102,7 +99,7 @@ test2()
static int
test3()
{
FilterDef *f1;
SFilterDef f1;
char name[40];
int i, n_filters = 1000;
@ -119,7 +116,7 @@ test3()
for (i = 0; i < n_filters; i++)
{
sprintf(name, "filter%d", i);
if ((f1 = filter_find(name)) == NULL)
if (!(f1 = filter_find(name)))
{
fprintf(stderr, "filter_find: test 3 failed.\n");
return 1;
@ -128,13 +125,13 @@ test3()
for (i = 0; i < n_filters; i++)
{
sprintf(name, "filter%d", i);
if ((f1 = filter_find(name)) == NULL)
if (!(f1 = filter_find(name)))
{
fprintf(stderr, "filter_find: test 3 failed.\n");
return 1;
}
filter_free(f1);
if ((f1 = filter_find(name)) != NULL)
if ((f1 = filter_find(name)))
{
fprintf(stderr,
"filter_find: test 3 failed - found deleted filter.\n");

View File

@ -19,46 +19,6 @@
#include <maxscale/modutil.h>
/**
* Detect loops in the filter chain.
*/
bool recursive_tee_usage(std::set<std::string>& services, SERVICE* service)
{
if (!services.insert(service->name).second)
{
/** The service name was already in the set */
return true;
}
for (int i = 0; i < service->n_filters; i++)
{
const char* module = filter_def_get_module_name(service->filters[i]);
if (strcmp(module, "tee") == 0)
{
/*
* Found a Tee filter, recurse down its path
* if the service name isn't already in the hashtable.
*/
Tee* inst = (Tee*)filter_def_get_instance(service->filters[i]);
if (inst == NULL)
{
/**
* This tee instance hasn't been initialized yet and full
* resolution of recursion cannot be done now.
*/
}
else if (recursive_tee_usage(services, inst->get_service()))
{
return true;
}
}
}
return false;
}
TeeSession::TeeSession(MXS_SESSION* session, LocalClient* client,
pcre2_code* match, pcre2_match_data* md_match,
pcre2_code* exclude, pcre2_match_data* md_exclude):
@ -73,15 +33,6 @@ TeeSession::TeeSession(MXS_SESSION* session, LocalClient* client,
TeeSession* TeeSession::create(Tee* my_instance, MXS_SESSION* session)
{
std::set<std::string> services;
if (recursive_tee_usage(services, my_instance->get_service()))
{
MXS_ERROR("%s: Recursive use of tee filter in service.",
session->service->name);
return NULL;
}
LocalClient* client = NULL;
pcre2_code* match = NULL;
pcre2_code* exclude = NULL;

View File

@ -1850,9 +1850,13 @@ convert_arg(char *arg, int arg_type)
break;
case ARG_TYPE_FILTER:
{
fix_object_name(arg);
rval = (unsigned long)filter_def_find(arg);
auto f = filter_find(arg);
// This will cause problems in the long run
rval = (unsigned long) (f ? f.get() : NULL);
break;
}
case ARG_TYPE_NUMERIC: