From a4975edbba6d74bc0dfce8ec1b47ec399a680702 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 7 Sep 2017 15:17:18 +0300 Subject: [PATCH] MXS-1389: Fix rule reloading and query parsing requirements Reloading of rules now properly uses the current rule file if no argument was provided. The rule version counter also used atomic operations for the sake of correctness. The rule parsing is now only required for DML type statements that should be fully parsed. --- maxscale-system-test/fw2/pass4 | 4 ++- maxscale-system-test/fw2/pass5 | 3 +- maxscale-system-test/fwf_reload.cpp | 2 +- .../modules/filter/dbfwfilter/dbfwfilter.cc | 13 ++------ .../modules/filter/dbfwfilter/dbfwfilter.hh | 1 - server/modules/filter/dbfwfilter/rules.hh | 30 ++++++++++++++++--- 6 files changed, 35 insertions(+), 18 deletions(-) diff --git a/maxscale-system-test/fw2/pass4 b/maxscale-system-test/fw2/pass4 index 6894c8432..3be6d33f1 100644 --- a/maxscale-system-test/fw2/pass4 +++ b/maxscale-system-test/fw2/pass4 @@ -1,4 +1,6 @@ -create or replace table test.t1(id int); +drop table if exists test.t1; +create table test.t1(id int); +drop function if exists my_function; create function my_function (arg int) returns int deterministic return arg * arg; select "sum(1)"; select (1); diff --git a/maxscale-system-test/fw2/pass5 b/maxscale-system-test/fw2/pass5 index 3bbec0fac..dd655a6e3 100644 --- a/maxscale-system-test/fw2/pass5 +++ b/maxscale-system-test/fw2/pass5 @@ -1 +1,2 @@ -create or replace table t1 (id int); +drop table if exists t1; +create table t1 (id int); diff --git a/maxscale-system-test/fwf_reload.cpp b/maxscale-system-test/fwf_reload.cpp index fa6833731..38c3d3c8a 100644 --- a/maxscale-system-test/fwf_reload.cpp +++ b/maxscale-system-test/fwf_reload.cpp @@ -20,7 +20,7 @@ int main(int argc, char *argv[]) char rules_dir[4096]; sprintf(rules_dir, "%s/fw/", test_dir); - int N = 10; + int N = 12; int i; Test->stop_maxscale(); diff --git a/server/modules/filter/dbfwfilter/dbfwfilter.cc b/server/modules/filter/dbfwfilter/dbfwfilter.cc index f73c65f36..5ec96e998 100644 --- a/server/modules/filter/dbfwfilter/dbfwfilter.cc +++ b/server/modules/filter/dbfwfilter/dbfwfilter.cc @@ -376,10 +376,9 @@ TIMERANGE* split_reverse_time(TIMERANGE* tr) bool dbfw_reload_rules(const MODULECMD_ARG *argv, json_t** output) { - bool rval = true; MXS_FILTER_DEF *filter = argv->argv[0].value.filter; Dbfw *inst = (Dbfw*)filter_def_get_instance(filter); - std::string filename; + std::string filename = inst->get_rule_file(); if (modulecmd_arg_is_present(argv, 1)) { @@ -1158,7 +1157,7 @@ std::string Dbfw::get_rule_file() const int Dbfw::get_rule_version() const { - return m_version; + return atomic_load_int32(&m_version); } bool Dbfw::do_reload_rules(std::string filename) @@ -1173,7 +1172,7 @@ bool Dbfw::do_reload_rules(std::string filename) { rval = true; m_filename = filename; - m_version++; + atomic_add(&m_version, 1); MXS_NOTICE("Reloaded rules from: %s", filename.c_str()); } else @@ -1197,12 +1196,6 @@ bool Dbfw::reload_rules(std::string filename) return do_reload_rules(filename); } -bool Dbfw::reload_rules() -{ - mxs::SpinLockGuard guard(m_lock); - return do_reload_rules(m_filename); -} - /** * Retrieve the user specific data for this session * diff --git a/server/modules/filter/dbfwfilter/dbfwfilter.hh b/server/modules/filter/dbfwfilter/dbfwfilter.hh index 6763e85c6..e7181159d 100644 --- a/server/modules/filter/dbfwfilter/dbfwfilter.hh +++ b/server/modules/filter/dbfwfilter/dbfwfilter.hh @@ -251,7 +251,6 @@ public: * error occurs, it is stored in the modulecmd error system. */ bool reload_rules(std::string filename); - bool reload_rules(); /** Diagnostic routines */ void diagnostics(DCB *dcb) const; diff --git a/server/modules/filter/dbfwfilter/rules.hh b/server/modules/filter/dbfwfilter/rules.hh index 24657aa41..a7ee17c0a 100644 --- a/server/modules/filter/dbfwfilter/rules.hh +++ b/server/modules/filter/dbfwfilter/rules.hh @@ -18,6 +18,28 @@ #include +namespace +{ + +static bool is_dml(GWBUF* buffer) +{ + qc_query_op_t optype = qc_get_operation(buffer); + + switch (optype) + { + case QUERY_OP_SELECT: + case QUERY_OP_UPDATE: + case QUERY_OP_INSERT: + case QUERY_OP_DELETE: + return true; + + default: + return false; + } +} + +} + /** * A structure used to identify individual rules and to store their contents * @@ -72,7 +94,7 @@ public: bool need_full_parsing(GWBUF* buffer) const { - return true; + return is_dml(buffer); } bool matches_query(DbfwSession* session, GWBUF* buffer, char** msg) const; @@ -98,7 +120,7 @@ public: bool need_full_parsing(GWBUF* buffer) const { - return true; + return is_dml(buffer); } bool matches_query(DbfwSession* session, GWBUF* buffer, char** msg) const; @@ -118,7 +140,7 @@ class ValueListRule: public Rule public: bool need_full_parsing(GWBUF* buffer) const { - return true; + return is_dml(buffer); } protected: @@ -227,7 +249,7 @@ public: bool need_full_parsing(GWBUF* buffer) const { - return true; + return is_dml(buffer); } bool matches_query(DbfwSession* session, GWBUF* buffer, char** msg) const;