Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PS-5846: MyRocks - Add support for default value expressions #1

Closed
wants to merge 1 commit into from

Conversation

voodam
Copy link

@voodam voodam commented Nov 17, 2021

  1. Add support for default value expressions in MyRocks.
  2. Add rocksdb.default_as_expr MTR test.

Also PS-4114 (percona#1912) and percona@56c96831f2a6 was added to safisly default_as_expr test.

1. Add support for default value expressions in MyRocks.
2. Add `rocksdb.default_as_expr` MTR test.
@inikep inikep deleted the branch inikep:FB8-PS-4894 January 10, 2022 11:18
@inikep inikep closed this Jan 10, 2022
inikep pushed a commit that referenced this pull request Jun 1, 2022
…NSHIP WITH THE BUFFER SIZE

Bug #33501541: Unmanageable Sort Buffer Behavior in 8.0.20+

Implement direct disk-to-disk copies of large packed addons during the
filesort merge phase; if a single row is so large that its addons do not
fit into its slice of the sort buffer during merging (even after
emptying that slice of all other rows), but the sort key _does_ fit,
simply sort the truncated row as usual, and then copy the rest of the
addon incrementally from the input to the output, 4 kB at a time, when
the row is to be written to the merge output. This is possible because
the addon itself doesn't need to be in RAM for the row to be compared
against other rows; only the sort key must.

This greatly relaxes the sort buffer requirements for successful merging,
especially when it comes to JSON rows or small blobs (which are
typically used as packed addons, not sort keys). The rules used to be:

 1. During initial chunk generation: The sort buffer must be at least
    as large as the largest row to be sorted.
 2. During merging: Merging is guaranteed to pass if the sort buffer is
    at least 15 times as large as the largest row (sort key + addons),
    but one may be lucky and pass with only the demands from #1.

Now, for sorts implemented using packed addons (which is the common case
for small blobs and JSON), the new rules are:

 1. Unchanged from #1 above.
 2. During merging: Merging is guaranteed to pass if the sort buffer is
    at least 15 times are large as the largest _sort key_ (plus 4-byte
    length marker), but one may be lucky and pass with only the demands
    from #1.

In practice, this means that filesort merging will almost never fail
due to insufficient buffer space anymore; the query will either fail
because a single row is too large in the sort step, or it will pass
nearly all of the time.

However, do note that while such merges will work, they will not always
be very performant, as having lots of 1-row merge chunks will mean many
merge passes and little work being done during the initial in-memory
sort. Thus, the main use of this functionality is to be able to do sorts
where there are a few rows with large JSON values or similar, but where
most fit comfortably into the buffer. Also note that since requirement
 #1 is unchanged, one still cannot sort e.g. 500 kB JSON values using the
default 256 kB sort buffer.

Older recommendations to keep sort buffers small at nearly any cost are
no longer valid, and have not been for a while. Sort buffers should be
sized to as much RAM as one can afford without interfering with other
tasks (such as the buffer pool, join buffers, or other concurrent
sorts), and small sorts are not affected by the maximum sort buffer size
being set to a larger value, as the sort buffer is incrementally
allocated.

Change-Id: I85745cd513402a42ed5fc4f5b7ddcf13c5793100
inikep pushed a commit that referenced this pull request Jun 1, 2022
…NSHIP WITH THE BUFFER SIZE

Bug #33501541: Unmanageable Sort Buffer Behavior in 8.0.20+

Implement direct disk-to-disk copies of large packed addons during the
filesort merge phase; if a single row is so large that its addons do not
fit into its slice of the sort buffer during merging (even after
emptying that slice of all other rows), but the sort key _does_ fit,
simply sort the truncated row as usual, and then copy the rest of the
addon incrementally from the input to the output, 4 kB at a time, when
the row is to be written to the merge output. This is possible because
the addon itself doesn't need to be in RAM for the row to be compared
against other rows; only the sort key must.

This greatly relaxes the sort buffer requirements for successful merging,
especially when it comes to JSON rows or small blobs (which are
typically used as packed addons, not sort keys). The rules used to be:

 1. During initial chunk generation: The sort buffer must be at least
    as large as the largest row to be sorted.
 2. During merging: Merging is guaranteed to pass if the sort buffer is
    at least 15 times as large as the largest row (sort key + addons),
    but one may be lucky and pass with only the demands from #1.

Now, for sorts implemented using packed addons (which is the common case
for small blobs and JSON), the new rules are:

 1. Unchanged from #1 above.
 2. During merging: Merging is guaranteed to pass if the sort buffer is
    at least 15 times are large as the largest _sort key_ (plus 4-byte
    length marker), but one may be lucky and pass with only the demands
    from #1.

In practice, this means that filesort merging will almost never fail
due to insufficient buffer space anymore; the query will either fail
because a single row is too large in the sort step, or it will pass
nearly all of the time.

However, do note that while such merges will work, they will not always
be very performant, as having lots of 1-row merge chunks will mean many
merge passes and little work being done during the initial in-memory
sort. Thus, the main use of this functionality is to be able to do sorts
where there are a few rows with large JSON values or similar, but where
most fit comfortably into the buffer. Also note that since requirement
 #1 is unchanged, one still cannot sort e.g. 500 kB JSON values using the
default 256 kB sort buffer.

Older recommendations to keep sort buffers small at nearly any cost are
no longer valid, and have not been for a while. Sort buffers should be
sized to as much RAM as one can afford without interfering with other
tasks (such as the buffer pool, join buffers, or other concurrent
sorts), and small sorts are not affected by the maximum sort buffer size
being set to a larger value, as the sort buffer is incrementally
allocated.

Change-Id: I85745cd513402a42ed5fc4f5b7ddcf13c5793100
inikep pushed a commit that referenced this pull request Jun 1, 2022
This reduces the number of issues of type "Single - argument constructor
may inadvertently be used as a type conversion constructor"
as reported by Flint++ tool.
Some of the reported issues were not addressed:
 - lines explicitly marked with NOLINT (like plugin/x/src/prepare_param_handler.h)
 - false positives (tool reports alignas() calls as constructors)
 - issues where objects were intended to be used with type conversion
 - constructors with "const std::string &" param, to accept literal strings as well

Change-Id: I7eb6fabea7137fc7e81143f06ec7636b22f6ea97
inikep pushed a commit that referenced this pull request Jun 1, 2022
Includes a partial implementation of span from C++20.

Change-Id: Ibae9a4aeed95135f4ef35a7ce7b095e6930c1d66
inikep pushed a commit that referenced this pull request Jun 1, 2022
Post push fix.

Remove include of unused header files "ndb_global.h" and <cassert>.

Use std::size_t instead of size_t.

Change-Id: I2c718d0889965ce5967d575172da8df4aa55b1d7
inikep pushed a commit that referenced this pull request Jun 1, 2022
Patch #1 caused several problems in mysql-trunk related to ndbinfo
initialization and upgrade, including the failure of the test
ndb_76_inplace_upgrade and the failure of all NDB MTR tests in Pushbuild
on Windows. This patch fixes these issues, including fixes for
bug#33726826 and bug#33730799.

In ndbinfo, revert the removal of ndb$blocks and ndb$index_stats and the
change of blocks and index_stats from views to tables.

Improve the ndbinfo schema upgrade & initialization logic to better handle
such a change in the future. This logic now runs in two passes: first it
drops the known tables and views from current and previous versions, then
it creates the tables and views for the current version.

Add a new class method NdbDictionary::printColumnTypeDescription(). This
is needed for the ndbinfo.columns table in patch #2 but was missing from
patch #1. Add boilerplate index lookup initialization code that was
also missing.

Fix ndbinfo prefix determination on Windows.

Change-Id: I422856bcad4baf5ae9b14c1e3a1f2871bd6c5f59
inikep pushed a commit that referenced this pull request Jun 7, 2022
A subset of binlog encryption tests was crashing with:

* thread percona#39, stop reason = signal SIGSTOP
    frame #0: 0x00007fff56063b66 libsystem_kernel.dylib`__pthread_kill + 10
    frame #1: 0x00007fff5622e080 libsystem_pthread.dylib`pthread_kill + 333
    frame #2: 0x000000010657442b mysqld-debug`my_write_core(sig=11) at stacktrace.cc:278
    frame #3: 0x0000000104d84334 mysqld-debug`::handle_fatal_signal(sig=11) at signal_handler.cc:254
    frame #4: 0x00007fff56221f5a libsystem_platform.dylib`_sigtramp + 26
    frame #5: 0x00007fff5622934d libsystem_pthread.dylib`pthread_mutex_lock + 1
    frame #6: 0x0000000106578d05 mysqld-debug`native_mutex_lock(mutex=0x0000000000000000) at thr_mutex.h:93
    frame #7: 0x0000000106578a57 mysqld-debug`safe_mutex_lock(mp=0x0000000000000000, try_lock=false, file="/Users/laurynas/percona/mysql-server/mysys/mf_iocache2.cc", line=113) at thr_mutex.cc:70
    frame #8: 0x000000010653cd3a mysqld-debug`my_mutex_lock(mp=0x00007ffb6b215038, file="/Users/laurynas/percona/mysql-server/mysys/mf_iocache2.cc", line=113) at thr_mutex.h:180
    frame #9: 0x000000010653b2cc mysqld-debug`inline_mysql_mutex_lock(that=0x00007ffb6b215038, src_file="/Users/laurynas/percona/mysql-server/mysys/mf_iocache2.cc", src_line=113) at mysql_mutex.h:267
  * frame #10: 0x000000010653b0d8 mysqld-debug`my_b_append_tell(info=0x00007ffb6b214fd8) at mf_iocache2.cc:113
    frame #11: 0x0000000105ed6a96 mysqld-debug`MYSQL_BIN_LOG::write_buffer(this=0x00007ffb6b214cb8, buf="", len=47, mi=0x00007ffb6b1f6a00) at binlog.cc:7128
    frame #12: 0x0000000105f4d54b mysqld-debug`queue_event(mi=0x00007ffb6b1f6a00, buf="", event_len=47, do_flush_mi=true) at rpl_slave.cc:7756
    frame percona#13: 0x0000000105f3a243 mysqld-debug`::handle_slave_io(arg=0x00007ffb6b1f6a00) at rpl_slave.cc:5382
    frame percona#14: 0x00000001065b87a5 mysqld-debug`pfs_spawn_thread(arg=0x00007ffb6a543af0) at pfs.cc:2836
    frame percona#15: 0x00007fff5622b661 libsystem_pthread.dylib`_pthread_body + 340
    frame percona#16: 0x00007fff5622b50d libsystem_pthread.dylib`_pthread_start + 377
    frame percona#17: 0x00007fff5622abf9 libsystem_pthread.dylib`thread_start + 13

This was caused by my_b_append_tell trying to lock a nullptr
IO_CACHE::append_buffer_lock. The lock was nullptr, because it's only
initialized for SEQ_READ_APPEND IO_CACHEs, whereas we have
WRITE_CACHE. This mismatch was introduced by WL#8599 [1] changing the
IO_CACHE type from the former to the latter.

Fix by using the correct API for the new IO_CACHE type: my_b_tell
instead of my_b_append_tell.

[1]:

commit dbd2ca2
Author: Joao Gramacho <joao.gramacho@oracle.com>
Date:   Tue Nov 1 06:45:39 2016 +0000

    WL#8599: Reduce contention in IO and SQL threads
    (...)
inikep pushed a commit that referenced this pull request Jun 7, 2022
create_table_info_t::create_table_def leaked memory in the case
enable_encryption(table) call failed:

worker[5] Sanitizer report from /tmp/results/PS/mysql-test/var/5/log/mysqld.2.err after tests:
 binlog_encryption.binlog_encryption_without_keyring group_replication.gr_change_master_hidden group_replication.gr_server_uuid_matches_group_name group_replication.gr_perfschema_connect_status group_replication.gr_single_primary_and_leader_election_on_error group_replication.gr_without_perfschema rpl.rpl_key_rotation
--------------------------------------------------------------------------
==14131==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 1136 byte(s) in 1 object(s) allocated from:
    #0 0x7fe9233f1602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
    #1 0xc692483 in ut_allocator<unsigned char>::allocate(unsigned long, unsigned char const*, unsigned int, bool, bool) storage/innobase/include/ut0new.h:608
    #2 0xc692483 in mem_heap_create_block_func(mem_block_info_t*, unsigned long, unsigned long) storage/innobase/mem/memory.cc:281
    #3 0xb99ff96 in mem_heap_create_func storage/innobase/include/mem0mem.ic:464
    #4 0xbae8604 in create_table_info_t::create_table_def(dd::Table const*) storage/innobase/handler/ha_innodb.cc:10349
    #5 0xbaee018 in create_table_info_t::create_table(dd::Table const*) storage/innobase/handler/ha_innodb.cc:12420
    #6 0xbaf1aba in int innobase_basic_ddl::create_impl<dd::Table>(THD*, char const*, TABLE*, HA_CREATE_INFO*, dd::Table*, bool, bool, bool, unsigned long, unsigned long) storage/innobase/handler/ha_innodb.cc:12805
    #7 0xbaf7e6a in ha_innobase::create(char const*, TABLE*, HA_CREATE_INFO*, dd::Table*) storage/innobase/handler/ha_innodb.cc:13756
    #8 0x2857f7a in ha_create_table(THD*, char const*, char const*, char const*, HA_CREATE_INFO*, List<Create_field> const*, bool, bool, dd::Table*) sql/handler.cc:5156
    #9 0x19d0d9f in rea_create_base_table sql/sql_table.cc:991
    #10 0x19d0d9f in create_table_impl sql/sql_table.cc:7118
    #11 0x19d37cf in mysql_create_table_no_lock(THD*, char const*, char const*, HA_CREATE_INFO*, Alter_info*, unsigned int, bool, bool*, handlerton**) sql/sql_table.cc:7200
    #12 0x19dffb2 in mysql_create_table(THD*, TABLE_LIST*, HA_CREATE_INFO*, Alter_info*) sql/sql_table.cc:7950
    percona#13 0x3b58b9b in Sql_cmd_create_table::execute(THD*) sql/sql_cmd_ddl_table.cc:319
    percona#14 0x15917c1 in mysql_execute_command(THD*, bool) sql/sql_parse.cc:4417
    percona#15 0x15b086e in mysql_parse(THD*, Parser_state*, bool) sql/sql_parse.cc:5139
    percona#16 0x8efc7fd in Query_log_event::do_apply_event(Relay_log_info const*, char const*, unsigned long) sql/log_event.cc:5295
    percona#17 0x8f7ea48 in Log_event::apply_event(Relay_log_info*) sql/log_event.cc:3882
    percona#18 0x91cb682 in apply_event_and_update_pos sql/rpl_slave.cc:4352
    percona#19 0x9215e69 in exec_relay_log_event sql/rpl_slave.cc:4812
    percona#20 0x9254685 in handle_slave_sql sql/rpl_slave.cc:6912
    percona#21 0xb1913a3 in pfs_spawn_thread storage/perfschema/pfs.cc:2836
    percona#22 0x7fe9231436b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)

Fix by adding the missing mem_heap_free(heap) call.
inikep pushed a commit that referenced this pull request Jun 7, 2022
Avoid undefined behavior in audit_log_update_thd_local by avoiding
passing NULL as source pointer to memcpy, even with zero length.

The UBSan report fixed is

/usr/include/x86_64-linux-gnu/bits/string3.h:53:71: runtime error: null pointer passed as argument 2, which is declared to never be null
    #0 0x7fe5aad56fb1 in memcpy /usr/include/x86_64-linux-gnu/bits/string3.h:53
    #1 0x7fe5aad56fb1 in audit_log_update_thd_local plugin/audit_log/audit_log.cc:987
    #2 0x7fe5aad56fb1 in audit_log_notify plugin/audit_log/audit_log.cc:1105
    #3 0x1ecac37 in plugins_dispatch sql/sql_audit.cc:1284
    #4 0x1ecac37 in event_class_dispatch sql/sql_audit.cc:1322
    #5 0x1ecb311 in event_class_dispatch_error sql/sql_audit.cc:1340
    #6 0x1ed21b1 in mysql_audit_notify(THD*, mysql_event_connection_subclass_t, char const*, int) sql/sql_audit.cc:438
    #7 0x1350071 in check_connection sql/sql_connect.cc:868
    #8 0x1350071 in login_connection sql/sql_connect.cc:929
    #9 0x1357881 in thd_prepare_connection(THD*, bool) sql/sql_connect.cc:1084
    #10 0x1e66347 in handle_connection sql/conn_handler/connection_handler_per_thread.cc:313
    #11 0xb1913a3 in pfs_spawn_thread storage/perfschema/pfs.cc:2836
    #12 0x7fe5d352f6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
    percona#13 0x7fe5d0bd741c in clone (/lib/x86_64-linux-gnu/libc.so.6+0x10741c)
inikep pushed a commit that referenced this pull request Jun 7, 2022
A subset of binlog encryption tests was crashing with:

* thread percona#39, stop reason = signal SIGSTOP
    frame #0: 0x00007fff56063b66 libsystem_kernel.dylib`__pthread_kill + 10
    frame #1: 0x00007fff5622e080 libsystem_pthread.dylib`pthread_kill + 333
    frame #2: 0x000000010657442b mysqld-debug`my_write_core(sig=11) at stacktrace.cc:278
    frame #3: 0x0000000104d84334 mysqld-debug`::handle_fatal_signal(sig=11) at signal_handler.cc:254
    frame #4: 0x00007fff56221f5a libsystem_platform.dylib`_sigtramp + 26
    frame #5: 0x00007fff5622934d libsystem_pthread.dylib`pthread_mutex_lock + 1
    frame #6: 0x0000000106578d05 mysqld-debug`native_mutex_lock(mutex=0x0000000000000000) at thr_mutex.h:93
    frame #7: 0x0000000106578a57 mysqld-debug`safe_mutex_lock(mp=0x0000000000000000, try_lock=false, file="/Users/laurynas/percona/mysql-server/mysys/mf_iocache2.cc", line=113) at thr_mutex.cc:70
    frame #8: 0x000000010653cd3a mysqld-debug`my_mutex_lock(mp=0x00007ffb6b215038, file="/Users/laurynas/percona/mysql-server/mysys/mf_iocache2.cc", line=113) at thr_mutex.h:180
    frame #9: 0x000000010653b2cc mysqld-debug`inline_mysql_mutex_lock(that=0x00007ffb6b215038, src_file="/Users/laurynas/percona/mysql-server/mysys/mf_iocache2.cc", src_line=113) at mysql_mutex.h:267
  * frame #10: 0x000000010653b0d8 mysqld-debug`my_b_append_tell(info=0x00007ffb6b214fd8) at mf_iocache2.cc:113
    frame #11: 0x0000000105ed6a96 mysqld-debug`MYSQL_BIN_LOG::write_buffer(this=0x00007ffb6b214cb8, buf="", len=47, mi=0x00007ffb6b1f6a00) at binlog.cc:7128
    frame #12: 0x0000000105f4d54b mysqld-debug`queue_event(mi=0x00007ffb6b1f6a00, buf="", event_len=47, do_flush_mi=true) at rpl_slave.cc:7756
    frame percona#13: 0x0000000105f3a243 mysqld-debug`::handle_slave_io(arg=0x00007ffb6b1f6a00) at rpl_slave.cc:5382
    frame percona#14: 0x00000001065b87a5 mysqld-debug`pfs_spawn_thread(arg=0x00007ffb6a543af0) at pfs.cc:2836
    frame percona#15: 0x00007fff5622b661 libsystem_pthread.dylib`_pthread_body + 340
    frame percona#16: 0x00007fff5622b50d libsystem_pthread.dylib`_pthread_start + 377
    frame percona#17: 0x00007fff5622abf9 libsystem_pthread.dylib`thread_start + 13

This was caused by my_b_append_tell trying to lock a nullptr
IO_CACHE::append_buffer_lock. The lock was nullptr, because it's only
initialized for SEQ_READ_APPEND IO_CACHEs, whereas we have
WRITE_CACHE. This mismatch was introduced by WL#8599 [1] changing the
IO_CACHE type from the former to the latter.

Fix by using the correct API for the new IO_CACHE type: my_b_tell
instead of my_b_append_tell.

[1]:

commit dbd2ca2
Author: Joao Gramacho <joao.gramacho@oracle.com>
Date:   Tue Nov 1 06:45:39 2016 +0000

    WL#8599: Reduce contention in IO and SQL threads
    (...)
inikep pushed a commit that referenced this pull request Jun 7, 2022
create_table_info_t::create_table_def leaked memory in the case
enable_encryption(table) call failed:

worker[5] Sanitizer report from /tmp/results/PS/mysql-test/var/5/log/mysqld.2.err after tests:
 binlog_encryption.binlog_encryption_without_keyring group_replication.gr_change_master_hidden group_replication.gr_server_uuid_matches_group_name group_replication.gr_perfschema_connect_status group_replication.gr_single_primary_and_leader_election_on_error group_replication.gr_without_perfschema rpl.rpl_key_rotation
--------------------------------------------------------------------------
==14131==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 1136 byte(s) in 1 object(s) allocated from:
    #0 0x7fe9233f1602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
    #1 0xc692483 in ut_allocator<unsigned char>::allocate(unsigned long, unsigned char const*, unsigned int, bool, bool) storage/innobase/include/ut0new.h:608
    #2 0xc692483 in mem_heap_create_block_func(mem_block_info_t*, unsigned long, unsigned long) storage/innobase/mem/memory.cc:281
    #3 0xb99ff96 in mem_heap_create_func storage/innobase/include/mem0mem.ic:464
    #4 0xbae8604 in create_table_info_t::create_table_def(dd::Table const*) storage/innobase/handler/ha_innodb.cc:10349
    #5 0xbaee018 in create_table_info_t::create_table(dd::Table const*) storage/innobase/handler/ha_innodb.cc:12420
    #6 0xbaf1aba in int innobase_basic_ddl::create_impl<dd::Table>(THD*, char const*, TABLE*, HA_CREATE_INFO*, dd::Table*, bool, bool, bool, unsigned long, unsigned long) storage/innobase/handler/ha_innodb.cc:12805
    #7 0xbaf7e6a in ha_innobase::create(char const*, TABLE*, HA_CREATE_INFO*, dd::Table*) storage/innobase/handler/ha_innodb.cc:13756
    #8 0x2857f7a in ha_create_table(THD*, char const*, char const*, char const*, HA_CREATE_INFO*, List<Create_field> const*, bool, bool, dd::Table*) sql/handler.cc:5156
    #9 0x19d0d9f in rea_create_base_table sql/sql_table.cc:991
    #10 0x19d0d9f in create_table_impl sql/sql_table.cc:7118
    #11 0x19d37cf in mysql_create_table_no_lock(THD*, char const*, char const*, HA_CREATE_INFO*, Alter_info*, unsigned int, bool, bool*, handlerton**) sql/sql_table.cc:7200
    #12 0x19dffb2 in mysql_create_table(THD*, TABLE_LIST*, HA_CREATE_INFO*, Alter_info*) sql/sql_table.cc:7950
    percona#13 0x3b58b9b in Sql_cmd_create_table::execute(THD*) sql/sql_cmd_ddl_table.cc:319
    percona#14 0x15917c1 in mysql_execute_command(THD*, bool) sql/sql_parse.cc:4417
    percona#15 0x15b086e in mysql_parse(THD*, Parser_state*, bool) sql/sql_parse.cc:5139
    percona#16 0x8efc7fd in Query_log_event::do_apply_event(Relay_log_info const*, char const*, unsigned long) sql/log_event.cc:5295
    percona#17 0x8f7ea48 in Log_event::apply_event(Relay_log_info*) sql/log_event.cc:3882
    percona#18 0x91cb682 in apply_event_and_update_pos sql/rpl_slave.cc:4352
    percona#19 0x9215e69 in exec_relay_log_event sql/rpl_slave.cc:4812
    percona#20 0x9254685 in handle_slave_sql sql/rpl_slave.cc:6912
    percona#21 0xb1913a3 in pfs_spawn_thread storage/perfschema/pfs.cc:2836
    percona#22 0x7fe9231436b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)

Fix by adding the missing mem_heap_free(heap) call.
inikep pushed a commit that referenced this pull request Jun 7, 2022
Avoid undefined behavior in audit_log_update_thd_local by avoiding
passing NULL as source pointer to memcpy, even with zero length.

The UBSan report fixed is

/usr/include/x86_64-linux-gnu/bits/string3.h:53:71: runtime error: null pointer passed as argument 2, which is declared to never be null
    #0 0x7fe5aad56fb1 in memcpy /usr/include/x86_64-linux-gnu/bits/string3.h:53
    #1 0x7fe5aad56fb1 in audit_log_update_thd_local plugin/audit_log/audit_log.cc:987
    #2 0x7fe5aad56fb1 in audit_log_notify plugin/audit_log/audit_log.cc:1105
    #3 0x1ecac37 in plugins_dispatch sql/sql_audit.cc:1284
    #4 0x1ecac37 in event_class_dispatch sql/sql_audit.cc:1322
    #5 0x1ecb311 in event_class_dispatch_error sql/sql_audit.cc:1340
    #6 0x1ed21b1 in mysql_audit_notify(THD*, mysql_event_connection_subclass_t, char const*, int) sql/sql_audit.cc:438
    #7 0x1350071 in check_connection sql/sql_connect.cc:868
    #8 0x1350071 in login_connection sql/sql_connect.cc:929
    #9 0x1357881 in thd_prepare_connection(THD*, bool) sql/sql_connect.cc:1084
    #10 0x1e66347 in handle_connection sql/conn_handler/connection_handler_per_thread.cc:313
    #11 0xb1913a3 in pfs_spawn_thread storage/perfschema/pfs.cc:2836
    #12 0x7fe5d352f6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
    percona#13 0x7fe5d0bd741c in clone (/lib/x86_64-linux-gnu/libc.so.6+0x10741c)
inikep pushed a commit that referenced this pull request Jun 10, 2022
A subset of binlog encryption tests was crashing with:

* thread percona#39, stop reason = signal SIGSTOP
    frame #0: 0x00007fff56063b66 libsystem_kernel.dylib`__pthread_kill + 10
    frame #1: 0x00007fff5622e080 libsystem_pthread.dylib`pthread_kill + 333
    frame #2: 0x000000010657442b mysqld-debug`my_write_core(sig=11) at stacktrace.cc:278
    frame #3: 0x0000000104d84334 mysqld-debug`::handle_fatal_signal(sig=11) at signal_handler.cc:254
    frame #4: 0x00007fff56221f5a libsystem_platform.dylib`_sigtramp + 26
    frame #5: 0x00007fff5622934d libsystem_pthread.dylib`pthread_mutex_lock + 1
    frame #6: 0x0000000106578d05 mysqld-debug`native_mutex_lock(mutex=0x0000000000000000) at thr_mutex.h:93
    frame #7: 0x0000000106578a57 mysqld-debug`safe_mutex_lock(mp=0x0000000000000000, try_lock=false, file="/Users/laurynas/percona/mysql-server/mysys/mf_iocache2.cc", line=113) at thr_mutex.cc:70
    frame #8: 0x000000010653cd3a mysqld-debug`my_mutex_lock(mp=0x00007ffb6b215038, file="/Users/laurynas/percona/mysql-server/mysys/mf_iocache2.cc", line=113) at thr_mutex.h:180
    frame #9: 0x000000010653b2cc mysqld-debug`inline_mysql_mutex_lock(that=0x00007ffb6b215038, src_file="/Users/laurynas/percona/mysql-server/mysys/mf_iocache2.cc", src_line=113) at mysql_mutex.h:267
  * frame #10: 0x000000010653b0d8 mysqld-debug`my_b_append_tell(info=0x00007ffb6b214fd8) at mf_iocache2.cc:113
    frame #11: 0x0000000105ed6a96 mysqld-debug`MYSQL_BIN_LOG::write_buffer(this=0x00007ffb6b214cb8, buf="", len=47, mi=0x00007ffb6b1f6a00) at binlog.cc:7128
    frame #12: 0x0000000105f4d54b mysqld-debug`queue_event(mi=0x00007ffb6b1f6a00, buf="", event_len=47, do_flush_mi=true) at rpl_slave.cc:7756
    frame percona#13: 0x0000000105f3a243 mysqld-debug`::handle_slave_io(arg=0x00007ffb6b1f6a00) at rpl_slave.cc:5382
    frame percona#14: 0x00000001065b87a5 mysqld-debug`pfs_spawn_thread(arg=0x00007ffb6a543af0) at pfs.cc:2836
    frame percona#15: 0x00007fff5622b661 libsystem_pthread.dylib`_pthread_body + 340
    frame percona#16: 0x00007fff5622b50d libsystem_pthread.dylib`_pthread_start + 377
    frame percona#17: 0x00007fff5622abf9 libsystem_pthread.dylib`thread_start + 13

This was caused by my_b_append_tell trying to lock a nullptr
IO_CACHE::append_buffer_lock. The lock was nullptr, because it's only
initialized for SEQ_READ_APPEND IO_CACHEs, whereas we have
WRITE_CACHE. This mismatch was introduced by WL#8599 [1] changing the
IO_CACHE type from the former to the latter.

Fix by using the correct API for the new IO_CACHE type: my_b_tell
instead of my_b_append_tell.

[1]:

commit dbd2ca2
Author: Joao Gramacho <joao.gramacho@oracle.com>
Date:   Tue Nov 1 06:45:39 2016 +0000

    WL#8599: Reduce contention in IO and SQL threads
    (...)
inikep pushed a commit that referenced this pull request Jun 10, 2022
create_table_info_t::create_table_def leaked memory in the case
enable_encryption(table) call failed:

worker[5] Sanitizer report from /tmp/results/PS/mysql-test/var/5/log/mysqld.2.err after tests:
 binlog_encryption.binlog_encryption_without_keyring group_replication.gr_change_master_hidden group_replication.gr_server_uuid_matches_group_name group_replication.gr_perfschema_connect_status group_replication.gr_single_primary_and_leader_election_on_error group_replication.gr_without_perfschema rpl.rpl_key_rotation
--------------------------------------------------------------------------
==14131==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 1136 byte(s) in 1 object(s) allocated from:
    #0 0x7fe9233f1602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
    #1 0xc692483 in ut_allocator<unsigned char>::allocate(unsigned long, unsigned char const*, unsigned int, bool, bool) storage/innobase/include/ut0new.h:608
    #2 0xc692483 in mem_heap_create_block_func(mem_block_info_t*, unsigned long, unsigned long) storage/innobase/mem/memory.cc:281
    #3 0xb99ff96 in mem_heap_create_func storage/innobase/include/mem0mem.ic:464
    #4 0xbae8604 in create_table_info_t::create_table_def(dd::Table const*) storage/innobase/handler/ha_innodb.cc:10349
    #5 0xbaee018 in create_table_info_t::create_table(dd::Table const*) storage/innobase/handler/ha_innodb.cc:12420
    #6 0xbaf1aba in int innobase_basic_ddl::create_impl<dd::Table>(THD*, char const*, TABLE*, HA_CREATE_INFO*, dd::Table*, bool, bool, bool, unsigned long, unsigned long) storage/innobase/handler/ha_innodb.cc:12805
    #7 0xbaf7e6a in ha_innobase::create(char const*, TABLE*, HA_CREATE_INFO*, dd::Table*) storage/innobase/handler/ha_innodb.cc:13756
    #8 0x2857f7a in ha_create_table(THD*, char const*, char const*, char const*, HA_CREATE_INFO*, List<Create_field> const*, bool, bool, dd::Table*) sql/handler.cc:5156
    #9 0x19d0d9f in rea_create_base_table sql/sql_table.cc:991
    #10 0x19d0d9f in create_table_impl sql/sql_table.cc:7118
    #11 0x19d37cf in mysql_create_table_no_lock(THD*, char const*, char const*, HA_CREATE_INFO*, Alter_info*, unsigned int, bool, bool*, handlerton**) sql/sql_table.cc:7200
    #12 0x19dffb2 in mysql_create_table(THD*, TABLE_LIST*, HA_CREATE_INFO*, Alter_info*) sql/sql_table.cc:7950
    percona#13 0x3b58b9b in Sql_cmd_create_table::execute(THD*) sql/sql_cmd_ddl_table.cc:319
    percona#14 0x15917c1 in mysql_execute_command(THD*, bool) sql/sql_parse.cc:4417
    percona#15 0x15b086e in mysql_parse(THD*, Parser_state*, bool) sql/sql_parse.cc:5139
    percona#16 0x8efc7fd in Query_log_event::do_apply_event(Relay_log_info const*, char const*, unsigned long) sql/log_event.cc:5295
    percona#17 0x8f7ea48 in Log_event::apply_event(Relay_log_info*) sql/log_event.cc:3882
    percona#18 0x91cb682 in apply_event_and_update_pos sql/rpl_slave.cc:4352
    percona#19 0x9215e69 in exec_relay_log_event sql/rpl_slave.cc:4812
    percona#20 0x9254685 in handle_slave_sql sql/rpl_slave.cc:6912
    percona#21 0xb1913a3 in pfs_spawn_thread storage/perfschema/pfs.cc:2836
    percona#22 0x7fe9231436b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)

Fix by adding the missing mem_heap_free(heap) call.
inikep pushed a commit that referenced this pull request Jun 10, 2022
Avoid undefined behavior in audit_log_update_thd_local by avoiding
passing NULL as source pointer to memcpy, even with zero length.

The UBSan report fixed is

/usr/include/x86_64-linux-gnu/bits/string3.h:53:71: runtime error: null pointer passed as argument 2, which is declared to never be null
    #0 0x7fe5aad56fb1 in memcpy /usr/include/x86_64-linux-gnu/bits/string3.h:53
    #1 0x7fe5aad56fb1 in audit_log_update_thd_local plugin/audit_log/audit_log.cc:987
    #2 0x7fe5aad56fb1 in audit_log_notify plugin/audit_log/audit_log.cc:1105
    #3 0x1ecac37 in plugins_dispatch sql/sql_audit.cc:1284
    #4 0x1ecac37 in event_class_dispatch sql/sql_audit.cc:1322
    #5 0x1ecb311 in event_class_dispatch_error sql/sql_audit.cc:1340
    #6 0x1ed21b1 in mysql_audit_notify(THD*, mysql_event_connection_subclass_t, char const*, int) sql/sql_audit.cc:438
    #7 0x1350071 in check_connection sql/sql_connect.cc:868
    #8 0x1350071 in login_connection sql/sql_connect.cc:929
    #9 0x1357881 in thd_prepare_connection(THD*, bool) sql/sql_connect.cc:1084
    #10 0x1e66347 in handle_connection sql/conn_handler/connection_handler_per_thread.cc:313
    #11 0xb1913a3 in pfs_spawn_thread storage/perfschema/pfs.cc:2836
    #12 0x7fe5d352f6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
    percona#13 0x7fe5d0bd741c in clone (/lib/x86_64-linux-gnu/libc.so.6+0x10741c)
inikep pushed a commit that referenced this pull request Jun 13, 2022
A subset of binlog encryption tests was crashing with:

* thread percona#39, stop reason = signal SIGSTOP
    frame #0: 0x00007fff56063b66 libsystem_kernel.dylib`__pthread_kill + 10
    frame #1: 0x00007fff5622e080 libsystem_pthread.dylib`pthread_kill + 333
    frame #2: 0x000000010657442b mysqld-debug`my_write_core(sig=11) at stacktrace.cc:278
    frame #3: 0x0000000104d84334 mysqld-debug`::handle_fatal_signal(sig=11) at signal_handler.cc:254
    frame #4: 0x00007fff56221f5a libsystem_platform.dylib`_sigtramp + 26
    frame #5: 0x00007fff5622934d libsystem_pthread.dylib`pthread_mutex_lock + 1
    frame #6: 0x0000000106578d05 mysqld-debug`native_mutex_lock(mutex=0x0000000000000000) at thr_mutex.h:93
    frame #7: 0x0000000106578a57 mysqld-debug`safe_mutex_lock(mp=0x0000000000000000, try_lock=false, file="/Users/laurynas/percona/mysql-server/mysys/mf_iocache2.cc", line=113) at thr_mutex.cc:70
    frame #8: 0x000000010653cd3a mysqld-debug`my_mutex_lock(mp=0x00007ffb6b215038, file="/Users/laurynas/percona/mysql-server/mysys/mf_iocache2.cc", line=113) at thr_mutex.h:180
    frame #9: 0x000000010653b2cc mysqld-debug`inline_mysql_mutex_lock(that=0x00007ffb6b215038, src_file="/Users/laurynas/percona/mysql-server/mysys/mf_iocache2.cc", src_line=113) at mysql_mutex.h:267
  * frame #10: 0x000000010653b0d8 mysqld-debug`my_b_append_tell(info=0x00007ffb6b214fd8) at mf_iocache2.cc:113
    frame #11: 0x0000000105ed6a96 mysqld-debug`MYSQL_BIN_LOG::write_buffer(this=0x00007ffb6b214cb8, buf="", len=47, mi=0x00007ffb6b1f6a00) at binlog.cc:7128
    frame #12: 0x0000000105f4d54b mysqld-debug`queue_event(mi=0x00007ffb6b1f6a00, buf="", event_len=47, do_flush_mi=true) at rpl_slave.cc:7756
    frame percona#13: 0x0000000105f3a243 mysqld-debug`::handle_slave_io(arg=0x00007ffb6b1f6a00) at rpl_slave.cc:5382
    frame percona#14: 0x00000001065b87a5 mysqld-debug`pfs_spawn_thread(arg=0x00007ffb6a543af0) at pfs.cc:2836
    frame percona#15: 0x00007fff5622b661 libsystem_pthread.dylib`_pthread_body + 340
    frame percona#16: 0x00007fff5622b50d libsystem_pthread.dylib`_pthread_start + 377
    frame percona#17: 0x00007fff5622abf9 libsystem_pthread.dylib`thread_start + 13

This was caused by my_b_append_tell trying to lock a nullptr
IO_CACHE::append_buffer_lock. The lock was nullptr, because it's only
initialized for SEQ_READ_APPEND IO_CACHEs, whereas we have
WRITE_CACHE. This mismatch was introduced by WL#8599 [1] changing the
IO_CACHE type from the former to the latter.

Fix by using the correct API for the new IO_CACHE type: my_b_tell
instead of my_b_append_tell.

[1]:

commit dbd2ca2
Author: Joao Gramacho <joao.gramacho@oracle.com>
Date:   Tue Nov 1 06:45:39 2016 +0000

    WL#8599: Reduce contention in IO and SQL threads
    (...)
inikep pushed a commit that referenced this pull request Jun 13, 2022
create_table_info_t::create_table_def leaked memory in the case
enable_encryption(table) call failed:

worker[5] Sanitizer report from /tmp/results/PS/mysql-test/var/5/log/mysqld.2.err after tests:
 binlog_encryption.binlog_encryption_without_keyring group_replication.gr_change_master_hidden group_replication.gr_server_uuid_matches_group_name group_replication.gr_perfschema_connect_status group_replication.gr_single_primary_and_leader_election_on_error group_replication.gr_without_perfschema rpl.rpl_key_rotation
--------------------------------------------------------------------------
==14131==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 1136 byte(s) in 1 object(s) allocated from:
    #0 0x7fe9233f1602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
    #1 0xc692483 in ut_allocator<unsigned char>::allocate(unsigned long, unsigned char const*, unsigned int, bool, bool) storage/innobase/include/ut0new.h:608
    #2 0xc692483 in mem_heap_create_block_func(mem_block_info_t*, unsigned long, unsigned long) storage/innobase/mem/memory.cc:281
    #3 0xb99ff96 in mem_heap_create_func storage/innobase/include/mem0mem.ic:464
    #4 0xbae8604 in create_table_info_t::create_table_def(dd::Table const*) storage/innobase/handler/ha_innodb.cc:10349
    #5 0xbaee018 in create_table_info_t::create_table(dd::Table const*) storage/innobase/handler/ha_innodb.cc:12420
    #6 0xbaf1aba in int innobase_basic_ddl::create_impl<dd::Table>(THD*, char const*, TABLE*, HA_CREATE_INFO*, dd::Table*, bool, bool, bool, unsigned long, unsigned long) storage/innobase/handler/ha_innodb.cc:12805
    #7 0xbaf7e6a in ha_innobase::create(char const*, TABLE*, HA_CREATE_INFO*, dd::Table*) storage/innobase/handler/ha_innodb.cc:13756
    #8 0x2857f7a in ha_create_table(THD*, char const*, char const*, char const*, HA_CREATE_INFO*, List<Create_field> const*, bool, bool, dd::Table*) sql/handler.cc:5156
    #9 0x19d0d9f in rea_create_base_table sql/sql_table.cc:991
    #10 0x19d0d9f in create_table_impl sql/sql_table.cc:7118
    #11 0x19d37cf in mysql_create_table_no_lock(THD*, char const*, char const*, HA_CREATE_INFO*, Alter_info*, unsigned int, bool, bool*, handlerton**) sql/sql_table.cc:7200
    #12 0x19dffb2 in mysql_create_table(THD*, TABLE_LIST*, HA_CREATE_INFO*, Alter_info*) sql/sql_table.cc:7950
    percona#13 0x3b58b9b in Sql_cmd_create_table::execute(THD*) sql/sql_cmd_ddl_table.cc:319
    percona#14 0x15917c1 in mysql_execute_command(THD*, bool) sql/sql_parse.cc:4417
    percona#15 0x15b086e in mysql_parse(THD*, Parser_state*, bool) sql/sql_parse.cc:5139
    percona#16 0x8efc7fd in Query_log_event::do_apply_event(Relay_log_info const*, char const*, unsigned long) sql/log_event.cc:5295
    percona#17 0x8f7ea48 in Log_event::apply_event(Relay_log_info*) sql/log_event.cc:3882
    percona#18 0x91cb682 in apply_event_and_update_pos sql/rpl_slave.cc:4352
    percona#19 0x9215e69 in exec_relay_log_event sql/rpl_slave.cc:4812
    percona#20 0x9254685 in handle_slave_sql sql/rpl_slave.cc:6912
    percona#21 0xb1913a3 in pfs_spawn_thread storage/perfschema/pfs.cc:2836
    percona#22 0x7fe9231436b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)

Fix by adding the missing mem_heap_free(heap) call.
inikep pushed a commit that referenced this pull request Jun 13, 2022
Avoid undefined behavior in audit_log_update_thd_local by avoiding
passing NULL as source pointer to memcpy, even with zero length.

The UBSan report fixed is

/usr/include/x86_64-linux-gnu/bits/string3.h:53:71: runtime error: null pointer passed as argument 2, which is declared to never be null
    #0 0x7fe5aad56fb1 in memcpy /usr/include/x86_64-linux-gnu/bits/string3.h:53
    #1 0x7fe5aad56fb1 in audit_log_update_thd_local plugin/audit_log/audit_log.cc:987
    #2 0x7fe5aad56fb1 in audit_log_notify plugin/audit_log/audit_log.cc:1105
    #3 0x1ecac37 in plugins_dispatch sql/sql_audit.cc:1284
    #4 0x1ecac37 in event_class_dispatch sql/sql_audit.cc:1322
    #5 0x1ecb311 in event_class_dispatch_error sql/sql_audit.cc:1340
    #6 0x1ed21b1 in mysql_audit_notify(THD*, mysql_event_connection_subclass_t, char const*, int) sql/sql_audit.cc:438
    #7 0x1350071 in check_connection sql/sql_connect.cc:868
    #8 0x1350071 in login_connection sql/sql_connect.cc:929
    #9 0x1357881 in thd_prepare_connection(THD*, bool) sql/sql_connect.cc:1084
    #10 0x1e66347 in handle_connection sql/conn_handler/connection_handler_per_thread.cc:313
    #11 0xb1913a3 in pfs_spawn_thread storage/perfschema/pfs.cc:2836
    #12 0x7fe5d352f6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
    percona#13 0x7fe5d0bd741c in clone (/lib/x86_64-linux-gnu/libc.so.6+0x10741c)
inikep pushed a commit that referenced this pull request Jun 13, 2022
A subset of binlog encryption tests was crashing with:

* thread percona#39, stop reason = signal SIGSTOP
    frame #0: 0x00007fff56063b66 libsystem_kernel.dylib`__pthread_kill + 10
    frame #1: 0x00007fff5622e080 libsystem_pthread.dylib`pthread_kill + 333
    frame #2: 0x000000010657442b mysqld-debug`my_write_core(sig=11) at stacktrace.cc:278
    frame #3: 0x0000000104d84334 mysqld-debug`::handle_fatal_signal(sig=11) at signal_handler.cc:254
    frame #4: 0x00007fff56221f5a libsystem_platform.dylib`_sigtramp + 26
    frame #5: 0x00007fff5622934d libsystem_pthread.dylib`pthread_mutex_lock + 1
    frame #6: 0x0000000106578d05 mysqld-debug`native_mutex_lock(mutex=0x0000000000000000) at thr_mutex.h:93
    frame #7: 0x0000000106578a57 mysqld-debug`safe_mutex_lock(mp=0x0000000000000000, try_lock=false, file="/Users/laurynas/percona/mysql-server/mysys/mf_iocache2.cc", line=113) at thr_mutex.cc:70
    frame #8: 0x000000010653cd3a mysqld-debug`my_mutex_lock(mp=0x00007ffb6b215038, file="/Users/laurynas/percona/mysql-server/mysys/mf_iocache2.cc", line=113) at thr_mutex.h:180
    frame #9: 0x000000010653b2cc mysqld-debug`inline_mysql_mutex_lock(that=0x00007ffb6b215038, src_file="/Users/laurynas/percona/mysql-server/mysys/mf_iocache2.cc", src_line=113) at mysql_mutex.h:267
  * frame #10: 0x000000010653b0d8 mysqld-debug`my_b_append_tell(info=0x00007ffb6b214fd8) at mf_iocache2.cc:113
    frame #11: 0x0000000105ed6a96 mysqld-debug`MYSQL_BIN_LOG::write_buffer(this=0x00007ffb6b214cb8, buf="", len=47, mi=0x00007ffb6b1f6a00) at binlog.cc:7128
    frame #12: 0x0000000105f4d54b mysqld-debug`queue_event(mi=0x00007ffb6b1f6a00, buf="", event_len=47, do_flush_mi=true) at rpl_slave.cc:7756
    frame percona#13: 0x0000000105f3a243 mysqld-debug`::handle_slave_io(arg=0x00007ffb6b1f6a00) at rpl_slave.cc:5382
    frame percona#14: 0x00000001065b87a5 mysqld-debug`pfs_spawn_thread(arg=0x00007ffb6a543af0) at pfs.cc:2836
    frame percona#15: 0x00007fff5622b661 libsystem_pthread.dylib`_pthread_body + 340
    frame percona#16: 0x00007fff5622b50d libsystem_pthread.dylib`_pthread_start + 377
    frame percona#17: 0x00007fff5622abf9 libsystem_pthread.dylib`thread_start + 13

This was caused by my_b_append_tell trying to lock a nullptr
IO_CACHE::append_buffer_lock. The lock was nullptr, because it's only
initialized for SEQ_READ_APPEND IO_CACHEs, whereas we have
WRITE_CACHE. This mismatch was introduced by WL#8599 [1] changing the
IO_CACHE type from the former to the latter.

Fix by using the correct API for the new IO_CACHE type: my_b_tell
instead of my_b_append_tell.

[1]:

commit dbd2ca2
Author: Joao Gramacho <joao.gramacho@oracle.com>
Date:   Tue Nov 1 06:45:39 2016 +0000

    WL#8599: Reduce contention in IO and SQL threads
    (...)
inikep pushed a commit that referenced this pull request Jun 13, 2022
create_table_info_t::create_table_def leaked memory in the case
enable_encryption(table) call failed:

worker[5] Sanitizer report from /tmp/results/PS/mysql-test/var/5/log/mysqld.2.err after tests:
 binlog_encryption.binlog_encryption_without_keyring group_replication.gr_change_master_hidden group_replication.gr_server_uuid_matches_group_name group_replication.gr_perfschema_connect_status group_replication.gr_single_primary_and_leader_election_on_error group_replication.gr_without_perfschema rpl.rpl_key_rotation
--------------------------------------------------------------------------
==14131==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 1136 byte(s) in 1 object(s) allocated from:
    #0 0x7fe9233f1602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
    #1 0xc692483 in ut_allocator<unsigned char>::allocate(unsigned long, unsigned char const*, unsigned int, bool, bool) storage/innobase/include/ut0new.h:608
    #2 0xc692483 in mem_heap_create_block_func(mem_block_info_t*, unsigned long, unsigned long) storage/innobase/mem/memory.cc:281
    #3 0xb99ff96 in mem_heap_create_func storage/innobase/include/mem0mem.ic:464
    #4 0xbae8604 in create_table_info_t::create_table_def(dd::Table const*) storage/innobase/handler/ha_innodb.cc:10349
    #5 0xbaee018 in create_table_info_t::create_table(dd::Table const*) storage/innobase/handler/ha_innodb.cc:12420
    #6 0xbaf1aba in int innobase_basic_ddl::create_impl<dd::Table>(THD*, char const*, TABLE*, HA_CREATE_INFO*, dd::Table*, bool, bool, bool, unsigned long, unsigned long) storage/innobase/handler/ha_innodb.cc:12805
    #7 0xbaf7e6a in ha_innobase::create(char const*, TABLE*, HA_CREATE_INFO*, dd::Table*) storage/innobase/handler/ha_innodb.cc:13756
    #8 0x2857f7a in ha_create_table(THD*, char const*, char const*, char const*, HA_CREATE_INFO*, List<Create_field> const*, bool, bool, dd::Table*) sql/handler.cc:5156
    #9 0x19d0d9f in rea_create_base_table sql/sql_table.cc:991
    #10 0x19d0d9f in create_table_impl sql/sql_table.cc:7118
    #11 0x19d37cf in mysql_create_table_no_lock(THD*, char const*, char const*, HA_CREATE_INFO*, Alter_info*, unsigned int, bool, bool*, handlerton**) sql/sql_table.cc:7200
    #12 0x19dffb2 in mysql_create_table(THD*, TABLE_LIST*, HA_CREATE_INFO*, Alter_info*) sql/sql_table.cc:7950
    percona#13 0x3b58b9b in Sql_cmd_create_table::execute(THD*) sql/sql_cmd_ddl_table.cc:319
    percona#14 0x15917c1 in mysql_execute_command(THD*, bool) sql/sql_parse.cc:4417
    percona#15 0x15b086e in mysql_parse(THD*, Parser_state*, bool) sql/sql_parse.cc:5139
    percona#16 0x8efc7fd in Query_log_event::do_apply_event(Relay_log_info const*, char const*, unsigned long) sql/log_event.cc:5295
    percona#17 0x8f7ea48 in Log_event::apply_event(Relay_log_info*) sql/log_event.cc:3882
    percona#18 0x91cb682 in apply_event_and_update_pos sql/rpl_slave.cc:4352
    percona#19 0x9215e69 in exec_relay_log_event sql/rpl_slave.cc:4812
    percona#20 0x9254685 in handle_slave_sql sql/rpl_slave.cc:6912
    percona#21 0xb1913a3 in pfs_spawn_thread storage/perfschema/pfs.cc:2836
    percona#22 0x7fe9231436b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)

Fix by adding the missing mem_heap_free(heap) call.
inikep pushed a commit that referenced this pull request Jun 13, 2022
Avoid undefined behavior in audit_log_update_thd_local by avoiding
passing NULL as source pointer to memcpy, even with zero length.

The UBSan report fixed is

/usr/include/x86_64-linux-gnu/bits/string3.h:53:71: runtime error: null pointer passed as argument 2, which is declared to never be null
    #0 0x7fe5aad56fb1 in memcpy /usr/include/x86_64-linux-gnu/bits/string3.h:53
    #1 0x7fe5aad56fb1 in audit_log_update_thd_local plugin/audit_log/audit_log.cc:987
    #2 0x7fe5aad56fb1 in audit_log_notify plugin/audit_log/audit_log.cc:1105
    #3 0x1ecac37 in plugins_dispatch sql/sql_audit.cc:1284
    #4 0x1ecac37 in event_class_dispatch sql/sql_audit.cc:1322
    #5 0x1ecb311 in event_class_dispatch_error sql/sql_audit.cc:1340
    #6 0x1ed21b1 in mysql_audit_notify(THD*, mysql_event_connection_subclass_t, char const*, int) sql/sql_audit.cc:438
    #7 0x1350071 in check_connection sql/sql_connect.cc:868
    #8 0x1350071 in login_connection sql/sql_connect.cc:929
    #9 0x1357881 in thd_prepare_connection(THD*, bool) sql/sql_connect.cc:1084
    #10 0x1e66347 in handle_connection sql/conn_handler/connection_handler_per_thread.cc:313
    #11 0xb1913a3 in pfs_spawn_thread storage/perfschema/pfs.cc:2836
    #12 0x7fe5d352f6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
    percona#13 0x7fe5d0bd741c in clone (/lib/x86_64-linux-gnu/libc.so.6+0x10741c)
inikep pushed a commit that referenced this pull request Jun 14, 2022
1. (Mostly) fix the audit log plugin:
- set correct flags for PFS memory instrumentation;
- fix undefined behavior in make_argv;
- fix plugin declaration having swapped deinit and check-deinit
  function pointers;
- fix an include in filter.cc;
- re-record some testcases.

2. Fix audit log plugin command filtering.

The initial 8.0 port dropped the lowercasing of passed filter strings
by mistake. Fix by restoring it. At the same time notice that MySQL
collations charsets are not interesting for command names, which are
7-bit ASCII, and so replace collation_unordered_set uses with simpler
malloc_unordered_set.

3. audit_log testcases under 8.0 tend to produce control characters more
often, due to CREATE USER ... IDENTIFIED AS hashes containing
them. There is no good handling option for them in XML 1.0, thus
1) as a lesser evil, print them as numeric sequences anyway, and
replace \0 with ?;
2) patch testcases not to produce control characters if the output is
to be consumed by an XML parser.

