Problem:
========
Purge threads computing virtual columns could crash due to:
1. Stale TABLE* pointers when tables are flushed/rebuilt during purge
2. open_purge_table() called close_thread_tables() on failure, making
MDL tickets invalid before purge could release them
3. Purge coordinator opened TABLE* but workers accessed it with wrong
TABLE->in_use
4. No retry mechanism when open_purge_table() failed due to concurrent
FLUSH TABLES, BACKUP STAGE, or ALTER TABLE operations
Solution:
========
1. Removed close_thread_tables() from open_purge_table(). Purge
coordinator thread should close explicitly in close_and_reopen()
2. Added retry logic: when open_purge_table() returns NULL due to
table flush/rebuild, set must_wait() flag and retry in
close_and_reopen()
3. Update close_and_reopen() with purge_table parameter to
close the failed table. Pass it to trx_purge_close_tables()
for proper cleanup
4. Properly set and reset TABLE::in_use during purge operations:
- Set to coordinator_thd in row_purge_parse_undo_rec() when opening
- Reset in trx_purge_close_table() when closing
5. The auto_increment initialization now happens unconditionally for
purge threads , ensuring the auto_increment counter is always
properly initialized when purge opens tables with virtual columns
Problem:
========
A single InnoDB purge worker thread can process undo logs from different
tables within the same batch. But get_purge_table(), open_purge_table()
incorrectly assumes that a 1:1 relationship between a purge worker thread
and a table within a single batch. Based on this wrong assumtion,
InnoDB attempts to reuse TABLE objects cached in thd->open_tables for
virtual column computation.
1) Purge worker opens Table A and caches the TABLE pointer in thd->open_tables.
2) Same purge worker moves to Table B in the same batch, get_purge_table()
retrieves the cached pointer for Table A instead of opening Table B.
3) Because innobase::open() is ignored for Table B, the virtual column
template is never initialized.
4) virtual column computation for Table B aborts the server
Solution:
========
- Introduced purge_table class which has the following
purge_table: Stores either TABLE* (for tables with indexed virtual
columns) or MDL_ticket* (for tables without) in a single union
using LSB as a flag.
For tables with indexed virtual columns: opens TABLE*, accesses
MDL_ticket* via TABLE->mdl_ticket
For tables without indexed virtual columns: stores only MDL_ticket*.
trx_purge_attach_undo_recs(): Coordinator opens both dict_table_t*
and TABLE* with proper MDL protection. Workers access cached
table pointers from purge_node_t->tables without opening
their own handles
purge_sys.coordinator_thd: Distinguish coordinator from workers
in cleanup logic. Skip innobase_reset_background_thd() for
coordinator thread to prevent premature table closure during
batch processing. Workers still call cleanup to release their
thread-local resources
trx_purge_close_tables():
Rewrite for purge coordinator thread
1) Close all dict_table_t* objects first
2) Call close_thread_tables() once for all TABLE* objects
3) Release MDL tickets last, after tables are closed
Added table->lock_mutex protection when reading (or) writing
vc_templ->mysql_table and mysql_table_query_id. Clear cached
TABLE* pointers before closing tables to prevent stale pointer
access
Declared open_purge_table() and close_thread_tables() in trx0purge.cc
Declared reset_thd() in row0purge.cc and dict0stats_bg.cc.
Removed innobase_reset_background_thd()
Remove the unscoped #pragma GCC diagnostic ignored
"-Wdeprecated-declarations" from include/violite.h and replace all
sprintf/vsprintf calls with snprintf/vsnprintf across the codebase.
Where possible, pass actual buffer sizes as function parameters
instead of using hardcoded constants or recomputing strlen().
Notable API changes:
- get_date(): added to_len parameter for buffer size
- nice_time(), end_timer() in client/mysql.cc: added buff_size
- generate_new_name(): added name_size parameter
- calc_md5(): added buffer_size parameter (fixes off-by-one)
- MD5_HASH_TO_STRING macro: added _size parameter
- Added MYSQL_UDF_MAX_RESULT_LENGTH constant in mysql_com.h
Also fixed a pre-existing bug in sql/rpl_mi.cc where the ellipsis
"..." was written to the wrong buffer (dbuff instead of buff).
Vendored code is excluded: extra/readline, extra/wolfssl,
wsrep-lib, libmariadb, storage/mroonga/vendor, zlib.
Problem was in wsrep_handle_mdl_conflict function was comparing
thd->lex->sql_command variable for granted MDL-lock.
There is two possible schedules:
(1) FLUSH TABLES ... FOR EXPORT that will take MDL-lock (granted_thd).
INSERT from other node is conflicting operation (request_thd)
and sees MDL-conflict. Because granted_thd has not executed anything
else thd->lex->sql_command == SQLCOM_FLUSH and this case was
correctly handled in wsrep_handle_mdl_conflict i.e. INSERT needs
to wait.
(2) FLUSH TABLES ... FOR EXPORT that will take MDL-lock (granted_thd).
SET SESSION wsrep_sync_wait=0; (granted_thd)
INSERT from other node is conflicting operation (request_thd)
However, thd->lex->sql_command is not stored to taken MDL-lock. Now
as granted_thd is executing SET thd->lex->sql_command != SQLCOM_FLUSH
and INSERT that is BF will abort it and that means also FTFE is
killed and MDL-lock relesed. This is incorrect as FTFE has written
file on filesystem and it can't be really killed.
In this fix wsrep_handle_mdl_conflict is refactored not to use
thd->lex->sql_command as a variable used for decisions. Instead
connection state can be determined also via THD members. E.g.:
* wsrep_thd_is_toi() || wsrep_thd_is_applying - ongoing TOI or applier
* wsrep_thd_is_BF - thread is brute force
* wsrep_thd_is_SR - thread is streaming replication thread
* thd->current_backup_stage != BACKUP_FINISHED - there's ongoing BACKUP
* thd->global_read_lock.is_acquired() - ongoing FTWRL
* thd->locked_tables_mode == LTM_LOCK_TABLES - ongoing FTFE or LOCK TABLES
Fixed by setting server_status SERVER_STATUS_RETURNED_ROW if send_data
is called for stored procedures.
This make the definition of Empty_queries well defined:
"Empty_queries" is the number of SELECT queries that returns 0 rows.
The test failed as the Jira ticket describes because of insufficient
main transaction isolation from an out-of-band dynamic "new" transaction.
Specifically the replicated transaction's temporary tables became
accessible to the "new" transaction.
The fixes ensure the "new" transaction can't reach any piece of
replication transaction execution context, the temporary table list
including.
This is safe as by so far start_new_trans class instances
need not any replication info, as it must've been anticipated by design.
THD class has an instance of Gap_time_tracker_data, which has a
reference to "Gap_time_tracker" using "bill_to" field.
During clean_up_after_query, reset bill_to field to NULL.
Let us access some data members of THD directly, instead of invoking
non-inline accessor functions. Note: my_thread_id will be used instead
of the potentially narrower ulong data type.
Also, let us remove some functions from sql_class.cc that were only
being used by InnoDB or RocksDB, for no reason. RocksDB always had
access to the internals of THD.
Reviewed by: Sergei Golubchik
Tested by: Saahil Alam
- Removed duplicate words, like "the the" and "to to"
- Removed duplicate lines (one double sort line found in mysql.cc)
- Fixed some typos found while searching for duplicate words.
Command used to find duplicate words:
egrep -rI "\s([a-zA-Z]+)\s+\1\s" | grep -v param
Thanks to Artjoms Rimdjonoks for the command and pointing out the
spelling errors.
MDEV-6247 added PROCESSLIST states for when a Replication
SQL thread processes Row events, including a WSRep variant
that dynamically includes the Galera Sequence Number.
MDEV-7409 further expanded on it by adding the table name to the states.
However, PROCESSLIST __cannot__ support generated states.
Because it loads the state texts asynchronously,
only permanently static strings are safe.
Even thread-local memory can become invalid when the thread terminates,
which can happen in the middle of generating a PROCESSLIST.
To prioritize memory safety, this commit reverts both variants to
static strings as the non-WSRep variant was before MDEV-7409.
* __Fully__ revert MDEV-7409 (d9898c9a71)
* Remove the WSRep override from MDEV-6247
* Remove `THD::wsrep_info` and its compiler
flag `WSREP_PROC_INFO` as they are now unused
This commit also includes small optimizations
from MDEV-36839’s previous draft, #4133.
Reviewed-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
Problem:
Empty queries are incremented if no rows are sent to the client in the
EXECUTE phase of select query. With cursor protocol, rows are not sent
during EXECUTE phase; they are sent later in FETCH phase. Hence,
queries executed with cursor protocol are always falsely treated as
empty in EXECUTE phase.
Fix:
For cursor protocol, empty queries are now counted during the FETCH
phase. This ensures counter correctly reflects whether any rows were
actually sent to the client.
Tests included in `mysql-test/main/show.test`.
MDEV-33813 caused a regressing in that when a disk got full when
writing to a MyISAM or Aria table the MariaDB connection would, instead
of doing a retry after 60 seconds, hang until the query was killed.
Fixed by changing mysql_coind_wait() top mysql_cond_timedwait()
Author: Thomas Stangner
THD::reset_sub_statement_state and THD::restore_sub_staement_state
swap auto_inc_intervals_forced(Discrete_intervals_list) of a THD class
with a local variable temporary to execute other things before restoring
at the end of Table_triggers_list::process_triggers under a
rpl_master_erroneous_autoinc(true) condition as exposed by the
rpl.rpl_trigger test.
The uninitialized data isn't used and the only required action is to
copy the data in one direction. As the intent is for the auto_inc_intervals_forced
value to be overwritten or unused, MEM_UNDEFINED is used on it to
ensure the previous state is considered invalid.
The other uses of reset_sub_statement_state in Item_sp::execute_impl
also follow the same pattern of taking a copy to restore within the
same function.
wait_for_prior_commit() can be called multiple times per event group,
only do my_error() the first time the call fails.
Remove redundant set_overwrite_status() calls.
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
Reviewed-by: Monty <monty@mariadb.org>
It was not possible to use a package body variable as a
fetch target:
CREATE PACKAGE BODY pkg AS
vc INT := 0;
FUNCTION f1 RETURN INT AS
CURSOR cur IS SELECT 1 AS c FROM DUAL;
BEGIN
OPEN cur;
FETCH cur INTO vc; -- this returned "Undeclared variable: vc" error.
CLOSE cur;
RETURN vc;
END;
END;
FETCH assumed that all fetch targets reside of the same sp_rcontext
instance with the cursor. This patch fixes the problem.
Now a cursor and its fetch target can reside in different sp_rcontext
instances.
Details:
- Adding a helper class sp_rcontext_addr
(a combination of Sp_rcontext_handler pointer and an offset in the rcontext)
- Adding a new class sp_fetch_target deriving from sp_rcontext_addr.
Fetch targets in "FETCH cur INTO target1, target2 ..." are now collected
into this structure instead of sp_variable.
sp_variable cannot be used any more to store fetch targets,
because it does not have a pointer to Sp_rcontext_handler
(it only has the current rcontext offset).
- Removing members sp_instr_set members m_rcontext_handler and m_offset.
Deriving sp_instr_set from sp_rcontext_addr instead.
- Renaming sp_instr_cfetch member "List<sp_variable> m_varlist"
to "List<sp_fetch_target> m_fetch_target_list".
- Fixing LEX::sp_add_cfetch() to return the pointer to the
created sp_fetch_target instance (instead of returning bool).
This helps to make the grammar in sql_yacc.c simpler
- Renaming LEX::sp_add_cfetch() to LEX::sp_add_instr_cfetch(),
as `if(sp_add_cfetch())` changed its meaning to the opposite,
to avoid automatic wrong merge from earlier versions.
- Chaning the "List<sp_variable> *vars" parameter to sp_cursor::fetch
to have the data type "List<sp_fetch_target> *".
- Changing the data type of "List<sp_variable> &vars" in
sp_cursor::Select_fetch_into_spvars::send_data_to_variable_list()
to "List<sp_fetch_target> &".
- Adding THD helper methods get_rcontext() and get_variable().
- Moving the code from sql_yacc.yy into a new LEX method
LEX::make_fetch_target().
- Simplifying the grammar in sql_yacc.yy using the new LEX method.
Changing the data type of the bison rule sp_fetch_list from "void"
to "List<sp_fetch_target> *".
Cannot add a foreign key on a table with a long UNIQUE multi-column index, that
contains a foreign key as a prefix.
Check that index algorithms match during the "generated" keys duplicate removal.
mysql_prepare_create_table: Extract a Key initialization part that
relates to length calculation and long unique index designation.
append_system_key_parts call also moves there.
Move this initialization before the duplicate elimination.
Extract WITHOUT OVERPLAPS check into a separate function. It had to be moved
earlier in the code to preserve the order of the error checks, as in the tests.
InnoDB transactions may be reused after committed:
- when taken from the transaction pool
- during a DDL operation execution
In this case wsrep flag on trx object is cleared, which may cause wrong
execution logic afterwards (wsrep-related hooks are not run).
Make trx->wsrep flag initialize from THD object only once on InnoDB transaction
start and don't change it throughout the transaction's lifetime.
The flag is reset at commit time as before.
Unconditionally set wsrep=OFF for THD objects that represent InnoDB background
threads.
Make Wsrep_schema::store_view() operate in its own transaction.
Fix streaming replication transactions' fragments rollback to not switch
THD->wsrep value during transaction's execution
(use THD->wsrep_ignore_table as a workaround).
Signed-off-by: Julius Goryavsky <julius.goryavsky@mariadb.com>
The problems were that:
1) resources was freed "asimetric" normal execution in send_eof,
in case of error in destructor.
2) destructor was not called in case of SP for result objects.
(so if the last SP execution ended with error resorces was not
freeded on reinit before execution (cleanup() called before next
execution) and destructor also was not called due to lack of
delete call for the object)
Result cleanup() renamed to reset_for_next_ps_execution() to better
reflect function().
All result method revised and freeing resources made "symetric".
Destructor of result object called for SP.
Added skipped invalidation in case of error in insert.
Removed misleading naming of reset(thd) (could be mixed with
with reset()).
This commit updates default memory allocations size used with MEM_ROOT
objects to minimize the number of calls to malloc().
Changes:
- Updated MEM_ROOT block sizes in sql_const.h
- Updated MALLOC_OVERHEAD to also take into account the extra memory
allocated by my_malloc()
- Updated init_alloc_root() to only take MALLOC_OVERHEAD into account as
buffer size, not MALLOC_OVERHEAD + sizeof(USED_MEM).
- Reset mem_root->first_block_usage if and only if first block was used.
- Increase MEM_ROOT buffers sized used by my_load_defaults, plugin_init,
Create_tmp_table, allocate_table_share, TABLE and TABLE_SHARE.
This decreases number of malloc calls during queries.
- Use a small buffer for THD->main_mem_root in THD::THD. This avoids
multiple malloc() call for new connections.
I tried the above changes on a complex select query with 12 tables.
The following shows the number of extra allocations that where used
to increase the size of the MEM_ROOT buffers.
Original code:
- Connection to MariaDB: 9 allocations
- First query run: 146 allocations
- Second query run: 24 allocations
Max memory allocated for thd when using with heap table: 61,262,408
Max memory allocated for thd when using Aria tmp table: 419,464
After changes:
Connection to MariaDB: 0 allocations
- First run: 25 allocations
- Second run: 7 allocations
Max memory allocated for thd when using with heap table: 61,347,424
Max memory allocated for thd when using Aria table: 529,168
The new code uses slightly more memory, but avoids memory fragmentation
and is slightly faster thanks to much fewer calls to malloc().
Reviewed-by: Sergei Golubchik <serg@mariadb.org>
Added Query_time (total time spent running queries) to status_variables.
Other things:
- Added SHOW_MICROSECOND_STATUS type that shows an ulonglong variable
in microseconds converted to a double (in seconds).
- Changed Busy_time and Cpu_time to use SHOW_MICROSECOND_STATUS, which
simplified the code and avoids some double divisions for each query.
Reviewed-by: Sergei Golubchik <serg@mariadb.org>
