commit 2605c4c9554a9ca23722e547d9aec942188063ea Author: Dmitry Lenev Date: Thu Feb 9 14:23:20 2023 +0100 Bug#44625 Triggers should not be loaded on SELECTs Avoid caching fully-loaded/parsed trigger bodies in the Table Cache for read-only statements. This is done by implementing lazy full-loading/parsing of trigger bodies, i.e. we don't parse and finalize their load by default, but only do this if operation that does data change comes. Once such operation completes we return the TABLE instance and associated with it triggers to the Table Cache. Later we try to prefer reusing TABLE instances with fully-loaded triggers for data change operations and TABLE instances sans trigger bodies for read-only operations. To achieve that Table Cache code was changed to keep two lists of unused TABLE objects for each tables instead of one - with fully-loaded triggers and without them. To ensure that spike in data change workload won't fill up the Table Cache by TABLE objects with fully-loaded triggers soft limit on number of such objects in the cache was introduced. If this limit is exceeded unused TABLE objects with fully-loaded/parsed triggers are evicted from the cache in LRU fashion. Users can control this soft limit using new global table_open_cache_triggers variable. By default this variable is set to 524288, so eviction of unused TABLE objects with fully-loaded triggers happens as before this patch. New status Table_open_cache_triggers_hits/misses/overflows variables were introduced to allow users to see how well TABLE objects with fully-loaded triggers are cached and if table_open_cache_triggers value needs to be adjusted. To support the above changes and to further reduce memory consumption by trigger-related objects this patch is also introducing Trigger objects which are bound to TABLE_SHARE and makes static metadata strings describing triggers associated with these objects only, rather than having multiple copies of these strings around (a copy for each Trigger object which is bound to each TABLE instance). These objects and their string attributes are allocated on share's memory root. They can't be used for trigger execution directly. Trigger objects which are bound to each TABLE instance are still there and used for trigger execution. But they are created from from corresponding Trigger objects bound to TABLE_SHARE and their static string attributes simply reference same strings allocated by TABLE_SHARE bound Trigger object. Also thanks to the above, we now perform initial phase of trigger loading, i.e. reading of information about triggers from data-dictionary (or its cache) and filling in static attributes of Trigger objects only once, during TABLE_SHARE creation/in open_table_def() call, rather than when each TABLE object is constructed from TABLE_SHARE. This patch reduces memory consumption by trigger objects even further, by getting rid of statically allocated 512-byte buffer to store parsing error messages in Table_trigger_dispatcher and Trigger classes. Since these buffers are not used in 99% of cases this looks like pure waste of memory. So these always-allocated fixed-size buffers are replaced with variable-size buffers which are allocated on memory root on demand, only when the table has a trigger with a parse error. Small change in behavior introduced by this patch: Unlike before SHOW CREATE TRIGGER now shows trigger definition even for triggers with un-parseable bodies. Such triggers might occur in a data-dictionary after upgrading to server version which makes some syntax used in them invalid. diff --git a/mysql-test/include/assert_trigger_cache_increment.inc b/mysql-test/include/assert_trigger_cache_increment.inc new file mode 100644 index 00000000000..4eb2a1f008a --- /dev/null +++ b/mysql-test/include/assert_trigger_cache_increment.inc @@ -0,0 +1,91 @@ +# ==== Purpose ==== +# +# Check that values of system status variables related to storing +# triggers in Table Cache (Table_open_cache_misses, +# Table_open_cache_trigger_hits, Table_open_cache_triggers_misses, +# Table_open_cache_triggers_overflows) have got expected increments +# since the last execution of this script, fail with debug info if +# they have not. +# +# ==== Usage ==== +# +# --let $expected_inc = { "tc_misses_inc": VALUE1, "trg_hits_inc": VALUE2, "trg_misses_inc" : VALUE3, "trg_overflows_inc" : VALUE4 } +# --source include/assert_trigger_cache_increment.inc +# +# $expected_inc +# The JSON object which specifies expected increments. +# +# At the end of the test, source include/destroy_json_functions.inc +# to remove all auxiliary .inc files created by this script. +# +# ==== Example ==== +# +# # Run SELECT reading from table which was absent in Table Cache so far. +# SELECT count(*) FROM t1; +# +# --let $expected_inc = { "tc_misses_inc": 1, "trg_hits_inc": 0, "trg_misses_inc" : 0, "trg_overflows_inc" : 0 } +# --source include/assert_trigger_cache_increment.inc +# +# Resulting output: +# include/assert.inc [Expected table cache misses increment : 1] +# include/assert.inc [Expected table cache triggers hits increment : 0] +# include/assert.inc [Expected table cache triggers misses increment : 0] +# include/assert.inc [Expected table cache triggers overflows increment : 0] +# + +# If missing, create helper scripts to iterate through bellow JSON array. +--let $has_json_functions = `SELECT INSTR('$json_function_files','sv_check_inc')` +if (!$has_json_functions) { + --let $json_label = sv_check_inc + --let $json_keys = paramname, psname, varname, atext + --source include/create_json_unpacking_iterator.inc +} + +# Array describing each status variable to be checked, what is the name +# of the corresponding script parameter with expected increment, what +# is name of variable to remember its value for future executions, and +# what is assertion text to be used for this variable. + +let $json_array = [ + { + "paramname" : "tc_misses_inc", + "psname": "Table_open_cache_misses", + "varname": "tc_misses_count", + "atext": "Expected table cache misses increment" + }, + { + "paramname" : "trg_hits_inc", + "psname": "Table_open_cache_triggers_hits", + "varname": "trg_hits_count", + "atext": "Expected table cache triggers hits increment" + }, + { + "paramname" : "trg_misses_inc", + "psname": "Table_open_cache_triggers_misses", + "varname": "trg_misses_count", + "atext": "Expected table cache triggers misses increment" + }, + { + "paramname" : "trg_overflows_inc", + "psname": "Table_open_cache_triggers_overflows", + "varname": "trg_overflows_count", + "atext": "Expected table cache triggers overflows increment" + } +]; + +# Iterate through the above array checking increment for each variable +# it describes. +--source $json_sv_check_inc_start + +while (!$json_sv_check_inc_done) { + --disable_query_log + --eval SET @oldvalue = ifnull(@$varname, 0) + --eval SELECT variable_value INTO @$varname FROM performance_schema.session_status WHERE variable_name ='$psname' + --let $expinc = `SELECT JSON_EXTRACT('$expected_inc', '$.$paramname')` + --enable_query_log + --let $assert_text = $atext : $expinc + --let $assert_cond = [SELECT @$varname - @oldvalue] = $expinc + --source include/assert.inc + + --source $json_sv_check_inc_next +} diff --git a/mysql-test/r/all_persisted_variables.result b/mysql-test/r/all_persisted_variables.result index f62d911bee4..e17a43a166c 100644 --- a/mysql-test/r/all_persisted_variables.result +++ b/mysql-test/r/all_persisted_variables.result @@ -40,7 +40,7 @@ include/assert.inc [Expect 500+ variables in the table. Due to open Bugs, we are # Test SET PERSIST -include/assert.inc [Expect 449 persisted variables in the table.] +include/assert.inc [Expect 450 persisted variables in the table.] ************************************************************ * 3. Restart server, it must preserve the persisted variable @@ -48,9 +48,9 @@ include/assert.inc [Expect 449 persisted variables in the table.] ************************************************************ # restart -include/assert.inc [Expect 449 persisted variables in persisted_variables table.] -include/assert.inc [Expect 449 persisted variables shown as PERSISTED in variables_info table.] -include/assert.inc [Expect 449 persisted variables with matching peristed and global values.] +include/assert.inc [Expect 450 persisted variables in persisted_variables table.] +include/assert.inc [Expect 450 persisted variables shown as PERSISTED in variables_info table.] +include/assert.inc [Expect 450 persisted variables with matching peristed and global values.] ************************************************************ * 4. Test RESET PERSIST IF EXISTS. Verify persisted variable diff --git a/mysql-test/r/mysqld--help-notwin.result b/mysql-test/r/mysqld--help-notwin.result index ef10f24ea5b..adea22a6901 100644 --- a/mysql-test/r/mysqld--help-notwin.result +++ b/mysql-test/r/mysqld--help-notwin.result @@ -1542,6 +1542,9 @@ The following options may be given as the first argument: cache instances) --table-open-cache-instances=# The number of table cache instances + --table-open-cache-triggers=# + The number of cached open tables with fully loaded + triggers --tablespace-definition-cache=# The number of cached tablespace definitions --tc-heuristic-recover=name @@ -2042,6 +2045,7 @@ sync-source-info 10000 sysdate-is-now FALSE table-encryption-privilege-check FALSE table-open-cache-instances 16 +table-open-cache-triggers 524288 tablespace-definition-cache 256 tc-heuristic-recover OFF temptable-max-mmap 1073741824 diff --git a/mysql-test/r/mysqld--help-win.result b/mysql-test/r/mysqld--help-win.result index 4c02b326b04..a31ae29bebd 100644 --- a/mysql-test/r/mysqld--help-win.result +++ b/mysql-test/r/mysqld--help-win.result @@ -1553,6 +1553,9 @@ The following options may be given as the first argument: cache instances) --table-open-cache-instances=# The number of table cache instances + --table-open-cache-triggers=# + The number of cached open tables with fully loaded + triggers --tablespace-definition-cache=# The number of cached tablespace definitions --tc-heuristic-recover=name @@ -2057,6 +2060,7 @@ sync-source-info 10000 sysdate-is-now FALSE table-encryption-privilege-check FALSE table-open-cache-instances 16 +table-open-cache-triggers 524288 tablespace-definition-cache 256 tc-heuristic-recover OFF temptable-max-mmap 1073741824 diff --git a/mysql-test/r/status.result b/mysql-test/r/status.result index c4c2192f1e1..6013d97187b 100644 --- a/mysql-test/r/status.result +++ b/mysql-test/r/status.result @@ -256,6 +256,9 @@ Variable_name Value Table_open_cache_hits 0 Table_open_cache_misses 0 Table_open_cache_overflows 0 +Table_open_cache_triggers_hits 0 +Table_open_cache_triggers_misses 0 +Table_open_cache_triggers_overflows 0 # The first statement accessing t1 after flush should result # in table cache miss. select * from t1; @@ -265,6 +268,9 @@ Variable_name Value Table_open_cache_hits 1 Table_open_cache_misses 16 Table_open_cache_overflows 0 +Table_open_cache_triggers_hits 0 +Table_open_cache_triggers_misses 0 +Table_open_cache_triggers_overflows 0 # The second statement accessing the same table should # result in table cache hit. select * from t1; @@ -274,6 +280,9 @@ Variable_name Value Table_open_cache_hits 2 Table_open_cache_misses 16 Table_open_cache_overflows 0 +Table_open_cache_triggers_hits 0 +Table_open_cache_triggers_misses 0 +Table_open_cache_triggers_overflows 0 # Again table cache miss if accessing different table. select * from t2; j @@ -282,6 +291,9 @@ Variable_name Value Table_open_cache_hits 18 Table_open_cache_misses 17 Table_open_cache_overflows 0 +Table_open_cache_triggers_hits 0 +Table_open_cache_triggers_misses 0 +Table_open_cache_triggers_overflows 0 # And cache hit then accessing it second time. select * from t2; j @@ -290,6 +302,9 @@ Variable_name Value Table_open_cache_hits 19 Table_open_cache_misses 17 Table_open_cache_overflows 0 +Table_open_cache_triggers_hits 0 +Table_open_cache_triggers_misses 0 +Table_open_cache_triggers_overflows 0 # The below statement should result in 2 cache hits and # 4 cache misses since it needs 6 table instances in total. select * from t1 as a, t2 as b, t1 as c, t2 as d, t1 as e, t2 as f; @@ -299,6 +314,9 @@ Variable_name Value Table_open_cache_hits 21 Table_open_cache_misses 21 Table_open_cache_overflows 0 +Table_open_cache_triggers_hits 0 +Table_open_cache_triggers_misses 0 +Table_open_cache_triggers_overflows 0 # Reduce size of table cache to check that status # variable tracking cache overflows works. set @@global.table_open_cache= 4; @@ -313,6 +331,9 @@ Variable_name Value Table_open_cache_hits 22 Table_open_cache_misses 21 Table_open_cache_overflows 19 +Table_open_cache_triggers_hits 0 +Table_open_cache_triggers_misses 0 +Table_open_cache_triggers_overflows 0 # This statement should result in 4 cache hits, 2 cache misses/ # overflows. select * from t1 as a, t2 as b, t1 as c, t2 as d, t1 as e, t2 as f; @@ -322,6 +343,9 @@ Variable_name Value Table_open_cache_hits 25 Table_open_cache_misses 24 Table_open_cache_overflows 22 +Table_open_cache_triggers_hits 0 +Table_open_cache_triggers_misses 0 +Table_open_cache_triggers_overflows 0 # Finally, the below statement should result in 1 cache miss # and 1 overflow since it accesses table which is not yet in # cache and table cache is full. @@ -332,6 +356,9 @@ Variable_name Value Table_open_cache_hits 25 Table_open_cache_misses 41 Table_open_cache_overflows 39 +Table_open_cache_triggers_hits 0 +Table_open_cache_triggers_misses 0 +Table_open_cache_triggers_overflows 0 # Cleanup set @@global.table_open_cache= @old_table_open_cache; drop tables t1, t2, t3; diff --git a/mysql-test/r/table_open_cache_triggers_functionality.result b/mysql-test/r/table_open_cache_triggers_functionality.result new file mode 100644 index 00000000000..294aff7304d --- /dev/null +++ b/mysql-test/r/table_open_cache_triggers_functionality.result @@ -0,0 +1,175 @@ +# +# Test coverage for limit on number of table objects in Table Cache +# with fully-loaded triggers. +# + +# Create some test tables with and without triggers. +CREATE TABLE t1 (i INT); +INSERT INTO t1 VALUES (1), (2), (3); +CREATE TRIGGER bi_t1 BEFORE INSERT ON t1 FOR EACH ROW SET @a = @a + 1; +CREATE TABLE t2 (j INT); +INSERT INTO t2 VALUES (1); +CREATE TRIGGER ai_t2 AFTER INSERT ON t2 FOR EACH ROW SET @b = @b + 1; +CREATE TABLE t3 (k INT); +INSERT INTO t3 VALUES (1); +CREATE TRIGGER bi_t3 BEFORE INSERT ON t3 FOR EACH ROW SET @c = @c + 1; +CREATE TABLE t4 (l INT); +INSERT INTO t4 VALUES (1); + +# Set small soft limit on number of table objects in Table Cache with +# fully-loaded triggers, so we can observe it kicking in. +SET @save_table_open_cache_triggers = @@global.table_open_cache_triggers; +SET GLOBAL table_open_cache_triggers = 2; +# To make further test repeatable reset Table Cache. +FLUSH TABLES; +# But Ensure that it contains necessary DD tables. +# Reset status variables to get 0 as their base value. +FLUSH STATUS; + +# Read-only statements should not need table objects with fully-loaded +# triggers and thus change status variables tracking their usage. +# Otherwise absence of object in table cache causes TC miss. +# presence - TC hit. Read-only statement requiring 2 table objects +# when only 1 is present in TC causes 1 hit and 1 miss. +SELECT count(*) FROM t1; +count(*) +3 +include/assert.inc [Expected table cache misses increment : 1] +include/assert.inc [Expected table cache triggers hits increment : 0] +include/assert.inc [Expected table cache triggers misses increment : 0] +include/assert.inc [Expected table cache triggers overflows increment : 0] +SELECT count(*) FROM t1 AS a, t1 AS b; +count(*) +9 +include/assert.inc [Expected table cache misses increment : 1] +include/assert.inc [Expected table cache triggers hits increment : 0] +include/assert.inc [Expected table cache triggers misses increment : 0] +include/assert.inc [Expected table cache triggers overflows increment : 0] +SELECT count(*) FROM t4; +count(*) +1 +include/assert.inc [Expected table cache misses increment : 1] +include/assert.inc [Expected table cache triggers hits increment : 0] +include/assert.inc [Expected table cache triggers misses increment : 0] +include/assert.inc [Expected table cache triggers overflows increment : 0] + +# Updating statement needs fully-loaded triggers, so there will be +# a table cache hit, but also trigger miss and trigger will be +# loaded. +INSERT INTO t1 VALUES (4); +include/assert.inc [Expected table cache misses increment : 0] +include/assert.inc [Expected table cache triggers hits increment : 0] +include/assert.inc [Expected table cache triggers misses increment : 1] +include/assert.inc [Expected table cache triggers overflows increment : 0] + +# Repeating the statement will cause table cache and trigger hits. +INSERT INTO t1 VALUES (5); +include/assert.inc [Expected table cache misses increment : 0] +include/assert.inc [Expected table cache triggers hits increment : 1] +include/assert.inc [Expected table cache triggers misses increment : 0] +include/assert.inc [Expected table cache triggers overflows increment : 0] + +# When same statement needs 2 instances of the table, one for +# updating and another for read-only purposes, then for updating +# we should try to return object with loaded triggers, and object +# without triggers we also have for read-only part. +INSERT INTO t1 SELECT * FROM t1; +include/assert.inc [Expected table cache misses increment : 0] +include/assert.inc [Expected table cache triggers hits increment : 1] +include/assert.inc [Expected table cache triggers misses increment : 0] +include/assert.inc [Expected table cache triggers overflows increment : 0] + +# Statement needing 2 table objects with triggers will cause one +# trigger hit and one trigger miss (causing trigger loading for +# one of cached table objects). +LOCK TABLES t1 AS a WRITE, t1 AS b WRITE; +include/assert.inc [Expected table cache misses increment : 0] +include/assert.inc [Expected table cache triggers hits increment : 1] +include/assert.inc [Expected table cache triggers misses increment : 1] +include/assert.inc [Expected table cache triggers overflows increment : 0] +UNLOCK TABLES; +include/assert.inc [Expected table cache misses increment : 0] +include/assert.inc [Expected table cache triggers hits increment : 0] +include/assert.inc [Expected table cache triggers misses increment : 0] +include/assert.inc [Expected table cache triggers overflows increment : 0] + +# Running read-only statement on unrelated table should not disturb +# trigger counters. Since only one table object for this table was +# used before it will cause single TC miss as expected. +SELECT count(*) FROM t4 AS a, t4 AS b; +count(*) +1 +include/assert.inc [Expected table cache misses increment : 1] +include/assert.inc [Expected table cache triggers hits increment : 0] +include/assert.inc [Expected table cache triggers misses increment : 0] +include/assert.inc [Expected table cache triggers overflows increment : 0] + +# Statement which needs 3 table objects for updating will cause 2 +# trigger hits and 1 trigger (and TC) miss . As result we temporarily +# exceed the soft limit on open tables with triggers. +LOCK TABLES t1 AS a WRITE, t1 AS b WRITE, t1 AS c WRITE; +include/assert.inc [Expected table cache misses increment : 1] +include/assert.inc [Expected table cache triggers hits increment : 2] +include/assert.inc [Expected table cache triggers misses increment : 1] +include/assert.inc [Expected table cache triggers overflows increment : 0] + +# Once we are done with using all 3 table objects, one table +# object with trigger gets evicted. 2 table objects with triggers +# is left. +UNLOCK TABLES; +include/assert.inc [Expected table cache misses increment : 0] +include/assert.inc [Expected table cache triggers hits increment : 0] +include/assert.inc [Expected table cache triggers misses increment : 0] +include/assert.inc [Expected table cache triggers overflows increment : 1] + +# Read-only statement will use table objects with triggers and +# should not disturb trigger counters. Since we have not used +# table t2 so far, there will be 1 TC miss. +SELECT count(*) FROM t1, t2; +count(*) +10 +include/assert.inc [Expected table cache misses increment : 1] +include/assert.inc [Expected table cache triggers hits increment : 0] +include/assert.inc [Expected table cache triggers misses increment : 0] +include/assert.inc [Expected table cache triggers overflows increment : 0] + +# Statement that uses t1 for updating and table t2 in read-only +# mode will reuse 1 table object with triggers (and also reuse object +# without triggers for t2), and won't disturb cache otherwise. +INSERT INTO t1 SELECT * FROM t2; +include/assert.inc [Expected table cache misses increment : 0] +include/assert.inc [Expected table cache triggers hits increment : 1] +include/assert.inc [Expected table cache triggers misses increment : 0] +include/assert.inc [Expected table cache triggers overflows increment : 0] + +# Statement that updates t2 will require loading of triggers (existing +# table object will be used for this), this will cause table object +# with triggers for table t1 eviction. +INSERT INTO t2 VALUES (2); +include/assert.inc [Expected table cache misses increment : 0] +include/assert.inc [Expected table cache triggers hits increment : 0] +include/assert.inc [Expected table cache triggers misses increment : 1] +include/assert.inc [Expected table cache triggers overflows increment : 1] + +# Statement that updates t3 will require new table object and triggers +# loaded for it. Since Table Cache already has 2 table objects with +# triggers it will cause eviction of one of these objects. According +# to LRU it should be object for t1. +INSERT INTO t3 VALUES (2); +include/assert.inc [Expected table cache misses increment : 1] +include/assert.inc [Expected table cache triggers hits increment : 0] +include/assert.inc [Expected table cache triggers misses increment : 1] +include/assert.inc [Expected table cache triggers overflows increment : 1] + +# Let us confirm that it is table object for t1 which is gone +# missing, by showing that it and its triggers need to be loaded again. +INSERT INTO t1 VALUES (6); +include/assert.inc [Expected table cache misses increment : 1] +include/assert.inc [Expected table cache triggers hits increment : 0] +include/assert.inc [Expected table cache triggers misses increment : 1] +include/assert.inc [Expected table cache triggers overflows increment : 1] +# +# Clean up. +# +DROP TABLES t1, t2, t3, t4; +SET GLOBAL table_open_cache_triggers = @save_table_open_cache_triggers; diff --git a/mysql-test/suite/sys_vars/r/table_open_cache_triggers_basic.result b/mysql-test/suite/sys_vars/r/table_open_cache_triggers_basic.result new file mode 100644 index 00000000000..d0b569a3a53 --- /dev/null +++ b/mysql-test/suite/sys_vars/r/table_open_cache_triggers_basic.result @@ -0,0 +1,163 @@ +SET @original_value = @@global.table_open_cache_triggers; +# +# Test DEFAULT value. Use SET ... = DEFAULT to handle case, +# when the test suite overrides compiled-in default. +# +SELECT @@global.table_open_cache_triggers; +@@global.table_open_cache_triggers +524288 +SET @@global.table_open_cache_triggers = DEFAULT; +SELECT @@global.table_open_cache_triggers; +@@global.table_open_cache_triggers +524288 +# +# Test a few valid values. +# +SET @@global.table_open_cache_triggers = 1; +SELECT @@global.table_open_cache_triggers; +@@global.table_open_cache_triggers +1 +SET @@global.table_open_cache_triggers = 2; +SELECT @@global.table_open_cache_triggers; +@@global.table_open_cache_triggers +2 +SET @@global.table_open_cache_triggers = 524287; +SELECT @@global.table_open_cache_triggers; +@@global.table_open_cache_triggers +524287 +SET @@global.table_open_cache_triggers = 524288; +SELECT @@global.table_open_cache_triggers; +@@global.table_open_cache_triggers +524288 +# +# Test some invalid values. +# +SET @@global.table_open_cache_triggers = 0; +Warnings: +Warning 1292 Truncated incorrect table_open_cache_triggers value: '0' +SELECT @@global.table_open_cache_triggers; +@@global.table_open_cache_triggers +1 +SET @@global.table_open_cache_triggers = -1024; +Warnings: +Warning 1292 Truncated incorrect table_open_cache_triggers value: '-1024' +SELECT @@global.table_open_cache_triggers; +@@global.table_open_cache_triggers +1 +SET @@global.table_open_cache_triggers = 524289; +Warnings: +Warning 1292 Truncated incorrect table_open_cache_triggers value: '524289' +SELECT @@global.table_open_cache_triggers; +@@global.table_open_cache_triggers +524288 +SET @@global.table_open_cache_triggers = 42949672950; +Warnings: +Warning 1292 Truncated incorrect table_open_cache_triggers value: '42949672950' +SELECT @@global.table_open_cache_triggers; +@@global.table_open_cache_triggers +524288 +# Including values of wrong type. +SET @@global.table_open_cache_triggers = 21221204.10; +ERROR 42000: Incorrect argument type to variable 'table_open_cache_triggers' +SELECT @@global.table_open_cache_triggers; +@@global.table_open_cache_triggers +524288 +SET @@global.table_open_cache_triggers = ON; +ERROR 42000: Incorrect argument type to variable 'table_open_cache_triggers' +SELECT @@global.table_open_cache_triggers; +@@global.table_open_cache_triggers +524288 +SET @@global.table_open_cache_triggers = 'test'; +ERROR 42000: Incorrect argument type to variable 'table_open_cache_triggers' +SELECT @@global.table_open_cache_triggers; +@@global.table_open_cache_triggers +524288 +# +# Test variable scope. +# +SET @@session.table_open_cache_triggers = 1; +ERROR HY000: Variable 'table_open_cache_triggers' is a GLOBAL variable and should be set with SET GLOBAL +SELECT @@session.table_open_cache_triggers; +ERROR HY000: Variable 'table_open_cache_triggers' is a GLOBAL variable +# Check that value in p_s.global_variables matches variables value. +SET @@global.table_open_cache_triggers = 5; +SELECT @@global.table_open_cache_triggers = VARIABLE_VALUE FROM performance_schema.global_variables WHERE VARIABLE_NAME='table_open_cache_triggers'; +@@global.table_open_cache_triggers = VARIABLE_VALUE +1 +# Check if accessing variable without SCOPE points to the same global +# variable. +SET @@global.table_open_cache_triggers = 7; +SELECT @@table_open_cache_triggers = @@global.table_open_cache_triggers; +@@table_open_cache_triggers = @@global.table_open_cache_triggers +1 +# Check various alternative syntax forms which are prohibited. +SET @@table_open_cache_triggers = 1; +ERROR HY000: Variable 'table_open_cache_triggers' is a GLOBAL variable and should be set with SET GLOBAL +SET table_open_cache_triggers = 1; +ERROR HY000: Variable 'table_open_cache_triggers' is a GLOBAL variable and should be set with SET GLOBAL +SET global.table_open_cache_triggers = 1; +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'global.table_open_cache_triggers = 1' at line 1 +SELECT global.table_open_cache_triggers; +ERROR 42S02: Unknown table 'global' in field list +SELECT table_open_cache_triggers; +ERROR 42S22: Unknown column 'table_open_cache_triggers' in 'field list' +# But SET GLOBAL should work. +SET GLOBAL table_open_cache_triggers = 11; +SELECT @@global.table_open_cache_triggers; +@@global.table_open_cache_triggers +11 +# +# Check that SET PERSIST/SET PERSIST_ONLY work. +# +SET PERSIST table_open_cache_triggers = 13; +SELECT @@global.table_open_cache_triggers; +@@global.table_open_cache_triggers +13 +SELECT VARIABLE_VALUE FROM performance_schema.persisted_variables WHERE VARIABLE_NAME='table_open_cache_triggers'; +VARIABLE_VALUE +13 +SET PERSIST_ONLY table_open_cache_triggers = 17; +SELECT @@global.table_open_cache_triggers; +@@global.table_open_cache_triggers +13 +SELECT VARIABLE_VALUE FROM performance_schema.persisted_variables WHERE VARIABLE_NAME='table_open_cache_triggers'; +VARIABLE_VALUE +17 +# +# Test that either SYSTEM_VARIABLES_ADMIN or SUPER are required for +# setting this variable. +# +CREATE USER u1@localhost; +connect con1, localhost, u1; +SET @@global.table_open_cache_triggers = 19; +ERROR 42000: Access denied; you need (at least one of) the SUPER or SYSTEM_VARIABLES_ADMIN privilege(s) for this operation +SELECT @@global.table_open_cache_triggers; +@@global.table_open_cache_triggers +13 +connection default; +GRANT SYSTEM_VARIABLES_ADMIN ON *.* TO u1@localhost; +connection con1; +SET @@global.table_open_cache_triggers = 19; +SELECT @@global.table_open_cache_triggers; +@@global.table_open_cache_triggers +19 +disconnect con1; +connection default; +REVOKE SYSTEM_VARIABLES_ADMIN ON *.* FROM u1@localhost; +GRANT SUPER ON *.* TO u1@localhost; +Warnings: +Warning 1287 The SUPER privilege identifier is deprecated +connect con1, localhost, u1; +connection con1; +SET @@global.table_open_cache_triggers = 23; +SELECT @@global.table_open_cache_triggers; +@@global.table_open_cache_triggers +23 +disconnect con1; +connection default; +# +# Clean-up. +# +DROP USER u1@localhost; +RESET PERSIST table_open_cache_triggers; +SET @@global.table_open_cache_triggers = @original_value; diff --git a/mysql-test/suite/sys_vars/t/table_open_cache_triggers_basic.test b/mysql-test/suite/sys_vars/t/table_open_cache_triggers_basic.test new file mode 100644 index 00000000000..917f023a6a8 --- /dev/null +++ b/mysql-test/suite/sys_vars/t/table_open_cache_triggers_basic.test @@ -0,0 +1,153 @@ +################################################################################ +# # +# Variable Name: table_open_cache_triggers # +# Scope: GLOBAL # +# Access Type: Dynamic # +# Data Type: Numeric # +# Default Value: 524288 # +# Range: 1 - 524288 # +# # +# # +# Description: Tests for dynamic system variable "table_open_cache_triggers" # +# that cover the following aspects: # +# * Default Value # +# * Valid & Invalid values # +# * Scope & Access method # +# * Persistence support # +# * Privileges # +# # +################################################################################ + +SET @original_value = @@global.table_open_cache_triggers; + +--echo # +--echo # Test DEFAULT value. Use SET ... = DEFAULT to handle case, +--echo # when the test suite overrides compiled-in default. +--echo # +SELECT @@global.table_open_cache_triggers; +SET @@global.table_open_cache_triggers = DEFAULT; +SELECT @@global.table_open_cache_triggers; + +--echo # +--echo # Test a few valid values. +--echo # +SET @@global.table_open_cache_triggers = 1; +SELECT @@global.table_open_cache_triggers; +SET @@global.table_open_cache_triggers = 2; +SELECT @@global.table_open_cache_triggers; +SET @@global.table_open_cache_triggers = 524287; +SELECT @@global.table_open_cache_triggers; +SET @@global.table_open_cache_triggers = 524288; +SELECT @@global.table_open_cache_triggers; + + +--echo # +--echo # Test some invalid values. +--echo # +SET @@global.table_open_cache_triggers = 0; +SELECT @@global.table_open_cache_triggers; +SET @@global.table_open_cache_triggers = -1024; +SELECT @@global.table_open_cache_triggers; +SET @@global.table_open_cache_triggers = 524289; +SELECT @@global.table_open_cache_triggers; +SET @@global.table_open_cache_triggers = 42949672950; +SELECT @@global.table_open_cache_triggers; + +--echo # Including values of wrong type. +--error ER_WRONG_TYPE_FOR_VAR +SET @@global.table_open_cache_triggers = 21221204.10; +SELECT @@global.table_open_cache_triggers; +--error ER_WRONG_TYPE_FOR_VAR +SET @@global.table_open_cache_triggers = ON; +SELECT @@global.table_open_cache_triggers; +--error ER_WRONG_TYPE_FOR_VAR +SET @@global.table_open_cache_triggers = 'test'; +SELECT @@global.table_open_cache_triggers; + +--echo # +--echo # Test variable scope. +--echo # +--error ER_GLOBAL_VARIABLE +SET @@session.table_open_cache_triggers = 1; +--error ER_INCORRECT_GLOBAL_LOCAL_VAR +SELECT @@session.table_open_cache_triggers; + +--echo # Check that value in p_s.global_variables matches variables value. +SET @@global.table_open_cache_triggers = 5; +SELECT @@global.table_open_cache_triggers = VARIABLE_VALUE FROM performance_schema.global_variables WHERE VARIABLE_NAME='table_open_cache_triggers'; + +--echo # Check if accessing variable without SCOPE points to the same global +--echo # variable. +SET @@global.table_open_cache_triggers = 7; +SELECT @@table_open_cache_triggers = @@global.table_open_cache_triggers; + +--echo # Check various alternative syntax forms which are prohibited. +--error ER_GLOBAL_VARIABLE +SET @@table_open_cache_triggers = 1; +--error ER_GLOBAL_VARIABLE +SET table_open_cache_triggers = 1; +--error ER_PARSE_ERROR +SET global.table_open_cache_triggers = 1; +--error ER_UNKNOWN_TABLE +SELECT global.table_open_cache_triggers; +--error ER_BAD_FIELD_ERROR +SELECT table_open_cache_triggers; + +--echo # But SET GLOBAL should work. +SET GLOBAL table_open_cache_triggers = 11; +SELECT @@global.table_open_cache_triggers; + +--echo # +--echo # Check that SET PERSIST/SET PERSIST_ONLY work. +--echo # +SET PERSIST table_open_cache_triggers = 13; +SELECT @@global.table_open_cache_triggers; +SELECT VARIABLE_VALUE FROM performance_schema.persisted_variables WHERE VARIABLE_NAME='table_open_cache_triggers'; + +SET PERSIST_ONLY table_open_cache_triggers = 17; +SELECT @@global.table_open_cache_triggers; +SELECT VARIABLE_VALUE FROM performance_schema.persisted_variables WHERE VARIABLE_NAME='table_open_cache_triggers'; + +--echo # +--echo # Test that either SYSTEM_VARIABLES_ADMIN or SUPER are required for +--echo # setting this variable. +--echo # +--enable_connect_log +CREATE USER u1@localhost; + +--connect (con1, localhost, u1) +--error ER_SPECIFIC_ACCESS_DENIED_ERROR +SET @@global.table_open_cache_triggers = 19; +SELECT @@global.table_open_cache_triggers; + +--connection default +GRANT SYSTEM_VARIABLES_ADMIN ON *.* TO u1@localhost; + +--connection con1 +SET @@global.table_open_cache_triggers = 19; +SELECT @@global.table_open_cache_triggers; + +--disconnect con1 +--source include/wait_until_disconnected.inc + +--connection default +REVOKE SYSTEM_VARIABLES_ADMIN ON *.* FROM u1@localhost; +GRANT SUPER ON *.* TO u1@localhost; + +--connect (con1, localhost, u1) +--connection con1 +SET @@global.table_open_cache_triggers = 23; +SELECT @@global.table_open_cache_triggers; + +--disconnect con1 +--source include/wait_until_disconnected.inc + +--connection default +--disable_connect_log + +--echo # +--echo # Clean-up. +--echo # +DROP USER u1@localhost; +RESET PERSIST table_open_cache_triggers; +SET @@global.table_open_cache_triggers = @original_value; diff --git a/mysql-test/t/all_persisted_variables.test b/mysql-test/t/all_persisted_variables.test index badb0f245ef..144023c4252 100644 --- a/mysql-test/t/all_persisted_variables.test +++ b/mysql-test/t/all_persisted_variables.test @@ -41,7 +41,7 @@ call mtr.add_suppression("\\[Warning\\] .*MY-\\d+.* Changing innodb_extend_and_i call mtr.add_suppression("Failed to initialize TLS for channel: mysql_main"); let $total_global_vars=`SELECT COUNT(*) FROM performance_schema.global_variables where variable_name NOT LIKE 'ndb_%' AND variable_name NOT LIKE 'debug_%'`; -let $total_persistent_vars=449; +let $total_persistent_vars=450; --echo *************************************************************** --echo * 0. Verify that variables present in performance_schema.global diff --git a/mysql-test/t/table_open_cache_triggers_functionality-master.opt b/mysql-test/t/table_open_cache_triggers_functionality-master.opt new file mode 100644 index 00000000000..0fac11d0be1 --- /dev/null +++ b/mysql-test/t/table_open_cache_triggers_functionality-master.opt @@ -0,0 +1 @@ +--table_open_cache_instances=1 diff --git a/mysql-test/t/table_open_cache_triggers_functionality.test b/mysql-test/t/table_open_cache_triggers_functionality.test new file mode 100644 index 00000000000..b4c2bac99df --- /dev/null +++ b/mysql-test/t/table_open_cache_triggers_functionality.test @@ -0,0 +1,181 @@ +--echo # +--echo # Test coverage for limit on number of table objects in Table Cache +--echo # with fully-loaded triggers. +--echo # + +--echo +--echo # Create some test tables with and without triggers. +CREATE TABLE t1 (i INT); +INSERT INTO t1 VALUES (1), (2), (3); +CREATE TRIGGER bi_t1 BEFORE INSERT ON t1 FOR EACH ROW SET @a = @a + 1; +CREATE TABLE t2 (j INT); +INSERT INTO t2 VALUES (1); +CREATE TRIGGER ai_t2 AFTER INSERT ON t2 FOR EACH ROW SET @b = @b + 1; +CREATE TABLE t3 (k INT); +INSERT INTO t3 VALUES (1); +CREATE TRIGGER bi_t3 BEFORE INSERT ON t3 FOR EACH ROW SET @c = @c + 1; +CREATE TABLE t4 (l INT); +INSERT INTO t4 VALUES (1); + +--echo +--echo # Set small soft limit on number of table objects in Table Cache with +--echo # fully-loaded triggers, so we can observe it kicking in. +SET @save_table_open_cache_triggers = @@global.table_open_cache_triggers; +SET GLOBAL table_open_cache_triggers = 2; + +--echo # To make further test repeatable reset Table Cache. +FLUSH TABLES; +--echo # But Ensure that it contains necessary DD tables. +--disable_result_log +--disable_query_log +CREATE TABLE t0 (h INT); +INSERT INTO t0 VALUES (1); +CREATE TRIGGER bi_t0 BEFORE UPDATE ON t0 FOR EACH ROW SET @h = @h + 1; +SELECT COUNT(*) FROM t0; +DROP TABLE t0; +SELECT variable_value FROM performance_schema.session_status; +--enable_query_log +--enable_result_log +--echo # Reset status variables to get 0 as their base value. +FLUSH STATUS; + +--echo +--echo # Read-only statements should not need table objects with fully-loaded +--echo # triggers and thus change status variables tracking their usage. +--echo # Otherwise absence of object in table cache causes TC miss. +--echo # presence - TC hit. Read-only statement requiring 2 table objects +--echo # when only 1 is present in TC causes 1 hit and 1 miss. +SELECT count(*) FROM t1; + +--let $expected_inc = { "tc_misses_inc": 1, "trg_hits_inc": 0, "trg_misses_inc" : 0, "trg_overflows_inc" : 0 } +--source include/assert_trigger_cache_increment.inc + +SELECT count(*) FROM t1 AS a, t1 AS b; + +--let $expected_inc = { "tc_misses_inc": 1, "trg_hits_inc": 0, "trg_misses_inc" : 0, "trg_overflows_inc" : 0 } +--source include/assert_trigger_cache_increment.inc + +SELECT count(*) FROM t4; + +--let $expected_inc = { "tc_misses_inc": 1, "trg_hits_inc": 0, "trg_misses_inc" : 0, "trg_overflows_inc" : 0 } +--source include/assert_trigger_cache_increment.inc + +--echo +--echo # Updating statement needs fully-loaded triggers, so there will be +--echo # a table cache hit, but also trigger miss and trigger will be +--echo # loaded. +INSERT INTO t1 VALUES (4); + +--let $expected_inc = { "tc_misses_inc": 0, "trg_hits_inc": 0, "trg_misses_inc" : 1, "trg_overflows_inc" : 0 } +--source include/assert_trigger_cache_increment.inc + +--echo +--echo # Repeating the statement will cause table cache and trigger hits. +INSERT INTO t1 VALUES (5); + +--let $expected_inc = { "tc_misses_inc": 0, "trg_hits_inc": 1, "trg_misses_inc" : 0, "trg_overflows_inc" : 0 } +--source include/assert_trigger_cache_increment.inc + +--echo +--echo # When same statement needs 2 instances of the table, one for +--echo # updating and another for read-only purposes, then for updating +--echo # we should try to return object with loaded triggers, and object +--echo # without triggers we also have for read-only part. +INSERT INTO t1 SELECT * FROM t1; + +--let $expected_inc = { "tc_misses_inc": 0, "trg_hits_inc": 1, "trg_misses_inc" : 0, "trg_overflows_inc" : 0 } +--source include/assert_trigger_cache_increment.inc + +--echo +--echo # Statement needing 2 table objects with triggers will cause one +--echo # trigger hit and one trigger miss (causing trigger loading for +--echo # one of cached table objects). +LOCK TABLES t1 AS a WRITE, t1 AS b WRITE; + +--let $expected_inc = { "tc_misses_inc": 0, "trg_hits_inc": 1, "trg_misses_inc" : 1, "trg_overflows_inc" : 0 } +--source include/assert_trigger_cache_increment.inc + +UNLOCK TABLES; + +--let $expected_inc = { "tc_misses_inc": 0, "trg_hits_inc": 0, "trg_misses_inc" : 0, "trg_overflows_inc" : 0 } +--source include/assert_trigger_cache_increment.inc + +--echo +--echo # Running read-only statement on unrelated table should not disturb +--echo # trigger counters. Since only one table object for this table was +--echo # used before it will cause single TC miss as expected. +SELECT count(*) FROM t4 AS a, t4 AS b; + +--let $expected_inc = { "tc_misses_inc": 1, "trg_hits_inc": 0, "trg_misses_inc" : 0, "trg_overflows_inc" : 0 } +--source include/assert_trigger_cache_increment.inc + +--echo +--echo # Statement which needs 3 table objects for updating will cause 2 +--echo # trigger hits and 1 trigger (and TC) miss . As result we temporarily +--echo # exceed the soft limit on open tables with triggers. +LOCK TABLES t1 AS a WRITE, t1 AS b WRITE, t1 AS c WRITE; + +--let $expected_inc = { "tc_misses_inc": 1, "trg_hits_inc": 2, "trg_misses_inc" : 1, "trg_overflows_inc" : 0 } +--source include/assert_trigger_cache_increment.inc + +--echo +--echo # Once we are done with using all 3 table objects, one table +--echo # object with trigger gets evicted. 2 table objects with triggers +--echo # is left. +UNLOCK TABLES; + +--let $expected_inc = { "tc_misses_inc": 0, "trg_hits_inc": 0, "trg_misses_inc" : 0, "trg_overflows_inc" : 1 } +--source include/assert_trigger_cache_increment.inc + +--echo +--echo # Read-only statement will use table objects with triggers and +--echo # should not disturb trigger counters. Since we have not used +--echo # table t2 so far, there will be 1 TC miss. +SELECT count(*) FROM t1, t2; + +--let $expected_inc = { "tc_misses_inc": 1, "trg_hits_inc": 0, "trg_misses_inc" : 0, "trg_overflows_inc" : 0 } +--source include/assert_trigger_cache_increment.inc + +--echo +--echo # Statement that uses t1 for updating and table t2 in read-only +--echo # mode will reuse 1 table object with triggers (and also reuse object +--echo # without triggers for t2), and won't disturb cache otherwise. +INSERT INTO t1 SELECT * FROM t2; + +--let $expected_inc = { "tc_misses_inc": 0, "trg_hits_inc": 1, "trg_misses_inc" : 0, "trg_overflows_inc" : 0 } +--source include/assert_trigger_cache_increment.inc + +--echo +--echo # Statement that updates t2 will require loading of triggers (existing +--echo # table object will be used for this), this will cause table object +--echo # with triggers for table t1 eviction. +INSERT INTO t2 VALUES (2); + +--let $expected_inc = { "tc_misses_inc": 0, "trg_hits_inc": 0, "trg_misses_inc" : 1, "trg_overflows_inc" : 1 } +--source include/assert_trigger_cache_increment.inc + +--echo +--echo # Statement that updates t3 will require new table object and triggers +--echo # loaded for it. Since Table Cache already has 2 table objects with +--echo # triggers it will cause eviction of one of these objects. According +--echo # to LRU it should be object for t1. +INSERT INTO t3 VALUES (2); + +--let $expected_inc = { "tc_misses_inc": 1, "trg_hits_inc": 0, "trg_misses_inc" : 1, "trg_overflows_inc" : 1 } +--source include/assert_trigger_cache_increment.inc + +--echo +--echo # Let us confirm that it is table object for t1 which is gone +--echo # missing, by showing that it and its triggers need to be loaded again. +INSERT INTO t1 VALUES (6); + +--let $expected_inc = { "tc_misses_inc": 1, "trg_hits_inc": 0, "trg_misses_inc" : 1, "trg_overflows_inc" : 1 } +--source include/assert_trigger_cache_increment.inc + +--echo # +--echo # Clean up. +--echo # +DROP TABLES t1, t2, t3, t4; +SET GLOBAL table_open_cache_triggers = @save_table_open_cache_triggers; +# Clean up JSON helper scripts created by assert_trigger_cache_increment.inc +--source include/destroy_json_functions.inc diff --git a/sql/dd_table_share.cc b/sql/dd_table_share.cc index 10671210503..55fca19b202 100644 --- a/sql/dd_table_share.cc +++ b/sql/dd_table_share.cc @@ -51,6 +51,7 @@ #include "sql/dd/collection.h" #include "sql/dd/dd_table.h" // dd::FIELD_NAME_SEPARATOR_CHAR #include "sql/dd/dd_tablespace.h" // dd::get_tablespace_name +#include "sql/dd/dd_trigger.h" // dd::load_triggers // TODO: Avoid exposing dd/impl headers in public files. #include "sql/dd/impl/utils.h" // dd::eat_str #include "sql/dd/properties.h" // dd::Properties @@ -2279,6 +2280,24 @@ static bool fill_check_constraints_from_dd(TABLE_SHARE *share, return false; } +/** + Fill information about triggers from dd::Table object to the TABLE_SHARE. +*/ +static bool fill_triggers_from_dd(THD *thd, TABLE_SHARE *share, + const dd::Table *tab_obj) { + assert(share->triggers == nullptr); + + if (tab_obj->has_trigger()) { + share->triggers = new (&share->mem_root) List; + if (share->triggers == nullptr) return true; // OOM + if (dd::load_triggers(thd, &share->mem_root, share->db.str, + share->table_name.str, *tab_obj, share->triggers)) + return true; // OOM. + } + + return false; +} + bool open_table_def(THD *thd, TABLE_SHARE *share, const dd::Table &table_def) { DBUG_TRACE; @@ -2292,7 +2311,8 @@ bool open_table_def(THD *thd, TABLE_SHARE *share, const dd::Table &table_def) { fill_indexes_from_dd(thd, share, &table_def) || fill_partitioning_from_dd(thd, share, &table_def) || fill_foreign_keys_from_dd(share, &table_def) || - fill_check_constraints_from_dd(share, &table_def)); + fill_check_constraints_from_dd(share, &table_def) || + fill_triggers_from_dd(thd, share, &table_def)); thd->mem_root = old_root; diff --git a/sql/mysqld.cc b/sql/mysqld.cc index dda66f3ff6f..465d812b897 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -1321,6 +1321,8 @@ ulong back_log, connect_timeout, server_id; ulong table_cache_size; ulong table_cache_instances; ulong table_cache_size_per_instance; +ulong table_cache_triggers; +ulong table_cache_triggers_per_instance; ulong schema_def_size; ulong stored_program_def_size; ulong table_def_size; @@ -8737,6 +8739,8 @@ static void adjust_table_cache_size(ulong requested_open_files) { } table_cache_size_per_instance = table_cache_size / table_cache_instances; + table_cache_triggers_per_instance = + table_cache_triggers / table_cache_instances; } static void adjust_table_def_size() { @@ -9880,6 +9884,15 @@ SHOW_VAR status_vars[] = { {"Table_open_cache_overflows", (char *)offsetof(System_status_var, table_open_cache_overflows), SHOW_LONGLONG_STATUS, SHOW_SCOPE_ALL}, + {"Table_open_cache_triggers_hits", + (char *)offsetof(System_status_var, table_open_cache_triggers_hits), + SHOW_LONGLONG_STATUS, SHOW_SCOPE_ALL}, + {"Table_open_cache_triggers_misses", + (char *)offsetof(System_status_var, table_open_cache_triggers_misses), + SHOW_LONGLONG_STATUS, SHOW_SCOPE_ALL}, + {"Table_open_cache_triggers_overflows", + (char *)offsetof(System_status_var, table_open_cache_triggers_overflows), + SHOW_LONGLONG_STATUS, SHOW_SCOPE_ALL}, {"Tc_log_max_pages_used", (char *)&tc_log_max_pages_used, SHOW_LONG, SHOW_SCOPE_GLOBAL}, {"Tc_log_page_size", (char *)&tc_log_page_size, SHOW_LONG_NOFLUSH, diff --git a/sql/sp_head.cc b/sql/sp_head.cc index 15b8c0e87cf..2554652f39b 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -185,7 +185,7 @@ proc_param). - #Table_trigger_dispatcher::create_trigger() - - #Table_trigger_dispatcher::check_n_load() + - #dd::load_triggers() See the C++ class #Table_trigger_dispatcher in general. diff --git a/sql/sql_base.cc b/sql/sql_base.cc index f93f0edaca3..82f84034860 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -143,7 +143,6 @@ #include "sql/system_variables.h" #include "sql/table.h" // Table_ref #include "sql/table_cache.h" // table_cache_manager -#include "sql/table_trigger_dispatcher.h" // Table_trigger_dispatcher #include "sql/thd_raii.h" #include "sql/transaction.h" // trans_rollback_stmt #include "sql/transaction_info.h" @@ -329,8 +328,7 @@ static bool table_def_shutdown_in_progress = false; static bool check_and_update_table_version(THD *thd, Table_ref *tables, TABLE_SHARE *table_share); -static bool open_table_entry_fini(THD *thd, TABLE_SHARE *share, - const dd::Table *table, TABLE *entry); +static bool open_table_entry_fini(THD *thd, TABLE_SHARE *share, TABLE *entry); static bool auto_repair_table(THD *thd, Table_ref *table_list); static TABLE *find_temporary_table(THD *thd, const char *table_key, size_t table_key_length); @@ -3143,9 +3141,14 @@ retry_share : { /* Try to get unused TABLE object or at least pointer to TABLE_SHARE from the table cache. + + For cases when table is going to be updated try to get TABLE object, + which has trigger bodies fully loaded/ready for use. */ if (!table_list->is_view()) - table = tc->get_table(thd, key, key_length, &share); + table = + tc->get_table(thd, key, key_length, + table_list->mdl_request.is_write_lock_request(), &share); if (table) { /* We have found an unused TABLE object. */ @@ -3419,7 +3422,7 @@ share_found: } } - if (open_table_entry_fini(thd, share, table_def, table)) { + if (open_table_entry_fini(thd, share, table)) { closefrm(table, false); destroy(table); my_free(table); @@ -3441,6 +3444,24 @@ share_found: thd->status_var.table_open_cache_misses++; table_found: + if (table_list->mdl_request.is_write_lock_request() && table->triggers) { + /* + For tables which are going to be updated and have triggers we need + to ensure that trigger bodies are fully loaded and ready for execution. + */ + if (table->triggers->has_load_been_finalized()) { + // Common case. We've got TABLE instance with fully ready triggers. + thd->status_var.table_open_cache_triggers_hits++; + } else { + /* + Rare case. We've got either fresh TABLE object or TABLE object, + which was used only by read-only statements so far. + */ + thd->status_var.table_open_cache_triggers_misses++; + if (table->triggers->finalize_load(thd)) return true; + } + } + table->mdl_ticket = mdl_ticket; table->next = thd->open_tables; /* Link into simple list */ @@ -3779,23 +3800,11 @@ static bool tdc_open_view(THD *thd, Table_ref *table_list, } /** - Finalize the process of TABLE creation by loading table triggers - and taking action if a HEAP table content was emptied implicitly. + Finalize the process of TABLE creation by taking action if a HEAP table + content was emptied implicitly. */ -static bool open_table_entry_fini(THD *thd, TABLE_SHARE *share, - const dd::Table *table, TABLE *entry) { - if (table != nullptr && table->has_trigger()) { - Table_trigger_dispatcher *d = Table_trigger_dispatcher::create(entry); - - if (!d || d->check_n_load(thd, *table)) { - destroy(d); - return true; - } - - entry->triggers = d; - } - +static bool open_table_entry_fini(THD *thd, TABLE_SHARE *share, TABLE *entry) { /* If we are here, there was no fatal error (but error may be still uninitialized). @@ -4744,8 +4753,12 @@ static bool open_and_process_routine( tc->lock(); + /* + We don't need TABLE object with fully loaded triggers, since it is + not going to be used it for update, but only to get TABLE_SHARE. + */ table = tc->get_table(thd, rt->part_mdl_key(), - rt->part_mdl_key_length(), &share); + rt->part_mdl_key_length(), false, &share); if (table) { assert(table->s == share); diff --git a/sql/sql_show.cc b/sql/sql_show.cc index 730cb0c936a..1238b77e1fb 100644 --- a/sql/sql_show.cc +++ b/sql/sql_show.cc @@ -5297,6 +5297,32 @@ static bool acquire_mdl_for_table(THD *thd, const char *db_name, return false; } +/** + Helper to lookup Trigger object by trigger name in a TABLE_SHARE. + + @param share TABLE_SHARE in which list of Trigger object lookup to + be performed. + @param name Name of trigger to find. + + @return Pointer to Trigger object, or nullptr if no trigger with such + name was found. +*/ + +static Trigger *find_trigger_in_share(TABLE_SHARE *share, + const LEX_STRING &name) { + Trigger *t; + List_iterator_fast it(*(share->triggers)); + + while ((t = it++) != nullptr) { + if (!my_strnncoll(dd::Trigger::name_collation(), + pointer_cast(t->get_trigger_name().str), + t->get_trigger_name().length, + pointer_cast(name.str), name.length)) + return t; + } + return nullptr; +} + /** SHOW CREATE TRIGGER high-level implementation. @@ -5350,12 +5376,12 @@ bool show_create_trigger(THD *thd, const sp_name *trg_name) { /* Perform closing actions and return error status. */ } - if (!lst->table->triggers) { + if (!lst->table->s->triggers) { my_error(ER_TRG_DOES_NOT_EXIST, MYF(0)); goto exit; } - trigger = lst->table->triggers->find_trigger(trg_name->m_name); + trigger = find_trigger_in_share(lst->table->s, trg_name->m_name); if (!trigger) { my_error(ER_TRG_CORRUPTED_FILE, MYF(0), trg_name->m_db.str, diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc index 79b4ae93f9d..7a8fca3e943 100644 --- a/sql/sys_vars.cc +++ b/sql/sys_vars.cc @@ -5111,6 +5111,29 @@ static Sys_var_ulong Sys_table_cache_instances( */ sys_var::PARSE_EARLY); +static bool fix_table_cache_triggers(sys_var *, THD *, enum_var_type) { + /* + Similarly to table_open_cache parameter table_open_cache_triggers value + needs to be divided by number of table cache instances to get per-instance + soft limit on number of TABLE objects with fully loaded triggers in a + table cache. + */ + table_cache_triggers_per_instance = + table_cache_triggers / table_cache_instances; + return false; +} + +static Sys_var_ulong Sys_table_cache_triggers( + "table_open_cache_triggers", + "The number of cached open tables with fully loaded triggers", + GLOBAL_VAR(table_cache_triggers), CMD_LINE(REQUIRED_ARG), + /* Use 1 as lower bound to be consistent with table_open_cache variable.*/ + VALID_RANGE(1, 512 * 1024), DEFAULT(512 * 1024), BLOCK_SIZE(1), + NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(nullptr), + ON_UPDATE(fix_table_cache_triggers), nullptr, + /* See explanation for Sys_table_cache_instances. */ + sys_var::PARSE_EARLY); + /** Modify the thread size cache size. */ diff --git a/sql/system_variables.h b/sql/system_variables.h index 4ca0a5243b8..079095ead5e 100644 --- a/sql/system_variables.h +++ b/sql/system_variables.h @@ -539,6 +539,9 @@ struct System_status_var { ulonglong table_open_cache_hits; ulonglong table_open_cache_misses; ulonglong table_open_cache_overflows; + ulonglong table_open_cache_triggers_hits; + ulonglong table_open_cache_triggers_misses; + ulonglong table_open_cache_triggers_overflows; ulonglong select_full_join_count; ulonglong select_full_range_join_count; ulonglong select_range_count; diff --git a/sql/table.cc b/sql/table.cc index 61b9243a747..fabe8aab914 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -3155,6 +3155,18 @@ int open_table_from_share(THD *thd, TABLE_SHARE *share, const char *alias, } } + /* + If table has triggers create Table_trigger_dispacher object with some + initial state. Do not finalize trigger parsing/loading until it is + really required. + We need to create Table_trigger_dispatcher now as some places in code + use TABLE::triggers != nullptr check to determine presence of triggers. + */ + if (share->triggers != nullptr) { + outparam->triggers = Table_trigger_dispatcher::create(outparam); + if (outparam->triggers == nullptr) goto err; // OOM + } + /* The table struct is now initialized; Open the table */ error = 2; if (db_stat) { diff --git a/sql/table.h b/sql/table.h index a52f4883d0a..33340b38a4a 100644 --- a/sql/table.h +++ b/sql/table.h @@ -97,6 +97,7 @@ class Table_cache_element; class Table_ref; class Table_trigger_dispatcher; class Temp_table_param; +class Trigger; class handler; class partition_info; enum enum_stats_auto_recalc : int; @@ -1029,6 +1030,16 @@ struct TABLE_SHARE { // List of check constraint share instances. Sql_check_constraint_share_list *check_constraint_share_list{nullptr}; + /** + List of trigger descriptions for the table loaded from the data-dictionary. + Nullptr - if table doesn't have triggers. + + @note The purpose of Trigger objects in this list is to serve as template + for per-TABLE-object Trigger objects (and store static metadata). + They can't be used directly for execution of triggers. + */ + List *triggers{nullptr}; + /** Schema's read only mode - ON (true) or OFF (false). This is filled in when the share is initialized with meta data from DD. If the schema is @@ -1413,6 +1424,18 @@ struct TABLE { */ friend class Table_cache_element; + /** + Links for the LRU list of unused TABLE objects with fully loaded triggers + in the specific instance of Table_cache. + */ + TABLE *triggers_lru_next{nullptr}, **triggers_lru_prev{nullptr}; + + /* + Give Table_cache access to the above two members to allow using them + for linking TABLE objects in a list. + */ + friend class Table_cache; + public: /** A bitmap marking the hidden generated columns that exists for functional diff --git a/sql/table_cache.cc b/sql/table_cache.cc index 23587063e09..ae2e0ebedd6 100644 --- a/sql/table_cache.cc +++ b/sql/table_cache.cc @@ -55,6 +55,7 @@ bool Table_cache::init() { mysql_mutex_init(m_lock_key, &m_lock, MY_MUTEX_INIT_FAST); m_unused_tables = nullptr; m_table_count = 0; + m_table_triggers_count = 0; return false; } @@ -93,18 +94,29 @@ void Table_cache::check_unused() { for (const auto &hp : m_cache) { Table_cache_element *el = hp.second.get(); - Table_cache_element::TABLE_list::Iterator it(el->free_tables); - TABLE *entry; - while ((entry = it++)) { - /* We must not have TABLEs in the free list that have their file closed. - */ - assert(entry->db_stat && entry->file); + auto check_free_tables = + [&count](const Table_cache_element::TABLE_list &free_tables) { + Table_cache_element::TABLE_list::Iterator it(free_tables); + TABLE *entry; + while ((entry = it++)) { + /* + We must not have TABLEs in the free list that have their file + closed. + */ + assert(entry->db_stat && entry->file); + + if (entry->in_use) + DBUG_PRINT("error", + ("Used table is in share's list of unused tables")); + count--; + } + }; + + check_free_tables(el->free_tables_slim); + check_free_tables(el->free_tables_full_triggers); - if (entry->in_use) - DBUG_PRINT("error", ("Used table is in share's list of unused tables")); - count--; - } - it.init(el->used_tables); + Table_cache_element::TABLE_list::Iterator it(el->used_tables); + TABLE *entry; while ((entry = it++)) { if (!entry->in_use) DBUG_PRINT("error", ("Unused table is in share's list of used tables")); @@ -143,6 +155,18 @@ void Table_cache::print_tables() { for (const auto &key_and_value : m_cache) { Table_cache_element *el = key_and_value.second.get(); + auto print_free_tables = + [&unused](const Table_cache_element::TABLE_list &free_tables) { + Table_cache_element::TABLE_list::Iterator it(free_tables); + TABLE *entry; + while ((entry = it++)) { + unused++; + printf("%-14.14s %-32s%6ld%8ld%6d %s\n", entry->s->db.str, + entry->s->table_name.str, entry->s->version(), 0L, + entry->db_stat ? 1 : 0, "Not in use"); + } + }; + Table_cache_element::TABLE_list::Iterator it(el->used_tables); TABLE *entry; while ((entry = it++)) { @@ -151,13 +175,9 @@ void Table_cache::print_tables() { entry->in_use->thread_id(), entry->db_stat ? 1 : 0, lock_descriptions[(int)entry->reginfo.lock_type]); } - it.init(el->free_tables); - while ((entry = it++)) { - unused++; - printf("%-14.14s %-32s%6ld%8ld%6d %s\n", entry->s->db.str, - entry->s->table_name.str, entry->s->version(), 0L, - entry->db_stat ? 1 : 0, "Not in use"); - } + + print_free_tables(el->free_tables_slim); + print_free_tables(el->free_tables_full_triggers); } if (m_unused_tables != nullptr) { @@ -300,10 +320,28 @@ void Table_cache_manager::free_table(THD *thd [[maybe_unused]], memcpy(&cache_el, share->cache_element, table_cache_instances * sizeof(Table_cache_element *)); + auto remove_and_close_free_tables = + [](Table_cache &cache, Table_cache_element::TABLE_list &free_list) { + Table_cache_element::TABLE_list::Iterator it(free_list); + TABLE *table; + while ((table = it++)) { + cache.remove_table(table); + intern_close_table(table); + } + }; + for (uint i = 0; i < table_cache_instances; i++) { if (cache_el[i]) { - Table_cache_element::TABLE_list::Iterator it(cache_el[i]->free_tables); - TABLE *table; + /* + Since freeing last TABLE object for the share will destroy all + related Table_cache_element objects and hence their list members, + we need to remember the fact whether unused TABLE objects lists + are empty (and avoid iterating through them) before proceeding to + freeing TABLE objects. + */ + bool has_free_tables_slim = !cache_el[i]->free_tables_slim.is_empty(); + bool has_free_tables_full_triggers = + !cache_el[i]->free_tables_full_triggers.is_empty(); #ifndef NDEBUG if (remove_type == TDC_RT_REMOVE_ALL) @@ -311,6 +349,7 @@ void Table_cache_manager::free_table(THD *thd [[maybe_unused]], else if (remove_type == TDC_RT_REMOVE_NOT_OWN || remove_type == TDC_RT_REMOVE_NOT_OWN_KEEP_SHARE) { Table_cache_element::TABLE_list::Iterator it2(cache_el[i]->used_tables); + TABLE *table; while ((table = it2++)) { if (table->in_use != thd) assert(0); } @@ -318,15 +357,18 @@ void Table_cache_manager::free_table(THD *thd [[maybe_unused]], #endif if (remove_type == TDC_RT_MARK_FOR_REOPEN) { Table_cache_element::TABLE_list::Iterator it2(cache_el[i]->used_tables); + TABLE *table; while ((table = it2++)) { table->invalidate_stats(); } } - while ((table = it++)) { - m_table_cache[i].remove_table(table); - intern_close_table(table); - } + if (has_free_tables_slim) + remove_and_close_free_tables(m_table_cache[i], + cache_el[i]->free_tables_slim); + if (has_free_tables_full_triggers) + remove_and_close_free_tables(m_table_cache[i], + cache_el[i]->free_tables_full_triggers); } } } diff --git a/sql/table_cache.h b/sql/table_cache.h index d7430341adc..59c96bc16ff 100644 --- a/sql/table_cache.h +++ b/sql/table_cache.h @@ -26,6 +26,8 @@ #include #include #include + +#include #include #include #include @@ -44,10 +46,12 @@ #include "sql/sql_plist.h" #include "sql/system_variables.h" #include "sql/table.h" +#include "sql/table_trigger_dispatcher.h" class Table_cache_element; -extern ulong table_cache_size_per_instance, table_cache_instances; +extern ulong table_cache_size_per_instance, table_cache_instances, + table_cache_triggers, table_cache_triggers_per_instance; /** Cache for open TABLE objects. @@ -116,6 +120,24 @@ class Table_cache { */ uint m_table_count; + /** + LRU-organized list containing all TABLE instances with fully-loaded + triggers in this table cache which are not in use by any thread. + */ + I_P_List, + I_P_List_null_counter, I_P_List_fast_push_back> + m_unused_triggers_lru; + + /** + Total number of TABLE instances in this table cache with fully-loaded + triggers (both in use and unused). + + @sa notify_triggers_load() for rationale behind use of atomic here. + */ + std::atomic m_table_triggers_count; + #ifdef HAVE_PSI_INTERFACE static PSI_mutex_key m_lock_key; static PSI_mutex_info m_mutex_keys[]; @@ -145,7 +167,7 @@ class Table_cache { void assert_owner() { mysql_mutex_assert_owner(&m_lock); } inline TABLE *get_table(THD *thd, const char *key, size_t key_length, - TABLE_SHARE **share); + bool is_update, TABLE_SHARE **share); inline void release_table(THD *thd, TABLE *table); @@ -157,6 +179,17 @@ class Table_cache { void free_all_unused_tables(); + /** + Notify the table cache that for one of its TABLE objects we have + finalized loading and parsing triggers. + + @note We use atomic to make it MT-safe without introducing overhead + from lock()/unlock() pair. + */ + void notify_triggers_load() { m_table_triggers_count++; } + + uint loaded_triggers_tables() const { return m_table_triggers_count; } + #ifndef NDEBUG void print_tables(); #endif @@ -236,7 +269,14 @@ class Table_cache_element { TABLE_list; TABLE_list used_tables; - TABLE_list free_tables; + /** + List of unused TABLE objects for tables without triggers or unused TABLE + objects for which triggers were not fully-loaded, so they can only can be + used by read-only statements. + */ + TABLE_list free_tables_slim; + /** List of unused TABLE objects with fully-loaded triggers. */ + TABLE_list free_tables_full_triggers; TABLE_SHARE *share; public: @@ -275,6 +315,8 @@ class Table_cache_iterator { /** Add table to the tail of unused tables list for table cache (i.e. as the most recently used table in this list). + If necessary, do the same thing for list of unused tables with + fully-loaded triggers. */ void Table_cache::link_unused_table(TABLE *table) { @@ -286,9 +328,16 @@ void Table_cache::link_unused_table(TABLE *table) { } else m_unused_tables = table->next = table->prev = table; check_unused(); + + if (table->triggers && table->triggers->has_load_been_finalized()) + m_unused_triggers_lru.push_back(table); } -/** Remove table from the unused tables list for table cache. */ +/** + Remove table from the unused tables list for the table cache. + If necessary, do the same thing for list of unused tables with + fully-loaded triggers for the table cache. +*/ void Table_cache::unlink_unused_table(TABLE *table) { table->next->prev = table->prev; @@ -298,6 +347,9 @@ void Table_cache::unlink_unused_table(TABLE *table) { if (table == m_unused_tables) m_unused_tables = nullptr; } check_unused(); + + if (table->triggers && table->triggers->has_load_been_finalized()) + m_unused_triggers_lru.remove(table); } /** @@ -316,8 +368,14 @@ void Table_cache::free_unused_tables_if_necessary(THD *thd) { Note that we might need to free more than one TABLE object, and thus need the below loop, in case when table_cache_size is changed dynamically, at server run time. + + We also might need to get rid of TABLE instances with fully-loaded triggers + if there are too many of them. Unfortunately, there is no good way to + "unload" triggers, so we have to get rid of the whole TABLE object. */ - if (m_table_count > table_cache_size_per_instance && m_unused_tables) { + if ((m_table_count > table_cache_size_per_instance && m_unused_tables) || + (m_table_triggers_count > table_cache_triggers_per_instance && + !m_unused_triggers_lru.is_empty())) { mysql_mutex_lock(&LOCK_open); while (m_table_count > table_cache_size_per_instance && m_unused_tables) { TABLE *table_to_free = m_unused_tables; @@ -325,6 +383,13 @@ void Table_cache::free_unused_tables_if_necessary(THD *thd) { intern_close_table(table_to_free); thd->status_var.table_open_cache_overflows++; } + while (m_table_triggers_count > table_cache_triggers_per_instance && + !m_unused_triggers_lru.is_empty()) { + TABLE *table_to_free = m_unused_triggers_lru.front(); + remove_table(table_to_free); + intern_close_table(table_to_free); + thd->status_var.table_open_cache_triggers_overflows++; + } mysql_mutex_unlock(&LOCK_open); } } @@ -400,15 +465,21 @@ void Table_cache::remove_table(TABLE *table) { el->used_tables.remove(table); } else { /* Remove from per-table chain of unused TABLE objects. */ - el->free_tables.remove(table); + if (table->triggers && table->triggers->has_load_been_finalized()) + el->free_tables_full_triggers.remove(table); + else + el->free_tables_slim.remove(table); /* And per-cache unused chain. */ unlink_unused_table(table); } m_table_count--; + if (table->triggers && table->triggers->has_load_been_finalized()) + m_table_triggers_count--; - if (el->used_tables.is_empty() && el->free_tables.is_empty()) { + if (el->used_tables.is_empty() && el->free_tables_full_triggers.is_empty() && + el->free_tables_slim.is_empty()) { std::string key(table->s->table_cache_key.str, table->s->table_cache_key.length); m_cache.erase(key); @@ -426,6 +497,10 @@ void Table_cache::remove_table(TABLE *table) { @param thd Thread context. @param key Key identifying table. @param key_length Length of key for the table. + @param is_update Indicates whether statement is going to use + TABLE object for updating the table, so it + is better to obtain TABLE instance with + fully-loaded triggers. @param[out] share NULL - if table cache doesn't contain any information about the table (i.e. doesn't have neither used nor unused TABLE objects for it). @@ -442,7 +517,7 @@ void Table_cache::remove_table(TABLE *table) { */ TABLE *Table_cache::get_table(THD *thd, const char *key, size_t key_length, - TABLE_SHARE **share) { + bool is_update, TABLE_SHARE **share) { TABLE *table; assert_owner(); @@ -456,14 +531,33 @@ TABLE *Table_cache::get_table(THD *thd, const char *key, size_t key_length, *share = el->share; - if ((table = el->free_tables.front())) { - assert(!table->in_use); - + /* + Obtain (get first and unlink) table from list of unused TABLE objects for + this table in this cache. + */ + if (!is_update) { + /* + For read-only statements we prefer TABLE objects which don't have + triggers fully-loaded. If successful this should leave unused TABLEs + with fully-loaded triggers for read-write statements. + If there are no TABLE instances sans fully-loaded triggers available + we will resort to using one with them. It is still better than + doing full-blown TABLE construction process. + */ + table = el->free_tables_slim.pop_front(); + if (!table) table = el->free_tables_full_triggers.pop_front(); + } else { /* - Unlink table from list of unused TABLE objects for this - table in this cache. + For read-write statements try to get TABLE object with fully-loaded + triggers. If there is no such object try to use sans them. + If necessary trigger load will be finalized later. */ - el->free_tables.remove(table); + table = el->free_tables_full_triggers.pop_front(); + if (!table) table = el->free_tables_slim.pop_front(); + } + + if (table) { + assert(!table->in_use); /* Unlink table from unused tables list for this cache. */ unlink_unused_table(table); @@ -509,7 +603,10 @@ void Table_cache::release_table(THD *thd, TABLE *table) { /* Remove TABLE from the list of used objects for the table in this cache. */ el->used_tables.remove(table); /* Add TABLE to the list of unused objects for the table in this cache. */ - el->free_tables.push_front(table); + if (table->triggers && table->triggers->has_load_been_finalized()) + el->free_tables_full_triggers.push_front(table); + else + el->free_tables_slim.push_front(table); /* Also link it last in the list of unused TABLE objects for the cache. */ link_unused_table(table); diff --git a/sql/table_trigger_dispatcher.cc b/sql/table_trigger_dispatcher.cc index 3032dd67cc1..17d6bdab1fd 100644 --- a/sql/table_trigger_dispatcher.cc +++ b/sql/table_trigger_dispatcher.cc @@ -52,6 +52,7 @@ #include "sql/sql_list.h" #include "sql/sql_parse.h" // create_default_definer #include "sql/table.h" +#include "sql/table_cache.h" // table_cache_manager #include "sql/thr_malloc.h" #include "sql/trigger.h" #include "sql/trigger_chain.h" @@ -88,9 +89,9 @@ Table_trigger_dispatcher::Table_trigger_dispatcher(TABLE *subject_table) m_record1_field(nullptr), m_new_field(nullptr), m_old_field(nullptr), - m_has_unparseable_trigger(false) { + m_parse_error_message(nullptr), + m_load_finalized(false) { memset(m_trigger_map, 0, sizeof(m_trigger_map)); - m_parse_error_message[0] = 0; } Table_trigger_dispatcher::~Table_trigger_dispatcher() { @@ -316,27 +317,34 @@ bool Table_trigger_dispatcher::prepare_record1_accessors() { } /** - Load triggers for the table. + Finalize load of triggers for the table by creating Trigger objects to + be associated with TABLE/Table_trigger_dispatcher (rather than with + TABLE_SHARE), parsing trigger bodies, creating trigger chains, preparing + sp_head objects and row accessors. @param thd current thread context - @param table table object. @return Operation status. @retval false Success @retval true Failure */ -bool Table_trigger_dispatcher::check_n_load(THD *thd, const dd::Table &table) { +bool Table_trigger_dispatcher::finalize_load(THD *thd) { assert(m_subject_table); - // Load triggers from Data Dictionary. - List triggers; - if (dd::load_triggers(thd, &m_subject_table->mem_root, - m_subject_table->s->db.str, - m_subject_table->s->table_name.str, table, &triggers)) - return true; + /* + Create Trigger objects to be bound to TABLE from those stored + in TABLE_SHARE. + */ + List_iterator_fast it_share(*m_subject_table->s->triggers); + const Trigger *t_share; + while ((t_share = it_share++)) { + Trigger *t_clone = t_share->clone_shallow(&m_subject_table->mem_root); + if (!t_clone || triggers.push_back(t_clone, &m_subject_table->mem_root)) + return true; + } // 'false' flag for 'is_upgrade' as we read Trigger from DD. parse_triggers(thd, &triggers, false); @@ -377,6 +385,10 @@ bool Table_trigger_dispatcher::check_n_load(THD *thd, const dd::Table &table) { sp->setup_trigger_fields(thd, this, t->get_subject_table_grant(), false); } + m_load_finalized = true; + + table_cache_manager.get_cache(thd)->notify_triggers_load(); + return false; } @@ -492,8 +504,8 @@ void Table_trigger_dispatcher::parse_triggers(THD *thd, List *triggers, /* In case we are upgrading, call set_parse_error_message() to set - m_has_unparseable_trigger in case of fatal errors too. As return type - of this function is void, we use m_has_unparseable_trigger to check + m_parse_error_message in case of fatal errors too. As return type + of this function is void, we use m_parse_error_message to check for any errors in Trigger upgrade upgrade. */ if (is_upgrade && fatal_parse_error) { @@ -693,3 +705,13 @@ bool Table_trigger_dispatcher::mark_fields(enum_trigger_event_type event) { m_subject_table->file->column_bitmaps_signal(); return false; } + +void Table_trigger_dispatcher::set_parse_error_message( + const char *error_message) { + if (!m_parse_error_message) { + m_parse_error_message = + strdup_root(&m_subject_table->mem_root, error_message); + // Play safe even in case of OOM. + if (!m_parse_error_message) m_parse_error_message = ER_DEFAULT(ER_DA_OOM); + } +} diff --git a/sql/table_trigger_dispatcher.h b/sql/table_trigger_dispatcher.h index 5d676052896..e6db06acd28 100644 --- a/sql/table_trigger_dispatcher.h +++ b/sql/table_trigger_dispatcher.h @@ -34,6 +34,7 @@ #include "my_sys.h" #include "mysql_com.h" // MYSQL_ERRMSG_SIZE #include "mysqld_error.h" // ER_PARSE_ERROR +#include "sql/sql_list.h" // List #include "sql/table_trigger_field_support.h" // Table_trigger_field_support #include "sql/trigger_def.h" // enum_trigger_action_time_type @@ -53,6 +54,10 @@ class Table_ref; template class List; +namespace table_cache_unittest { +class Mock_share; +} + /////////////////////////////////////////////////////////////////////////// /** @@ -63,7 +68,7 @@ class Table_trigger_dispatcher : public Table_trigger_field_support { public: static Table_trigger_dispatcher *create(TABLE *subject_table); - bool check_n_load(THD *thd, const dd::Table &table); + bool finalize_load(THD *thd); /* During upgrade from 5.7, we need to use the trigger chains to fix @@ -74,6 +79,8 @@ class Table_trigger_dispatcher : public Table_trigger_field_support { private: Table_trigger_dispatcher(TABLE *subject_table); + friend class table_cache_unittest::Mock_share; + public: ~Table_trigger_dispatcher() override; @@ -85,7 +92,7 @@ class Table_trigger_dispatcher : public Table_trigger_field_support { SQL-definition can not be parsed) for this table. */ bool check_for_broken_triggers() { - if (m_has_unparseable_trigger) { + if (m_parse_error_message) { my_message(ER_PARSE_ERROR, m_parse_error_message, MYF(0)); return true; } @@ -165,6 +172,13 @@ class Table_trigger_dispatcher : public Table_trigger_field_support { void parse_triggers(THD *thd, List *triggers, bool is_upgrade); + /** + Check whether we have finalized loading of triggers for the table + by parsing their bodies, creating sp_head objects and preparing + row-accessors. + */ + bool has_load_been_finalized() { return m_load_finalized; } + private: Trigger_chain *create_trigger_chain( MEM_ROOT *mem_root, enum_trigger_event_type event, @@ -175,19 +189,13 @@ class Table_trigger_dispatcher : public Table_trigger_field_support { /** Remember a parse error that occurred while parsing trigger definitions loaded from the Data Dictionary. This makes the Table_trigger_dispatcher - enter the error state flagged by m_has_unparseable_trigger == true. The + enter the error state flagged by m_parse_error_message != nullptr . The error message will be used whenever a statement invoking or manipulating triggers is issued against the Table_trigger_dispatcher's table. @param error_message The error message thrown by the parser. */ - void set_parse_error_message(const char *error_message) { - if (!m_has_unparseable_trigger) { - m_has_unparseable_trigger = true; - snprintf(m_parse_error_message, sizeof(m_parse_error_message), "%s", - error_message); - } - } + void set_parse_error_message(const char *error_message); private: /************************************************************************ @@ -228,28 +236,22 @@ class Table_trigger_dispatcher : public Table_trigger_field_support { Field **m_old_field; /** - This flag indicates that one of the triggers was not parsed successfully, - and as a precaution the object has entered the state where all trigger - operations result in errors until all the table triggers are dropped. It is + Error which occurred while parsing one of the triggers for the table, + nullptr - if there was no error for any of its triggers. + + Non-nullptr value indicates that as a precaution the object has entered + the state where all trigger operations result in errors (referencing + this error message saved) until all the table triggers are dropped. It is not safe to add triggers since it is unknown if the broken trigger has the same name or event type. Nor is it safe to invoke any trigger. The only safe operations are drop_trigger() and drop_all_triggers(). - We can't use the value of m_parse_error_message as a flag to inform that - a trigger has a parse error since for multi-byte locale the first byte of - message can be 0 but the message still be meaningful. It means that just a - comparison against m_parse_error_message[0] can not be done safely. - @see Table_trigger_dispatcher::set_parse_error() */ - bool m_has_unparseable_trigger; + const char *m_parse_error_message; - /** - This error will be displayed when the user tries to manipulate or invoke - triggers on a table that has broken triggers. It is set once per statement - and thus will contain the first parse error encountered in the trigger file. - */ - char m_parse_error_message[MYSQL_ERRMSG_SIZE]; + /** Indicates whether we have finalized loading of triggers for the table. */ + bool m_load_finalized; }; /////////////////////////////////////////////////////////////////////////// diff --git a/sql/trigger.cc b/sql/trigger.cc index 51c21a9d221..d0707e24539 100644 --- a/sql/trigger.cc +++ b/sql/trigger.cc @@ -317,6 +317,22 @@ Trigger *Trigger::create_from_dd( trg_time_type, action_order, created_timestamp); } +/** + Create a new Trigger object as a shallow clone of existing Trigger object. + + @note Allows to produce Trigger objects to be associated with specific TABLE + instance from Trigger objects associated with TABLE_SHARE. +*/ +Trigger *Trigger::clone_shallow(MEM_ROOT *mem_root) const { + // Do not create shallow clones of Trigger objects associated with TABLE. + assert(m_sp == nullptr); + return new (mem_root) Trigger( + m_trigger_name, mem_root, m_db_name, m_subject_table_name, m_definition, + m_definition_utf8, m_sql_mode, m_definer_user, m_definer_host, + m_client_cs_name, m_connection_cl_name, m_db_cl_name, m_event, + m_action_time, m_action_order, m_created_timestamp); +} + /** Trigger constructor. */ @@ -347,9 +363,7 @@ Trigger::Trigger( m_action_order(action_order), m_trigger_name(trigger_name), m_sp(nullptr), - m_has_parse_error(false) { - m_parse_error_message[0] = 0; - + m_parse_error_message(nullptr) { construct_definer_value(mem_root, &m_definer, definer_user, definer_host); } @@ -369,7 +383,7 @@ Trigger::~Trigger() { sp_head::destroy(m_sp); } */ bool Trigger::execute(THD *thd) { - if (m_has_parse_error) return true; + if (has_parse_error()) return true; bool err_status; Sub_statement_state statement_state; @@ -670,3 +684,13 @@ void Trigger::print_upgrade_warning(THD *thd) { ER_THD(thd, ER_WARN_TRIGGER_DOESNT_HAVE_CREATED), get_db_name().str, get_subject_table_name().str, get_trigger_name().str); } + +/** + Mark trigger as having a parse error and remember the message for future use. +*/ + +void Trigger::set_parse_error_message(const char *error_message) { + m_parse_error_message = strdup_root(m_mem_root, error_message); + // Play safe even in case of OOM. + if (!m_parse_error_message) m_parse_error_message = ER_DEFAULT(ER_DA_OOM); +} diff --git a/sql/trigger.h b/sql/trigger.h index 4d0030021b9..25be90b3b71 100644 --- a/sql/trigger.h +++ b/sql/trigger.h @@ -50,10 +50,23 @@ typedef ulonglong sql_mode_t; This class represents a trigger object. Trigger can be created, initialized, parsed and executed. - Trigger attributes are usually stored on the memory root of the subject table. + Trigger attributes are usually stored on the memory root of the subject table + TABLE object or its TABLE_SHARE (depending on whether it is something specific + to the TABLE instance, e.g. sp_head, or static metadata that can be shared by + all TABLE instances, e.g. subject table name). Trigger object however can exist when the subject table does not. In this case, trigger attributes are stored on a separate memory root. + @note We create separate sets of Trigger objects for both TABLE_SHARE and + TABLE instances. The set for the former is used to store static + information about table's triggers and is directly associated with + TABLE_SHARE object. The set for the latter is used primarily for + trigger execution, and is asssociated with TABLE object with the help + of Table_triggers_dispatcher class. Attributes representing static + properties in Trigger instances of the latter type reference + attributes/memory belonging to attributes of Trigger objects associated + with the TABLE_SHARE. + Trigger objects are created in two ways: 1. loading from Data Dictionary (by Trigger_loader) @@ -86,6 +99,8 @@ class Trigger { enum_trigger_action_time_type trg_time_type, uint action_order, my_timeval created_timestamp); + Trigger *clone_shallow(MEM_ROOT *mem_root) const; + /** Constructs CREATE TRIGGER statement taking into account a value of the DEFINER clause. @@ -196,7 +211,7 @@ class Trigger { GRANT_INFO *get_subject_table_grant() { return &m_subject_table_grant; } - bool has_parse_error() const { return m_has_parse_error; } + bool has_parse_error() const { return m_parse_error_message; } const char *get_parse_error_message() const { return m_parse_error_message; } @@ -242,16 +257,14 @@ class Trigger { m_definition_utf8 = trigger_def_utf8; } - void set_parse_error_message(const char *error_message) { - m_has_parse_error = true; - snprintf(m_parse_error_message, sizeof(m_parse_error_message), "%s", - error_message); - } + void set_parse_error_message(const char *error_message); /** Memory root to store all data of this Trigger object. - This can be a pointer to the subject table memory root, or it can be a + This can be a pointer to the subject table share memory root (if this + Trigger object is associated with TABLE_SHARE), table memory root + (if this Trigger object is associated with TABLE object), or it can be a pointer to a dedicated memory root if subject table does not exist. */ MEM_ROOT *m_mem_root; @@ -265,7 +278,9 @@ class Trigger { private: /************************************************************************ * Mandatory trigger attributes loaded from data dictionary. - * All these strings are allocated on m_mem_root. + * All these strings are allocated on TABLE_SHARE's memory root + * (for both cases when Trigger object is bound to TABLE_SHARE object + * and to TABLE object) or dedicated memory root pointed by m_mem_root. ***********************************************************************/ /// Database name. @@ -325,7 +340,7 @@ class Trigger { private: /************************************************************************ - * All these strings are allocated on the trigger table's mem-root. + * All these strings are allocated on the TABLE_SHARE's mem-root. ***********************************************************************/ /// Trigger name. @@ -333,7 +348,7 @@ class Trigger { private: /************************************************************************ - * Other attributes. + * Other attributes. Allocated on m_mem_root if necessary. ***********************************************************************/ /// Grant information for the trigger. @@ -342,16 +357,15 @@ class Trigger { /// Pointer to the sp_head corresponding to the trigger. sp_head *m_sp; - /// This flags specifies whether the trigger has parse error or not. - bool m_has_parse_error; - /** + Parse error for trigger, if it has one, nullptr - if not. + This error will be displayed when the user tries to manipulate or invoke triggers on a table that has broken triggers. It will get set only once per statement and thus will contain the first parse error encountered in the trigger file. */ - char m_parse_error_message[MYSQL_ERRMSG_SIZE]; + const char *m_parse_error_message; }; /////////////////////////////////////////////////////////////////////////// diff --git a/unittest/gunit/table_cache-t.cc b/unittest/gunit/table_cache-t.cc index ec04ff5d378..8ade3d1ea9c 100644 --- a/unittest/gunit/table_cache-t.cc +++ b/unittest/gunit/table_cache-t.cc @@ -96,6 +96,7 @@ class TableCacheSingleCacheTest : public TableCacheBasicTest { */ table_cache_instances = CachesNumber(); table_cache_size_per_instance = 100; + table_cache_triggers_per_instance = 100; ASSERT_FALSE(table_def_init()); } void TearDown() override { @@ -163,6 +164,17 @@ class Mock_share : public TABLE_SHARE { return result; } + TABLE *create_table_with_triggers(THD *thd) { + // Make calling Table_trigger_dispatcher::finalize_load() safe. + if (triggers == nullptr) triggers = new (&m_mem_root) List; + + TABLE *result = create_table(thd); + + result->triggers = new (&m_mem_root) Table_trigger_dispatcher(result); + + return result; + } + void destroy_table(TABLE *table) { my_free(table); } }; @@ -285,8 +297,9 @@ TEST_F(TableCacheSingleCacheTest, CacheAddAndRemove) { // cache. OTOH it should contain info about table share of table_1. TABLE *table_2; TABLE_SHARE *share_2; - table_2 = table_cache->get_table(thd, share_1.table_cache_key.str, - share_1.table_cache_key.length, &share_2); + table_2 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, false, &share_2); EXPECT_TRUE(table_2 == nullptr); EXPECT_TRUE(share_2 == &share_1); @@ -299,8 +312,9 @@ TEST_F(TableCacheSingleCacheTest, CacheAddAndRemove) { // We must be able to release TABLE into table cache and reuse it after // this. table_cache->release_table(thd, table_1); - table_2 = table_cache->get_table(thd, share_1.table_cache_key.str, - share_1.table_cache_key.length, &share_2); + table_2 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, false, &share_2); EXPECT_TRUE(table_2 == table_1); EXPECT_TRUE(share_2 == &share_1); @@ -309,8 +323,9 @@ TEST_F(TableCacheSingleCacheTest, CacheAddAndRemove) { // Once TABLE is removed from the cache the latter should become empty. EXPECT_EQ(0U, table_cache->cached_tables()); - table_2 = table_cache->get_table(thd, share_1.table_cache_key.str, - share_1.table_cache_key.length, &share_2); + table_2 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, false, &share_2); EXPECT_TRUE(table_2 == nullptr); EXPECT_TRUE(share_2 == nullptr); @@ -327,8 +342,9 @@ TEST_F(TableCacheSingleCacheTest, CacheAddAndRemove) { // Once TABLE is removed from cache the latter should become empty. EXPECT_EQ(0U, table_cache->cached_tables()); - table_2 = table_cache->get_table(thd, share_1.table_cache_key.str, - share_1.table_cache_key.length, &share_2); + table_2 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, false, &share_2); EXPECT_TRUE(table_2 == nullptr); EXPECT_TRUE(share_2 == nullptr); @@ -419,8 +435,9 @@ TEST_F(TableCacheSingleCacheTest, CacheGetAndRelease) { TABLE_SHARE *share_2; // There should be no TABLE in cache, nor information about share. - table_1 = table_cache->get_table(thd, share_1.table_cache_key.str, - share_1.table_cache_key.length, &share_2); + table_1 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, false, &share_2); EXPECT_TRUE(table_1 == nullptr); EXPECT_TRUE(share_2 == nullptr); @@ -429,15 +446,17 @@ TEST_F(TableCacheSingleCacheTest, CacheGetAndRelease) { // There should be no unused TABLE in cache, but there should be // information about the share. - table_2 = table_cache->get_table(thd, share_1.table_cache_key.str, - share_1.table_cache_key.length, &share_2); + table_2 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, false, &share_2); EXPECT_TRUE(table_2 == nullptr); EXPECT_TRUE(share_2 == &share_1); // There should be even no information about the share for which // TABLE was not added to cache. - table_2 = table_cache->get_table(thd, share_0.table_cache_key.str, - share_0.table_cache_key.length, &share_2); + table_2 = + table_cache->get_table(thd, share_0.table_cache_key.str, + share_0.table_cache_key.length, false, &share_2); EXPECT_TRUE(table_2 == nullptr); EXPECT_TRUE(share_2 == nullptr); @@ -446,8 +465,9 @@ TEST_F(TableCacheSingleCacheTest, CacheGetAndRelease) { // Still there should be no unused TABLE in cache, but there should // be information about the share. - table_3 = table_cache->get_table(thd, share_1.table_cache_key.str, - share_1.table_cache_key.length, &share_2); + table_3 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, false, &share_2); EXPECT_TRUE(table_3 == nullptr); EXPECT_TRUE(share_2 == &share_1); @@ -455,14 +475,16 @@ TEST_F(TableCacheSingleCacheTest, CacheGetAndRelease) { // After releasing one of TABLE objects it should be possible to get // unused TABLE from cache. - table_3 = table_cache->get_table(thd, share_1.table_cache_key.str, - share_1.table_cache_key.length, &share_2); + table_3 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, false, &share_2); EXPECT_TRUE(table_3 == table_1); EXPECT_TRUE(share_2 == &share_1); // But only once! - table_3 = table_cache->get_table(thd, share_1.table_cache_key.str, - share_1.table_cache_key.length, &share_2); + table_3 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, false, &share_2); EXPECT_TRUE(table_3 == nullptr); EXPECT_TRUE(share_2 == &share_1); @@ -472,21 +494,25 @@ TEST_F(TableCacheSingleCacheTest, CacheGetAndRelease) { table_cache->release_table(thd, table_1); table_cache->release_table(thd, table_2); - table_3 = table_cache->get_table(thd, share_0.table_cache_key.str, - share_0.table_cache_key.length, &share_2); + table_3 = + table_cache->get_table(thd, share_0.table_cache_key.str, + share_0.table_cache_key.length, false, &share_2); EXPECT_TRUE(table_3 == nullptr); EXPECT_TRUE(share_2 == nullptr); - table_3 = table_cache->get_table(thd, share_1.table_cache_key.str, - share_1.table_cache_key.length, &share_2); + table_3 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, false, &share_2); EXPECT_TRUE(table_3 != nullptr); EXPECT_TRUE(share_2 == &share_1); - table_3 = table_cache->get_table(thd, share_1.table_cache_key.str, - share_1.table_cache_key.length, &share_2); + table_3 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, false, &share_2); EXPECT_TRUE(table_3 != nullptr); EXPECT_TRUE(share_2 == &share_1); - table_3 = table_cache->get_table(thd, share_1.table_cache_key.str, - share_1.table_cache_key.length, &share_2); + table_3 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, false, &share_2); EXPECT_TRUE(table_3 == nullptr); EXPECT_TRUE(share_2 == &share_1); @@ -500,6 +526,528 @@ TEST_F(TableCacheSingleCacheTest, CacheGetAndRelease) { table_cache->unlock(); } +TEST_F(TableCacheSingleCacheTest, CacheGetAndReleaseWithTriggers) { + THD *thd = get_thd(0); + + Table_cache *table_cache = table_cache_manager.get_cache(thd); + + table_cache->lock(); + + TABLE *table_1, *table_2, *table_3, *table_4; + Mock_share share_1("share_1"), share_0("share_0"); + TABLE_SHARE *share_2; + + // There should be no TABLE in cache, nor information about share. + table_1 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, false, &share_2); + EXPECT_TRUE(table_1 == nullptr); + EXPECT_TRUE(share_2 == nullptr); + + // There should be no TABLE in cache for update either. + table_1 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, true, &share_2); + EXPECT_TRUE(table_1 == nullptr); + EXPECT_TRUE(share_2 == nullptr); + + // Counter of TABLE objects with fully-loaded triggers should be zero. + EXPECT_EQ(0U, table_cache->loaded_triggers_tables()); + + table_1 = share_1.create_table_with_triggers(thd); + add_used_table(table_cache, thd, table_1); + + // Counter of TABLE objects with fully-loaded triggers should still be zero. + EXPECT_EQ(0U, table_cache->loaded_triggers_tables()); + + // There should be no unused TABLE in cache, but there should be + // information about the share. + table_2 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, false, &share_2); + EXPECT_TRUE(table_2 == nullptr); + EXPECT_TRUE(share_2 == &share_1); + + // There should be no unused TABLE for update either (but again we + // should be able to get information about the share). + table_2 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, true, &share_2); + EXPECT_TRUE(table_2 == nullptr); + EXPECT_TRUE(share_2 == &share_1); + + table_cache->release_table(thd, table_1); + + // Release doesn't increase counter of TABLE objects with fully-loaded + // triggers. + EXPECT_EQ(0U, table_cache->loaded_triggers_tables()); + + // After releasing the TABLE object it should be possible to get + // unused TABLE from cache for update, even though table triggers + // are not fully loaded. + table_2 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, true, &share_2); + EXPECT_TRUE(table_2 == table_1); + EXPECT_TRUE(share_2 == &share_1); + EXPECT_FALSE(table_1->triggers->has_load_been_finalized()); + + // Pretend that we have fully loaded triggers for this object. + + // Number of TABLE objects with fully-loaded triggers is 0 before the load. + EXPECT_EQ(0U, table_cache->loaded_triggers_tables()); + + table_cache->unlock(); + table_1->triggers->finalize_load(thd); + EXPECT_TRUE(table_1->triggers->has_load_been_finalized()); + table_cache->lock(); + + // Number of TABLE objects with fully-loaded triggers is 1 after the load. + EXPECT_EQ(1U, table_cache->loaded_triggers_tables()); + + table_cache->release_table(thd, table_1); + + // After the TABLE object with fully loaded triggers is released + // to the cache it should be possible to get unused TABLE from + // cache ready for update. + table_2 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, true, &share_2); + EXPECT_TRUE(table_2 == table_1); + EXPECT_TRUE(share_2 == &share_1); + EXPECT_TRUE(table_1->triggers->has_load_been_finalized()); + + table_cache->release_table(thd, table_1); + + // After releasing the TABLE object again, it should be possible to get + // the same unused TABLE from cache even for read-only load. + table_2 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, false, &share_2); + EXPECT_TRUE(table_2 == table_1); + EXPECT_TRUE(share_2 == &share_1); + // Even though we have requested TABLE for read-only operation it + // still has its triggers fully loaded. + EXPECT_TRUE(table_1->triggers->has_load_been_finalized()); + + // Get another TABLE object for the share, but do not finalize loading + // of its triggers yet. + table_2 = share_1.create_table_with_triggers(thd); + add_used_table(table_cache, thd, table_2); + + // Getting fresh TABLE doesn't increase counter of TABLEs with fully-loaded + // triggers. + EXPECT_EQ(1U, table_cache->loaded_triggers_tables()); + + // There should be no unused TABLE for either read-only or update. + // But we should be able to get information about the share. + table_3 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, false, &share_2); + EXPECT_TRUE(table_3 == nullptr); + EXPECT_TRUE(share_2 == &share_1); + + table_3 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, true, &share_2); + EXPECT_TRUE(table_3 == nullptr); + EXPECT_TRUE(share_2 == &share_1); + + // Once this TABLE object without fully loaded triggers is released + // it becomes usable for both read-only and update operations. + table_cache->release_table(thd, table_2); + + table_3 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, false, &share_2); + EXPECT_TRUE(table_3 == table_2); + EXPECT_TRUE(share_2 == &share_1); + EXPECT_FALSE(table_3->triggers->has_load_been_finalized()); + + table_cache->release_table(thd, table_2); + table_3 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, true, &share_2); + EXPECT_TRUE(table_3 == table_2); + EXPECT_TRUE(share_2 == &share_1); + EXPECT_FALSE(table_3->triggers->has_load_been_finalized()); + + // Pretend that we have aborted update early and didn't load triggers. + table_cache->release_table(thd, table_2); + + // Release TABLE object with fully loaded triggers as well. + table_cache->release_table(thd, table_1); + + // At this point we have two unused TABLE objects one with fully-loaded + // triggers and one without. + + // This is confirmed by value of counter of TABLEs with fully-loaded triggers. + EXPECT_EQ(1U, table_cache->loaded_triggers_tables()); + + // First, let us check that requests for TABLEs for read-only statements + // prefer objects without fully-loaded triggers. + table_3 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, false, &share_2); + EXPECT_TRUE(table_3 == table_2); + EXPECT_TRUE(share_2 == &share_1); + EXPECT_FALSE(table_3->triggers->has_load_been_finalized()); + + // However, if such object not available TABLE with fully-loaded triggers + // will do as well. + table_3 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, false, &share_2); + EXPECT_TRUE(table_3 == table_1); + EXPECT_TRUE(share_2 == &share_1); + EXPECT_TRUE(table_3->triggers->has_load_been_finalized()); + + // Rinse and repeat to demonstrate that this behavior is not dependent + // on the order in which TABLE objects returned to cache. + table_cache->release_table(thd, table_1); + table_cache->release_table(thd, table_2); + + table_3 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, false, &share_2); + EXPECT_TRUE(table_3 == table_2); + EXPECT_TRUE(share_2 == &share_1); + EXPECT_FALSE(table_3->triggers->has_load_been_finalized()); + + table_3 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, false, &share_2); + EXPECT_TRUE(table_3 == table_1); + EXPECT_TRUE(share_2 == &share_1); + EXPECT_TRUE(table_3->triggers->has_load_been_finalized()); + + // Requests for TABLE objects for updating statements should prefer + // TABLE objects with fully-loaded triggers. + table_cache->release_table(thd, table_1); + table_cache->release_table(thd, table_2); + + table_3 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, true, &share_2); + EXPECT_TRUE(table_3 == table_1); + EXPECT_TRUE(share_2 == &share_1); + EXPECT_TRUE(table_3->triggers->has_load_been_finalized()); + + // However, if there are no such unused TABLE objects, TABLE without + // fully-loaded triggers will do. + table_3 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, true, &share_2); + EXPECT_TRUE(table_3 == table_2); + EXPECT_TRUE(share_2 == &share_1); + EXPECT_FALSE(table_3->triggers->has_load_been_finalized()); + + // Repeat requests to show that this behavior doesn't depend on + // the order in which tables were released. + table_cache->release_table(thd, table_2); + table_cache->release_table(thd, table_1); + + table_3 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, true, &share_2); + EXPECT_TRUE(table_3 == table_1); + EXPECT_TRUE(share_2 == &share_1); + EXPECT_TRUE(table_3->triggers->has_load_been_finalized()); + + table_3 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, true, &share_2); + EXPECT_TRUE(table_3 == table_2); + EXPECT_TRUE(share_2 == &share_1); + EXPECT_FALSE(table_3->triggers->has_load_been_finalized()); + + // Simulate trigger loading for the second TABLE object. + EXPECT_EQ(1U, table_cache->loaded_triggers_tables()); + table_cache->unlock(); + table_2->triggers->finalize_load(thd); + EXPECT_TRUE(table_2->triggers->has_load_been_finalized()); + table_cache->lock(); + // Counter of TABLEs with fully-loaded triggers should have been + // increased. + EXPECT_EQ(2U, table_cache->loaded_triggers_tables()); + + table_cache->release_table(thd, table_1); + table_cache->release_table(thd, table_2); + + // Releasing objects to the cache doesn't change counter of TABLEs + // with fully-loaded triggers. + EXPECT_EQ(2U, table_cache->loaded_triggers_tables()); + + // Both unused TABLEs can be used for updating statements. + table_3 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, true, &share_2); + EXPECT_TRUE(table_3 == table_1 || table_3 == table_2); + EXPECT_TRUE(share_2 == &share_1); + EXPECT_TRUE(table_3->triggers->has_load_been_finalized()); + + table_4 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, true, &share_2); + EXPECT_TRUE(table_4 == table_1 || table_4 == table_2); + EXPECT_TRUE(table_4 != table_3); + EXPECT_TRUE(share_2 == &share_1); + EXPECT_TRUE(table_4->triggers->has_load_been_finalized()); + + // Number of TABLE objects with fully loaded triggers stays the same. + EXPECT_EQ(2U, table_cache->loaded_triggers_tables()); + + table_cache->release_table(thd, table_1); + table_cache->release_table(thd, table_2); + + // Also both unused TABLEs can be used for read-only statements. + table_3 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, true, &share_2); + EXPECT_TRUE(table_3 == table_1 || table_3 == table_2); + EXPECT_TRUE(share_2 == &share_1); + EXPECT_TRUE(table_3->triggers->has_load_been_finalized()); + + table_4 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, true, &share_2); + EXPECT_TRUE(table_4 == table_1 || table_4 == table_2); + EXPECT_TRUE(table_4 != table_3); + EXPECT_TRUE(share_2 == &share_1); + EXPECT_TRUE(table_4->triggers->has_load_been_finalized()); + + // Clean-up + table_cache->remove_table(table_1); + table_cache->remove_table(table_2); + + share_1.destroy_table(table_1); + share_1.destroy_table(table_2); + + table_cache->unlock(); +} + +TEST_F(TableCacheSingleCacheTest, CacheOverflowWithTriggers) { + THD *thd = get_thd(0); + + // Set cache size low so it will overflow quickly. + table_cache_size_per_instance = 4; + table_cache_triggers_per_instance = 2; + + Mock_share share_1("share_1"); + Mock_share share_2("share_2"); + TABLE *table_1 = share_1.create_table_with_triggers(thd); + TABLE *table_2 = share_1.create_table_with_triggers(thd); + TABLE *table_3 = share_1.create_table_with_triggers(thd); + TABLE *table_4 = share_2.create_table_with_triggers(thd); + TABLE *table_5 = share_2.create_table_with_triggers(thd); + TABLE *table_6, *table_7; + TABLE_SHARE *share_3; + + Table_cache *table_cache = table_cache_manager.get_cache(thd); + + table_cache->lock(); + add_used_table(table_cache, thd, table_1); + add_used_table(table_cache, thd, table_2); + add_used_table(table_cache, thd, table_3); + + // There should be two TABLE instances in the cache. + EXPECT_EQ(3U, table_cache->cached_tables()); + // But no tables with fully-loaded triggers. + EXPECT_EQ(0U, table_cache->loaded_triggers_tables()); + + // Simulate loading of triggers for 2 instances. + table_cache->unlock(); + table_1->triggers->finalize_load(thd); + table_2->triggers->finalize_load(thd); + EXPECT_TRUE(table_1->triggers->has_load_been_finalized()); + EXPECT_TRUE(table_2->triggers->has_load_been_finalized()); + table_cache->lock(); + + // There should be two TABLE instances with loaded triggers now. + EXPECT_EQ(2U, table_cache->loaded_triggers_tables()); + + table_cache->release_table(thd, table_1); + table_cache->release_table(thd, table_2); + table_cache->release_table(thd, table_3); + + // Still there should be 3 TABLE instances in the cache + // and 2 TABLE instances with triggers loaded. + EXPECT_EQ(3U, table_cache->cached_tables()); + EXPECT_EQ(2U, table_cache->loaded_triggers_tables()); + + // Add one more TABLE to the cache (sans triggers). + add_used_table(table_cache, thd, table_4); + + // Number of TABLE instances increases, number of TABLE with + // triggers stays the same. + EXPECT_EQ(4U, table_cache->cached_tables()); + EXPECT_EQ(2U, table_cache->loaded_triggers_tables()); + + // Simulate loading triggers for table for share_2. + table_cache->unlock(); + table_4->triggers->finalize_load(thd); + EXPECT_TRUE(table_4->triggers->has_load_been_finalized()); + table_cache->lock(); + + // Number of TABLE instances stays the same. + EXPECT_EQ(4U, table_cache->cached_tables()); + // Number of TABLE instances with triggers increases and crosses + // threshold, since loading of triggers doesn't cause freeing + // TABLE objects per se. + EXPECT_EQ(3U, table_cache->loaded_triggers_tables()); + + // Release TABLE to the cache, this causes LRU TABLE with + // triggers (table_1) expelled. + table_cache->release_table(thd, table_4); + + // Total number of TABLE instances decreases. + EXPECT_EQ(3U, table_cache->cached_tables()); + // Number of TABLE instances with fully-loaded triggers goes below + // threshold. + EXPECT_EQ(2U, table_cache->loaded_triggers_tables()); + + // Let us check that it is exactly LRU TABLE with triggers (table_1) that + // got expelled. Acquire two TABLE objects with triggers which should have + // remained. + table_6 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, true, &share_3); + EXPECT_TRUE(table_6 == table_2); + EXPECT_TRUE(share_3 == &share_1); + EXPECT_TRUE(table_6->triggers->has_load_been_finalized()); + + table_6 = + table_cache->get_table(thd, share_2.table_cache_key.str, + share_2.table_cache_key.length, true, &share_3); + EXPECT_TRUE(table_6 == table_4); + EXPECT_TRUE(share_3 == &share_2); + EXPECT_TRUE(table_6->triggers->has_load_been_finalized()); + + // Total number of TABLE objects and objects with triggers should stay + // the same. + EXPECT_EQ(3U, table_cache->cached_tables()); + EXPECT_EQ(2U, table_cache->loaded_triggers_tables()); + + // Acquire remaining TABLE object for share_1 and load triggers for it. + table_6 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, true, &share_3); + EXPECT_TRUE(table_6 == table_3); + EXPECT_TRUE(share_3 == &share_1); + EXPECT_FALSE(table_6->triggers->has_load_been_finalized()); + + table_cache->unlock(); + table_3->triggers->finalize_load(thd); + EXPECT_TRUE(table_3->triggers->has_load_been_finalized()); + table_cache->lock(); + + // Total number of TABLE objects stays the same. + EXPECT_EQ(3U, table_cache->cached_tables()); + // Number of TABLE objects with fully loaded triggers exceeds threshold, + // since all 3 of them are used. + EXPECT_EQ(3U, table_cache->loaded_triggers_tables()); + + // Release all three TABLEs with triggers. + table_cache->release_table(thd, table_4); + table_cache->release_table(thd, table_2); + table_cache->release_table(thd, table_3); + + // The first table that gets released is expelled. + EXPECT_EQ(2U, table_cache->cached_tables()); + EXPECT_EQ(2U, table_cache->loaded_triggers_tables()); + + // Two remaining TABLE objects (table_2 and table_3) should be still + // reachable. + table_6 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, true, &share_3); + EXPECT_TRUE(table_6 == table_2 || table_6 == table_3); + EXPECT_TRUE(share_3 == &share_1); + EXPECT_TRUE(table_6->triggers->has_load_been_finalized()); + + table_7 = + table_cache->get_table(thd, share_1.table_cache_key.str, + share_1.table_cache_key.length, true, &share_3); + EXPECT_TRUE(table_7 == table_2 || table_7 == table_3); + EXPECT_TRUE(table_7 != table_6); + EXPECT_TRUE(share_3 == &share_1); + EXPECT_TRUE(table_7->triggers->has_load_been_finalized()); + + // Total and number of TABLEs with triggers stay the same. + EXPECT_EQ(2U, table_cache->cached_tables()); + EXPECT_EQ(2U, table_cache->loaded_triggers_tables()); + + // Add one more TABLE object to cache. + add_used_table(table_cache, thd, table_5); + + // Total number of TABLEs gets increase. + EXPECT_EQ(3U, table_cache->cached_tables()); + EXPECT_EQ(2U, table_cache->loaded_triggers_tables()); + + // Simulate loading of triggers for this last table. + table_cache->unlock(); + table_5->triggers->finalize_load(thd); + EXPECT_TRUE(table_5->triggers->has_load_been_finalized()); + table_cache->lock(); + + // Number of TABLEs with triggers increases and exceeds threshold, + // but it is OK they are all used. + EXPECT_EQ(3U, table_cache->cached_tables()); + EXPECT_EQ(3U, table_cache->loaded_triggers_tables()); + + // Now let us add two more used TABLE instances. Note that table_1 + // and table_4 point to trash at this point. + table_1 = share_1.create_table_with_triggers(thd); + table_4 = share_2.create_table_with_triggers(thd); + add_used_table(table_cache, thd, table_1); + add_used_table(table_cache, thd, table_4); + + // Total number of TABLEs and TABLEs with triggers exceed threshold now. + // This is OK since they are all used. + EXPECT_EQ(5U, table_cache->cached_tables()); + EXPECT_EQ(3U, table_cache->loaded_triggers_tables()); + + // Release TABLE with triggers, it will be expelled immediately (there + // should not be double free). + table_cache->release_table(thd, table_2); + + // Both total and with triggers counters are decremented. + EXPECT_EQ(4U, table_cache->cached_tables()); + EXPECT_EQ(2U, table_cache->loaded_triggers_tables()); + + // Now create and add the TABLE as used without triggers back. + table_2 = share_1.create_table_with_triggers(thd); + add_used_table(table_cache, thd, table_2); + + // Total number of TABLEs is incremented. + EXPECT_EQ(5U, table_cache->cached_tables()); + EXPECT_EQ(2U, table_cache->loaded_triggers_tables()); + + // Release TABLE sans triggers now. + table_cache->release_table(thd, table_1); + + // Only total number of TABLEs gets decremented. + EXPECT_EQ(4U, table_cache->cached_tables()); + EXPECT_EQ(2U, table_cache->loaded_triggers_tables()); + + // Clean-up. + table_cache->remove_table(table_2); + table_cache->remove_table(table_3); + table_cache->remove_table(table_4); + table_cache->remove_table(table_5); + + // Cache should be empty after that + EXPECT_EQ(0U, table_cache->cached_tables()); + EXPECT_EQ(0U, table_cache->loaded_triggers_tables()); + + table_cache->unlock(); + + share_1.destroy_table(table_2); + share_1.destroy_table(table_3); + share_2.destroy_table(table_4); + share_2.destroy_table(table_5); +} + /* Test for Table_cache_manager/Table_cache::free_all_unused_tables(). */