4. Avoid undefined behavior in audit_log_update_thd_local by avoiding
passing NULL as source pointer to memcpy, even with zero length.

The UBSan report fixed is

/usr/include/x86_64-linux-gnu/bits/string3.h:53:71: runtime error: null pointer passed as argument 2, which is declared to never be null
    #0 0x7fe5aad56fb1 in memcpy /usr/include/x86_64-linux-gnu/bits/string3.h:53
    #1 0x7fe5aad56fb1 in audit_log_update_thd_local plugin/audit_log/audit_log.cc:987
    #2 0x7fe5aad56fb1 in audit_log_notify plugin/audit_log/audit_log.cc:1105
    #3 0x1ecac37 in plugins_dispatch sql/sql_audit.cc:1284
    #4 0x1ecac37 in event_class_dispatch sql/sql_audit.cc:1322
    #5 0x1ecb311 in event_class_dispatch_error sql/sql_audit.cc:1340
    #6 0x1ed21b1 in mysql_audit_notify(THD*, mysql_event_connection_subclass_t, char const*, int) sql/sql_audit.cc:438
    #7 0x1350071 in check_connection sql/sql_connect.cc:868
    #8 0x1350071 in login_connection sql/sql_connect.cc:929
    #9 0x1357881 in thd_prepare_connection(THD*, bool) sql/sql_connect.cc:1084
    #10 0x1e66347 in handle_connection sql/conn_handler/connection_handler_per_thread.cc:313
    #11 0xb1913a3 in pfs_spawn_thread storage/perfschema/pfs.cc:2836
    #12 0x7fe5d352f6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
    percona#13 0x7fe5d0bd741c in clone (/lib/x86_64-linux-gnu/libc.so.6+0x10741c)
