In sql_class.cc, 'row_count', of type 'ha_rows', was used as last argument for
ER_TRUNCATED_WRONG_VALUE_FOR_FIELD which is
"Incorrect %-.32s value: '%-.128s' for column '%.192s' at row %ld".
So 'ha_rows' was used as 'long'.
On SPARC32 Solaris builds, 'long' is 4 bytes and 'ha_rows' is 'longlong' i.e. 8 bytes.
So the printf-like code was reading only the first 4 bytes.
Because the CPU is big-endian, 1LL is 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x01
so the first four bytes yield 0. So the warning message had "row 0" instead of
"row 1" in test outfile_loaddata.test:
-Warning 1366 Incorrect string value: '\xE1\xE2\xF7' for column 'b' at row 1
+Warning 1366 Incorrect string value: '\xE1\xE2\xF7' for column 'b' at row 0
All error-messaging functions which internally invoke some printf-life function
are potential candidate for such mistakes.
One apparently easy way to catch such mistakes is to use
ATTRIBUTE_FORMAT (from my_attribute.h).
But this works only when call site has both:
a) the format as a string literal
b) the types of arguments.
So:
func(ER(ER_BLAH), 10);
will silently not be checked, because ER(ER_BLAH) is not known at
compile time (it is known at run-time, and depends on the chosen
language).
And
func("%s", a va_list argument);
has the same problem, as the *real* type of arguments is not
known at this site at compile time (it's known in some caller).
Moreover,
func(ER(ER_BLAH));
though possibly correct (if ER(ER_BLAH) has no '%' markers), will not
compile (gcc says "error: format not a string literal and no format
arguments").
Consequences:
1) ATTRIBUTE_FORMAT is here added only to functions which in practice
take "string literal" formats: "my_error_reporter" and "print_admin_msg".
2) it cannot be added to the other functions: my_error(),
push_warning_printf(), Table_check_intact::report_error(),
general_log_print().
To do a one-time check of functions listed in (2), the following
"static code analysis" has been done:
1) replace
my_error(ER_xxx, arguments for substitution in format)
with the equivalent
my_printf_error(ER_xxx,ER(ER_xxx), arguments for substitution in
format),
so that we have ER(ER_xxx) and the arguments *in the same call site*
2) add ATTRIBUTE_FORMAT to push_warning_printf(),
Table_check_intact::report_error(), general_log_print()
3) replace ER(xxx) with the hard-coded English text found in
errmsg.txt (like: ER(ER_UNKNOWN_ERROR) is replaced with
"Unknown error"), so that a call site has the format as string literal
4) this way, ATTRIBUTE_FORMAT can effectively do its job
5) compile, fix errors detected by ATTRIBUTE_FORMAT
6) revert steps 1-2-3.
The present patch has no compiler error when submitted again to the
static code analysis above.
It cannot catch all problems though: see Field::set_warning(), in
which a call to push_warning_printf() has a variable error
(thus, not replacable by a string literal); I checked set_warning() calls
by hand though.
See also WL 5883 for one proposal to avoid such bugs from appearing
again in the future.
The issues fixed in the patch are:
a) mismatch in types (like 'int' passed to '%ld')
b) more arguments passed than specified in the format.
This patch resolves mismatches by changing the type/number of arguments,
not by changing error messages of sql/share/errmsg.txt. The latter would be wrong,
per the following old rule: errmsg.txt must be as stable as possible; no insertions
or deletions of messages, no changes of type or number of printf-like format specifiers,
are allowed, as long as the change impacts a message already released in a GA version.
If this rule is not followed:
- Connectors, which use error message numbers, will be confused (by insertions/deletions
of messages)
- using errmsg.sys of MySQL 5.1.n with mysqld of MySQL 5.1.(n+1)
could produce wrong messages or crash; such usage can easily happen if
installing 5.1.(n+1) while /etc/my.cnf still has --language=/path/to/5.1.n/xxx;
or if copying mysqld from 5.1.(n+1) into a 5.1.n installation.
When fixing b), I have verified that the superfluous arguments were not used in the format
in the first 5.1 GA (5.1.30 'bteam@astra04-20081114162938-z8mctjp6st27uobm').
Had they been used, then passing them today, even if the message doesn't use them
anymore, would have been necessary, as explained above.
include/my_getopt.h:
this function pointer is used only with "string literal" formats, so we can add
ATTRIBUTE_FORMAT.
mysql-test/collections/default.experimental:
test should pass now
sql/derror.cc:
by having a format as string literal, ATTRIBUTE_FORMAT check becomes effective.
sql/events.cc:
Change justified by the following excerpt from sql/share/errmsg.txt:
ER_EVENT_SAME_NAME
eng "Same old and new event name"
ER_EVENT_SET_VAR_ERROR
eng "Error during starting/stopping of the scheduler. Error code %u"
sql/field.cc:
ER_TOO_BIG_SCALE 42000 S1009
eng "Too big scale %d specified for column '%-.192s'. Maximum is %lu."
ER_TOO_BIG_PRECISION 42000 S1009
eng "Too big precision %d specified for column '%-.192s'. Maximum is %lu."
ER_TOO_BIG_DISPLAYWIDTH 42000 S1009
eng "Display width out of range for column '%-.192s' (max = %lu)"
sql/ha_ndbcluster.cc:
ER_OUTOFMEMORY HY001 S1001
eng "Out of memory; restart server and try again (needed %d bytes)"
(sizeof() returns size_t)
sql/ha_ndbcluster_binlog.cc:
Too many arguments for:
ER_GET_ERRMSG
eng "Got error %d '%-.100s' from %s"
Patch by Jonas Oreland.
sql/ha_partition.cc:
print_admin_msg() is used only with a literal as format, so ATTRIBUTE_FORMAT
works.
sql/handler.cc:
ER_OUTOFMEMORY HY001 S1001
eng "Out of memory; restart server and try again (needed %d bytes)"
(sizeof() returns size_t)
sql/item_create.cc:
ER_TOO_BIG_SCALE 42000 S1009
eng "Too big scale %d specified for column '%-.192s'. Maximum is %lu."
ER_TOO_BIG_PRECISION 42000 S1009
eng "Too big precision %d specified for column '%-.192s'. Maximum is %lu."
'c_len' and 'c_dec' are char*, passed as %d !! We don't know their value
(as strtoul() failed), but they are likely big, so we use INT_MAX.
'len' is ulong.
sql/item_func.cc:
ER_WARN_DATA_OUT_OF_RANGE 22003
eng "Out of range value for column '%s' at row %ld"
ER_CANT_FIND_UDF
eng "Can't load function '%-.192s'"
sql/item_strfunc.cc:
ER_TOO_BIG_FOR_UNCOMPRESS
eng "Uncompressed data size too large; the maximum size is %d (probably, length of uncompressed data was corrupted)"
max_allowed_packet is ulong.
sql/mysql_priv.h:
sql_print_message_func is a function _pointer_.
sql/sp_head.cc:
ER_SP_RECURSION_LIMIT
eng "Recursive limit %d (as set by the max_sp_recursion_depth variable) was exceeded for routine %.192s"
max_sp_recursion_depth is ulong
sql/sql_acl.cc:
ER_PASSWORD_NO_MATCH 42000
eng "Can't find any matching row in the user table"
ER_CANT_CREATE_USER_WITH_GRANT 42000
eng "You are not allowed to create a user with GRANT"
sql/sql_base.cc:
ER_NOT_KEYFILE
eng "Incorrect key file for table '%-.200s'; try to repair it"
ER_TOO_MANY_TABLES
eng "Too many tables; MySQL can only use %d tables in a join"
MAX_TABLES is size_t.
sql/sql_binlog.cc:
ER_UNKNOWN_ERROR
eng "Unknown error"
sql/sql_class.cc:
ER_TRUNCATED_WRONG_VALUE_FOR_FIELD
eng "Incorrect %-.32s value: '%-.128s' for column '%.192s' at row %ld"
WARN_DATA_TRUNCATED 01000
eng "Data truncated for column '%s' at row %ld"
sql/sql_connect.cc:
ER_HANDSHAKE_ERROR 08S01
eng "Bad handshake"
ER_BAD_HOST_ERROR 08S01
eng "Can't get hostname for your address"
sql/sql_insert.cc:
ER_WRONG_VALUE_COUNT_ON_ROW 21S01
eng "Column count doesn't match value count at row %ld"
sql/sql_parse.cc:
ER_WARN_HOSTNAME_WONT_WORK
eng "MySQL is started in --skip-name-resolve mode; you must restart it without this switch for this grant to work"
ER_TOO_HIGH_LEVEL_OF_NESTING_FOR_SELECT
eng "Too high level of nesting for select"
ER_UNKNOWN_ERROR
eng "Unknown error"
sql/sql_partition.cc:
ER_OUTOFMEMORY HY001 S1001
eng "Out of memory; restart server and try again (needed %d bytes)"
sql/sql_plugin.cc:
ER_OUTOFMEMORY HY001 S1001
eng "Out of memory; restart server and try again (needed %d bytes)"
sql/sql_prepare.cc:
ER_OUTOFMEMORY HY001 S1001
eng "Out of memory; restart server and try again (needed %d bytes)"
ER_UNKNOWN_STMT_HANDLER
eng "Unknown prepared statement handler (%.*s) given to %s"
length value (for '%.*s') must be 'int', per the doc of printf()
and the code of my_vsnprintf().
sql/sql_show.cc:
ER_OUTOFMEMORY HY001 S1001
eng "Out of memory; restart server and try again (needed %d bytes)"
sql/sql_table.cc:
ER_TOO_BIG_FIELDLENGTH 42000 S1009
eng "Column length too big for column '%-.192s' (max = %lu); use BLOB or TEXT instead"
sql/table.cc:
ER_NOT_FORM_FILE
eng "Incorrect information in file: '%-.200s'"
ER_COL_COUNT_DOESNT_MATCH_PLEASE_UPDATE
eng "Column count of mysql.%s is wrong. Expected %d, found %d. Created with MySQL %d, now running %d. Please use mysql_upgrade to fix this error."
table->s->mysql_version is ulong.
sql/unireg.cc:
ER_TOO_LONG_TABLE_COMMENT
eng "Comment for table '%-.64s' is too long (max = %lu)"
ER_TOO_LONG_FIELD_COMMENT
eng "Comment for field '%-.64s' is too long (max = %lu)"
ER_TOO_BIG_ROWSIZE 42000
eng "Row size too large. The maximum row size for the used table type, not counting BLOBs, is %ld. You have to change some columns to TEXT or BLOBs"
In RBR and in case of converting blob fields, the space allocated
while unpacking into the conversion field was not freed after
copying from it into the real field.
We fix this by freeing the conversion field when the conversion
table is not needed anymore (on close_tables_to_lock).
(aka BUG#11766883)
- fix review comments
- Rewrite last usage of handler::get_tablespace_name to use
table->s->tablespace directly
- Remove(revert) the addition of default implementation for
handler::get_tablespace_name
- Add comments describing the new TABLE_SHARE members default_storage_media
and tablespace
- Fix usage of incorrect mask for column_format bits, i.e COLUMN_FORMAT_MASK
- Add new "format section" in extra data segment with additional table and
column properties. This was originally introduced in 5.1.20 based MySQL Cluster
- Remove hardcoded STORAGE DISK for table and instead
output the real storage format used. Keep both TABLESPACE
and STORAGE inside same version guard.
- Implement default version of handler::get_tablespace_name() since tablespace
is now available in share and it's unnecessary for each handler to implement.
(the function could actually be removed totally now).
- Add test for combinations of TABLESPACE and STORAGE with CREATE TABLE
and ALTER TABLE
- Add test to show that 5.5 now can read a .frm file created by MySQL Cluster
7.0.22. Although it does not yet show the column level attributes, they are read.
"set optimizer_switch to e or d causes invalid memory writes/valgrind warnings":
due to prefix support, the argument "e" was overwritten with its full value
"engine_condition_pushdown", which caused a buffer overrun.
This was wrong usage of find_type(); other wrong usages are fixed here too.
Please start reading with the comment of typelib.c.
client/mysqldump.c:
A bug: find_type() expects a bitmap as 3rd argument
(each bit is a flag controlling a behaviour of the function);
here it was instead passed the length of the string to search!
That could give random behaviour of find_type()
depending on the string.
We rather need to pass a correct flag to find_type().
The correct flag is FIND_TYPE_BASIC (0).
Flag 8 is not needed as buff cannot have a comma (see how buff is filled).
Flag 1 looks like a superfluous restriction.
Flag 4 is not user-friendly (why use
--compatible=2 rather than --compatible=mysql40 ?, and
we probably not commit to "2" always meaning "mysql40"
until the end of times).
include/mysql.h.pp:
This isn't a problematic API change as we go from char* to const char*:
existing code will run unchanged.
include/typelib.h:
named constants. Not an enum to not significantly change
the declaration of find_type() which would be an API change
(typelib.h is included in mysql.h).
mysql-test/r/mysqldump.result:
correct result (see the two requested modes in SQL_MODE)
mysql-test/suite/sys_vars/t/optimizer_switch_basic.test:
test for BUG#59894. The second SET used to crash.
mysql-test/t/mysqldump.test:
we had no test for multiple modes in --compatible, which is
supported according to --help
mysys/typelib.c:
Fix for BUG#59894. parse_name() is asked to match "e" with a row
of the TYPELIB (the TYPELIB lists permitted flags of optimizer_switch;
and comes from optimizer_switch_names[] of sys_vars.cc).
find_type() is capable of supporting prefixes, but if it is not
passed flag 2 in third argument, it will overwrite its first
argument (the string to search for) with the complete name,
here overwriting "e" with "engine_condition_pushdown". But
as this "e" was a buffer allocated in an Item, it was not big
enough to host the longer name, thus the crash.
We don't need to know the complete flag's name; the output used
from find_type() is just the flag's number (== function's return
code). So we can pass flag 2 to find_type() in parse_name().
After doing this fix and the other fixes in this patch, all usages
of find_type() were using flag 2; in most usages the string to search for,
is not guaranteed to be long enough to host the complete name
(it is either directly from argv, or from alloc_root/my_malloc
done in an earlier call).
Thus, flag 2 is here made implicit: callers need not pass it anymore,
it is always automatically turned on.
This allows to eliminate an oddity: parse_name() took a const char**,
and then removed "const" before calling find_type(), which could
theoretically modify the pointed data, thus lying on constness.
Last, constants for find_type() are now named.
sql-common/client.c:
Two bugs:
1) The enum was not in sync with the array (due to a bad porting of WL 1054;
the extra OPT_ values are about options present in 5.1 and deleted in 5.5);
added a compile_time_assert() to make sure this doesn't happen again
2) find_type() was writing past the end of opt_arg; as opt_arg was allocated
with alloc_root() with no extra space, this was an overrun; it could be seen
when
** building with -DWITH_VALGRIND -DHAVE_purify -DEXTRA_DEBUG
** making execution go through the faulty code; this faulty
code is executed only if the client asks to read a configuration
file like this:
mysql_options(mysql, MYSQL_READ_DEFAULT_FILE, "/tmp/cnf.cnf");
so by adding such line to the start of mysql_client_test.c::client_connect(),
we could see the valgrind warning:
==30548== Invalid write of size 1
==30548== at 0x4C2624C: strcpy (mc_replace_strmem.c:303)
==30548== by 0x48DC29: find_type (typelib.c:120)
==30548== by 0x465686: mysql_read_default_options (client.c:1344)
==30548== by 0x46830F: mysql_real_connect (client.c:2971)
==30548== by 0x409339: client_connect (mysql_client_test.c:331)
==30548== by 0x463A7F: main (mysql_client_test.c:19902)
==30548== Address 0x61875ad is 0 bytes after a block of size 29 alloc'd
==30548== at 0x4C25153: malloc (vg_replace_malloc.c:195)
==30548== by 0x49BFF1: my_malloc (my_malloc.c:38)
==30548== by 0x49C65C: alloc_root (my_alloc.c:166)
==30548== by 0x48EF97: handle_default_option (default.c:381)
==30548== by 0x49068C: search_default_file_with_ext (default.c:992)
==30548== by 0x48F929: search_default_file (default.c:670)
==30548== by 0x48EDC4: my_search_option_files (default.c:312)
==30548== by 0x48F4B1: my_load_defaults (default.c:576)
==30548== by 0x46517A: mysql_read_default_options (client.c:1207)
==30548== by 0x46830F: mysql_real_connect (client.c:2971)
==30548== by 0x409339: client_connect (mysql_client_test.c:331)
==30548== by 0x463A7F: main (mysql_client_test.c:19902)
This is fixed by having find_type() not overwrite anymore.
sql/sql_help.cc:
cast not needed anymore.
sql/table.cc:
cast not needed anymore.
There could be memory leaks if ALTER ... PARTITION command fails.
Problem was that the list of items to free was not set in
the partition info structure when fix_partition_func call failed
during ALTER ... PARTITION.
Solved by always setting the list in the partition info struct.
sql/table.cc:
item_free_list is not set if (!work_part_info_used) and
fix_partition_func failed. Which may result in a mem leak.
bug #57006 "Deadlock between HANDLER and FLUSH TABLES WITH READ
LOCK" and bug #54673 "It takes too long to get readlock for
'FLUSH TABLES WITH READ LOCK'".
The first bug manifested itself as a deadlock which occurred
when a connection, which had some table open through HANDLER
statement, tried to update some data through DML statement
while another connection tried to execute FLUSH TABLES WITH
READ LOCK concurrently.
What happened was that FTWRL in the second connection managed
to perform first step of GRL acquisition and thus blocked all
upcoming DML. After that it started to wait for table open
through HANDLER statement to be flushed. When the first connection
tried to execute DML it has started to wait for GRL/the second
connection creating deadlock.
The second bug manifested itself as starvation of FLUSH TABLES
WITH READ LOCK statements in cases when there was a constant
stream of concurrent DML statements (in two or more
connections).
This has happened because requests for protection against GRL
which were acquired by DML statements were ignoring presence of
pending GRL and thus the latter was starved.
This patch solves both these problems by re-implementing GRL
using metadata locks.
Similar to the old implementation acquisition of GRL in new
implementation is two-step. During the first step we block
all concurrent DML and DDL statements by acquiring global S
metadata lock (each DML and DDL statement acquires global IX
lock for its duration). During the second step we block commits
by acquiring global S lock in COMMIT namespace (commit code
acquires global IX lock in this namespace).
Note that unlike in old implementation acquisition of
protection against GRL in DML and DDL is semi-automatic.
We assume that any statement which should be blocked by GRL
will either open and acquires write-lock on tables or acquires
metadata locks on objects it is going to modify. For any such
statement global IX metadata lock is automatically acquired
for its duration.
The first problem is solved because waits for GRL become
visible to deadlock detector in metadata locking subsystem
and thus deadlocks like one in the first bug become impossible.
The second problem is solved because global S locks which
are used for GRL implementation are given preference over
IX locks which are acquired by concurrent DML (and we can
switch to fair scheduling in future if needed).
Important change:
FTWRL/GRL no longer blocks DML and DDL on temporary tables.
Before this patch behavior was not consistent in this respect:
in some cases DML/DDL statements on temporary tables were
blocked while in others they were not. Since the main use cases
for FTWRL are various forms of backups and temporary tables are
not preserved during backups we have opted for consistently
allowing DML/DDL on temporary tables during FTWRL/GRL.
Important change:
This patch changes thread state names which are used when
DML/DDL of FTWRL is waiting for global read lock. It is now
either "Waiting for global read lock" or "Waiting for commit
lock" depending on the stage on which FTWRL is.
Incompatible change:
To solve deadlock in events code which was exposed by this
patch we have to replace LOCK_event_metadata mutex with
metadata locks on events. As result we have to prohibit
DDL on events under LOCK TABLES.
This patch also adds extensive test coverage for interaction
of DML/DDL and FTWRL.
Performance of new and old global read lock implementations
in sysbench tests were compared. There were no significant
difference between new and old implementations.
mysql-test/include/check_ftwrl_compatible.inc:
Added helper script which allows to check that a statement is
compatible with FLUSH TABLES WITH READ LOCK.
mysql-test/include/check_ftwrl_incompatible.inc:
Added helper script which allows to check that a statement is
incompatible with FLUSH TABLES WITH READ LOCK.
mysql-test/include/handler.inc:
Adjusted test case to the fact that now DROP TABLE closes
open HANDLERs for the table to be dropped before checking
if there active FTWRL in this connection.
mysql-test/include/wait_show_condition.inc:
Fixed small error in the timeout message. The correct name
of variable used as parameter for this script is "$condition"
and not "$wait_condition".
mysql-test/r/delayed.result:
Added test coverage for scenario which triggered assert in
metadata locking subsystem.
mysql-test/r/events_2.result:
Updated test results after prohibiting event DDL operations
under LOCK TABLES.
mysql-test/r/flush.result:
Added test coverage for bug #57006 "Deadlock between HANDLER
and FLUSH TABLES WITH READ LOCK".
mysql-test/r/flush_read_lock.result:
Added test coverage for various aspects of FLUSH TABLES WITH
READ LOCK functionality.
mysql-test/r/flush_read_lock_kill.result:
Adjusted test case after replacing custom global read lock
implementation with one based on metadata locks. Use new
debug_sync point. Do not disable concurrent inserts as now
InnoDB we always use InnoDB table.
mysql-test/r/handler_innodb.result:
Adjusted test case to the fact that now DROP TABLE closes
open HANDLERs for the table to be dropped before checking
if there active FTWRL in this connection.
mysql-test/r/handler_myisam.result:
Adjusted test case to the fact that now DROP TABLE closes
open HANDLERs for the table to be dropped before checking
if there active FTWRL in this connection.
mysql-test/r/mdl_sync.result:
Adjusted test case after replacing custom global read lock
implementation with one based on metadata locks. Replaced
usage of GRL-specific debug_sync's with appropriate sync
points in MDL subsystem.
mysql-test/suite/perfschema/r/dml_setup_instruments.result:
Updated test results after removing global
COND_global_read_lock condition variable.
mysql-test/suite/perfschema/r/func_file_io.result:
Ensure that this test doesn't affect subsequent tests.
At the end of its execution enable back P_S instrumentation
which this test disables at some point.
mysql-test/suite/perfschema/r/func_mutex.result:
Ensure that this test doesn't affect subsequent tests.
At the end of its execution enable back P_S instrumentation
which this test disables at some point.
mysql-test/suite/perfschema/r/global_read_lock.result:
Adjusted test case to take into account that new GRL
implementation is based on MDL.
mysql-test/suite/perfschema/r/server_init.result:
Adjusted test case after replacing custom global read
lock implementation with one based on MDL and replacing
LOCK_event_metadata mutex with metadata lock.
mysql-test/suite/perfschema/t/func_file_io.test:
Ensure that this test doesn't affect subsequent tests.
At the end of its execution enable back P_S instrumentation
which this test disables at some point.
mysql-test/suite/perfschema/t/func_mutex.test:
Ensure that this test doesn't affect subsequent tests.
At the end of its execution enable back P_S instrumentation
which this test disables at some point.
mysql-test/suite/perfschema/t/global_read_lock.test:
Adjusted test case to take into account that new GRL
implementation is based on MDL.
mysql-test/suite/perfschema/t/server_init.test:
Adjusted test case after replacing custom global read
lock implementation with one based on MDL and replacing
LOCK_event_metadata mutex with metadata lock.
mysql-test/suite/rpl/r/rpl_tmp_table_and_DDL.result:
Updated test results after prohibiting event DDL under
LOCK TABLES.
mysql-test/t/delayed.test:
Added test coverage for scenario which triggered assert in
metadata locking subsystem.
mysql-test/t/events_2.test:
Updated test case after prohibiting event DDL operations
under LOCK TABLES.
mysql-test/t/flush.test:
Added test coverage for bug #57006 "Deadlock between HANDLER
and FLUSH TABLES WITH READ LOCK".
mysql-test/t/flush_block_commit.test:
Adjusted test case after changing thread state name which
is used when COMMIT waits for FLUSH TABLES WITH READ LOCK
from "Waiting for release of readlock" to "Waiting for commit
lock".
mysql-test/t/flush_block_commit_notembedded.test:
Adjusted test case after changing thread state name which is
used when DML waits for FLUSH TABLES WITH READ LOCK. Now we
use "Waiting for global read lock" in this case.
mysql-test/t/flush_read_lock.test:
Added test coverage for various aspects of FLUSH TABLES WITH
READ LOCK functionality.
mysql-test/t/flush_read_lock_kill-master.opt:
We no longer need to use make_global_read_lock_block_commit_loop
debug tag in this test. Instead we rely on an appropriate
debug_sync point in MDL code.
mysql-test/t/flush_read_lock_kill.test:
Adjusted test case after replacing custom global read lock
implementation with one based on metadata locks. Use new
debug_sync point. Do not disable concurrent inserts as now
InnoDB we always use InnoDB table.
mysql-test/t/lock_multi.test:
Adjusted test case after changing thread state names which
are used when DML or DDL waits for FLUSH TABLES WITH READ
LOCK to "Waiting for global read lock".
mysql-test/t/mdl_sync.test:
Adjusted test case after replacing custom global read lock
implementation with one based on metadata locks. Replaced
usage of GRL-specific debug_sync's with appropriate sync
points in MDL subsystem. Updated thread state names which
are used when DDL waits for FTWRL.
mysql-test/t/trigger_notembedded.test:
Adjusted test case after changing thread state names which
are used when DML or DDL waits for FLUSH TABLES WITH READ
LOCK to "Waiting for global read lock".
sql/event_data_objects.cc:
Removed Event_queue_element::status/last_executed_changed
members and Event_queue_element::update_timing_fields()
method. We no longer use this class for updating mysql.events
once event is chosen for execution. Accesses to instances of
this class in scheduler thread require protection by
Event_queue::LOCK_event_queue mutex and we try to avoid
updating table while holding this lock.
sql/event_data_objects.h:
Removed Event_queue_element::status/last_executed_changed
members and Event_queue_element::update_timing_fields()
method. We no longer use this class for updating mysql.events
once event is chosen for execution. Accesses to instances of
this class in scheduler thread require protection by
Event_queue::LOCK_event_queue mutex and we try to avoid
updating table while holding this lock.
sql/event_db_repository.cc:
- Changed Event_db_repository methods to not release all
metadata locks once they are done updating mysql.events
table. This allows to keep metadata lock protecting
against GRL and lock protecting particular event around
until corresponding DDL statement is written to the binary
log.
- Removed logic for conditional update of "status" and
"last_executed" fields from update_timing_fields_for_event()
method. In the only case when this method is called now
"last_executed" is always modified and tracking change
of "status" is too much hassle.
sql/event_db_repository.h:
Removed logic for conditional update of "status" and
"last_executed" fields from Event_db_repository::
update_timing_fields_for_event() method.
In the only case when this method is called now "last_executed"
is always modified and tracking change of "status" field is
too much hassle.
sql/event_queue.cc:
Changed event scheduler code not to update mysql.events
table while holding Event_queue::LOCK_event_queue mutex.
Doing so led to a deadlock with a new GRL implementation.
This deadlock didn't occur with old implementation due to
fact that code acquiring protection against GRL ignored
pending GRL requests (which lead to GRL starvation).
One of goals of new implementation is to disallow GRL
starvation and so we have to solve problem with this
deadlock in a different way.
sql/events.cc:
Changed methods of Events class to acquire protection
against GRL while perfoming DDL statement and keep it
until statement is written to the binary log.
Unfortunately this step together with new GRL implementation
exposed deadlock involving Events::LOCK_event_metadata
and GRL. To solve it Events::LOCK_event_metadata mutex was
replaced with a metadata lock on event. As a side-effect
events DDL has to be prohibited under LOCK TABLES even in
cases when mysql.events table was explicitly locked for
write.
sql/events.h:
Replaced Events::LOCK_event_metadata mutex with a metadata
lock on event.
sql/ha_ndbcluster.cc:
Updated code after replacing custom global read lock
implementation with one based on MDL. Since MDL subsystem
should now be able to detect deadlocks involving metadata
locks and GRL there is no need for special handling of
active GRL.
sql/handler.cc:
Replaced custom implementation of global read lock with
one based on metadata locks. Consequently when doing
commit instead of calling method of Global_read_lock
class to acquire protection against GRL we simply acquire
IX in COMMIT namespace.
sql/lock.cc:
Replaced custom implementation of global read lock with
one based on metadata locks. This step allows to expose
wait for GRL to deadlock detector of MDL subsystem and
thus succesfully resolve deadlocks similar to one behind
bug #57006 "Deadlock between HANDLER and FLUSH TABLES
WITH READ LOCK". It also solves problem with GRL starvation
described in bug #54673 "It takes too long to get readlock
for 'FLUSH TABLES WITH READ LOCK'" since metadata locks used
by GRL give preference to FTWRL statement instead of DML
statements (if needed in future this can be changed to
fair scheduling).
Similar to old implementation of acquisition of GRL is
two-step. During the first step we block all concurrent
DML and DDL statements by acquiring global S metadata lock
(each DML and DDL statement acquires global IX lock for
its duration). During the second step we block commits by
acquiring global S lock in COMMIT namespace (commit code
acquires global IX lock in this namespace).
Note that unlike in old implementation acquisition of
protection against GRL in DML and DDL is semi-automatic.
We assume that any statement which should be blocked by GRL
will either open and acquires write-lock on tables or acquires
metadata locks on objects it is going to modify. For any such
statement global IX metadata lock is automatically acquired
for its duration.
To support this change:
- Global_read_lock::lock/unlock_global_read_lock and
make_global_read_lock_block_commit methods were changed
accordingly.
- Global_read_lock::wait_if_global_read_lock() and
start_waiting_global_read_lock() methods were dropped.
It is now responsibility of code acquiring metadata locks
opening tables to acquire protection against GRL by
explicitly taking global IX lock with statement duration.
- Global variables, mutex and condition variable used by
old implementation was removed.
- lock_routine_name() was changed to use statement duration for
its global IX lock. It was also renamed to lock_object_name()
as it now also used to take metadata locks on events.
- Global_read_lock::set_explicit_lock_duration() was added which
allows not to release locks used for GRL when leaving prelocked
mode.
sql/lock.h:
- Renamed lock_routine_name() to lock_object_name() and changed
its signature to allow its usage for events.
- Removed broadcast_refresh() function. It is no longer needed
with new GRL implementation.
sql/log_event.cc:
Release metadata locks with statement duration at the end
of processing legacy event for LOAD DATA. This ensures that
replication thread processing such event properly releases
its protection against global read lock.
sql/mdl.cc:
Changed MDL subsystem to support new MDL-based implementation
of global read lock.
Added COMMIT and EVENTS namespaces for metadata locks. Changed
thread state name for GLOBAL namespace to "Waiting for global
read lock".
Optimized MDL_map::find_or_insert() method to avoid taking
m_mutex mutex when looking up MDL_lock objects for GLOBAL
or COMMIT namespaces. We keep pre-created MDL_lock objects
for these namespaces around and simply return pointers to
these global objects when needed.
Changed MDL_lock/MDL_scoped_lock to properly handle
notification of insert delayed handler threads when FTWRL
takes global S lock.
Introduced concept of lock duration. In addition to locks with
transaction duration which work in the way which is similar to
how locks worked before (i.e. they are released at the end of
transaction), locks with statement and explicit duration were
introduced.
Locks with statement duration are automatically released at the
end of statement. Locks with explicit duration require explicit
release and obsolete concept of transactional sentinel.
* Changed MDL_request and MDL_ticket classes to support notion
of duration.
* Changed MDL_context to keep locks with different duration in
different lists. Changed code handling ticket list to take
this into account.
* Changed methods responsible for releasing locks to take into
account duration of tickets. Particularly public
MDL_context::release_lock() method now only can release
tickets with explicit duration (there is still internal
method which allows to specify duration). To release locks
with statement or transaction duration one have to use
release_statement/transactional_locks() methods.
* Concept of savepoint for MDL subsystem now has to take into
account locks with statement duration. Consequently
MDL_savepoint class was introduced and methods working with
savepoints were updated accordingly.
* Added methods which allow to set duration for one or all
locks in the context.
sql/mdl.h:
Changed MDL subsystem to support new MDL-based implementation
of global read lock.
Added COMMIT and EVENTS namespaces for metadata locks.
Introduced concept of lock duration. In addition to locks with
transaction duration which work in the way which is similar to
how locks worked before (i.e. they are released at the end of
transaction), locks with statement and explicit duration were
introduced.
Locks with statement duration are automatically released at the
end of statement. Locks with explicit duration require explicit
release and obsolete concept of transactional sentinel.
* Changed MDL_request and MDL_ticket classes to support notion
of duration.
* Changed MDL_context to keep locks with different duration in
different lists. Changed code handling ticket list to take
this into account.
* Changed methods responsible for releasing locks to take into
account duration of tickets. Particularly public
MDL_context::release_lock() method now only can release
tickets with explicit duration (there is still internal
method which allows to specify duration). To release locks
with statement or transaction duration one have to use
release_statement/transactional_locks() methods.
* Concept of savepoint for MDL subsystem now has to take into
account locks with statement duration. Consequently
MDL_savepoint class was introduced and methods working with
savepoints were updated accordingly.
* Added methods which allow to set duration for one or all
locks in the context.
sql/mysqld.cc:
Removed global mutex and condition variables which were used
by old implementation of GRL.
Also we no longer need to initialize Events::LOCK_event_metadata
mutex as it was replaced with metadata locks on events.
sql/mysqld.h:
Removed global variable, mutex and condition variables which
were used by old implementation of GRL.
sql/rpl_rli.cc:
When slave thread closes tables which were open for handling
of RBR events ensure that it releases global IX lock which
was acquired as protection against GRL.
sql/sp.cc:
Adjusted code to the new signature of lock_object/routine_name(),
to the fact that one now needs specify duration of lock when
initializing MDL_request and to the fact that savepoints for MDL
subsystem are now represented by MDL_savepoint class.
sql/sp_head.cc:
Ensure that statements in stored procedures release statement
metadata locks and thus release their protectiong against GRL
in proper moment in time.
Adjusted code to the fact that one now needs specify duration
of lock when initializing MDL_request.
sql/sql_admin.cc:
Adjusted code to the fact that one now needs specify duration
of lock when initializing MDL_request.
sql/sql_base.cc:
- Implemented support for new approach to acquiring protection
against global read lock. We no longer acquire such protection
explicitly on the basis of statement flags. Instead we always
rely on code which is responsible for acquiring metadata locks
on object to be changed acquiring this protection. This is
achieved by acquiring global IX metadata lock with statement
duration. Code doing this also responsible for checking that
current connection has no active GRL by calling an
Global_read_lock::can_acquire_protection() method.
Changed code in open_table() and lock_table_names()
accordingly.
Note that as result of this change DDL and DML on temporary
tables is always compatible with GRL (before it was
incompatible in some cases and compatible in other cases).
- To speed-up code acquiring protection against GRL introduced
m_has_protection_against_grl member in Open_table_context
class. It indicates that protection was already acquired
sometime during open_tables() execution and new attempts
can be skipped.
- Thanks to new GRL implementation calls to broadcast_refresh()
became unnecessary and were removed.
- Adjusted code to the fact that one now needs specify duration
of lock when initializing MDL_request and to the fact that
savepoints for MDL subsystem are now represented by
MDL_savepoint class.
sql/sql_base.h:
Adjusted code to the fact that savepoints for MDL subsystem are
now represented by MDL_savepoint class.
Also introduced Open_table_context::m_has_protection_against_grl
member which allows to avoid acquiring protection against GRL
while opening tables if such protection was already acquired.
sql/sql_class.cc:
Changed THD::leave_locked_tables_mode() after transactional
sentinel for metadata locks was obsoleted by introduction of
locks with explicit duration.
sql/sql_class.h:
- Adjusted code to the fact that savepoints for MDL subsystem
are now represented by MDL_savepoint class.
- Changed Global_read_lock class according to changes in
global read lock implementation:
* wait_if_global_read_lock and start_waiting_global_read_lock
are now gone. Instead code needing protection against GRL
has to acquire global IX metadata lock with statement
duration itself. To help it new can_acquire_protection()
was introduced. Also as result of the above change
m_protection_count member is gone too.
* Added m_mdl_blocks_commits_lock member to store metadata
lock blocking commits.
* Adjusted code to the fact that concept of transactional
sentinel was obsoleted by concept of lock duration.
- Removed CF_PROTECT_AGAINST_GRL flag as it is no longer
necessary. New GRL implementation acquires protection
against global read lock automagically when statement
acquires metadata locks on tables or other objects it
is going to change.
sql/sql_db.cc:
Adjusted code to the fact that one now needs specify duration
of lock when initializing MDL_request.
sql/sql_handler.cc:
Removed call to broadcast_refresh() function. It is no longer
needed with new GRL implementation.
Adjusted code after introducing duration concept for metadata
locks. Particularly to the fact transactional sentinel was
replaced with explicit duration.
sql/sql_handler.h:
Renamed mysql_ha_move_tickets_after_trans_sentinel() to
mysql_ha_set_explicit_lock_duration() after transactional
sentinel was obsoleted by locks with explicit duration.
sql/sql_insert.cc:
Adjusted code handling delaying inserts after switching to
new GRL implementation. Now connection thread initiating
delayed insert has to acquire global IX lock in addition
to metadata lock on table being inserted into. This IX lock
protects against GRL and similarly to SW lock on table being
inserted into has to be passed to handler thread in order to
avoid deadlocks.
sql/sql_lex.cc:
LEX::protect_against_global_read_lock member is no longer
necessary since protection against GRL is automatically
taken by code acquiring metadata locks/opening tables.
sql/sql_lex.h:
LEX::protect_against_global_read_lock member is no longer
necessary since protection against GRL is automatically
taken by code acquiring metadata locks/opening tables.
sql/sql_parse.cc:
- Implemented support for new approach to acquiring protection
against global read lock. We no longer acquire such protection
explicitly on the basis of statement flags. Instead we always
rely on code which is responsible for acquiring metadata locks
on object to be changed acquiring this protection. This is
achieved by acquiring global IX metadata lock with statement
duration. This lock is automatically released at the end of
statement execution.
- Changed implementation of CREATE/DROP PROCEDURE/FUNCTION not
to release metadata locks and thus protection against of GRL
in the middle of statement execution.
- Adjusted code to the fact that one now needs specify duration
of lock when initializing MDL_request and to the fact that
savepoints for MDL subsystem are now represented by
MDL_savepoint class.
sql/sql_prepare.cc:
Adjusted code to the to the fact that savepoints for MDL
subsystem are now represented by MDL_savepoint class.
sql/sql_rename.cc:
With new GRL implementation there is no need to explicitly
acquire protection against GRL before renaming tables.
This happens automatically in code which acquires metadata
locks on tables being renamed.
sql/sql_show.cc:
Adjusted code to the fact that one now needs specify duration
of lock when initializing MDL_request and to the fact that
savepoints for MDL subsystem are now represented by
MDL_savepoint class.
sql/sql_table.cc:
- With new GRL implementation there is no need to explicitly
acquire protection against GRL before dropping tables.
This happens automatically in code which acquires metadata
locks on tables being dropped.
- Changed mysql_alter_table() not to release lock on new table
name explicitly and to rely on automatic release of locks
at the end of statement instead. This was necessary since
now MDL_context::release_lock() is supported only for locks
for explicit duration.
sql/sql_trigger.cc:
With new GRL implementation there is no need to explicitly
acquire protection against GRL before changing table triggers.
This happens automatically in code which acquires metadata
locks on tables which triggers are to be changed.
sql/sql_update.cc:
Fix bug exposed by GRL testing. During prepare phase acquire
only S metadata locks instead of SW locks to keep prepare of
multi-UPDATE compatible with concurrent LOCK TABLES WRITE
and global read lock.
sql/sql_view.cc:
With new GRL implementation there is no need to explicitly
acquire protection against GRL before creating view.
This happens automatically in code which acquires metadata
lock on view to be created.
sql/sql_yacc.yy:
LEX::protect_against_global_read_lock member is no longer
necessary since protection against GRL is automatically
taken by code acquiring metadata locks/opening tables.
sql/table.cc:
Adjusted code to the fact that one now needs specify duration
of lock when initializing MDL_request.
sql/table.h:
Adjusted code to the fact that one now needs specify duration
of lock when initializing MDL_request.
sql/transaction.cc:
Replaced custom implementation of global read lock with
one based on metadata locks. Consequently when doing
commit instead of calling method of Global_read_lock
class to acquire protection against GRL we simply acquire
IX in COMMIT namespace.
Also adjusted code to the fact that MDL savepoint is now
represented by MDL_savepoint class.
LOAD DATA into partitioned MyISAM table
Problem was that both partitioning and myisam
used the same table_share->mutex for different protections
(auto inc and repair).
Solved by adding a specific mutex for the partitioning
auto_increment.
Also adding destroying the ha_data structure in
free_table_share (which is to be propagated
into 5.5).
This is a 5.1 ONLY patch, already fixed in 5.5+.
MDL deadlock detector".
Deadlock could have occurred when workload containing mix
of DML, DDL and FLUSH TABLES statements affecting same
set of tables was executed in heavily concurrent environment.
This deadlock occurred when several connections tried to
perform deadlock detection in metadata locking subsystem.
The first connection started traversing wait-for graph,
encountered sub-graph representing wait for flush, acquired
LOCK_open and dived into sub-graph inspection. When it has
encounterd sub-graph corresponding to wait for metadata lock
and blocked while trying to acquire rd-lock on
MDL_lock::m_rwlock (*) protecting this subgraph, since some
other thread had wr-lock on it. When this wr-lock was released
it could have happened (if there was other pending wr-lock
against this rwlock) that rd-lock from the first connection
was left unsatisfied but at the same time new rd-lock request
from the second connection sneaked in and was satisfied (for
this to be possible second rd- request should come exactly
after wr-lock is released but before pending wr-lock manages
to grab rwlock, which is possible both on Linux and in our
own rwlock implementation). If this second connection
continued traversing wait-for graph and encountered sub-graph
representing wait for flush it tried to acquire LOCK_open
and thus deadlock was created.
This patch tries to workaround this problem but not allowing
deadlock detector to lock LOCK_open mutex if some other thread
doing deadlock detection already owns it and current search
depth is greater than 0. Instead deadlock is reported.
Other possible solutions are either known to have negative
effects on performance or require much more time for proper
implementation and testing.
No test case is provided as this bug is very hard to repeat
in MTR environment but is repeatable with the help of RQG
tests.
sql/mdl.cc:
Moved Deadlock_detection_visitor::m_current_search_depth to
parent class to make it available in
TABLE_SHARE::visit_subgraph().
Added MDL_wait_for_graph_visitor::abort_traversal() method
which allows to abort traversal of a wait-for graph and
report a deadlock.
sql/mdl.h:
Moved Deadlock_detection_visitor::m_current_search_depth to
parent class to make it available in
TABLE_SHARE::visit_subgraph().
Added MDL_wait_for_graph_visitor::abort_traversal() method
which allows to abort traversal of a wait-for graph and
report a deadlock.
sql/sql_base.cc:
Added dd_owns_lock_open counter and mutex protecting it to
track number of connections which do deadlock detection and
own or try to acquire LOCK_open.
sql/sql_base.h:
Added dd_owns_lock_open counter and mutex protecting it to
track number of connections which do deadlock detection and
own or try to acquire LOCK_open.
sql/table.cc:
Workaround bug #56405 but not allowing MDL deadlock detector
to lock LOCK_open mutex if some other thread doing deadlock
detection already owns it and current search depth is greater
than 0. Instead report deadlock.
to allow temp table operations) -- prerequisite patch #1.
Move a piece of code that initialiazes TABLE instance
after it was successfully opened into a separate function.
This function will be reused in the following patches.
TABLES <list> WITH READ LOCK are incompatible".
The problem was that FLUSH TABLES <list> WITH READ LOCK
which was issued when other connection has acquired global
read lock using FLUSH TABLES WITH READ LOCK was blocked
and has to wait until global read lock is released.
This issue stemmed from the fact that FLUSH TABLES <list>
WITH READ LOCK implementation has acquired X metadata locks
on tables to be flushed. Since these locks required acquiring
of global IX lock this statement was incompatible with global
read lock.
This patch addresses problem by using SNW metadata type of
lock for tables to be flushed by FLUSH TABLES <list> WITH
READ LOCK. It is OK to acquire them without global IX lock
as long as we won't try to upgrade those locks. Since SNW
locks allow concurrent statements using same table FLUSH
TABLE <list> WITH READ LOCK now has to wait until old
versions of tables to be flushed go away after acquiring
metadata locks. Since such waiting can lead to deadlock
MDL deadlock detector was extended to take into account
waits for flush and resolve such deadlocks.
As a bonus code in open_tables() which was responsible for
waiting old versions of tables to go away was refactored.
Now when we encounter old version of table in open_table()
we don't back-off and wait for all old version to go away,
but instead wait for this particular table to be flushed.
Such approach supported by deadlock detection should reduce
number of scenarios in which FLUSH TABLES aborts concurrent
multi-statement transactions.
Note that active FLUSH TABLES <list> WITH READ LOCK still
blocks concurrent FLUSH TABLES WITH READ LOCK statement
as the former keeps tables open and thus prevents the
latter statement from doing flush.
mysql-test/include/handler.inc:
Adjusted test case after changing status which is set
when FLUSH TABLES waits for tables to be flushed from
"Flushing tables" to "Waiting for table".
mysql-test/r/flush.result:
Added test which checks that "flush tables <list> with
read lock" is compatible with active "flush tables with
read lock" but not vice-versa. This test also covers
bug #52044 "FLUSH TABLES WITH READ LOCK and FLUSH TABLES
<list> WITH READ LOCK are incompatible".
mysql-test/r/mdl_sync.result:
Added scenarios in which wait for table to be flushed
causes deadlocks to the coverage of MDL deadlock detector.
mysql-test/suite/perfschema/r/dml_setup_instruments.result:
Adjusted test results after removal of COND_refresh
condition variable.
mysql-test/suite/perfschema/r/server_init.result:
Adjusted test and its results after removal of COND_refresh
condition variable.
mysql-test/suite/perfschema/t/server_init.test:
Adjusted test and its results after removal of COND_refresh
condition variable.
mysql-test/t/flush.test:
Added test which checks that "flush tables <list> with
read lock" is compatible with active "flush tables with
read lock" but not vice-versa. This test also covers
bug #52044 "FLUSH TABLES WITH READ LOCK and FLUSH TABLES
<list> WITH READ LOCK are incompatible".
mysql-test/t/kill.test:
Adjusted test case after changing status which is set
when FLUSH TABLES waits for tables to be flushed from
"Flushing tables" to "Waiting for table".
mysql-test/t/lock_multi.test:
Adjusted test case after changing status which is set
when FLUSH TABLES waits for tables to be flushed from
"Flushing tables" to "Waiting for table".
mysql-test/t/mdl_sync.test:
Added scenarios in which wait for table to be flushed
causes deadlocks to the coverage of MDL deadlock detector.
sql/ha_ndbcluster.cc:
Adjusted code after adding one more parameter for
close_cached_tables() call - timeout for waiting for
table to be flushed.
sql/ha_ndbcluster_binlog.cc:
Adjusted code after adding one more parameter for
close_cached_tables() call - timeout for waiting for
table to be flushed.
sql/lock.cc:
Removed COND_refresh condition variable. See comment
for sql_base.cc for details.
sql/mdl.cc:
Now MDL deadlock detector takes into account information
about waits for table flushes when searching for deadlock.
To implement this change:
- Declaration of enum_deadlock_weight and
Deadlock_detection_visitor were moved to mdl.h header
to make them available to the code in table.cc which
implements deadlock detector traversal through edges
of waiters graph representing waiting for flush.
- Since now MDL_context may wait not only for metadata
lock but also for table to be flushed an abstract
Wait_for_edge class was introduced. Its descendants
MDL_ticket and Flush_ticket incapsulate specifics
of inspecting waiters graph when following through
edge representing wait of particular type.
We no longer require global IX metadata lock when acquiring
SNW or SNRW locks. Such locks are needed only when metadata
locks of these types are upgraded to X locks. This allows
to use SNW locks in FLUSH TABLES <list> WITH READ LOCK
implementation and keep the latter compatible with global
read lock.
sql/mdl.h:
Now MDL deadlock detector takes into account information
about waits for table flushes when searching for deadlock.
To implement this change:
- Declaration of enum_deadlock_weight and
Deadlock_detection_visitor were moved to mdl.h header
to make them available to the code in table.cc which
implements deadlock detector traversal through edges
of waiters graph representing waiting for flush.
- Since now MDL_context may wait not only for metadata
lock but also for table to be flushed an abstract
Wait_for_edge class was introduced. Its descendants
MDL_ticket and Flush_ticket incapsulate specifics
of inspecting waiters graph when following through
edge representing wait of particular type.
- Deadlock_detection_visitor now has m_table_shares_visited
member which allows to support recursive locking for
LOCK_open. This is required when deadlock detector
inspects waiters graph which contains several edges
representing waits for flushes or needs to come through
the such edge more than once.
sql/mysqld.cc:
Removed COND_refresh condition variable. See comment
for sql_base.cc for details.
sql/mysqld.h:
Removed COND_refresh condition variable. See comment
for sql_base.cc for details.
sql/sql_base.cc:
Changed approach to how threads are waiting for table
to be flushed. Now thread that wants to wait for old
table to go away subscribes for notification by adding
Flush_ticket to table's share and waits using
MDL_context::m_wait object. Once table gets flushed
(i.e. all tables are closed and table share is ready
to be destroyed) all such waiters are notified
individually.
Thanks to this change MDL deadlock detector can take
such waits into account.
To implement this/as result of this change:
- tdc_wait_for_old_versions() was replaced with
tdc_wait_for_old_version() which waits for individual
old share to go away and which is called by open_table()
after finding out that share is outdated. We don't
need to perform back-off before such waiting thanks
to the fact that deadlock detector now sees such waits.
- As result Open_table_ctx::m_mdl_requests became
unnecessary and was removed. We no longer allocate
copies of MDL_request objects on MEM_ROOT when
MYSQL_OPEN_FORCE_SHARED/SHARED_HIGH_PRIO flags are
in effect.
- close_cached_tables() and tdc_wait_for_old_version()
share code which implements waiting for share to be
flushed - the both use TABLE_SHARE::wait_until_flush()
method. Thanks to this close_cached_tables() supports
timeouts and has extra parameter for this.
- Open_table_context::OT_MDL_CONFLICT enum element was
renamed to OT_CONFLICT as it is now also used in cases
when back-off is required to resolve deadlock caused
by waiting for flush and not metadata lock.
- In cases when we discover that current connection tries
to open tables from different generation we now simply
back-off and restart process of opening tables. To
support this Open_table_context::OT_REOPEN_TABLES enum
element was added.
- COND_refresh condition variable became unnecessary and
was removed.
- mysql_notify_thread_having_shared_lock() no longer wakes
up connections waiting for flush as all such connections
can be waken up by deadlock detector if necessary.
sql/sql_base.h:
- close_cached_tables() now has one more parameter -
timeout for waiting for table to be flushed.
- Open_table_context::OT_MDL_CONFLICT enum element was
renamed to OT_CONFLICT as it is now also used in cases
when back-off is required to resolve deadlock caused
by waiting for flush and not metadata lock.
Added new OT_REOPEN_TABLES enum element to be used in
cases when we need to restart open tables process even
in the middle of transaction.
- Open_table_ctx::m_mdl_requests became unnecessary and
was removed.
sql/sql_class.h:
Added assert ensuring that we won't use LOCK_open mutex
with THD::enter_cond(). Otherwise deadlocks can arise in
MDL deadlock detector.
sql/sql_parse.cc:
Changed FLUSH TABLES <list> WITH READ LOCK to take SNW
metadata locks instead of X locks on tables to be flushed.
Since we no longer require global IX lock to be taken
when SNW locks are taken this makes this statement
compatible with FLUSH TABLES WITH READ LOCK statement.
Since SNW locks allow other connections to have table
opened FLUSH TABLES <list> WITH READ LOCK now has to
wait during open_tables() for old version to go away.
Such waits can lead to deadlocks which will be detected
by MDL deadlock detector which now takes waits for table
to be flushed into account.
Also adjusted code after adding one more parameter for
close_cached_tables() call - timeout for waiting for
table to be flushed.
sql/sql_yacc.yy:
FLUSH TABLES <list> WITH READ LOCK now needs only SNW
metadata locks on tables.
sql/sys_vars.cc:
Adjusted code after adding one more parameter for
close_cached_tables() call - timeout for waiting for
table to be flushed.
sql/table.cc:
Implemented new approach to how threads are waiting for
table to be flushed. Now thread that wants to wait for
old table to go away subscribes for notification by
adding Flush_ticket to table's share and waits using
MDL_context::m_wait object. Once table gets flushed
(i.e. all tables are closed and table share is ready
to be destroyed) all such waiters are notified
individually. This change allows to make such waits
visible inside of MDL deadlock detector.
To do it:
- Added list of waiters/Flush_tickets to TABLE_SHARE
class.
- Changed free_table_share() to postpone freeing of
share memory until last waiter goes away and to
wake up subscribed waiters.
- Added TABLE_SHARE::wait_until_flushed() method which
implements subscription to the list of waiters for
table to be flushed and waiting for this event.
Implemented interface which allows to expose waits for
flushes to MDL deadlock detector:
- Introduced Flush_ticket class a descendant of
Wait_for_edge class.
- Added TABLE_SHARE::find_deadlock() method which allows
deadlock detector to find out what contexts are still
using old version of table in question (i.e. to find
out what contexts are waited for by owner of
Flush_ticket).
sql/table.h:
In order to support new strategy of waiting for table flush
(see comment for table.cc for details) added list of
waiters/Flush_tickets to TABLE_SHARE class.
Implemented interface which allows to expose waits for
flushes to MDL deadlock detector:
- Introduced Flush_ticket class a descendant of
Wait_for_edge class.
- Added TABLE_SHARE::find_deadlock() method which allows
deadlock detector to find out what contexts are still
using old version of table in question (i.e. to find
out what contexts are waited for by owner of
Flush_ticket).
Fix warnings flagged by the new warning option -Wunused-but-set-variable
that was added to GCC 4.6 and that is enabled by -Wunused and -Wall. The
option causes a warning whenever a local variable is assigned to but is
later unused. It also warns about meaningless pointer dereferences.
client/mysql.cc:
Meaningless pointer dereferences.
client/mysql_upgrade.c:
Check whether reading from the file succeeded.
extra/comp_err.c:
Unused.
extra/yassl/src/yassl_imp.cpp:
Skip instead of reading data that is discarded.
include/my_pthread.h:
Variable is only used in debug builds.
include/mysys_err.h:
Add new error messages.
mysys/errors.c:
Add new error message for permission related functions.
mysys/mf_iocache.c:
Variable is only checked under THREAD.
mysys/my_copy.c:
Raise a error if chmod or chown fails.
mysys/my_redel.c:
Raise a error if chmod or chown fails.
regex/engine.c:
Use a equivalent variable for the assert.
server-tools/instance-manager/instance_options.cc:
Unused.
sql/field.cc:
Unused.
sql/item.cc:
Unused.
sql/log.cc:
Do not ignore the return value of freopen: only set buffer if
reopening succeeds.
Adjust doxygen comment to the right function.
Pass message lenght to log function.
sql/mysqld.cc:
Do not ignore the return value of freopen: only set buffer if
reopening succeeds.
sql/partition_info.cc:
Unused.
sql/slave.cc:
No need to set pointer to the address of '\0'.
sql/spatial.cc:
Unused. Left for historical purposes.
sql/sql_acl.cc:
Unused.
sql/sql_base.cc:
Pointers are always set to the same variables.
sql/sql_parse.cc:
End statement if reading fails.
Store the buffer after it has actually been updated.
sql/sql_repl.cc:
No need to set pointer to the address of '\0'.
sql/sql_show.cc:
Put variable under the same ifdef block.
sql/udf_example.c:
Set null pointer flag appropriately.
storage/csv/ha_tina.cc:
Meaningless dereferences.
storage/example/ha_example.cc:
Return the error since it's available.
storage/myisam/mi_locking.c:
Remove unused and dead code.
Essentially, the problem is that safemalloc is excruciatingly
slow as it checks all allocated blocks for overrun at each
memory management primitive, yielding a almost exponential
slowdown for the memory management functions (malloc, realloc,
free). The overrun check basically consists of verifying some
bytes of a block for certain magic keys, which catches some
simple forms of overrun. Another minor problem is violation
of aliasing rules and that its own internal list of blocks
is prone to corruption.
Another issue with safemalloc is rather the maintenance cost
as the tool has a significant impact on the server code.
Given the magnitude of memory debuggers available nowadays,
especially those that are provided with the platform malloc
implementation, maintenance of a in-house and largely obsolete
memory debugger becomes a burden that is not worth the effort
due to its slowness and lack of support for detecting more
common forms of heap corruption.
Since there are third-party tools that can provide the same
functionality at a lower or comparable performance cost, the
solution is to simply remove safemalloc. Third-party tools
can provide the same functionality at a lower or comparable
performance cost.
The removal of safemalloc also allows a simplification of the
malloc wrappers, removing quite a bit of kludge: redefinition
of my_malloc, my_free and the removal of the unused second
argument of my_free. Since free() always check whether the
supplied pointer is null, redudant checks are also removed.
Also, this patch adds unit testing for my_malloc and moves
my_realloc implementation into the same file as the other
memory allocation primitives.
client/mysqldump.c:
Pass my_free directly as its signature is compatible with the
callback type -- which wasn't the case for free_table_ent.
use limit efficiently
Bug #36569: UPDATE ... WHERE ... ORDER BY... always does a
filesort even if not required
Also two bugs reported after QA review (before the commit
of bugs above to public trees, no documentation needed):
Bug #53737: Performance regressions after applying patch
for bug 36569
Bug #53742: UPDATEs have no effect after applying patch
for bug 36569
Execution of single-table UPDATE and DELETE statements did not use the
same optimizer as was used in the compilation of SELECT statements.
Instead, it had an optimizer of its own that did not take into account
that you can omit sorting by retrieving rows using an index.
Extra optimization has been added: when applicable, single-table
UPDATE/DELETE statements use an existing index instead of filesort. A
corresponding SELECT query would do the former.
Also handling of the DESC ordering expression has been added when
reverse index scan is applicable.
From now on most single table UPDATE and DELETE statements show the
same disk access patterns as the corresponding SELECT query. We verify
this by comparing the result of SHOW STATUS LIKE 'Sort%
Currently the get_index_for_order function
a) checks quick select index (if any) for compatibility with the
ORDER expression list or
b) chooses the cheapest available compatible index, but only if
the index scan is cheaper than filesort.
Second way is implemented by the new test_if_cheaper_ordering
function (extracted part the test_if_skip_sort_order()).
mysql-test/r/log_state.result:
Updated result for optimized query, bug #36569.
mysql-test/r/single_delete_update.result:
Test case for bug #30584, bug #36569 and bug #53742.
mysql-test/r/update.result:
Updated result for optimized query, bug #30584.
Note:
"Handler_read_last 1" omitted, see bug 52312:
lost Handler_read_last status variable.
mysql-test/t/single_delete_update.test:
Test case for bug #30584, bug #36569 and bug #53742.
sql/opt_range.cc:
Bug #30584, bug #36569: UPDATE/DELETE ... WHERE ... ORDER BY...
always does a filesort even if not required
* get_index_for_order() has been rewritten entirely and moved
to sql_select.cc
New QUICK_RANGE_SELECT::make_reverse method has been added.
sql/opt_range.h:
Bug #30584, bug #36569: UPDATE/DELETE ... WHERE ... ORDER BY...
always does a filesort even if not required
* get_index_for_order() has been rewritten entirely and moved
to sql_select.cc
New functions:
* QUICK_SELECT_I::make_reverse()
* SQL_SELECT::set_quick()
sql/records.cc:
Bug #30584, bug #36569: UPDATE/DELETE ... WHERE ... ORDER BY...
always does a filesort even if not required
* init_read_record_idx() has been modified to allow reverse index scan
New functions:
* rr_index_last()
* rr_index_desc()
sql/records.h:
Bug #30584, bug #36569: UPDATE/DELETE ... WHERE ... ORDER BY...
always does a filesort even if not required
init_read_record_idx() has been modified to allow reverse index scan
sql/sql_delete.cc:
Bug #30584, bug #36569: UPDATE/DELETE ... WHERE ... ORDER BY...
always does a filesort even if not required
mysql_delete: an optimization has been added to skip
unnecessary sorting with ORDER BY clause where select
result ordering is acceptable.
sql/sql_select.cc:
Bug #30584, bug #36569, bug #53737, bug #53742:
UPDATE/DELETE ... WHERE ... ORDER BY... always does a filesort
even if not required
The const_expression_in_where function has been modified
to accept both Item and Field pointers.
New functions:
* get_index_for_order()
* test_if_cheaper_ordering() has been extracted from
test_if_skip_sort_order() to share with get_index_for_order()
* simple_remove_const()
sql/sql_select.h:
Bug #30584, bug #36569: UPDATE/DELETE ... WHERE ... ORDER BY...
always does a filesort even if not required
New functions:
* test_if_cheaper_ordering()
* simple_remove_const()
* get_index_for_order()
sql/sql_update.cc:
Bug #30584, bug #36569: UPDATE/DELETE ... WHERE ... ORDER BY...
always does a filesort even if not required
mysql_update: an optimization has been added to skip
unnecessary sorting with ORDER BY clause where a select
result ordering is acceptable.
sql/table.cc:
Bug #30584, bug #36569: UPDATE/DELETE ... WHERE ... ORDER BY...
always does a filesort even if not required
New functions:
* TABLE::update_const_key_parts()
* is_simple_order()
sql/table.h:
Bug #30584, bug #36569: UPDATE/DELETE ... WHERE ... ORDER BY...
always does a filesort even if not required
New functions:
* TABLE::update_const_key_parts()
* is_simple_order()
strict aliasing violations.
Essentially, the problem is that large parts of the server were
developed in simpler times (last decades, pre C99 standard) when
strict aliasing and compilers supporting such optimizations were
rare to non-existent. Thus, when compiling the server with a modern
compiler that uses strict aliasing rules to perform optimizations,
there are several places in the code that might trigger undefined
behavior.
As evinced by some recent bugs, GCC does a somewhat good of job
misoptimizing such code, but on the other hand also gives warnings
about suspicious code. One problem is that the warnings aren't
always accurate, yet we can't afford to just shut them off as we
might miss real cases. False-positive cases are aggravated mostly
by casts that are likely to trigger undefined behavior.
The solution is to start a cleanup process focused on fixing and
reducing the amount of strict-aliasing related warnings produced
by GCC and others compilers. A good deal of noise reduction can
be achieved by just removing useless casts that are product of
historical cruft and are likely to trigger undefined behavior if
dereferenced.
client/mysql.cc:
Remove now-unnecessary casts.
Break up large strings.
client/mysql_upgrade.c:
Remove now-unnecessary casts.
client/mysqladmin.cc:
Remove now-unnecessary casts.
Break up large strings.
client/mysqlbinlog.cc:
Remove now-unnecessary casts.
client/mysqlcheck.c:
Remove now-unnecessary casts.
client/mysqldump.c:
Remove now-unnecessary casts.
client/mysqlimport.c:
Remove now-unnecessary casts.
client/mysqlshow.c:
Remove now-unnecessary casts.
client/mysqlslap.c:
Remove now-unnecessary casts.
client/mysqltest.cc:
Remove now-unnecessary casts.
extra/comp_err.c:
Remove now-unnecessary casts.
extra/my_print_defaults.c:
Remove now-unnecessary casts.
Break up large strings.
extra/mysql_waitpid.c:
Remove now-unnecessary casts.
extra/perror.c:
Remove now-unnecessary casts.
extra/resolve_stack_dump.c:
Remove now-unnecessary casts.
extra/resolveip.c:
Remove now-unnecessary casts.
include/my_getopt.h:
Use a void pointer type as the opaque type to avoid problems with type
incompatibility -- GCC issues warnings when the type name is not type
compatible with a operand. As a side bonus, a explicit cast won't be
necessary anymore.
include/sslopt-longopts.h:
Remove now-unnecessary casts.
Break up large strings.
mysys/my_getopt.c:
Update opaque type and introduce a type definition for the
argument to my_getopt_register_get_addr.
server-tools/instance-manager/options.cc:
Remove now-unnecessary casts.
sql/mysqld.cc:
Remove now-unnecessary casts.
Break up large strings.
Update mysql_getopt_value prototype (the old prototype
was different from the definition anyway).
sql/sql_plugin.cc:
The type of a pointer to a function must be compatible with the
pointed-to function type, otherwise the behavior is undefined.
sql/table.cc:
The variable buf pointer to pointer to pointer to constant char
could improperly alias a incompatible type in call to fix_type_
pointers. Since this was actually dead code, it is simply removed.
sql/unireg.cc:
Remove call to get_form_pos. The code creates a new FRM file which
is always truncated and writes the form position as 0. Hence, no
need to retrieve it, we now for sure it is 0.
storage/archive/archive_reader.c:
Remove now-unnecessary casts.
storage/myisam/ft_nlq_search.c:
Read weight directly from the buffer.
storage/myisam/fulltext.h:
Add explanation about the type duality of a key buffer.
Add accessor macro to retrieve a FT float value.
storage/myisam/mi_test1.c:
Remove now-unnecessary casts.
storage/myisam/myisam_ftdump.c:
Read weight directly from the buffer.
storage/myisam/myisamchk.c:
Remove now-unnecessary casts.
storage/myisam/myisamlog.c:
A pointer to char was used to alias a pointer to pointer to
unsigned char, thus violating strict aliasing rules.
storage/myisam/myisampack.c:
Remove now-unnecessary casts.
strings/decimal.c:
Remove aliasing violation, printing the value is enough for
debugging purposes.
tests/mysql_client_test.c:
Remove now-unnecessary casts.
Conflicts:
Text conflict in mysql-test/r/archive.result
Contents conflict in mysql-test/r/innodb_bug38231.result
Text conflict in mysql-test/r/mdl_sync.result
Text conflict in mysql-test/suite/binlog/t/disabled.def
Text conflict in mysql-test/suite/rpl_ndb/r/rpl_ndb_binlog_format_errors.result
Text conflict in mysql-test/t/archive.test
Contents conflict in mysql-test/t/innodb_bug38231.test
Text conflict in mysql-test/t/mdl_sync.test
Text conflict in sql/sp_head.cc
Text conflict in sql/sql_show.cc
Text conflict in sql/table.cc
Text conflict in sql/table.h