inikep added a commit that referenced this pull request Jun 14, 2022
A subset of binlog encryption tests was crashing with:

* thread percona#39, stop reason = signal SIGSTOP
    frame #0: 0x00007fff56063b66 libsystem_kernel.dylib`__pthread_kill + 10
    frame #1: 0x00007fff5622e080 libsystem_pthread.dylib`pthread_kill + 333
    frame #2: 0x000000010657442b mysqld-debug`my_write_core(sig=11) at stacktrace.cc:278
    frame #3: 0x0000000104d84334 mysqld-debug`::handle_fatal_signal(sig=11) at signal_handler.cc:254
    frame #4: 0x00007fff56221f5a libsystem_platform.dylib`_sigtramp + 26
    frame #5: 0x00007fff5622934d libsystem_pthread.dylib`pthread_mutex_lock + 1
    frame #6: 0x0000000106578d05 mysqld-debug`native_mutex_lock(mutex=0x0000000000000000) at thr_mutex.h:93
    frame #7: 0x0000000106578a57 mysqld-debug`safe_mutex_lock(mp=0x0000000000000000, try_lock=false, file="/Users/laurynas/percona/mysql-server/mysys/mf_iocache2.cc", line=113) at thr_mutex.cc:70
    frame #8: 0x000000010653cd3a mysqld-debug`my_mutex_lock(mp=0x00007ffb6b215038, file="/Users/laurynas/percona/mysql-server/mysys/mf_iocache2.cc", line=113) at thr_mutex.h:180
    frame #9: 0x000000010653b2cc mysqld-debug`inline_mysql_mutex_lock(that=0x00007ffb6b215038, src_file="/Users/laurynas/percona/mysql-server/mysys/mf_iocache2.cc", src_line=113) at mysql_mutex.h:267
  * frame #10: 0x000000010653b0d8 mysqld-debug`my_b_append_tell(info=0x00007ffb6b214fd8) at mf_iocache2.cc:113
    frame #11: 0x0000000105ed6a96 mysqld-debug`MYSQL_BIN_LOG::write_buffer(this=0x00007ffb6b214cb8, buf="", len=47, mi=0x00007ffb6b1f6a00) at binlog.cc:7128
    frame #12: 0x0000000105f4d54b mysqld-debug`queue_event(mi=0x00007ffb6b1f6a00, buf="", event_len=47, do_flush_mi=true) at rpl_slave.cc:7756
    frame percona#13: 0x0000000105f3a243 mysqld-debug`::handle_slave_io(arg=0x00007ffb6b1f6a00) at rpl_slave.cc:5382
    frame percona#14: 0x00000001065b87a5 mysqld-debug`pfs_spawn_thread(arg=0x00007ffb6a543af0) at pfs.cc:2836
    frame percona#15: 0x00007fff5622b661 libsystem_pthread.dylib`_pthread_body + 340
    frame percona#16: 0x00007fff5622b50d libsystem_pthread.dylib`_pthread_start + 377
    frame percona#17: 0x00007fff5622abf9 libsystem_pthread.dylib`thread_start + 13

This was caused by my_b_append_tell trying to lock a nullptr
IO_CACHE::append_buffer_lock. The lock was nullptr, because it's only
initialized for SEQ_READ_APPEND IO_CACHEs, whereas we have
WRITE_CACHE. This mismatch was introduced by WL#8599 [1] changing the
IO_CACHE type from the former to the latter.

Fix by using the correct API for the new IO_CACHE type: my_b_tell
instead of my_b_append_tell.

[1]:

commit dbd2ca2
Author: Joao Gramacho <joao.gramacho@oracle.com>
Date:   Tue Nov 1 06:45:39 2016 +0000

    WL#8599: Reduce contention in IO and SQL threads
    (...)
inikep added a commit that referenced this pull request Jun 14, 2022
1. Fix merge error that broke row_log_online_op by making it write
unencrypted blocks even with encryption enabled

2. Plug a memory leak in log_online_setup_bitmap_file_range introduced by
a rewrite in 8.0 to use my_dir: use my_dirend.

3. Plug a memory leak in log_online_read_init introduced by a rewrite in
8.0 to use my_dir: use my_dirend.

4. create_table_info_t::create_table_def leaked memory in the case
enable_encryption(table) call failed:

worker[5] Sanitizer report from /tmp/results/PS/mysql-test/var/5/log/mysqld.2.err after tests:
 binlog_encryption.binlog_encryption_without_keyring group_replication.gr_change_master_hidden group_replication.gr_server_uuid_matches_group_name group_replication.gr_perfschema_connect_status group_replication.gr_single_primary_and_leader_election_on_error group_replication.gr_without_perfschema rpl.rpl_key_rotation
--------------------------------------------------------------------------
==14131==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 1136 byte(s) in 1 object(s) allocated from:
    #0 0x7fe9233f1602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
    #1 0xc692483 in ut_allocator<unsigned char>::allocate(unsigned long, unsigned char const*, unsigned int, bool, bool) storage/innobase/include/ut0new.h:608
    #2 0xc692483 in mem_heap_create_block_func(mem_block_info_t*, unsigned long, unsigned long) storage/innobase/mem/memory.cc:281
    #3 0xb99ff96 in mem_heap_create_func storage/innobase/include/mem0mem.ic:464
    #4 0xbae8604 in create_table_info_t::create_table_def(dd::Table const*) storage/innobase/handler/ha_innodb.cc:10349
    #5 0xbaee018 in create_table_info_t::create_table(dd::Table const*) storage/innobase/handler/ha_innodb.cc:12420
    #6 0xbaf1aba in int innobase_basic_ddl::create_impl<dd::Table>(THD*, char const*, TABLE*, HA_CREATE_INFO*, dd::Table*, bool, bool, bool, unsigned long, unsigned long) storage/innobase/handler/ha_innodb.cc:12805
    #7 0xbaf7e6a in ha_innobase::create(char const*, TABLE*, HA_CREATE_INFO*, dd::Table*) storage/innobase/handler/ha_innodb.cc:13756
    #8 0x2857f7a in ha_create_table(THD*, char const*, char const*, char const*, HA_CREATE_INFO*, List<Create_field> const*, bool, bool, dd::Table*) sql/handler.cc:5156
    #9 0x19d0d9f in rea_create_base_table sql/sql_table.cc:991
    #10 0x19d0d9f in create_table_impl sql/sql_table.cc:7118
    #11 0x19d37cf in mysql_create_table_no_lock(THD*, char const*, char const*, HA_CREATE_INFO*, Alter_info*, unsigned int, bool, bool*, handlerton**) sql/sql_table.cc:7200
    #12 0x19dffb2 in mysql_create_table(THD*, TABLE_LIST*, HA_CREATE_INFO*, Alter_info*) sql/sql_table.cc:7950
    percona#13 0x3b58b9b in Sql_cmd_create_table::execute(THD*) sql/sql_cmd_ddl_table.cc:319
    percona#14 0x15917c1 in mysql_execute_command(THD*, bool) sql/sql_parse.cc:4417
    percona#15 0x15b086e in mysql_parse(THD*, Parser_state*, bool) sql/sql_parse.cc:5139
    percona#16 0x8efc7fd in Query_log_event::do_apply_event(Relay_log_info const*, char const*, unsigned long) sql/log_event.cc:5295
    percona#17 0x8f7ea48 in Log_event::apply_event(Relay_log_info*) sql/log_event.cc:3882
    percona#18 0x91cb682 in apply_event_and_update_pos sql/rpl_slave.cc:4352
    percona#19 0x9215e69 in exec_relay_log_event sql/rpl_slave.cc:4812
    percona#20 0x9254685 in handle_slave_sql sql/rpl_slave.cc:6912
    percona#21 0xb1913a3 in pfs_spawn_thread storage/perfschema/pfs.cc:2836
    percona#22 0x7fe9231436b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)

Fix by adding the missing mem_heap_free(heap) call.

5. Fix merge error which resulted in a single LRU manager thread being
created as opposed to one per buffer pool instance. At the same time
trivially fix one of the multiple-buffer-pool-instance testcases.

6. Partially fix changed page tracking:
- avoid deadlock on Fil_shard mutex between server shutdown thread and
  changed page tracking by moving the
  Fil_system::wait_for_changed_page_tracker call outside this mutex
  critical section;
- extend recv_read_log_seg with a new argument bool online, which is
  false during recovery and true for changed page tracking reads. In
  function body, use it to guard
  log_background_threads_inactive_validate call (as the background
  threads are active during changed page tracking), and to acquire the
  log_writer mutex, which is not held for the changed page tracking;
- fix MIN_TRACKED_LSN to stop being OS_FILE_LOG_BLOCK_SIZE-too large;
- take last checkpoint LSN to start tracking from, instead of the
  larger of that LSN and MIN_TRACKED_LSN, as the checkpoint LSN is
  always valid at that point;
- strengthen asserts in log_online_add_to_parse_buf.
inikep pushed a commit that referenced this pull request Jun 14, 2022
1. (Mostly) fix the audit log plugin:
- set correct flags for PFS memory instrumentation;
- fix undefined behavior in make_argv;
- fix plugin declaration having swapped deinit and check-deinit
  function pointers;
- fix an include in filter.cc;
- re-record some testcases.

2. Fix audit log plugin command filtering.

The initial 8.0 port dropped the lowercasing of passed filter strings
by mistake. Fix by restoring it. At the same time notice that MySQL
collations charsets are not interesting for command names, which are
7-bit ASCII, and so replace collation_unordered_set uses with simpler
malloc_unordered_set.

3. audit_log testcases under 8.0 tend to produce control characters more
often, due to CREATE USER ... IDENTIFIED AS hashes containing
them. There is no good handling option for them in XML 1.0, thus
1) as a lesser evil, print them as numeric sequences anyway, and
replace \0 with ?;
2) patch testcases not to produce control characters if the output is
to be consumed by an XML parser.

4. audit_log_rotations and audit_log_rotate_on_size were messed up when set
at runtime.

5. Avoid undefined behavior in audit_log_update_thd_local by avoiding
passing NULL as source pointer to memcpy, even with zero length.

The UBSan report fixed is

/usr/include/x86_64-linux-gnu/bits/string3.h:53:71: runtime error: null pointer passed as argument 2, which is declared to never be null
    #0 0x7fe5aad56fb1 in memcpy /usr/include/x86_64-linux-gnu/bits/string3.h:53
    #1 0x7fe5aad56fb1 in audit_log_update_thd_local plugin/audit_log/audit_log.cc:987
    #2 0x7fe5aad56fb1 in audit_log_notify plugin/audit_log/audit_log.cc:1105
    #3 0x1ecac37 in plugins_dispatch sql/sql_audit.cc:1284
    #4 0x1ecac37 in event_class_dispatch sql/sql_audit.cc:1322
    #5 0x1ecb311 in event_class_dispatch_error sql/sql_audit.cc:1340
    #6 0x1ed21b1 in mysql_audit_notify(THD*, mysql_event_connection_subclass_t, char const*, int) sql/sql_audit.cc:438
    #7 0x1350071 in check_connection sql/sql_connect.cc:868
    #8 0x1350071 in login_connection sql/sql_connect.cc:929
    #9 0x1357881 in thd_prepare_connection(THD*, bool) sql/sql_connect.cc:1084
    #10 0x1e66347 in handle_connection sql/conn_handler/connection_handler_per_thread.cc:313
    #11 0xb1913a3 in pfs_spawn_thread storage/perfschema/pfs.cc:2836
    #12 0x7fe5d352f6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
    percona#13 0x7fe5d0bd741c in clone (/lib/x86_64-linux-gnu/libc.so.6+0x10741c)
inikep added a commit that referenced this pull request Jun 14, 2022
A subset of binlog encryption tests was crashing with:

* thread percona#39, stop reason = signal SIGSTOP
    frame #0: 0x00007fff56063b66 libsystem_kernel.dylib`__pthread_kill + 10
    frame #1: 0x00007fff5622e080 libsystem_pthread.dylib`pthread_kill + 333
    frame #2: 0x000000010657442b mysqld-debug`my_write_core(sig=11) at stacktrace.cc:278
    frame #3: 0x0000000104d84334 mysqld-debug`::handle_fatal_signal(sig=11) at signal_handler.cc:254
    frame #4: 0x00007fff56221f5a libsystem_platform.dylib`_sigtramp + 26
    frame #5: 0x00007fff5622934d libsystem_pthread.dylib`pthread_mutex_lock + 1
    frame #6: 0x0000000106578d05 mysqld-debug`native_mutex_lock(mutex=0x0000000000000000) at thr_mutex.h:93
    frame #7: 0x0000000106578a57 mysqld-debug`safe_mutex_lock(mp=0x0000000000000000, try_lock=false, file="/Users/laurynas/percona/mysql-server/mysys/mf_iocache2.cc", line=113) at thr_mutex.cc:70
    frame #8: 0x000000010653cd3a mysqld-debug`my_mutex_lock(mp=0x00007ffb6b215038, file="/Users/laurynas/percona/mysql-server/mysys/mf_iocache2.cc", line=113) at thr_mutex.h:180
    frame #9: 0x000000010653b2cc mysqld-debug`inline_mysql_mutex_lock(that=0x00007ffb6b215038, src_file="/Users/laurynas/percona/mysql-server/mysys/mf_iocache2.cc", src_line=113) at mysql_mutex.h:267
  * frame #10: 0x000000010653b0d8 mysqld-debug`my_b_append_tell(info=0x00007ffb6b214fd8) at mf_iocache2.cc:113
    frame #11: 0x0000000105ed6a96 mysqld-debug`MYSQL_BIN_LOG::write_buffer(this=0x00007ffb6b214cb8, buf="", len=47, mi=0x00007ffb6b1f6a00) at binlog.cc:7128
    frame #12: 0x0000000105f4d54b mysqld-debug`queue_event(mi=0x00007ffb6b1f6a00, buf="", event_len=47, do_flush_mi=true) at rpl_slave.cc:7756
    frame percona#13: 0x0000000105f3a243 mysqld-debug`::handle_slave_io(arg=0x00007ffb6b1f6a00) at rpl_slave.cc:5382
    frame percona#14: 0x00000001065b87a5 mysqld-debug`pfs_spawn_thread(arg=0x00007ffb6a543af0) at pfs.cc:2836
    frame percona#15: 0x00007fff5622b661 libsystem_pthread.dylib`_pthread_body + 340
    frame percona#16: 0x00007fff5622b50d libsystem_pthread.dylib`_pthread_start + 377
    frame percona#17: 0x00007fff5622abf9 libsystem_pthread.dylib`thread_start + 13

This was caused by my_b_append_tell trying to lock a nullptr
IO_CACHE::append_buffer_lock. The lock was nullptr, because it's only
initialized for SEQ_READ_APPEND IO_CACHEs, whereas we have
WRITE_CACHE. This mismatch was introduced by WL#8599 [1] changing the
IO_CACHE type from the former to the latter.

Fix by using the correct API for the new IO_CACHE type: my_b_tell
instead of my_b_append_tell.

[1]:

commit dbd2ca2
Author: Joao Gramacho <joao.gramacho@oracle.com>
Date:   Tue Nov 1 06:45:39 2016 +0000

    WL#8599: Reduce contention in IO and SQL threads
    (...)
inikep added a commit that referenced this pull request Jun 14, 2022
1. Fix merge error that broke row_log_online_op by making it write
unencrypted blocks even with encryption enabled

2. Plug a memory leak in log_online_setup_bitmap_file_range introduced by
a rewrite in 8.0 to use my_dir: use my_dirend.

3. Plug a memory leak in log_online_read_init introduced by a rewrite in
8.0 to use my_dir: use my_dirend.

4. create_table_info_t::create_table_def leaked memory in the case
enable_encryption(table) call failed:

worker[5] Sanitizer report from /tmp/results/PS/mysql-test/var/5/log/mysqld.2.err after tests:
 binlog_encryption.binlog_encryption_without_keyring group_replication.gr_change_master_hidden group_replication.gr_server_uuid_matches_group_name group_replication.gr_perfschema_connect_status group_replication.gr_single_primary_and_leader_election_on_error group_replication.gr_without_perfschema rpl.rpl_key_rotation
--------------------------------------------------------------------------
==14131==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 1136 byte(s) in 1 object(s) allocated from:
    #0 0x7fe9233f1602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
    #1 0xc692483 in ut_allocator<unsigned char>::allocate(unsigned long, unsigned char const*, unsigned int, bool, bool) storage/innobase/include/ut0new.h:608
    #2 0xc692483 in mem_heap_create_block_func(mem_block_info_t*, unsigned long, unsigned long) storage/innobase/mem/memory.cc:281
    #3 0xb99ff96 in mem_heap_create_func storage/innobase/include/mem0mem.ic:464
    #4 0xbae8604 in create_table_info_t::create_table_def(dd::Table const*) storage/innobase/handler/ha_innodb.cc:10349
    #5 0xbaee018 in create_table_info_t::create_table(dd::Table const*) storage/innobase/handler/ha_innodb.cc:12420
    #6 0xbaf1aba in int innobase_basic_ddl::create_impl<dd::Table>(THD*, char const*, TABLE*, HA_CREATE_INFO*, dd::Table*, bool, bool, bool, unsigned long, unsigned long) storage/innobase/handler/ha_innodb.cc:12805
    #7 0xbaf7e6a in ha_innobase::create(char const*, TABLE*, HA_CREATE_INFO*, dd::Table*) storage/innobase/handler/ha_innodb.cc:13756
    #8 0x2857f7a in ha_create_table(THD*, char const*, char const*, char const*, HA_CREATE_INFO*, List<Create_field> const*, bool, bool, dd::Table*) sql/handler.cc:5156
    #9 0x19d0d9f in rea_create_base_table sql/sql_table.cc:991
    #10 0x19d0d9f in create_table_impl sql/sql_table.cc:7118
    #11 0x19d37cf in mysql_create_table_no_lock(THD*, char const*, char const*, HA_CREATE_INFO*, Alter_info*, unsigned int, bool, bool*, handlerton**) sql/sql_table.cc:7200
    #12 0x19dffb2 in mysql_create_table(THD*, TABLE_LIST*, HA_CREATE_INFO*, Alter_info*) sql/sql_table.cc:7950
    percona#13 0x3b58b9b in Sql_cmd_create_table::execute(THD*) sql/sql_cmd_ddl_table.cc:319
    percona#14 0x15917c1 in mysql_execute_command(THD*, bool) sql/sql_parse.cc:4417
    percona#15 0x15b086e in mysql_parse(THD*, Parser_state*, bool) sql/sql_parse.cc:5139
    percona#16 0x8efc7fd in Query_log_event::do_apply_event(Relay_log_info const*, char const*, unsigned long) sql/log_event.cc:5295
    percona#17 0x8f7ea48 in Log_event::apply_event(Relay_log_info*) sql/log_event.cc:3882
    percona#18 0x91cb682 in apply_event_and_update_pos sql/rpl_slave.cc:4352
    percona#19 0x9215e69 in exec_relay_log_event sql/rpl_slave.cc:4812
    percona#20 0x9254685 in handle_slave_sql sql/rpl_slave.cc:6912
    percona#21 0xb1913a3 in pfs_spawn_thread storage/perfschema/pfs.cc:2836
    percona#22 0x7fe9231436b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)

Fix by adding the missing mem_heap_free(heap) call.

5. Fix merge error which resulted in a single LRU manager thread being
created as opposed to one per buffer pool instance. At the same time
trivially fix one of the multiple-buffer-pool-instance testcases.

6. Partially fix changed page tracking:
- avoid deadlock on Fil_shard mutex between server shutdown thread and
  changed page tracking by moving the
  Fil_system::wait_for_changed_page_tracker call outside this mutex
  critical section;
- extend recv_read_log_seg with a new argument bool online, which is
  false during recovery and true for changed page tracking reads. In
  function body, use it to guard
  log_background_threads_inactive_validate call (as the background
  threads are active during changed page tracking), and to acquire the
  log_writer mutex, which is not held for the changed page tracking;
- fix MIN_TRACKED_LSN to stop being OS_FILE_LOG_BLOCK_SIZE-too large;
- take last checkpoint LSN to start tracking from, instead of the
  larger of that LSN and MIN_TRACKED_LSN, as the checkpoint LSN is
  always valid at that point;
- strengthen asserts in log_online_add_to_parse_buf.
inikep pushed a commit that referenced this pull request Apr 10, 2024
Summary:
MySQL bypass infrastructure change

1. Adds a new hton override `handlerton::handle_single_table_select` to allow hijacking of SELECT statement execution from any storage engine.
2. Add lex parsing for bypass hints in optimizer hint style /*+ bypass */ and /*+ no_bypass */
3. MyRocks overrides handle_single_table_select but does nothing and simply fallback.

This is a port from from bypass feature branch - change #1 of 3. I'm planning to port the changes in the following order:

1. MySQL lexer + SELECT hijacking (this one).
2. SELECT Statement parsing and checking
3. Execution of SELECT statement in bypass

Reference Patch: facebook/mysql-5.6@2d19dc0

-----
Porting Notes:

* The entry points have changed due to MySQL 8.0 refactoring to SELECT path, and now it is in `Sql_cmd_dml::execute_inner`. I'll also see if overriding the entire execute would be better but this should be good enough for now.
* The check for whether we should go to bypass handler is mostly the same, except st_select_lex_t is now SELECT_LEX.
* The lex parsing for hints was mostly ported as-is. I was also debating whether I should just use optimizer hints instead of hacking the lexer, but decided against using optimizer hint for now so that migration from 5.6 will be easier. We can migrate to optimizer hint once 8.0 migration is complete.

Differential Revision: D22807040
inikep pushed a commit that referenced this pull request Apr 10, 2024
Summary:
This is an upstream bug: https://bugs.mysql.com/bug.php?id=92964

The issue is that if slave instance use sysvar_session_track_gtids == OWN_GTID and also enable MTS, the slave will lag behind master and its performance degrades over time.

The reason for this lag/perf degrade is caused by each slave worker keep adding its own gtid into its own Session_consistency_gtids_ctx.m_gtid_set during each transaction commit. Overtime, some of Session_consistency_gtids_ctx.m_gtid_set may have millions gno_interval and it will take long time to insert 1 gtid into Session_consistency_gtids_ctx.m_gtid_set(time complexity O(n)), see [Gtid_set::add_gno_interval](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/rpl_gtid_set.cc#L323).

There are couple related code to this issue:
- During slave worker thread start, there is a [check](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/session_tracker.cc#L1560) to not enable tracker for system threads but that check doesn't work---The THD system_thread field is initialized with default value(NON_SYSTEM_THREAD) during THD::THD.
```
  m_enabled = thd->variables.session_track_gtids != OFF &&
              /* No need to track GTIDs for system threads. */
              thd->system_thread == NON_SYSTEM_THREAD;
```
```
Call stack:
```
- in [Session_gtids_track::Update](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/session_tracker.cc#L1581), the else code doesn't unregister listener due to mysql/mysql-server@1e0844c try to fix another issue.

Solutions:
1.  [Current]add system threads check during collecting data, since THD will be fully initialized at that time.
 -Pro: simple change(easy to port) and perf is ok
2. add `thd->session_track_gtids=OFF; thd->session_tracker.get_tracker(SESSION_GTIDS_TRACKER)->update(thd);` after [init_slave_thread()](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/rpl_slave.cc#L6788)
-Pro: During init_slave_thread(), thd->system_thread will be set correctly value.
-Con: it need to add couple places, such as handle_slave_io, handle_slave_sql, handle_slave_worker
3. add `thd->session_track_gtids=OFF;thd->session_tracker.get_tracker(SESSION_GTIDS_TRACKER)->update(thd);` inside [init_slave_thread()](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/rpl_slave.cc#L6788)
   -Pro: only add once for slave/slave worker/IO threads
   -Con: [should_collect](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/rpl_context.cc#L53) check still return true, thus it is less perf than #1
4. add `thd->session_track_gtids=OFF;thd->rpl_thd_ctx.session_gtids_ctx().unregister_ctx_change_listener(thd->session_tracker.get_tracker(SESSION_GTIDS_TRACKER));` inside init_slave_thread()
   -Pro: perf is better than #3, on par as #1
   -Con: if there are other non-system threads(except IO/sql threads) do transactions, these threads will still collect its own gtids.
5. add another THD overload constructor which takes 'enum_thread_type' as a parameter whose default value is NON_SYSTEM_THREAD
 -Pro: it will initialize correct value with THD and won't enable those session_tracker
 -Con: similar to #2/#4, need to replace THD() with THD(enum_thread_type) couples places.

Differential Revision: D18072672
inikep pushed a commit that referenced this pull request Apr 10, 2024
…batch #1

Summary:
Disabling tests:
1. Transaction related - those are already disabled in 5.6 as they are not supported
2. hotbackup tests - we need Xtrabackup for MyRocks
3. partition tests - we need Percona's partition patch. Note these are partition specific tests - tests that only use partition are properly fixed up to work without partitions to ensure we have good coverage.
4. tests that are requires authentication plugin
5. Missing patches

In 8.0 the disabled.def aren't being used anymore and instead they are located in collections and needs to be specifically added.

Reviewed By: lloyd

Differential Revision: D17555081
inikep pushed a commit that referenced this pull request Apr 10, 2024
Summary:
MySQL bypass infrastructure change

1. Adds a new hton override `handlerton::handle_single_table_select` to allow hijacking of SELECT statement execution from any storage engine.
2. Add lex parsing for bypass hints in optimizer hint style /*+ bypass */ and /*+ no_bypass */
3. MyRocks overrides handle_single_table_select but does nothing and simply fallback.

This is a port from from bypass feature branch - change #1 of 3. I'm planning to port the changes in the following order:

1. MySQL lexer + SELECT hijacking (this one).
2. SELECT Statement parsing and checking
3. Execution of SELECT statement in bypass

Reference Patch: facebook/mysql-5.6@2d19dc0

-----
Porting Notes:

* The entry points have changed due to MySQL 8.0 refactoring to SELECT path, and now it is in `Sql_cmd_dml::execute_inner`. I'll also see if overriding the entire execute would be better but this should be good enough for now.
* The check for whether we should go to bypass handler is mostly the same, except st_select_lex_t is now SELECT_LEX.
* The lex parsing for hints was mostly ported as-is. I was also debating whether I should just use optimizer hints instead of hacking the lexer, but decided against using optimizer hint for now so that migration from 5.6 will be easier. We can migrate to optimizer hint once 8.0 migration is complete.

Differential Revision: D22807040

---------------------------------------------------------------------------------------------

[bypass] MySQL bypass change #2 - SELECT parsing [non-RocksDB part]

Summary:
This is change #2 that ports the SELECT statement parsing from feature-myrocks-bypass branch, including:

1. select_parser class that parses SELECT statements and validates the scenarios are supported, and error with proper message if not supported
2. You can control bypass policy with rocksdb_select_bypass_policy variable = 0 (ALWAYS OFF) / 1 (ALWAYS ON) / 2 (OPT IN) / 3 (OPT OUT).
3. A few metrics for number of SELECT bypass statements executed/rejected (due to not supported) /failed (something wrong in execution)

At this point this only dumps the SELECT statement parsed results without doing anything useful. The next change will be porting the query plan execution and point/range query execution part.

Please ignore the unused attributes for now - they'll be removed in next change once the execution part is ported.

Reference Patch:

facebook/mysql-5.6@e79b20a
facebook/mysql-5.6@8072e3d

 ---
Porting Notes:

There are a lot of small changes in this area but nothing too bad:
* Some bypass scenarios no longer fail or fail with different error message because of the new parse tree -> AST conversion process. For example, `SELECT *` now works correctly
* st_select_lex_t -> SELECT_LEX, and many members has changed name, such as where -> where_cond()
* protocol usage has changed to start_row, store, end_row in 8.0, which is much better.
* thd->query() is now a LEX_STRING
* SELECT option check (such as SELECT_DISTINCT) now uses active_options() and only check for SELECT_DISTINCT as there seems to be default value being set most likely due to side effect of having more parse tree processing being done. I checked the flags and it looks like only SELECT_DISTINCT are interesting and needs to be blocked.
* SELECT INTO OUTFILE check is changed to use results->needs_file_priviledge
* True/False const are now represented as Item_func_true/Item_func_false. For now I'm over-generalizing to functions that are const, and expect val_int/etc would do the right thing

Reviewed By: luqun

Differential Revision: D22808305

-----------------------------------------------------------------------------------------------

Introduce base select parser class

Summary:
To make it easier to add a new bypass rpc parser into existing bypass implementation (nosql_access.cc),
and make minimum change to bypass engine while adding a bypass rpc parser, I introduced a new base class,
base_select_parser. It includes all essential members and methods for parsing bypass sq/rpc queries.
Existing bypass sql parser (select_parser) and a new bypass rpc parser (not included in this diff)
inherits the base function.

Bypass RPC reference diff: D30493654

Reviewed By: yizhang82

Differential Revision: D31870628
inikep pushed a commit that referenced this pull request Apr 12, 2024
…batch #1

Summary:
Disabling tests:
1. Transaction related - those are already disabled in 5.6 as they are not supported
2. hotbackup tests - we need Xtrabackup for MyRocks
3. partition tests - we need Percona's partition patch. Note these are partition specific tests - tests that only use partition are properly fixed up to work without partitions to ensure we have good coverage.
4. tests that are requires authentication plugin
5. Missing patches

In 8.0 the disabled.def aren't being used anymore and instead they are located in collections and needs to be specifically added.

Reviewed By: lloyd

Differential Revision: D17555081
inikep pushed a commit that referenced this pull request Apr 12, 2024
Summary:
MySQL bypass infrastructure change

1. Adds a new hton override `handlerton::handle_single_table_select` to allow hijacking of SELECT statement execution from any storage engine.
2. Add lex parsing for bypass hints in optimizer hint style /*+ bypass */ and /*+ no_bypass */
3. MyRocks overrides handle_single_table_select but does nothing and simply fallback.

This is a port from from bypass feature branch - change #1 of 3. I'm planning to port the changes in the following order:

1. MySQL lexer + SELECT hijacking (this one).
2. SELECT Statement parsing and checking
3. Execution of SELECT statement in bypass

Reference Patch: facebook/mysql-5.6@2d19dc0

-----
Porting Notes:

* The entry points have changed due to MySQL 8.0 refactoring to SELECT path, and now it is in `Sql_cmd_dml::execute_inner`. I'll also see if overriding the entire execute would be better but this should be good enough for now.
* The check for whether we should go to bypass handler is mostly the same, except st_select_lex_t is now SELECT_LEX.
* The lex parsing for hints was mostly ported as-is. I was also debating whether I should just use optimizer hints instead of hacking the lexer, but decided against using optimizer hint for now so that migration from 5.6 will be easier. We can migrate to optimizer hint once 8.0 migration is complete.

Differential Revision: D22807040

---------------------------------------------------------------------------------------------

[bypass] MySQL bypass change #2 - SELECT parsing [non-RocksDB part]

Summary:
This is change #2 that ports the SELECT statement parsing from feature-myrocks-bypass branch, including:

1. select_parser class that parses SELECT statements and validates the scenarios are supported, and error with proper message if not supported
2. You can control bypass policy with rocksdb_select_bypass_policy variable = 0 (ALWAYS OFF) / 1 (ALWAYS ON) / 2 (OPT IN) / 3 (OPT OUT).
3. A few metrics for number of SELECT bypass statements executed/rejected (due to not supported) /failed (something wrong in execution)

At this point this only dumps the SELECT statement parsed results without doing anything useful. The next change will be porting the query plan execution and point/range query execution part.

Please ignore the unused attributes for now - they'll be removed in next change once the execution part is ported.

Reference Patch:

facebook/mysql-5.6@e79b20a
facebook/mysql-5.6@8072e3d

 ---
Porting Notes:

There are a lot of small changes in this area but nothing too bad:
* Some bypass scenarios no longer fail or fail with different error message because of the new parse tree -> AST conversion process. For example, `SELECT *` now works correctly
* st_select_lex_t -> SELECT_LEX, and many members has changed name, such as where -> where_cond()
* protocol usage has changed to start_row, store, end_row in 8.0, which is much better.
* thd->query() is now a LEX_STRING
* SELECT option check (such as SELECT_DISTINCT) now uses active_options() and only check for SELECT_DISTINCT as there seems to be default value being set most likely due to side effect of having more parse tree processing being done. I checked the flags and it looks like only SELECT_DISTINCT are interesting and needs to be blocked.
* SELECT INTO OUTFILE check is changed to use results->needs_file_priviledge
* True/False const are now represented as Item_func_true/Item_func_false. For now I'm over-generalizing to functions that are const, and expect val_int/etc would do the right thing

Reviewed By: luqun

Differential Revision: D22808305

-----------------------------------------------------------------------------------------------

Introduce base select parser class

Summary:
To make it easier to add a new bypass rpc parser into existing bypass implementation (nosql_access.cc),
and make minimum change to bypass engine while adding a bypass rpc parser, I introduced a new base class,
base_select_parser. It includes all essential members and methods for parsing bypass sq/rpc queries.
Existing bypass sql parser (select_parser) and a new bypass rpc parser (not included in this diff)
inherits the base function.

Bypass RPC reference diff: D30493654

Reviewed By: yizhang82

Differential Revision: D31870628
inikep added a commit that referenced this pull request Apr 12, 2024
Summary:
mysqlbinlog expects rows_query event only in RBR. It prints an
unflushed event warning when it parses a transaction which has rows_query but
not a rows events with STMT_END_F flag set. Since there is no direct way to
tell the binlog format in mysqlbinlog, we'll just forgo the warning for
rows_event which contain metadata.

Reference Patch: facebook/mysql-5.6@89e1c03ac

Reviewed By: lloyd

Differential Revision: D10054923

-----------------------------------------------------------------------

optimize Rows_query_log_event::has_trx_meta_data()

Summary:
Rows_query_log_event::has_trx_meta_data() is expensive as it does a bunch
of string manipulation to figure out if it is prepended by
TRX_META_DATA. In a random profile, it shows to take up 2.7% CPU cycles on
secondaries. The function is called multiple times from different
places during applying a trx from worker threads:
1. Rows_query_log_event::do_apply_event(): The 'apply' of this event
stashes the trx_meta_data_json into 'rli' _only_ if the event contains
trx_meta_data. This is the first invocation of
Rows_query_log_event::has_trx_meta_data()
2. Query_log_event::do_apply_event(): It needs to cleanup
the 'rows_query_ev' stashed in 'rli' _only_ if it has trx_meta_data.
Hence it calls Rows_query_log_event::has_trx_metadata() again
3. Log_event::apply_event(): This extracts the timestamp (for
'milli-secs behind master' calculation) from
trx_meta_data (stashed inside rli in #1) and needs to check if the event
contains trx_meta_data. Hence it calls
Rows_query_log_event::has_trx_metadata() again

It is best to cache this information in the event itself so that the cost
of Rows_query_log_event::has_trx_metadata() is paid only once. This
reduces the cost of this method by 3x.

In a subsequent diff, I intend to cache the result of
Query_log_event::extract_trx_metadata() since this method is also called
multiple times and it is expensive as it does a bunch of json parsing
and string construction/manipulation.

Reviewed By: abhinav04sharma

Differential Revision: D31373879
inikep added a commit that referenced this pull request Apr 12, 2024
Summary:
mysqlbinlog expects rows_query event only in RBR. It prints an
unflushed event warning when it parses a transaction which has rows_query but
not a rows events with STMT_END_F flag set. Since there is no direct way to
tell the binlog format in mysqlbinlog, we'll just forgo the warning for
rows_event which contain metadata.

Reference Patch: facebook/mysql-5.6@89e1c03ac

Reviewed By: lloyd

Differential Revision: D10054923

-----------------------------------------------------------------------

optimize Rows_query_log_event::has_trx_meta_data()

Summary:
Rows_query_log_event::has_trx_meta_data() is expensive as it does a bunch
of string manipulation to figure out if it is prepended by
TRX_META_DATA. In a random profile, it shows to take up 2.7% CPU cycles on
secondaries. The function is called multiple times from different
places during applying a trx from worker threads:
1. Rows_query_log_event::do_apply_event(): The 'apply' of this event
stashes the trx_meta_data_json into 'rli' _only_ if the event contains
trx_meta_data. This is the first invocation of
Rows_query_log_event::has_trx_meta_data()
2. Query_log_event::do_apply_event(): It needs to cleanup
the 'rows_query_ev' stashed in 'rli' _only_ if it has trx_meta_data.
Hence it calls Rows_query_log_event::has_trx_metadata() again
3. Log_event::apply_event(): This extracts the timestamp (for
'milli-secs behind master' calculation) from
trx_meta_data (stashed inside rli in #1) and needs to check if the event
contains trx_meta_data. Hence it calls
Rows_query_log_event::has_trx_metadata() again

It is best to cache this information in the event itself so that the cost
of Rows_query_log_event::has_trx_metadata() is paid only once. This
reduces the cost of this method by 3x.

In a subsequent diff, I intend to cache the result of
Query_log_event::extract_trx_metadata() since this method is also called
multiple times and it is expensive as it does a bunch of json parsing
and string construction/manipulation.

Reviewed By: abhinav04sharma

Differential Revision: D31373879
inikep pushed a commit that referenced this pull request Apr 15, 2024
Summary:
This is an upstream bug: https://bugs.mysql.com/bug.php?id=92964

The issue is that if slave instance use sysvar_session_track_gtids == OWN_GTID and also enable MTS, the slave will lag behind master and its performance degrades over time.

The reason for this lag/perf degrade is caused by each slave worker keep adding its own gtid into its own Session_consistency_gtids_ctx.m_gtid_set during each transaction commit. Overtime, some of Session_consistency_gtids_ctx.m_gtid_set may have millions gno_interval and it will take long time to insert 1 gtid into Session_consistency_gtids_ctx.m_gtid_set(time complexity O(n)), see [Gtid_set::add_gno_interval](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/rpl_gtid_set.cc#L323).

There are couple related code to this issue:
- During slave worker thread start, there is a [check](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/session_tracker.cc#L1560) to not enable tracker for system threads but that check doesn't work---The THD system_thread field is initialized with default value(NON_SYSTEM_THREAD) during THD::THD.
```
  m_enabled = thd->variables.session_track_gtids != OFF &&
              /* No need to track GTIDs for system threads. */
              thd->system_thread == NON_SYSTEM_THREAD;
```
```
Call stack:
```
- in [Session_gtids_track::Update](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/session_tracker.cc#L1581), the else code doesn't unregister listener due to mysql/mysql-server@1e0844c try to fix another issue.

Solutions:
1.  [Current]add system threads check during collecting data, since THD will be fully initialized at that time.
 -Pro: simple change(easy to port) and perf is ok
2. add `thd->session_track_gtids=OFF; thd->session_tracker.get_tracker(SESSION_GTIDS_TRACKER)->update(thd);` after [init_slave_thread()](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/rpl_slave.cc#L6788)
-Pro: During init_slave_thread(), thd->system_thread will be set correctly value.
-Con: it need to add couple places, such as handle_slave_io, handle_slave_sql, handle_slave_worker
3. add `thd->session_track_gtids=OFF;thd->session_tracker.get_tracker(SESSION_GTIDS_TRACKER)->update(thd);` inside [init_slave_thread()](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/rpl_slave.cc#L6788)
   -Pro: only add once for slave/slave worker/IO threads
   -Con: [should_collect](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/rpl_context.cc#L53) check still return true, thus it is less perf than #1
4. add `thd->session_track_gtids=OFF;thd->rpl_thd_ctx.session_gtids_ctx().unregister_ctx_change_listener(thd->session_tracker.get_tracker(SESSION_GTIDS_TRACKER));` inside init_slave_thread()
   -Pro: perf is better than #3, on par as #1
   -Con: if there are other non-system threads(except IO/sql threads) do transactions, these threads will still collect its own gtids.
5. add another THD overload constructor which takes 'enum_thread_type' as a parameter whose default value is NON_SYSTEM_THREAD
 -Pro: it will initialize correct value with THD and won't enable those session_tracker
 -Con: similar to #2/#4, need to replace THD() with THD(enum_thread_type) couples places.

Differential Revision: D18072672
inikep pushed a commit that referenced this pull request Apr 15, 2024
…batch #1

Summary:
Disabling tests:
1. Transaction related - those are already disabled in 5.6 as they are not supported
2. hotbackup tests - we need Xtrabackup for MyRocks
3. partition tests - we need Percona's partition patch. Note these are partition specific tests - tests that only use partition are properly fixed up to work without partitions to ensure we have good coverage.
4. tests that are requires authentication plugin
5. Missing patches

In 8.0 the disabled.def aren't being used anymore and instead they are located in collections and needs to be specifically added.

Reviewed By: lloyd

Differential Revision: D17555081
inikep pushed a commit that referenced this pull request Apr 15, 2024
Summary:
MySQL bypass infrastructure change

1. Adds a new hton override `handlerton::handle_single_table_select` to allow hijacking of SELECT statement execution from any storage engine.
2. Add lex parsing for bypass hints in optimizer hint style /*+ bypass */ and /*+ no_bypass */
3. MyRocks overrides handle_single_table_select but does nothing and simply fallback.

This is a port from from bypass feature branch - change #1 of 3. I'm planning to port the changes in the following order:

1. MySQL lexer + SELECT hijacking (this one).
2. SELECT Statement parsing and checking
3. Execution of SELECT statement in bypass

Reference Patch: facebook/mysql-5.6@2d19dc0

-----
Porting Notes:

* The entry points have changed due to MySQL 8.0 refactoring to SELECT path, and now it is in `Sql_cmd_dml::execute_inner`. I'll also see if overriding the entire execute would be better but this should be good enough for now.
* The check for whether we should go to bypass handler is mostly the same, except st_select_lex_t is now SELECT_LEX.
* The lex parsing for hints was mostly ported as-is. I was also debating whether I should just use optimizer hints instead of hacking the lexer, but decided against using optimizer hint for now so that migration from 5.6 will be easier. We can migrate to optimizer hint once 8.0 migration is complete.

Differential Revision: D22807040

---------------------------------------------------------------------------------------------

[bypass] MySQL bypass change #2 - SELECT parsing [non-RocksDB part]

Summary:
This is change #2 that ports the SELECT statement parsing from feature-myrocks-bypass branch, including:

1. select_parser class that parses SELECT statements and validates the scenarios are supported, and error with proper message if not supported
2. You can control bypass policy with rocksdb_select_bypass_policy variable = 0 (ALWAYS OFF) / 1 (ALWAYS ON) / 2 (OPT IN) / 3 (OPT OUT).
3. A few metrics for number of SELECT bypass statements executed/rejected (due to not supported) /failed (something wrong in execution)

At this point this only dumps the SELECT statement parsed results without doing anything useful. The next change will be porting the query plan execution and point/range query execution part.

Please ignore the unused attributes for now - they'll be removed in next change once the execution part is ported.

Reference Patch:

facebook/mysql-5.6@e79b20a
facebook/mysql-5.6@8072e3d

 ---
Porting Notes:

There are a lot of small changes in this area but nothing too bad:
* Some bypass scenarios no longer fail or fail with different error message because of the new parse tree -> AST conversion process. For example, `SELECT *` now works correctly
* st_select_lex_t -> SELECT_LEX, and many members has changed name, such as where -> where_cond()
* protocol usage has changed to start_row, store, end_row in 8.0, which is much better.
* thd->query() is now a LEX_STRING
* SELECT option check (such as SELECT_DISTINCT) now uses active_options() and only check for SELECT_DISTINCT as there seems to be default value being set most likely due to side effect of having more parse tree processing being done. I checked the flags and it looks like only SELECT_DISTINCT are interesting and needs to be blocked.
* SELECT INTO OUTFILE check is changed to use results->needs_file_priviledge
* True/False const are now represented as Item_func_true/Item_func_false. For now I'm over-generalizing to functions that are const, and expect val_int/etc would do the right thing

Reviewed By: luqun

Differential Revision: D22808305

-----------------------------------------------------------------------------------------------

Introduce base select parser class

Summary:
To make it easier to add a new bypass rpc parser into existing bypass implementation (nosql_access.cc),
and make minimum change to bypass engine while adding a bypass rpc parser, I introduced a new base class,
base_select_parser. It includes all essential members and methods for parsing bypass sq/rpc queries.
Existing bypass sql parser (select_parser) and a new bypass rpc parser (not included in this diff)
inherits the base function.

Bypass RPC reference diff: D30493654

Reviewed By: yizhang82

Differential Revision: D31870628
inikep added a commit that referenced this pull request Apr 15, 2024
Summary:
mysqlbinlog expects rows_query event only in RBR. It prints an
unflushed event warning when it parses a transaction which has rows_query but
not a rows events with STMT_END_F flag set. Since there is no direct way to
tell the binlog format in mysqlbinlog, we'll just forgo the warning for
rows_event which contain metadata.

Reference Patch: facebook/mysql-5.6@89e1c03ac

Reviewed By: lloyd

Differential Revision: D10054923

-----------------------------------------------------------------------

optimize Rows_query_log_event::has_trx_meta_data()

Summary:
Rows_query_log_event::has_trx_meta_data() is expensive as it does a bunch
of string manipulation to figure out if it is prepended by
TRX_META_DATA. In a random profile, it shows to take up 2.7% CPU cycles on
secondaries. The function is called multiple times from different
places during applying a trx from worker threads:
1. Rows_query_log_event::do_apply_event(): The 'apply' of this event
stashes the trx_meta_data_json into 'rli' _only_ if the event contains
trx_meta_data. This is the first invocation of
Rows_query_log_event::has_trx_meta_data()
2. Query_log_event::do_apply_event(): It needs to cleanup
the 'rows_query_ev' stashed in 'rli' _only_ if it has trx_meta_data.
Hence it calls Rows_query_log_event::has_trx_metadata() again
3. Log_event::apply_event(): This extracts the timestamp (for
'milli-secs behind master' calculation) from
trx_meta_data (stashed inside rli in #1) and needs to check if the event
contains trx_meta_data. Hence it calls
Rows_query_log_event::has_trx_metadata() again

It is best to cache this information in the event itself so that the cost
of Rows_query_log_event::has_trx_metadata() is paid only once. This
reduces the cost of this method by 3x.

In a subsequent diff, I intend to cache the result of
Query_log_event::extract_trx_metadata() since this method is also called
multiple times and it is expensive as it does a bunch of json parsing
and string construction/manipulation.

Reviewed By: abhinav04sharma

Differential Revision: D31373879
inikep pushed a commit that referenced this pull request Apr 16, 2024
Summary:
This is an upstream bug: https://bugs.mysql.com/bug.php?id=92964

The issue is that if slave instance use sysvar_session_track_gtids == OWN_GTID and also enable MTS, the slave will lag behind master and its performance degrades over time.

The reason for this lag/perf degrade is caused by each slave worker keep adding its own gtid into its own Session_consistency_gtids_ctx.m_gtid_set during each transaction commit. Overtime, some of Session_consistency_gtids_ctx.m_gtid_set may have millions gno_interval and it will take long time to insert 1 gtid into Session_consistency_gtids_ctx.m_gtid_set(time complexity O(n)), see [Gtid_set::add_gno_interval](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/rpl_gtid_set.cc#L323).

There are couple related code to this issue:
- During slave worker thread start, there is a [check](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/session_tracker.cc#L1560) to not enable tracker for system threads but that check doesn't work---The THD system_thread field is initialized with default value(NON_SYSTEM_THREAD) during THD::THD.
```
  m_enabled = thd->variables.session_track_gtids != OFF &&
              /* No need to track GTIDs for system threads. */
              thd->system_thread == NON_SYSTEM_THREAD;
```
```
Call stack:
```
- in [Session_gtids_track::Update](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/session_tracker.cc#L1581), the else code doesn't unregister listener due to mysql/mysql-server@1e0844c try to fix another issue.

Solutions:
1.  [Current]add system threads check during collecting data, since THD will be fully initialized at that time.
 -Pro: simple change(easy to port) and perf is ok
2. add `thd->session_track_gtids=OFF; thd->session_tracker.get_tracker(SESSION_GTIDS_TRACKER)->update(thd);` after [init_slave_thread()](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/rpl_slave.cc#L6788)
-Pro: During init_slave_thread(), thd->system_thread will be set correctly value.
-Con: it need to add couple places, such as handle_slave_io, handle_slave_sql, handle_slave_worker
3. add `thd->session_track_gtids=OFF;thd->session_tracker.get_tracker(SESSION_GTIDS_TRACKER)->update(thd);` inside [init_slave_thread()](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/rpl_slave.cc#L6788)
   -Pro: only add once for slave/slave worker/IO threads
   -Con: [should_collect](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/rpl_context.cc#L53) check still return true, thus it is less perf than #1
4. add `thd->session_track_gtids=OFF;thd->rpl_thd_ctx.session_gtids_ctx().unregister_ctx_change_listener(thd->session_tracker.get_tracker(SESSION_GTIDS_TRACKER));` inside init_slave_thread()
   -Pro: perf is better than #3, on par as #1
   -Con: if there are other non-system threads(except IO/sql threads) do transactions, these threads will still collect its own gtids.
5. add another THD overload constructor which takes 'enum_thread_type' as a parameter whose default value is NON_SYSTEM_THREAD
 -Pro: it will initialize correct value with THD and won't enable those session_tracker
 -Con: similar to #2/#4, need to replace THD() with THD(enum_thread_type) couples places.

Differential Revision: D18072672
inikep pushed a commit that referenced this pull request Apr 16, 2024
…batch #1

Summary:
Disabling tests:
1. Transaction related - those are already disabled in 5.6 as they are not supported
2. hotbackup tests - we need Xtrabackup for MyRocks
3. partition tests - we need Percona's partition patch. Note these are partition specific tests - tests that only use partition are properly fixed up to work without partitions to ensure we have good coverage.
4. tests that are requires authentication plugin
5. Missing patches

In 8.0 the disabled.def aren't being used anymore and instead they are located in collections and needs to be specifically added.

Reviewed By: lloyd

Differential Revision: D17555081
inikep pushed a commit that referenced this pull request Apr 16, 2024
Summary:
MySQL bypass infrastructure change

1. Adds a new hton override `handlerton::handle_single_table_select` to allow hijacking of SELECT statement execution from any storage engine.
2. Add lex parsing for bypass hints in optimizer hint style /*+ bypass */ and /*+ no_bypass */
3. MyRocks overrides handle_single_table_select but does nothing and simply fallback.

This is a port from from bypass feature branch - change #1 of 3. I'm planning to port the changes in the following order:

1. MySQL lexer + SELECT hijacking (this one).
2. SELECT Statement parsing and checking
3. Execution of SELECT statement in bypass

Reference Patch: facebook/mysql-5.6@2d19dc0

-----
Porting Notes:

* The entry points have changed due to MySQL 8.0 refactoring to SELECT path, and now it is in `Sql_cmd_dml::execute_inner`. I'll also see if overriding the entire execute would be better but this should be good enough for now.
* The check for whether we should go to bypass handler is mostly the same, except st_select_lex_t is now SELECT_LEX.
* The lex parsing for hints was mostly ported as-is. I was also debating whether I should just use optimizer hints instead of hacking the lexer, but decided against using optimizer hint for now so that migration from 5.6 will be easier. We can migrate to optimizer hint once 8.0 migration is complete.

Differential Revision: D22807040

---------------------------------------------------------------------------------------------

[bypass] MySQL bypass change #2 - SELECT parsing [non-RocksDB part]

Summary:
This is change #2 that ports the SELECT statement parsing from feature-myrocks-bypass branch, including:

1. select_parser class that parses SELECT statements and validates the scenarios are supported, and error with proper message if not supported
2. You can control bypass policy with rocksdb_select_bypass_policy variable = 0 (ALWAYS OFF) / 1 (ALWAYS ON) / 2 (OPT IN) / 3 (OPT OUT).
3. A few metrics for number of SELECT bypass statements executed/rejected (due to not supported) /failed (something wrong in execution)

At this point this only dumps the SELECT statement parsed results without doing anything useful. The next change will be porting the query plan execution and point/range query execution part.

Please ignore the unused attributes for now - they'll be removed in next change once the execution part is ported.

Reference Patch:

facebook/mysql-5.6@e79b20a
facebook/mysql-5.6@8072e3d

 ---
Porting Notes:

There are a lot of small changes in this area but nothing too bad:
* Some bypass scenarios no longer fail or fail with different error message because of the new parse tree -> AST conversion process. For example, `SELECT *` now works correctly
* st_select_lex_t -> SELECT_LEX, and many members has changed name, such as where -> where_cond()
* protocol usage has changed to start_row, store, end_row in 8.0, which is much better.
* thd->query() is now a LEX_STRING
* SELECT option check (such as SELECT_DISTINCT) now uses active_options() and only check for SELECT_DISTINCT as there seems to be default value being set most likely due to side effect of having more parse tree processing being done. I checked the flags and it looks like only SELECT_DISTINCT are interesting and needs to be blocked.
* SELECT INTO OUTFILE check is changed to use results->needs_file_priviledge
* True/False const are now represented as Item_func_true/Item_func_false. For now I'm over-generalizing to functions that are const, and expect val_int/etc would do the right thing

Reviewed By: luqun

Differential Revision: D22808305

-----------------------------------------------------------------------------------------------

Introduce base select parser class

Summary:
To make it easier to add a new bypass rpc parser into existing bypass implementation (nosql_access.cc),
and make minimum change to bypass engine while adding a bypass rpc parser, I introduced a new base class,
base_select_parser. It includes all essential members and methods for parsing bypass sq/rpc queries.
Existing bypass sql parser (select_parser) and a new bypass rpc parser (not included in this diff)
inherits the base function.

Bypass RPC reference diff: D30493654

Reviewed By: yizhang82

Differential Revision: D31870628
inikep added a commit that referenced this pull request Apr 16, 2024
Summary:
mysqlbinlog expects rows_query event only in RBR. It prints an
unflushed event warning when it parses a transaction which has rows_query but
not a rows events with STMT_END_F flag set. Since there is no direct way to
tell the binlog format in mysqlbinlog, we'll just forgo the warning for
rows_event which contain metadata.

Reference Patch: facebook/mysql-5.6@89e1c03ac

Reviewed By: lloyd

Differential Revision: D10054923

-----------------------------------------------------------------------

optimize Rows_query_log_event::has_trx_meta_data()

Summary:
Rows_query_log_event::has_trx_meta_data() is expensive as it does a bunch
of string manipulation to figure out if it is prepended by
TRX_META_DATA. In a random profile, it shows to take up 2.7% CPU cycles on
secondaries. The function is called multiple times from different
places during applying a trx from worker threads:
1. Rows_query_log_event::do_apply_event(): The 'apply' of this event
stashes the trx_meta_data_json into 'rli' _only_ if the event contains
trx_meta_data. This is the first invocation of
Rows_query_log_event::has_trx_meta_data()
2. Query_log_event::do_apply_event(): It needs to cleanup
the 'rows_query_ev' stashed in 'rli' _only_ if it has trx_meta_data.
Hence it calls Rows_query_log_event::has_trx_metadata() again
3. Log_event::apply_event(): This extracts the timestamp (for
'milli-secs behind master' calculation) from
trx_meta_data (stashed inside rli in #1) and needs to check if the event
contains trx_meta_data. Hence it calls
Rows_query_log_event::has_trx_metadata() again

It is best to cache this information in the event itself so that the cost
of Rows_query_log_event::has_trx_metadata() is paid only once. This
reduces the cost of this method by 3x.

In a subsequent diff, I intend to cache the result of
Query_log_event::extract_trx_metadata() since this method is also called
multiple times and it is expensive as it does a bunch of json parsing
and string construction/manipulation.

Reviewed By: abhinav04sharma

Differential Revision: D31373879
inikep pushed a commit that referenced this pull request Apr 17, 2024
Summary:
This is an upstream bug: https://bugs.mysql.com/bug.php?id=92964

The issue is that if slave instance use sysvar_session_track_gtids == OWN_GTID and also enable MTS, the slave will lag behind master and its performance degrades over time.

The reason for this lag/perf degrade is caused by each slave worker keep adding its own gtid into its own Session_consistency_gtids_ctx.m_gtid_set during each transaction commit. Overtime, some of Session_consistency_gtids_ctx.m_gtid_set may have millions gno_interval and it will take long time to insert 1 gtid into Session_consistency_gtids_ctx.m_gtid_set(time complexity O(n)), see [Gtid_set::add_gno_interval](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/rpl_gtid_set.cc#L323).

There are couple related code to this issue:
- During slave worker thread start, there is a [check](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/session_tracker.cc#L1560) to not enable tracker for system threads but that check doesn't work---The THD system_thread field is initialized with default value(NON_SYSTEM_THREAD) during THD::THD.
```
  m_enabled = thd->variables.session_track_gtids != OFF &&
              /* No need to track GTIDs for system threads. */
              thd->system_thread == NON_SYSTEM_THREAD;
```
```
Call stack:
```
- in [Session_gtids_track::Update](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/session_tracker.cc#L1581), the else code doesn't unregister listener due to mysql/mysql-server@1e0844c try to fix another issue.

Solutions:
1.  [Current]add system threads check during collecting data, since THD will be fully initialized at that time.
 -Pro: simple change(easy to port) and perf is ok
2. add `thd->session_track_gtids=OFF; thd->session_tracker.get_tracker(SESSION_GTIDS_TRACKER)->update(thd);` after [init_slave_thread()](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/rpl_slave.cc#L6788)
-Pro: During init_slave_thread(), thd->system_thread will be set correctly value.
-Con: it need to add couple places, such as handle_slave_io, handle_slave_sql, handle_slave_worker
3. add `thd->session_track_gtids=OFF;thd->session_tracker.get_tracker(SESSION_GTIDS_TRACKER)->update(thd);` inside [init_slave_thread()](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/rpl_slave.cc#L6788)
   -Pro: only add once for slave/slave worker/IO threads
   -Con: [should_collect](https://github.com/facebook/mysql-5.6/blob/fb-mysql-8.0.13/sql/rpl_context.cc#L53) check still return true, thus it is less perf than #1
4. add `thd->session_track_gtids=OFF;thd->rpl_thd_ctx.session_gtids_ctx().unregister_ctx_change_listener(thd->session_tracker.get_tracker(SESSION_GTIDS_TRACKER));` inside init_slave_thread()
   -Pro: perf is better than #3, on par as #1
   -Con: if there are other non-system threads(except IO/sql threads) do transactions, these threads will still collect its own gtids.
5. add another THD overload constructor which takes 'enum_thread_type' as a parameter whose default value is NON_SYSTEM_THREAD
 -Pro: it will initialize correct value with THD and won't enable those session_tracker
 -Con: similar to #2/#4, need to replace THD() with THD(enum_thread_type) couples places.

Differential Revision: D18072672
inikep pushed a commit that referenced this pull request Apr 17, 2024
…batch #1

Summary:
Disabling tests:
1. Transaction related - those are already disabled in 5.6 as they are not supported
2. hotbackup tests - we need Xtrabackup for MyRocks
3. partition tests - we need Percona's partition patch. Note these are partition specific tests - tests that only use partition are properly fixed up to work without partitions to ensure we have good coverage.
4. tests that are requires authentication plugin
5. Missing patches

In 8.0 the disabled.def aren't being used anymore and instead they are located in collections and needs to be specifically added.

Reviewed By: lloyd

Differential Revision: D17555081
inikep pushed a commit that referenced this pull request Apr 17, 2024
Summary:
MySQL bypass infrastructure change

1. Adds a new hton override `handlerton::handle_single_table_select` to allow hijacking of SELECT statement execution from any storage engine.
2. Add lex parsing for bypass hints in optimizer hint style /*+ bypass */ and /*+ no_bypass */
3. MyRocks overrides handle_single_table_select but does nothing and simply fallback.

This is a port from from bypass feature branch - change #1 of 3. I'm planning to port the changes in the following order:

1. MySQL lexer + SELECT hijacking (this one).
2. SELECT Statement parsing and checking
3. Execution of SELECT statement in bypass

Reference Patch: facebook/mysql-5.6@2d19dc0

-----
Porting Notes:

* The entry points have changed due to MySQL 8.0 refactoring to SELECT path, and now it is in `Sql_cmd_dml::execute_inner`. I'll also see if overriding the entire execute would be better but this should be good enough for now.
* The check for whether we should go to bypass handler is mostly the same, except st_select_lex_t is now SELECT_LEX.
* The lex parsing for hints was mostly ported as-is. I was also debating whether I should just use optimizer hints instead of hacking the lexer, but decided against using optimizer hint for now so that migration from 5.6 will be easier. We can migrate to optimizer hint once 8.0 migration is complete.

Differential Revision: D22807040

---------------------------------------------------------------------------------------------

[bypass] MySQL bypass change #2 - SELECT parsing [non-RocksDB part]

Summary:
This is change #2 that ports the SELECT statement parsing from feature-myrocks-bypass branch, including:

1. select_parser class that parses SELECT statements and validates the scenarios are supported, and error with proper message if not supported
2. You can control bypass policy with rocksdb_select_bypass_policy variable = 0 (ALWAYS OFF) / 1 (ALWAYS ON) / 2 (OPT IN) / 3 (OPT OUT).
3. A few metrics for number of SELECT bypass statements executed/rejected (due to not supported) /failed (something wrong in execution)

At this point this only dumps the SELECT statement parsed results without doing anything useful. The next change will be porting the query plan execution and point/range query execution part.

Please ignore the unused attributes for now - they'll be removed in next change once the execution part is ported.

Reference Patch:

facebook/mysql-5.6@e79b20a
facebook/mysql-5.6@8072e3d

 ---
Porting Notes:

There are a lot of small changes in this area but nothing too bad:
* Some bypass scenarios no longer fail or fail with different error message because of the new parse tree -> AST conversion process. For example, `SELECT *` now works correctly
* st_select_lex_t -> SELECT_LEX, and many members has changed name, such as where -> where_cond()
* protocol usage has changed to start_row, store, end_row in 8.0, which is much better.
* thd->query() is now a LEX_STRING
* SELECT option check (such as SELECT_DISTINCT) now uses active_options() and only check for SELECT_DISTINCT as there seems to be default value being set most likely due to side effect of having more parse tree processing being done. I checked the flags and it looks like only SELECT_DISTINCT are interesting and needs to be blocked.
* SELECT INTO OUTFILE check is changed to use results->needs_file_priviledge
* True/False const are now represented as Item_func_true/Item_func_false. For now I'm over-generalizing to functions that are const, and expect val_int/etc would do the right thing

Reviewed By: luqun

Differential Revision: D22808305

-----------------------------------------------------------------------------------------------

Introduce base select parser class

Summary:
To make it easier to add a new bypass rpc parser into existing bypass implementation (nosql_access.cc),
and make minimum change to bypass engine while adding a bypass rpc parser, I introduced a new base class,
base_select_parser. It includes all essential members and methods for parsing bypass sq/rpc queries.
Existing bypass sql parser (select_parser) and a new bypass rpc parser (not included in this diff)
inherits the base function.

Bypass RPC reference diff: D30493654

Reviewed By: yizhang82

Differential Revision: D31870628
inikep added a commit that referenced this pull request Apr 17, 2024
Summary:
mysqlbinlog expects rows_query event only in RBR. It prints an
unflushed event warning when it parses a transaction which has rows_query but
not a rows events with STMT_END_F flag set. Since there is no direct way to
tell the binlog format in mysqlbinlog, we'll just forgo the warning for
rows_event which contain metadata.

Reference Patch: facebook/mysql-5.6@89e1c03ac

Reviewed By: lloyd

Differential Revision: D10054923

-----------------------------------------------------------------------

optimize Rows_query_log_event::has_trx_meta_data()

Summary:
Rows_query_log_event::has_trx_meta_data() is expensive as it does a bunch
of string manipulation to figure out if it is prepended by
TRX_META_DATA. In a random profile, it shows to take up 2.7% CPU cycles on
secondaries. The function is called multiple times from different
places during applying a trx from worker threads:
1. Rows_query_log_event::do_apply_event(): The 'apply' of this event
stashes the trx_meta_data_json into 'rli' _only_ if the event contains
trx_meta_data. This is the first invocation of
Rows_query_log_event::has_trx_meta_data()
2. Query_log_event::do_apply_event(): It needs to cleanup
the 'rows_query_ev' stashed in 'rli' _only_ if it has trx_meta_data.
Hence it calls Rows_query_log_event::has_trx_metadata() again
3. Log_event::apply_event(): This extracts the timestamp (for
'milli-secs behind master' calculation) from
trx_meta_data (stashed inside rli in #1) and needs to check if the event
contains trx_meta_data. Hence it calls
Rows_query_log_event::has_trx_metadata() again

It is best to cache this information in the event itself so that the cost
of Rows_query_log_event::has_trx_metadata() is paid only once. This
reduces the cost of this method by 3x.

In a subsequent diff, I intend to cache the result of
Query_log_event::extract_trx_metadata() since this method is also called
multiple times and it is expensive as it does a bunch of json parsing
and string construction/manipulation.

Reviewed By: abhinav04sharma

Differential Revision: D31373879
inikep pushed a commit that referenced this pull request Jun 14, 2024
Post push fix.

Do not allow ndb_mgm_listen_event to return a socket that uses TLS since
user can not access the corresponding SSL object thorugh the public
MgmAPI.

Change-Id: I2a741efe4f80db750419101ecabb03fb5e025346
inikep pushed a commit that referenced this pull request Jun 14, 2024
Post push fix.

Make NdbSocket::ssl_readln return 0 on timeout.

Change-Id: I4cad95abd319883c16f2c28eff5cf2b6761731d6
inikep pushed a commit that referenced this pull request Jun 14, 2024
Post push fix.

Add missing socket close in testMgmd -n StartTls.

Change-Id: Ia446b522ad2698f63d588d3c52122df8735765c7
inikep pushed a commit that referenced this pull request Jun 14, 2024
Problem
================================

Group Replication ASAN run failing without any symptom of a
leak, but with shutdown issues:

worker[6] Shutdown report from
/dev/shm/mtr-3771884/var-gr-debug/6/log/mysqld.1.err after tests:
 group_replication.gr_flush_logs
group_replication.gr_delayed_initialization_thread_handler_error
group_replication.gr_sbr_verifications
group_replication.gr_server_uuid_matches_group_name_bootstrap
group_replication.gr_stop_async_on_stop_gr
group_replication.gr_certifier_message_same_member
group_replication.gr_ssl_mode_verify_identity_error_xcom

Analysis and Fix
================================

It ended up being a leak on gr_ssl_mode_verify_identity_error_xcom test:
Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7f1709fbe1c7 in operator new(unsigned long)
      ../../../../src/libsanitizer/asan/asan_new_delete.cpp:99
    #1 0x7f16ea0df799 in xcom_tcp_server_startup(Xcom_network_provider*)
      (/export/home/tmp/BUG35594709/mysql-trunk/BIN-ASAN/plugin_output_directory
        /group_replication.so+0x65d799)
    #2 0x7f170751e2b2  (/lib/x86_64-linux-gnu/libstdc++.so.6+0xdc2b2)

This happens because we delegated incoming connections
cleanup to the external consumer in incoming_connection_task.
Since it calls incoming_connection() from
Network_provider_manager, in case of a concurrent stop,
a connection could be left orphan in the shared atomic
due to the lack of an Active Provider, thus creating a
memory leak.

The solution is to make this cleanup on
Network_provider_manager, on both stop_provider() and in
stop_all_providers() methods, thus ensuring that no
incoming connection leaks.

Change-Id: I2367c37608ad075dee63785e9f908af5e81374ca
inikep pushed a commit that referenced this pull request Jun 14, 2024
BUG#35949017 Schema dist setup lockup
Bug#35948153 Problem setting up events due to stale NdbApi dictionary cache [#2]
Bug#35948153 Problem setting up events due to stale NdbApi dictionary cache [#1]
Bug#32550019 Missing check for ndb_schema_result leads to schema dist timeout

Change-Id: I4a32197992bf8b6899892f21587580788f828f34
inikep pushed a commit that referenced this pull request Jun 14, 2024
When built with ASAN, a use-after-free is reported for the TcpPortPool.

AddressSanitizer: heap-use-after-free on address 0x60200019f190 at pc
0x00000076a18d bp 0x7fff51e7d1d0 sp 0x7fff51e7d1c0

    #4 0x770b73 in UniqueId::ProcessUniqueIds::erase(unsigned int)
       ../router/tests/helpers/tcp_port_pool.h:112
    #5 0x770c48 in UniqueId::~UniqueId()
       ../router/tests/helpers/tcp_port_pool.cc:234
    ...
    #12 0x82faa3 in testing::UnitTest::~UnitTest()
	../extra/googletest/googletest-release-1.12.0/googletest/src/gtest.cc:5496
    percona#13 0x7f5fe085ace8 in __run_exit_handlers (/lib64/libc.so.6+0x39ce8)

0x60200019f190 is located 0 bytes inside of 16-byte region
[0x60200019f190,0x60200019f1a0)
freed by thread T0 here:
    #0 0x7f5fe3cbd10f in operator delete(void*, unsigned long)
       (/lib64/libasan.so.6+0xb710f)
    #1 0x7f5fe085ace8 in __run_exit_handlers (/lib64/libc.so.6+0x39ce8)

Background
==========

__run_exit_handlers destroys "static" and "global" variables in reverse
order of their creation.

googletest's unit-tests are a static, and the TcpPortPool also has
ProcessUniqueId's which contains the process-wide unique-ids.

At construct: unittest -> tcp-port-pool -> proces-unique-ids
At destruct : process-unique-ids -> tcp-port-pool -> 💥

The use-after-free happens as the process-unique-ids static is
destructed before the tcp-port-pool which tries to its Ids from the
process-unique-ids.

Change
======

- extend the lifetime of the process-unique-ids to after the last use of
  the tcp-port-pool via a std::shared_ptr<>

Change-Id: I75b8b781e1d240f18ca72f2c86182639a7699f06
inikep pushed a commit that referenced this pull request Jun 14, 2024
…nt on Windows and posix [#1]

When passing arguments to NdbProcess::create it will become important
when introducing quoting to distinguish spaces that are port of the
argument value or beeing an argument separator.

This patch removes current uses of space as separator in arguments to
NdbProcess::create.

Change-Id: I1d1bab27e183fc33632bfd9974010129a8970365
inikep pushed a commit that referenced this pull request Jun 14, 2024
Problem:
Starting ´ndb_mgmd --bind-address´ may potentially cause abnormal
program termination in MgmtSrvr destructor when ndb_mgmd restart itself.

  Core was generated by `ndb_mgmd --defa'.
  Program terminated with signal SIGABRT,   Aborted.
  #0  0x00007f8ce4066b8f in raise () from /lib64/libc.so.6
  #1  0x00007f8ce4039ea5 in abort () from /lib64/libc.so.6
  #2  0x00007f8ce40a7d97 in __libc_message () from /lib64/libc.so.6
  #3  0x00007f8ce40af08c in malloc_printerr () from /lib64/libc.so.6
  #4  0x00007f8ce40b132d in _int_free () from /lib64/libc.so.6
  #5  0x00000000006e9ffe in MgmtSrvr::~MgmtSrvr (this=0x28de4b0) at
mysql/8.0/storage/ndb/src/mgmsrv/MgmtSrvr.cpp:
890
  #6  0x00000000006ea09e in MgmtSrvr::~MgmtSrvr (this=0x2) at mysql/8.0/
storage/ndb/src/mgmsrv/MgmtSrvr.cpp:849
  #7  0x0000000000700d94 in mgmd_run () at
mysql/8.0/storage/ndb/src/mgmsrv/main.cpp:260
  #8  0x0000000000700775 in mgmd_main (argc=<optimized out>,
argv=0x28041d0) at mysql/8.0/storage/ndb/src/
mgmsrv/main.cpp:479

Analysis:
While starting up, the ndb_mgmd will allocate memory for bind_address in
order to potentially rewrite the parameter. When ndb_mgmd restart itself
the memory will be released and dangling pointer causing double free.

Fix:
Drop support for bind_address=[::], it is not documented anywhere, is
not useful and doesn't work.
This means the need to rewrite bind_address is gone and bind_address
argument need neither alloc or free.

Change-Id: I7797109b9d8391394587188d64d4b1f398887e94
inikep pushed a commit that referenced this pull request Jun 14, 2024
…ocal DDL

         executed

https://perconadev.atlassian.net/browse/PS-9018

Merge remote-tracking branch 'venki/PS-9018-8.0-gca' into HEAD

Problem
-------
In high concurrency scenarios, MySQL replica can enter into a deadlock due to a
race condition between the replica applier thread and the client thread
performing a binlog group commit.

Analysis
--------
It needs at least 3 threads for this deadlock to happen

1. One client thread
2. Two replica applier threads

How this deadlock happens?
--------------------------
0. Binlog is enabled on replica, but log_replica_updates is disabled.

1. Initially, both "Commit Order" and "Binlog Flush" queues are empty.

2. Replica applier thread 1 enters the group commit pipeline to register in the
   "Commit Order" queue since `log-replica-updates` is disabled on the replica
   node.

3. Since both "Commit Order" and "Binlog Flush" queues are empty, the applier
   thread 1

   3.1. Becomes leader (In Commit_stage_manager::enroll_for()).

   3.2. Registers in the commit order queue.

   3.3. Acquires the lock MYSQL_BIN_LOG::LOCK_log.

   3.4. Commit Order queue is emptied, but the lock MYSQL_BIN_LOG::LOCK_log is
        not yet released.

   NOTE: SE commit for applier thread is already done by the time it reaches
         here.

4. Replica applier thread 2 enters the group commit pipeline to register in the
   "Commit Order" queue since `log-replica-updates` is disabled on the replica
   node.

5. Since the "Commit Order" queue is empty (emptied by applier thread 1 in 3.4), the
   applier thread 2

   5.1. Becomes leader (In Commit_stage_manager::enroll_for())

   5.2. Registers in the commit order queue.

   5.3. Tries to acquire the lock MYSQL_BIN_LOG::LOCK_log. Since it is held by applier
        thread 1 it will wait until the lock is released.

6. Client thread enters the group commit pipeline to register in the
   "Binlog Flush" queue.

7. Since "Commit Order" queue is not empty (there is applier thread 2 in the
   queue), it enters the conditional wait `m_stage_cond_leader` with an
   intention to become the leader for both the "Binlog Flush" and
   "Commit Order" queues.

8. Applier thread 1 releases the lock MYSQL_BIN_LOG::LOCK_log and proceeds to update
   the GTID by calling gtid_state->update_commit_group() from
   Commit_order_manager::flush_engine_and_signal_threads().

9. Applier thread 2 acquires the lock MYSQL_BIN_LOG::LOCK_log.

   9.1. It checks if there is any thread waiting in the "Binlog Flush" queue
        to become the leader. Here it finds the client thread waiting to be
        the leader.

   9.2. It releases the lock MYSQL_BIN_LOG::LOCK_log and signals on the
        cond_var `m_stage_cond_leader` and enters a conditional wait until the
        thread's `tx_commit_pending` is set to false by the client thread
       (will be done in the
       Commit_stage_manager::process_final_stage_for_ordered_commit_group()
       called by client thread from fetch_and_process_flush_stage_queue()).

10. The client thread wakes up from the cond_var `m_stage_cond_leader`.  The
    thread has now become a leader and it is its responsibility to update GTID
    of applier thread 2.

    10.1. It acquires the lock MYSQL_BIN_LOG::LOCK_log.

    10.2. Returns from `enroll_for()` and proceeds to process the
          "Commit Order" and "Binlog Flush" queues.

    10.3. Fetches the "Commit Order" and "Binlog Flush" queues.

    10.4. Performs the storage engine flush by calling ha_flush_logs() from
          fetch_and_process_flush_stage_queue().

    10.5. Proceeds to update the GTID of threads in "Commit Order" queue by
          calling gtid_state->update_commit_group() from
          Commit_stage_manager::process_final_stage_for_ordered_commit_group().

11. At this point, we will have

    - Client thread performing GTID update on behalf if applier thread 2 (from step 10.5), and
    - Applier thread 1 performing GTID update for itself (from step 8).

    Due to the lack of proper synchronization between the above two threads,
    there exists a time window where both threads can call
    gtid_state->update_commit_group() concurrently.

    In subsequent steps, both threads simultaneously try to modify the contents
    of the array `commit_group_sidnos` which is used to track the lock status of
    sidnos. This concurrent access to `update_commit_group()` can cause a
    lock-leak resulting in one thread acquiring the sidno lock and not
    releasing at all.

-----------------------------------------------------------------------------------------------------------
Client thread                                           Applier Thread 1
-----------------------------------------------------------------------------------------------------------
update_commit_group() => global_sid_lock->rdlock();     update_commit_group() => global_sid_lock->rdlock();

calls update_gtids_impl_lock_sidnos()                   calls update_gtids_impl_lock_sidnos()

set commit_group_sidno[2] = true                        set commit_group_sidno[2] = true

                                                        lock_sidno(2) -> successful

lock_sidno(2) -> waits

                                                        update_gtids_impl_own_gtid() -> Add the thd->owned_gtid in `executed_gtids()`

                                                        if (commit_group_sidnos[2]) {
                                                          unlock_sidno(2);
                                                          commit_group_sidnos[2] = false;
                                                        }

                                                        Applier thread continues..

lock_sidno(2) -> successful

update_gtids_impl_own_gtid() -> Add the thd->owned_gtid in `executed_gtids()`

if (commit_group_sidnos[2]) { <=== this check fails and lock is not released.
  unlock_sidno(2);
  commit_group_sidnos[2] = false;
}

Client thread continues without releasing the lock
-----------------------------------------------------------------------------------------------------------

12. As the above lock-leak can also happen the other way i.e, the applier
    thread fails to unlock, there can be different consequences hereafter.

13. If the client thread continues without releasing the lock, then at a later
    stage, it can enter into a deadlock with the applier thread performing a
    GTID update with stack trace.

    Client_thread
    -------------
    #1  __GI___lll_lock_wait
    #2  ___pthread_mutex_lock
    #3  native_mutex_lock                                       <= waits for commit lock while holding sidno lock
    #4  Commit_stage_manager::enroll_for
    #5  MYSQL_BIN_LOG::change_stage
    #6  MYSQL_BIN_LOG::ordered_commit
    #7  MYSQL_BIN_LOG::commit
    #8  ha_commit_trans
    #9  trans_commit_implicit
    #10 mysql_create_like_table
    #11 Sql_cmd_create_table::execute
    #12 mysql_execute_command
    percona#13 dispatch_sql_command

    Applier thread
    --------------
    #1  ___pthread_mutex_lock
    #2  native_mutex_lock
    #3  safe_mutex_lock
    #4  Gtid_state::update_gtids_impl_lock_sidnos               <= waits for sidno lock
    #5  Gtid_state::update_commit_group
    #6  Commit_order_manager::flush_engine_and_signal_threads   <= acquires commit lock here
    #7  Commit_order_manager::finish
    #8  Commit_order_manager::wait_and_finish
    #9  ha_commit_low
    #10 trx_coordinator::commit_in_engines
    #11 MYSQL_BIN_LOG::commit
    #12 ha_commit_trans
    percona#13 trans_commit
    percona#14 Xid_log_event::do_commit
    percona#15 Xid_apply_log_event::do_apply_event_worker
    percona#16 Slave_worker::slave_worker_exec_event
    percona#17 slave_worker_exec_job_group
    percona#18 handle_slave_worker

14. If the applier thread continues without releasing the lock, then at a later
    stage, it can perform recursive locking while setting the GTID for the next
    transaction (in set_gtid_next()).

    In debug builds the above case hits the assertion
    `safe_mutex_assert_not_owner()` meaning the lock is already acquired by the
    replica applier thread when it tries to re-acquire the lock.

Solution
--------
In the above problematic example, when seen from each thread
individually, we can conclude that there is no problem in the order of lock
acquisition, thus there is no need to change the lock order.

However, the root cause for this problem is that multiple threads can
concurrently access to the array `Gtid_state::commit_group_sidnos`.

In its initial implementation, it was expected that threads should
hold the `MYSQL_BIN_LOG::LOCK_commit` before modifying its contents. But it
was not considered when upstream implemented WL#7846 (MTS:
slave-preserve-commit-order when log-slave-updates/binlog is disabled).

With this patch, we now ensure that `MYSQL_BIN_LOG::LOCK_commit` is acquired
when the client thread (binlog flush leader) when it tries to perform GTID
update on behalf of threads waiting in "Commit Order" queue, thus providing a
guarantee that `Gtid_state::commit_group_sidnos` array is never accessed
without the protection of `MYSQL_BIN_LOG::LOCK_commit`.
inikep pushed a commit that referenced this pull request Jun 14, 2024
This worklog introduces dynamic offload of Queries to RAPID in following
ways:

When system variable rapid_use_dynamic_offload is 0/false , then we
fall back to normal cost threshold classifier, which also implies that
when use secondary engine is set to forced, eligible queries will go to
secondary engine, regardless of cost threshold or this classifier.

When rapid_use_dynamic_offload is 1/true, then we proceed with looking
for optimal execution engine for this queries, if secondary engine is
found more optimal, then query is offloaded, otherwise it is sent back
to mysql. This is handled in following scenarios:

1. Static Scenario: When there's no Change propagation or Queue on RAPID
side, this introduces decision tree which has > 85 % precision in
training which queries should be faster on mysql or which queries
should be faster on mysql, and accepts or rejects queries.  the decision
tree takes around 20-100 microseconds for fast queries, hence
minimal overhead, for bigger queries this introduces overhead of
upto maximum observed 700 microseconds, these end up with long execution
time, hence not a problem. For very fast queries, defined here by having
cost < 10 and of the form point select, dynamic offload is not applied,
since 100 % of these queries  (out of 16667 samples) are faster on
MySQL. Additionally, routing these "very fast queries" through dynamic
offload leads to performance regressions due to 3 phase optimisation.

2. Dynamic Scenario: When there's CP or queuing on RAPID, this worklog
 introduces dynamic feature normalization to factor into account
 extra catch up time RAPID needs, and factoring in that, attempts to
 verify if RAPID is still the best engine for execution. If queue is
 too long or CP is too long, this mechanism wants to progressively start
 shifting queries to mysql, moving gradually towards the heavier queries

The steps in this worklog with respect to query lifecycle in server with
secondary_engine = ON, are described below:

query
   |
Primary Tentatively optimisation -> mysql optimises for Innodb
   |
secondary_engine_pre_prepare_hook -> following Rapid function called:
   |  RapidCachePrimaryInfoAtPrimaryTentativelyStep
   |  If dynamic offload is enabled and query is not "very fast":
   |   This caches features from mysql plan in rapid_statement_context
   |   to be used for dynamic offload.
   |  If dynamic offload is disabled or the query is "very fast":
   |   This function invokes standary mysql cost threshold classifier,
   |   which decides if query needs further RAPID optimisation.
   |
   |
   |-> if returns False, then query proceeds to Innodb for execution
   |-> if returns true, step below is called
   |
 Secondary optimisation -> mysql optimises for RAPID
   |
prepare_secondary_engine -> following Rapid function is called:
   |   RapidPrepareEstimateQueryCosts
   |     In this function, Dynamic offload combines mysql plan features
   |      retrieved from rapid_statement_context
   |     and RAPID info such as rapid base table cardinality,
   |     dict encoding projection, varlen projection size, rapid queue
   |     size in to decide if query should be offloaded to RAPID.
   |
   |->if returns True, then query proceeds to Innodb for execution
   |->if returns False, step below is called
   |
optimize_secondary_engine -> following Rapid function is called
   |    RapidOptimize
   |     In this function, Dynamic offload retrieves info from
   |     rapid_statement_context and additionally looks at Change
   |     propagation lag to decide if query should be offloaded to rapid
   |
   |->if returns True, then query proceeds to Innodb for execution
   |->if returns False, then query goes to Rapid Execution.

Following new MYSQL ERR log messages are printed with this WL, when
dynamic offload is enabled, and query is not a "very fast query".

1. SelOffload allow decision 1 : as secondary not forced 1 and enable
 var value 1 and transactional enabled 1 and( big shape detected 0
  or small shape detected 1 ) inno: 10737418240 , rpd: 4294967296 ,
   no lh table: 1

   Message such as this shows if dynamic offload is used to classify
   this query or not. If not, why not, using each of the conditions.
   1 = pass, 0 = not pass.

2. myqid=65 Selective offload classifier #1#1#1
    f_mysql_total_ts_nrows <= 2105.5 : 0.173916, f_MySQLCost <=
    68.3899040222168 : 0.028218, f_count_all_base_tables = 0 ,
    f_count_ref_index_ts = 0 ,f_BaseTableSumNrows <= 278177.5 :
    0.173916 are_all_ts_index_ref = true outcome=0

   Line such as this serialises what leg of decision tree decided
   outcome of this query 0 -> back to mysql 1 -> keep on rapid.
   each leg is uniquely searchable via identifier such as #1#1#1 here.

This worklog additionally introduces python scripts to run queries on
mysql client with multiple queries and multiple dmls at once, in
various modes such as simulator mode and standard benchmark modes.

By Default this WL is enabled, but before release it will be disabled.
This is tracked via BUG#36343189 #no-close.

Perf mode unittests will be enabled on jenkins after this wl.
Further cleanup will be done via BUG#36368437 #no-close.

Bugs tackled via this WL: 	BUG#35738194, Enh#34132523, Bug#36343208

Unrelated bugs fixed: BUG#35987975

Old gerrit review : 25567 (abandoned due to 1000 update limit reached)

Change-Id: Ie5f9fdcd8b55a669d04b389d3aec5f6b33f0fe2e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants