Azure trigger test #3
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
inikep
force-pushed
the
PS-5.7-azure-trigger
branch
from
72782fa to
7aef65b
Compare
inikep
force-pushed
the
PS-5.7-azure-trigger2
branch
from
659f0ec to
7aef65b
Compare
inikep
force-pushed
the
PS-5.7-azure-trigger2
branch
from
f3c4e8e to
3cd484f
Compare
inikep
force-pushed
the
PS-5.7-azure-trigger
branch
from
7aef65b to
2383669
Compare
inikep
pushed a commit
that referenced
this pull request
Nov 28, 2022
*Problem:*
ASAN complains about stack-buffer-overflow on function `mysql_heartbeat`:
```
==90890==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fe746d06d14 at pc 0x7fe760f5b017 bp 0x7fe746d06cd0 sp 0x7fe746d06478
WRITE of size 24 at 0x7fe746d06d14 thread T16777215
Address 0x7fe746d06d14 is located in stack of thread T26 at offset 340 in frame
#0 0x7fe746d0a55c in mysql_heartbeat(void*) /home/yura/ws/percona-server/plugin/daemon_example/daemon_example.cc:62
This frame has 4 object(s):
[48, 56) 'result' (line 66)
[80, 112) '_db_stack_frame_' (line 63)
[144, 200) 'tm_tmp' (line 67)
[240, 340) 'buffer' (line 65) <== Memory access at offset 340 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
(longjmp and C++ exceptions *are* supported)
Thread T26 created by T25 here:
#0 0x7fe760f5f6d5 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:216
#1 0x557ccbbcb857 in my_thread_create /home/yura/ws/percona-server/mysys/my_thread.c:104
#2 0x7fe746d0b21a in daemon_example_plugin_init /home/yura/ws/percona-server/plugin/daemon_example/daemon_example.cc:148
#3 0x557ccb4c69c7 in plugin_initialize /home/yura/ws/percona-server/sql/sql_plugin.cc:1279
#4 0x557ccb4d19cd in mysql_install_plugin /home/yura/ws/percona-server/sql/sql_plugin.cc:2279
#5 0x557ccb4d218f in Sql_cmd_install_plugin::execute(THD*) /home/yura/ws/percona-server/sql/sql_plugin.cc:4664
#6 0x557ccb47695e in mysql_execute_command(THD*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5160
#7 0x557ccb47977c in mysql_parse(THD*, Parser_state*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5952
#8 0x557ccb47b6c2 in dispatch_command(THD*, COM_DATA const*, enum_server_command) /home/yura/ws/percona-server/sql/sql_parse.cc:1544
#9 0x557ccb47de1d in do_command(THD*) /home/yura/ws/percona-server/sql/sql_parse.cc:1065
#10 0x557ccb6ac294 in handle_connection /home/yura/ws/percona-server/sql/conn_handler/connection_handler_per_thread.cc:325
#11 0x557ccbbfabb0 in pfs_spawn_thread /home/yura/ws/percona-server/storage/perfschema/pfs.cc:2198
#12 0x7fe760ab544f in start_thread nptl/pthread_create.c:473
```
The reason is that `my_thread_cancel` is used to finish the daemon thread. This is not and orderly way of finishing the thread. ASAN does not register the stack variables are not used anymore which generates the error above.
This is a benign error as all the variables are on the stack.
*Solution*:
Finish the thread in orderly way by using a signalling variable.
inikep
pushed a commit
that referenced
this pull request
Nov 29, 2022
-- Patch #1: Persist secondary load information -- Problem: We need a way of knowing which tables were loaded to HeatWave after MySQL restarts due to a crash or a planned shutdown. Solution: Add a new "secondary_load" flag to the `options` column of mysql.tables. This flag is toggled after a successful secondary load or unload. The information about this flag is also reflected in INFORMATION_SCHEMA.TABLES.CREATE_OPTIONS. -- Patch #2 -- The second patch in this worklog triggers the table reload from InnoDB after MySQL restart. The recovery framework recognizes that the system restarted by checking whether tables are present in the Global State. If there are no tables present, the framework will access the Data Dictionary and find which tables were loaded before the restart. This patch introduces the "Data Dictionary Worker" - a MySQL service recovery worker whose task is to query the INFORMATION_SCHEMA.TABLES table from a separate thread and find all tables whose secondary_load flag is set to 1. All tables that were found in the Data Dictionary will be appended to the list of tables that have to be reloaded by the framework from InnoDB. If an error occurs during restart recovery we will not mark the recovery as failed. This is done because the types of failures that can occur when the tables are reloaded after a restart are less critical compared to previously existing recovery situations. Additionally, this code will soon have to be adapted for the next worklog in this area so we are proceeding with the simplest solution that makes sense. A Global Context variable m_globalStateEmpty is added which indicates whether the Global State should be recovered from an external source. -- Patch #3 -- This patch adds the "rapid_reload_on_restart" system variable. This variable is used to control whether tables should be reloaded after a restart of mysqld or the HeatWave plugin. This variable is persistable (i.e., SET PERSIST RAPID_RELOAD_ON_RESTART = TRUE/FALSE). The default value of this variable is set to false. The variable can be modified in OFF, IDLE, and SUSPENDED states. -- Patch #4 -- This patch refactors the recovery code by removing all recovery-related code from ha_rpd.cc and moving it to separate files: - ha_rpd_session_factory.h/cc: These files contain the MySQLAdminSessionFactory class, which is used to create admin sessions in separate threads that can be used to issue SQL queries. - ha_rpd_recovery.h/cc: These files contain the MySQLServiceRecoveryWorker, MySQLServiceRecoveryJob and ObjectStoreRecoveryJob classes which were previously defined in ha_rpd.cc. This file also contains a function that creates the RecoveryWorkerFactory object. This object is passed to the constructor of the Recovery Framework and is used to communicate with the other section of the code located in rpdrecoveryfwk.h/cc. This patch also renames rpdrecvryfwk to rpdrecoveryfwk for better readability. The include relationship between the files is shown on the following diagram: rpdrecoveryfwk.h◄──────────────rpdrecoveryfwk.cc ▲ ▲ │ │ │ │ │ └──────────────────────────┐ │ │ ha_rpd_recovery.h◄─────────────ha_rpd_recovery.cc──┐ ▲ │ │ │ │ │ │ │ │ │ ▼ │ ha_rpd.cc───────────────────────►ha_rpd.h │ ▲ │ │ │ ┌───────────────────────────────┘ │ │ ▼ ha_rpd_session_factory.cc──────►ha_rpd_session_factory.h Other changes: - In agreement with Control Plane, the external Global State is now invalidated during recovery framework startup if: 1) Recovery framework recognizes that it should load the Global State from an external source AND, 2) rapid_reload_on_restart is set to OFF. - Addressed review comments for Patch #3, rapid_reload_on_restart is now also settable while plugin is ON. - Provide a single entry point for processing external Global State before starting the recovery framework loop. - Change when the Data Dictionary is read. Now we will no longer wait for the HeatWave nodes to connect before querying the Data Dictionary. We will query it when the recovery framework starts, before accepting any actions in the recovery loop. - Change the reload flow by inserting fake global state entries for tables that need to be reloaded instead of manually adding them to a list of tables scheduled for reload. This method will be used for the next phase where we will recover from Object Storage so both recovery methods will now follow the same flow. - Update secondary_load_dd_flag added in Patch #1. - Increase timeout in wait_for_server_bootup to 300s to account for long MySQL version upgrades. - Add reload_on_restart and reload_on_restart_dbg tests to the rapid suite. - Add PLUGIN_VAR_PERSIST_AS_READ_ONLY flag to "rapid_net_orma_port" and "rapid_reload_on_restart" definitions, enabling their initialization from persisted values along with "rapid_bootstrap" when it is persisted as ON. - Fix numerous clang-tidy warnings in recovery code. - Prevent suspended_basic and secondary_load_dd_flag tests to run on ASAN builds due to an existing issue when reinstalling the RAPID plugin. -- Bug#33752387 -- Problem: A shutdown of MySQL causes a crash in queries fired by DD worker. Solution: Prevent MySQL from killing DD worker's queries by instantiating a DD_kill_immunizer before the queries are fired. -- Patch #5 -- Problem: A table can be loaded before the DD Worker queries the Data Dictionary. This means that table will be wrongly processed as part of the external global state. Solution: If the table is present in the current in-memory global state we will not consider it as part of the external global state and we will not process it by the recovery framework. -- Bug#34197659 -- Problem: If a table reload after restart causes OOM the cluster will go into RECOVERYFAILED state. Solution: Recognize when the tables are being reloaded after restart and do not move the cluster into RECOVERYFAILED. In that case only the current reload will fail and the reload for other tables will be attempted. Change-Id: Ic0c2a763bc338ea1ae6a7121ff3d55b456271bf0
inikep
pushed a commit
that referenced
this pull request
Nov 29, 2022
**Problem:**
The tests fail under ASAN:
```
==470513==ERROR: AddressSanitizer: heap-use-after-free on address 0x632000054e20 at pc 0x556599b68016 bp 0x7ffc630afb30 sp 0x7ffc630afb20
READ of size 8 at 0x632000054e20 thread T0
#0 0x556599b68015 in destroy_rwlock(PFS_rwlock*) /tmp/ps/storage/perfschema/pfs_instr.cc:430
#1 0x556599b30b82 in pfs_destroy_rwlock_v2(PSI_rwlock*) /tmp/ps/storage/perfschema/pfs.cc:2596
#2 0x7fa44336d62e in inline_mysql_rwlock_destroy /tmp/ps/include/mysql/psi/mysql_rwlock.h:289
#3 0x7fa44336da39 in vtoken_lock_cleanup::~vtoken_lock_cleanup() /tmp/ps/plugin/version_token/version_token.cc:517
#4 0x7fa46a7188a6 in __run_exit_handlers /build/glibc-SzIz7B/glibc-2.31/stdlib/exit.c:108
#5 0x7fa46a718a5f in __GI_exit /build/glibc-SzIz7B/glibc-2.31/stdlib/exit.c:139
#6 0x556596531da2 in mysqld_exit /tmp/ps/sql/mysqld.cc:2512
#7 0x55659655d579 in mysqld_main(int, char**) /tmp/ps/sql/mysqld.cc:8505
#8 0x55659609c5b5 in main /tmp/ps/sql/main.cc:25
#9 0x7fa46a6f6082 in __libc_start_main ../csu/libc-start.c:308
#10 0x55659609c4ed in _start (/tmp/results/PS/runtime_output_directory/mysqld+0x3c1b4ed)
0x632000054e20 is located 50720 bytes inside of 90112-byte region [0x632000048800,0x63200005e800)
freed by thread T0 here:
#0 0x7fa46b5f940f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:122
#1 0x556599b617eb in pfs_free(PFS_builtin_memory_class*, unsigned long, void*) /tmp/ps/storage/perfschema/pfs_global.cc:113
#2 0x556599b61a15 in pfs_free_array(PFS_builtin_memory_class*, unsigned long, unsigned long, void*) /tmp/ps/storage/perfschema/pfs_global.cc:177
#3 0x556599b6f28b in PFS_buffer_default_allocator<PFS_rwlock>::free_array(PFS_buffer_default_array<PFS_rwlock>*) /tmp/ps/storage/perfschema/pfs_buffer_container.h:172
#4 0x556599b75628 in PFS_buffer_scalable_container<PFS_rwlock, 1024, 1024, PFS_buffer_default_array<PFS_rwlock>, PFS_buffer_default_allocator<PFS_rwlock> >::cleanup() /tmp/ps/storage/perfschema/pfs_buffer_container.h:452
#5 0x556599b6d591 in cleanup_instruments() /tmp/ps/storage/perfschema/pfs_instr.cc:231
#6 0x556599b8c3f1 in cleanup_performance_schema /tmp/ps/storage/perfschema/pfs_server.cc:343
#7 0x556599b8dcfc in shutdown_performance_schema() /tmp/ps/storage/perfschema/pfs_server.cc:374
#8 0x556596531d96 in mysqld_exit /tmp/ps/sql/mysqld.cc:2500
#9 0x55659655d579 in mysqld_main(int, char**) /tmp/ps/sql/mysqld.cc:8505
#10 0x55659609c5b5 in main /tmp/ps/sql/main.cc:25
#11 0x7fa46a6f6082 in __libc_start_main ../csu/libc-start.c:308
previously allocated by thread T0 here:
#0 0x7fa46b5fa6e5 in __interceptor_posix_memalign ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:217
#1 0x556599b6167e in pfs_malloc(PFS_builtin_memory_class*, unsigned long, int) /tmp/ps/storage/perfschema/pfs_global.cc:68
#2 0x556599b6187a in pfs_malloc_array(PFS_builtin_memory_class*, unsigned long, unsigned long, int) /tmp/ps/storage/perfschema/pfs_global.cc:155
#3 0x556599b6fa9e in PFS_buffer_default_allocator<PFS_rwlock>::alloc_array(PFS_buffer_default_array<PFS_rwlock>*) /tmp/ps/storage/perfschema/pfs_buffer_container.h:159
#4 0x556599b6ff12 in PFS_buffer_scalable_container<PFS_rwlock, 1024, 1024, PFS_buffer_default_array<PFS_rwlock>, PFS_buffer_default_allocator<PFS_rwlock> >::allocate(pfs_dirty_state*) /tmp/ps/storage/perfschema/pfs_buffer_container.h:602
#5 0x556599b69abc in create_rwlock(PFS_rwlock_class*, void const*) /tmp/ps/storage/perfschema/pfs_instr.cc:402
#6 0x556599b341f5 in pfs_init_rwlock_v2(unsigned int, void const*) /tmp/ps/storage/perfschema/pfs.cc:2578
#7 0x556599b9487b in inline_mysql_rwlock_init /tmp/ps/include/mysql/psi/mysql_rwlock.h:261
#8 0x556599b94ba7 in init_pfs_tls_channels_instrumentation() /tmp/ps/storage/perfschema/pfs_tls_channel.cc:209
#9 0x556599b8ca44 in initialize_performance_schema(PFS_global_param*, PSI_thread_bootstrap**, PSI_mutex_bootstrap**, PSI_rwlock_bootstrap**, PSI_cond_bootstrap**, PSI_file_bootstrap**, PSI_socket_bootstrap**, PSI_table_bootstrap**, PSI_mdl_bootstrap**, PSI_idle_bootstrap**, PSI_stage_bootstrap**, PSI_statement_bootstrap**, PSI_transaction_bootstrap**, PSI_memory_bootstrap**, PSI_error_bootstrap**, PSI_data_lock_bootstrap**, PSI_system_bootstrap**, PSI_tls_channel_bootstrap**) /tmp/ps/storage/perfschema/pfs_server.cc:266
#10 0x55659655a585 in mysqld_main(int, char**) /tmp/ps/sql/mysqld.cc:7497
#11 0x55659609c5b5 in main /tmp/ps/sql/main.cc:25
#12 0x7fa46a6f6082 in __libc_start_main ../csu/libc-start.c:308
SUMMARY: AddressSanitizer: heap-use-after-free /tmp/ps/storage/perfschema/pfs_instr.cc:430 in destroy_rwlock(PFS_rwlock*)
Shadow bytes around the buggy address:
0x0c6480002970: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c6480002980: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c6480002990: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c64800029a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c64800029b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x0c64800029c0: fd fd fd fd[fd]fd fd fd fd fd fd fd fd fd fd fd
0x0c64800029d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c64800029e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c64800029f0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c6480002a00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c6480002a10: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
Shadow gap: cc
==470513==ABORTING
```
The reason of the error is Percona's change on
5ae4d27 which causes the static
variables of the plugin not to be deallocated.
This causes `void cleanup_instruments()` to be called before
`vtoken_lock_cleanup::~vtoken_lock_cleanup()`, which finds
the memory of the object to have been deallocated.
**Solution:**
Do not run the tests under ASAN or Valgrind.
inikep
pushed a commit
that referenced
this pull request
Nov 29, 2022
**Problem:**
The following leak is detected when running the test
`encryption.upgrade_crypt_data_57_v1`:
```
==388399==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 70 byte(s) in 1 object(s) allocated from:
#0 0x7f5f87812808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
#1 0x55f098875d2c in ut::detail::malloc(unsigned long) /home/ldonoso/src/release-8.0.29-20/storage/innobase/include/detail/ut/allocator_traits.h:71
#2 0x55f098875db5 in ut::detail::Alloc_fn::malloc(unsigned long) /home/ldonoso/src/release-8.0.29-20/storage/innobase/include/detail/ut/allocator_traits.h:88
#3 0x55f0988aa4b9 in void* ut::detail::Alloc_fn::alloc<false>(unsigned long) /home/ldonoso/src/release-8.0.29-20/storage/innobase/include/detail/ut/allocator_traits.h:97
#4 0x55f09889b7a3 in void* ut::detail::Alloc_pfs::alloc<false>(unsigned long, unsigned int) /home/ldonoso/src/release-8.0.29-20/storage/innobase/include/detail/ut/alloc.h:275
#5 0x55f09889bb9a in std::enable_if<ut::detail::Alloc_pfs::is_pfs_instrumented_v, void*>::type ut::detail::Alloc_<ut::detail::Alloc_pfs>::alloc<false, ut::detail::Alloc_pfs>(unsigned long, unsigned int) /home/ldonoso/src/release-8.0.29-20/storage/innobase/include/detail/ut/alloc.h:438
#6 0x55f0988767dd in ut::malloc_withkey(ut::PSI_memory_key_t, unsigned long) /home/ldonoso/src/release-8.0.29-20/storage/innobase/include/ut0new.h:604
#7 0x55f09937dd3c in rec_copy_prefix_to_buf_old /home/ldonoso/src/release-8.0.29-20/storage/innobase/rem/rem0rec.cc:1206
#8 0x55f09937dfd3 in rec_copy_prefix_to_buf(unsigned char const*, dict_index_t const*, unsigned long, unsigned char**, unsigned long*) /home/ldonoso/src/release-8.0.29-20/storage/innobase/rem/rem0rec.cc:1233
#9 0x55f098ae0ae3 in dict_index_copy_rec_order_prefix(dict_index_t const*, unsigned char const*, unsigned long*, unsigned char**, unsigned long*) /home/ldonoso/src/release-8.0.29-20/storage/innobase/dict/dict0dict.cc:3764
#10 0x55f098c3d0ba in btr_pcur_t::store_position(mtr_t*) /home/ldonoso/src/release-8.0.29-20/storage/innobase/btr/btr0pcur.cc:141
#11 0x55f098c027b6 in dict_getnext_system_low /home/ldonoso/src/release-8.0.29-20/storage/innobase/dict/dict0load.cc:256
#12 0x55f098c02933 in dict_getnext_system(btr_pcur_t*, mtr_t*) /home/ldonoso/src/release-8.0.29-20/storage/innobase/dict/dict0load.cc:298
percona#13 0x55f098c0c05b in dict_check_sys_tables /home/ldonoso/src/release-8.0.29-20/storage/innobase/dict/dict0load.cc:1573
percona#14 0x55f098c1770d in dict_load_tablespaces_for_upgrade() /home/ldonoso/src/release-8.0.29-20/storage/innobase/dict/dict0load.cc:3233
percona#15 0x55f0987e9ed1 in innobase_init_files /home/ldonoso/src/release-8.0.29-20/storage/innobase/handler/ha_innodb.cc:6072
percona#16 0x55f098819ed3 in innobase_ddse_dict_init /home/ldonoso/src/release-8.0.29-20/storage/innobase/handler/ha_innodb.cc:13985
percona#17 0x55f097fa5c10 in dd::bootstrap::DDSE_dict_init(THD*, dict_init_mode_t, unsigned int) /home/ldonoso/src/release-8.0.29-20/sql/dd/impl/bootstrap/bootstrapper.cc:742
percona#18 0x55f0986696a6 in dd::upgrade_57::do_pre_checks_and_initialize_dd(THD*) /home/ldonoso/src/release-8.0.29-20/sql/dd/upgrade_57/upgrade.cc:922
percona#19 0x55f09550e082 in handle_bootstrap /home/ldonoso/src/release-8.0.29-20/sql/bootstrap.cc:327
percona#20 0x55f0997416e7 in pfs_spawn_thread /home/ldonoso/src/release-8.0.29-20/storage/perfschema/pfs.cc:2943
percona#21 0x7f5f876a1608 in start_thread /build/glibc-SzIz7B/glibc-2.31/nptl/pthread_create.c:477
SUMMARY: AddressSanitizer: 70 byte(s) leaked in 1 allocation(s).
```
**Solution:**
The cause of the leak raises from the traversing of `pcur`. When
traversing is exhausted `pcur.close()` is automatically called and all
`pcur` resources are deallocated.
Percona adds some early returns to the traverse, hence sometimes the
traversing is not exhausted and `pcur.close()` is not called.
The solution is calling `pcur.close()` explicitly. `close()` is an
idempotent function so it is not a bug if it is called several times as
a result of this change.
inikep
pushed a commit
that referenced
this pull request
Jan 12, 2023
Upstream commit ID : fb-mysql-5.6.35/8cb1dc836b68f1f13e8b2655b2b8cb2d57f400b3 PS-5217 : Merge fb-prod201803 Summary: Original report: https://jira.mariadb.org/browse/MDEV-15816 To reproduce this bug just following below steps, client 1: USE test; CREATE TABLE t1 (i INT) ENGINE=MyISAM; HANDLER t1 OPEN h; CREATE TABLE t2 (i INT) ENGINE=RocksDB; LOCK TABLES t2 WRITE; client 2: FLUSH TABLES WITH READ LOCK; client 1: INSERT INTO t2 VALUES (1); So client 1 acquired the lock and set m_lock_rows = RDB_LOCK_WRITE. Then client 2 calls store_lock(TL_IGNORE) and m_lock_rows was wrongly set to RDB_LOCK_NONE, as below ``` #0 myrocks::ha_rocksdb::store_lock (this=0x7fffbc03c7c8, thd=0x7fffc0000ba0, to=0x7fffc0011220, lock_type=TL_IGNORE) #1 get_lock_data (thd=0x7fffc0000ba0, table_ptr=0x7fffe84b7d20, count=1, flags=2) #2 mysql_lock_abort_for_thread (thd=0x7fffc0000ba0, table=0x7fffbc03bbc0) #3 THD::notify_shared_lock (this=0x7fffc0000ba0, ctx_in_use=0x7fffbc000bd8, needs_thr_lock_abort=true) #4 MDL_lock::notify_conflicting_locks (this=0x555557a82380, ctx=0x7fffc0000cc8) #5 MDL_context::acquire_lock (this=0x7fffc0000cc8, mdl_request=0x7fffe84b8350, lock_wait_timeout=2) #6 Global_read_lock::lock_global_read_lock (this=0x7fffc0003fe0, thd=0x7fffc0000ba0) ``` Finally, client 1 "INSERT INTO..." hits the Assertion 'm_lock_rows == RDB_LOCK_WRITE' failed in myrocks::ha_rocksdb::write_row() Fix this bug by not setting m_locks_rows if lock_type == TL_IGNORE. Closes facebook/mysql-5.6#838 Pull Request resolved: facebook/mysql-5.6#871 Differential Revision: D9417382 Pulled By: lth fbshipit-source-id: c36c164e06c
inikep
pushed a commit
that referenced
this pull request
Jan 12, 2023
Upstream commit ID : fb-mysql-5.6.35/77032004ad23d21a4c386f8136ecfbb071ea42d6 PS-6865 : Merge fb-prod201903 Summary: Currently during primary key's value encode, its ttl value can be from either one of these 3 cases 1. ttl column in primary key 2. non-ttl column a. old record(update case) b. current timestamp 3. ttl column in non-key field Workflow #1: first in Rdb_key_def::pack_record() find and store pk_offset, then in value encode try to parse key slice to fetch ttl value by using pk_offset. Workflow #3: fetch ttl value from ttl column The change is to merge #1 and #3 by always fetching TTL value from ttl column, not matter whether the ttl column is in primary key or not. Of course, remove pk_offset, since it isn't used. BTW, for secondary keys, its ttl value is always from m_ttl_bytes, which is stored by primary value encoding. Reviewed By: yizhang82 Differential Revision: D14662716 fbshipit-source-id: 6b4e5f044fd
inikep
pushed a commit
that referenced
this pull request
Jan 12, 2023
Upstream commit ID : fb-mysql-5.6.35/e025cf1c47e63aada985d78e4083f2e02fba434f
PS-7731 : Merge percona-202102
Summary:
Today in `SELECT count(*)` MyRocks would still decode every single column due to this check, despite the readset being empty:
```
// bitmap is cleared on index merge, but it still needs to decode columns
bool field_requested =
decode_all_fields || m_verify_row_debug_checksums ||
bitmap_is_set(field_map, m_table->field[i]->field_index);
```
As a result MyRocks is significantly slower than InnoDB in this particular scenario.
Turns out in index merge, when it tries to reset, it calls ha_index_init with an empty column_bitmap, so our field decoders didn't know it needs to decode anything, so the entire query would return nothing. This is discussed in [this commit](facebook/mysql-5.6@70f2bcd), and [issue 624](facebook/mysql-5.6#624) and [PR 626](facebook/mysql-5.6#626). So the workaround we had at that time is to simply treat empty map as implicitly everything, and the side effect is massively slowed down count(*).
We have a few options to address this:
1. Fix index merge optimizer - looking at the code in QUICK_RANGE_SELECT::init_ror_merged_scan, it actually fixes up the column_bitmap properly, but after init/reset, so the fix would simply be moving the bitmap set code up. For secondary keys, prepare_for_position will automatically call `mark_columns_used_by_index_no_reset(s->primary_key, read_set)` if HA_PRIMARY_KEY_REQUIRED_FOR_POSITION is set (true for both InnoDB and MyRocks), so we would know correctly that we need to unpack PK when walking SK during index merge.
2. Overriding `column_bitmaps_signal` and setup decoders whenever the bitmap changes - however this doesn't work by itself. Because no storage engine today actually use handler::column_bitmaps_signal this path haven't been tested properly in index merge. In this case, QUICK_RANGE_SELECT::init_ror_merged_scan should call set_column_bitmaps_no_signal to avoid resetting the correct read/write set of head since head is used as first handler (reuses_handler=true) and subsequent place holders for read/write set updates (reuse_handler=false).
3. Follow InnoDB's solution - InnoDB delays it actually initialize its template again in index_read for the 2nd time (relying on `prebuilt->sql_stat_start`), and during index_read `QUICK_RANGE_SELECT::column_bitmap` is already fixed up and the table read/write set is switched to it, so the new template would be built correctly.
In order to make it easier to maintain and port, after discussing with Manuel, I'm going with a simplified version of #3 that delays decoder creation until the first read operation (index_*, rnd_*, range_read_*, multi_range_read_*), and setting the delay flag in index_init / rnd_init / multi_range_read_init.
Also, I ran into a bug with truncation_partition where Rdb_converter's tbl_def is stale (we only update ha_rocksdb::m_tbl_def), but it is fine because it is not being used after table open. But my change moves the lookup_bitmap initialization into Rdb_converter which takes a dependency on Rdb_converter::m_tbl_def so now we need to reset it properly.
Reference Patch: facebook/mysql-5.6@44d6a8d
---------
Porting Note: Due to 8.0's new counting infra (handler::record & handler::record_with_index), this only helps PK counting. Will send out a better fix that works better with 8.0 new counting infra.
Reviewed By: Pushapgl
Differential Revision: D26265470
fbshipit-source-id: f142be681ab
inikep
added a commit
that referenced
this pull request
Jan 12, 2023
PS-5741: Incorrect use of memset_s in keyring_vault.
Fixed the usage of memset_s. The arguments should be:
void memset_s(void *dest, size_t dest_max, int c, size_t n)
where the 2nd argument is size of buffer and the 3rd is
argument is character to fill.
---------------------------------------------------------------------------
PS-7769 - Fix use-after-return error in audit_log_exclude_accounts_validate
---
*Problem:*
`st_mysql_value::val_str` might return a pointer to `buf` which after
the function called is deleted. Therefore the value in `save`, after
reuturnin from the function, is invalid.
In this particular case, the error is not manifesting as val_str`
returns memory allocated with `thd_strmake` and it does not use `buf`.
*Solution:*
Allocate memory with `thd_strmake` so the memory in `save` is not local.
---------------------------------------------------------------------------
Fix test main.bug12969156 when WITH_ASAN=ON
*Problem:*
ASAN complains about stack-buffer-overflow on function `mysql_heartbeat`:
```
==90890==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fe746d06d14 at pc 0x7fe760f5b017 bp 0x7fe746d06cd0 sp 0x7fe746d06478
WRITE of size 24 at 0x7fe746d06d14 thread T16777215
Address 0x7fe746d06d14 is located in stack of thread T26 at offset 340 in frame
#0 0x7fe746d0a55c in mysql_heartbeat(void*) /home/yura/ws/percona-server/plugin/daemon_example/daemon_example.cc:62
This frame has 4 object(s):
[48, 56) 'result' (line 66)
[80, 112) '_db_stack_frame_' (line 63)
[144, 200) 'tm_tmp' (line 67)
[240, 340) 'buffer' (line 65) <== Memory access at offset 340 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
(longjmp and C++ exceptions *are* supported)
Thread T26 created by T25 here:
#0 0x7fe760f5f6d5 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:216
#1 0x557ccbbcb857 in my_thread_create /home/yura/ws/percona-server/mysys/my_thread.c:104
#2 0x7fe746d0b21a in daemon_example_plugin_init /home/yura/ws/percona-server/plugin/daemon_example/daemon_example.cc:148
#3 0x557ccb4c69c7 in plugin_initialize /home/yura/ws/percona-server/sql/sql_plugin.cc:1279
#4 0x557ccb4d19cd in mysql_install_plugin /home/yura/ws/percona-server/sql/sql_plugin.cc:2279
#5 0x557ccb4d218f in Sql_cmd_install_plugin::execute(THD*) /home/yura/ws/percona-server/sql/sql_plugin.cc:4664
#6 0x557ccb47695e in mysql_execute_command(THD*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5160
#7 0x557ccb47977c in mysql_parse(THD*, Parser_state*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5952
#8 0x557ccb47b6c2 in dispatch_command(THD*, COM_DATA const*, enum_server_command) /home/yura/ws/percona-server/sql/sql_parse.cc:1544
#9 0x557ccb47de1d in do_command(THD*) /home/yura/ws/percona-server/sql/sql_parse.cc:1065
#10 0x557ccb6ac294 in handle_connection /home/yura/ws/percona-server/sql/conn_handler/connection_handler_per_thread.cc:325
#11 0x557ccbbfabb0 in pfs_spawn_thread /home/yura/ws/percona-server/storage/perfschema/pfs.cc:2198
#12 0x7fe760ab544f in start_thread nptl/pthread_create.c:473
```
The reason is that `my_thread_cancel` is used to finish the daemon thread. This is not and orderly way of finishing the thread. ASAN does not register the stack variables are not used anymore which generates the error above.
This is a benign error as all the variables are on the stack.
*Solution*:
Finish the thread in orderly way by using a signalling variable.
---------------------------------------------------------------------------
PS-8204: Fix XML escape rules for audit plugin
https://jira.percona.com/browse/PS-8204
There was a wrong length specified for some XML
escape rules. As a result of this terminating null symbol from
replacement rule was copied into resulting string. This lead to
quer text truncation in audit log file.
In addition added empty replacement rules for '\b' and 'f' symbols
which just remove them from resulting string. These symboles are
not supported in XML 1.0.
inikep
pushed a commit
that referenced
this pull request
Jan 16, 2023
Upstream commit ID : fb-mysql-5.6.35/8cb1dc836b68f1f13e8b2655b2b8cb2d57f400b3 PS-5217 : Merge fb-prod201803 Summary: Original report: https://jira.mariadb.org/browse/MDEV-15816 To reproduce this bug just following below steps, client 1: USE test; CREATE TABLE t1 (i INT) ENGINE=MyISAM; HANDLER t1 OPEN h; CREATE TABLE t2 (i INT) ENGINE=RocksDB; LOCK TABLES t2 WRITE; client 2: FLUSH TABLES WITH READ LOCK; client 1: INSERT INTO t2 VALUES (1); So client 1 acquired the lock and set m_lock_rows = RDB_LOCK_WRITE. Then client 2 calls store_lock(TL_IGNORE) and m_lock_rows was wrongly set to RDB_LOCK_NONE, as below ``` #0 myrocks::ha_rocksdb::store_lock (this=0x7fffbc03c7c8, thd=0x7fffc0000ba0, to=0x7fffc0011220, lock_type=TL_IGNORE) #1 get_lock_data (thd=0x7fffc0000ba0, table_ptr=0x7fffe84b7d20, count=1, flags=2) #2 mysql_lock_abort_for_thread (thd=0x7fffc0000ba0, table=0x7fffbc03bbc0) #3 THD::notify_shared_lock (this=0x7fffc0000ba0, ctx_in_use=0x7fffbc000bd8, needs_thr_lock_abort=true) #4 MDL_lock::notify_conflicting_locks (this=0x555557a82380, ctx=0x7fffc0000cc8) #5 MDL_context::acquire_lock (this=0x7fffc0000cc8, mdl_request=0x7fffe84b8350, lock_wait_timeout=2) #6 Global_read_lock::lock_global_read_lock (this=0x7fffc0003fe0, thd=0x7fffc0000ba0) ``` Finally, client 1 "INSERT INTO..." hits the Assertion 'm_lock_rows == RDB_LOCK_WRITE' failed in myrocks::ha_rocksdb::write_row() Fix this bug by not setting m_locks_rows if lock_type == TL_IGNORE. Closes facebook/mysql-5.6#838 Pull Request resolved: facebook/mysql-5.6#871 Differential Revision: D9417382 Pulled By: lth fbshipit-source-id: c36c164e06c
inikep
pushed a commit
that referenced
this pull request
Jan 16, 2023
Upstream commit ID : fb-mysql-5.6.35/77032004ad23d21a4c386f8136ecfbb071ea42d6 PS-6865 : Merge fb-prod201903 Summary: Currently during primary key's value encode, its ttl value can be from either one of these 3 cases 1. ttl column in primary key 2. non-ttl column a. old record(update case) b. current timestamp 3. ttl column in non-key field Workflow #1: first in Rdb_key_def::pack_record() find and store pk_offset, then in value encode try to parse key slice to fetch ttl value by using pk_offset. Workflow #3: fetch ttl value from ttl column The change is to merge #1 and #3 by always fetching TTL value from ttl column, not matter whether the ttl column is in primary key or not. Of course, remove pk_offset, since it isn't used. BTW, for secondary keys, its ttl value is always from m_ttl_bytes, which is stored by primary value encoding. Reviewed By: yizhang82 Differential Revision: D14662716 fbshipit-source-id: 6b4e5f044fd
inikep
pushed a commit
that referenced
this pull request
Jan 16, 2023
Upstream commit ID : fb-mysql-5.6.35/e025cf1c47e63aada985d78e4083f2e02fba434f
PS-7731 : Merge percona-202102
Summary:
Today in `SELECT count(*)` MyRocks would still decode every single column due to this check, despite the readset being empty:
```
// bitmap is cleared on index merge, but it still needs to decode columns
bool field_requested =
decode_all_fields || m_verify_row_debug_checksums ||
bitmap_is_set(field_map, m_table->field[i]->field_index);
```
As a result MyRocks is significantly slower than InnoDB in this particular scenario.
Turns out in index merge, when it tries to reset, it calls ha_index_init with an empty column_bitmap, so our field decoders didn't know it needs to decode anything, so the entire query would return nothing. This is discussed in [this commit](facebook/mysql-5.6@70f2bcd), and [issue 624](facebook/mysql-5.6#624) and [PR 626](facebook/mysql-5.6#626). So the workaround we had at that time is to simply treat empty map as implicitly everything, and the side effect is massively slowed down count(*).
We have a few options to address this:
1. Fix index merge optimizer - looking at the code in QUICK_RANGE_SELECT::init_ror_merged_scan, it actually fixes up the column_bitmap properly, but after init/reset, so the fix would simply be moving the bitmap set code up. For secondary keys, prepare_for_position will automatically call `mark_columns_used_by_index_no_reset(s->primary_key, read_set)` if HA_PRIMARY_KEY_REQUIRED_FOR_POSITION is set (true for both InnoDB and MyRocks), so we would know correctly that we need to unpack PK when walking SK during index merge.
2. Overriding `column_bitmaps_signal` and setup decoders whenever the bitmap changes - however this doesn't work by itself. Because no storage engine today actually use handler::column_bitmaps_signal this path haven't been tested properly in index merge. In this case, QUICK_RANGE_SELECT::init_ror_merged_scan should call set_column_bitmaps_no_signal to avoid resetting the correct read/write set of head since head is used as first handler (reuses_handler=true) and subsequent place holders for read/write set updates (reuse_handler=false).
3. Follow InnoDB's solution - InnoDB delays it actually initialize its template again in index_read for the 2nd time (relying on `prebuilt->sql_stat_start`), and during index_read `QUICK_RANGE_SELECT::column_bitmap` is already fixed up and the table read/write set is switched to it, so the new template would be built correctly.
In order to make it easier to maintain and port, after discussing with Manuel, I'm going with a simplified version of #3 that delays decoder creation until the first read operation (index_*, rnd_*, range_read_*, multi_range_read_*), and setting the delay flag in index_init / rnd_init / multi_range_read_init.
Also, I ran into a bug with truncation_partition where Rdb_converter's tbl_def is stale (we only update ha_rocksdb::m_tbl_def), but it is fine because it is not being used after table open. But my change moves the lookup_bitmap initialization into Rdb_converter which takes a dependency on Rdb_converter::m_tbl_def so now we need to reset it properly.
Reference Patch: facebook/mysql-5.6@44d6a8d
---------
Porting Note: Due to 8.0's new counting infra (handler::record & handler::record_with_index), this only helps PK counting. Will send out a better fix that works better with 8.0 new counting infra.
Reviewed By: Pushapgl
Differential Revision: D26265470
fbshipit-source-id: f142be681ab
inikep
added a commit
that referenced
this pull request
Jan 16, 2023
PS-5741: Incorrect use of memset_s in keyring_vault.
Fixed the usage of memset_s. The arguments should be:
void memset_s(void *dest, size_t dest_max, int c, size_t n)
where the 2nd argument is size of buffer and the 3rd is
argument is character to fill.
---------------------------------------------------------------------------
PS-7769 - Fix use-after-return error in audit_log_exclude_accounts_validate
---
*Problem:*
`st_mysql_value::val_str` might return a pointer to `buf` which after
the function called is deleted. Therefore the value in `save`, after
reuturnin from the function, is invalid.
In this particular case, the error is not manifesting as val_str`
returns memory allocated with `thd_strmake` and it does not use `buf`.
*Solution:*
Allocate memory with `thd_strmake` so the memory in `save` is not local.
---------------------------------------------------------------------------
Fix test main.bug12969156 when WITH_ASAN=ON
*Problem:*
ASAN complains about stack-buffer-overflow on function `mysql_heartbeat`:
```
==90890==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fe746d06d14 at pc 0x7fe760f5b017 bp 0x7fe746d06cd0 sp 0x7fe746d06478
WRITE of size 24 at 0x7fe746d06d14 thread T16777215
Address 0x7fe746d06d14 is located in stack of thread T26 at offset 340 in frame
#0 0x7fe746d0a55c in mysql_heartbeat(void*) /home/yura/ws/percona-server/plugin/daemon_example/daemon_example.cc:62
This frame has 4 object(s):
[48, 56) 'result' (line 66)
[80, 112) '_db_stack_frame_' (line 63)
[144, 200) 'tm_tmp' (line 67)
[240, 340) 'buffer' (line 65) <== Memory access at offset 340 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
(longjmp and C++ exceptions *are* supported)
Thread T26 created by T25 here:
#0 0x7fe760f5f6d5 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:216
#1 0x557ccbbcb857 in my_thread_create /home/yura/ws/percona-server/mysys/my_thread.c:104
#2 0x7fe746d0b21a in daemon_example_plugin_init /home/yura/ws/percona-server/plugin/daemon_example/daemon_example.cc:148
#3 0x557ccb4c69c7 in plugin_initialize /home/yura/ws/percona-server/sql/sql_plugin.cc:1279
#4 0x557ccb4d19cd in mysql_install_plugin /home/yura/ws/percona-server/sql/sql_plugin.cc:2279
#5 0x557ccb4d218f in Sql_cmd_install_plugin::execute(THD*) /home/yura/ws/percona-server/sql/sql_plugin.cc:4664
#6 0x557ccb47695e in mysql_execute_command(THD*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5160
#7 0x557ccb47977c in mysql_parse(THD*, Parser_state*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5952
#8 0x557ccb47b6c2 in dispatch_command(THD*, COM_DATA const*, enum_server_command) /home/yura/ws/percona-server/sql/sql_parse.cc:1544
#9 0x557ccb47de1d in do_command(THD*) /home/yura/ws/percona-server/sql/sql_parse.cc:1065
#10 0x557ccb6ac294 in handle_connection /home/yura/ws/percona-server/sql/conn_handler/connection_handler_per_thread.cc:325
#11 0x557ccbbfabb0 in pfs_spawn_thread /home/yura/ws/percona-server/storage/perfschema/pfs.cc:2198
#12 0x7fe760ab544f in start_thread nptl/pthread_create.c:473
```
The reason is that `my_thread_cancel` is used to finish the daemon thread. This is not and orderly way of finishing the thread. ASAN does not register the stack variables are not used anymore which generates the error above.
This is a benign error as all the variables are on the stack.
*Solution*:
Finish the thread in orderly way by using a signalling variable.
---------------------------------------------------------------------------
PS-8204: Fix XML escape rules for audit plugin
https://jira.percona.com/browse/PS-8204
There was a wrong length specified for some XML
escape rules. As a result of this terminating null symbol from
replacement rule was copied into resulting string. This lead to
quer text truncation in audit log file.
In addition added empty replacement rules for '\b' and 'f' symbols
which just remove them from resulting string. These symboles are
not supported in XML 1.0.
inikep
added a commit
that referenced
this pull request
Jan 24, 2023
PS-5741: Incorrect use of memset_s in keyring_vault.
Fixed the usage of memset_s. The arguments should be:
void memset_s(void *dest, size_t dest_max, int c, size_t n)
where the 2nd argument is size of buffer and the 3rd is
argument is character to fill.
---------------------------------------------------------------------------
PS-7769 - Fix use-after-return error in audit_log_exclude_accounts_validate
---
*Problem:*
`st_mysql_value::val_str` might return a pointer to `buf` which after
the function called is deleted. Therefore the value in `save`, after
reuturnin from the function, is invalid.
In this particular case, the error is not manifesting as val_str`
returns memory allocated with `thd_strmake` and it does not use `buf`.
*Solution:*
Allocate memory with `thd_strmake` so the memory in `save` is not local.
---------------------------------------------------------------------------
Fix test main.bug12969156 when WITH_ASAN=ON
*Problem:*
ASAN complains about stack-buffer-overflow on function `mysql_heartbeat`:
```
==90890==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fe746d06d14 at pc 0x7fe760f5b017 bp 0x7fe746d06cd0 sp 0x7fe746d06478
WRITE of size 24 at 0x7fe746d06d14 thread T16777215
Address 0x7fe746d06d14 is located in stack of thread T26 at offset 340 in frame
#0 0x7fe746d0a55c in mysql_heartbeat(void*) /home/yura/ws/percona-server/plugin/daemon_example/daemon_example.cc:62
This frame has 4 object(s):
[48, 56) 'result' (line 66)
[80, 112) '_db_stack_frame_' (line 63)
[144, 200) 'tm_tmp' (line 67)
[240, 340) 'buffer' (line 65) <== Memory access at offset 340 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
(longjmp and C++ exceptions *are* supported)
Thread T26 created by T25 here:
#0 0x7fe760f5f6d5 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:216
#1 0x557ccbbcb857 in my_thread_create /home/yura/ws/percona-server/mysys/my_thread.c:104
#2 0x7fe746d0b21a in daemon_example_plugin_init /home/yura/ws/percona-server/plugin/daemon_example/daemon_example.cc:148
#3 0x557ccb4c69c7 in plugin_initialize /home/yura/ws/percona-server/sql/sql_plugin.cc:1279
#4 0x557ccb4d19cd in mysql_install_plugin /home/yura/ws/percona-server/sql/sql_plugin.cc:2279
#5 0x557ccb4d218f in Sql_cmd_install_plugin::execute(THD*) /home/yura/ws/percona-server/sql/sql_plugin.cc:4664
#6 0x557ccb47695e in mysql_execute_command(THD*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5160
#7 0x557ccb47977c in mysql_parse(THD*, Parser_state*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5952
#8 0x557ccb47b6c2 in dispatch_command(THD*, COM_DATA const*, enum_server_command) /home/yura/ws/percona-server/sql/sql_parse.cc:1544
#9 0x557ccb47de1d in do_command(THD*) /home/yura/ws/percona-server/sql/sql_parse.cc:1065
#10 0x557ccb6ac294 in handle_connection /home/yura/ws/percona-server/sql/conn_handler/connection_handler_per_thread.cc:325
#11 0x557ccbbfabb0 in pfs_spawn_thread /home/yura/ws/percona-server/storage/perfschema/pfs.cc:2198
#12 0x7fe760ab544f in start_thread nptl/pthread_create.c:473
```
The reason is that `my_thread_cancel` is used to finish the daemon thread. This is not and orderly way of finishing the thread. ASAN does not register the stack variables are not used anymore which generates the error above.
This is a benign error as all the variables are on the stack.
*Solution*:
Finish the thread in orderly way by using a signalling variable.
---------------------------------------------------------------------------
PS-8204: Fix XML escape rules for audit plugin
https://jira.percona.com/browse/PS-8204
There was a wrong length specified for some XML
escape rules. As a result of this terminating null symbol from
replacement rule was copied into resulting string. This lead to
quer text truncation in audit log file.
In addition added empty replacement rules for '\b' and 'f' symbols
which just remove them from resulting string. These symboles are
not supported in XML 1.0.
inikep
pushed a commit
that referenced
this pull request
Jan 26, 2023
Upstream commit ID : fb-mysql-5.6.35/8cb1dc836b68f1f13e8b2655b2b8cb2d57f400b3 PS-5217 : Merge fb-prod201803 Summary: Original report: https://jira.mariadb.org/browse/MDEV-15816 To reproduce this bug just following below steps, client 1: USE test; CREATE TABLE t1 (i INT) ENGINE=MyISAM; HANDLER t1 OPEN h; CREATE TABLE t2 (i INT) ENGINE=RocksDB; LOCK TABLES t2 WRITE; client 2: FLUSH TABLES WITH READ LOCK; client 1: INSERT INTO t2 VALUES (1); So client 1 acquired the lock and set m_lock_rows = RDB_LOCK_WRITE. Then client 2 calls store_lock(TL_IGNORE) and m_lock_rows was wrongly set to RDB_LOCK_NONE, as below ``` #0 myrocks::ha_rocksdb::store_lock (this=0x7fffbc03c7c8, thd=0x7fffc0000ba0, to=0x7fffc0011220, lock_type=TL_IGNORE) #1 get_lock_data (thd=0x7fffc0000ba0, table_ptr=0x7fffe84b7d20, count=1, flags=2) #2 mysql_lock_abort_for_thread (thd=0x7fffc0000ba0, table=0x7fffbc03bbc0) #3 THD::notify_shared_lock (this=0x7fffc0000ba0, ctx_in_use=0x7fffbc000bd8, needs_thr_lock_abort=true) #4 MDL_lock::notify_conflicting_locks (this=0x555557a82380, ctx=0x7fffc0000cc8) #5 MDL_context::acquire_lock (this=0x7fffc0000cc8, mdl_request=0x7fffe84b8350, lock_wait_timeout=2) #6 Global_read_lock::lock_global_read_lock (this=0x7fffc0003fe0, thd=0x7fffc0000ba0) ``` Finally, client 1 "INSERT INTO..." hits the Assertion 'm_lock_rows == RDB_LOCK_WRITE' failed in myrocks::ha_rocksdb::write_row() Fix this bug by not setting m_locks_rows if lock_type == TL_IGNORE. Closes facebook/mysql-5.6#838 Pull Request resolved: facebook/mysql-5.6#871 Differential Revision: D9417382 Pulled By: lth fbshipit-source-id: c36c164e06c
inikep
pushed a commit
that referenced
this pull request
Jan 26, 2023
Upstream commit ID : fb-mysql-5.6.35/77032004ad23d21a4c386f8136ecfbb071ea42d6 PS-6865 : Merge fb-prod201903 Summary: Currently during primary key's value encode, its ttl value can be from either one of these 3 cases 1. ttl column in primary key 2. non-ttl column a. old record(update case) b. current timestamp 3. ttl column in non-key field Workflow #1: first in Rdb_key_def::pack_record() find and store pk_offset, then in value encode try to parse key slice to fetch ttl value by using pk_offset. Workflow #3: fetch ttl value from ttl column The change is to merge #1 and #3 by always fetching TTL value from ttl column, not matter whether the ttl column is in primary key or not. Of course, remove pk_offset, since it isn't used. BTW, for secondary keys, its ttl value is always from m_ttl_bytes, which is stored by primary value encoding. Reviewed By: yizhang82 Differential Revision: D14662716 fbshipit-source-id: 6b4e5f044fd
inikep
pushed a commit
that referenced
this pull request
Jan 26, 2023
Upstream commit ID : fb-mysql-5.6.35/e025cf1c47e63aada985d78e4083f2e02fba434f
PS-7731 : Merge percona-202102
Summary:
Today in `SELECT count(*)` MyRocks would still decode every single column due to this check, despite the readset being empty:
```
// bitmap is cleared on index merge, but it still needs to decode columns
bool field_requested =
decode_all_fields || m_verify_row_debug_checksums ||
bitmap_is_set(field_map, m_table->field[i]->field_index);
```
As a result MyRocks is significantly slower than InnoDB in this particular scenario.
Turns out in index merge, when it tries to reset, it calls ha_index_init with an empty column_bitmap, so our field decoders didn't know it needs to decode anything, so the entire query would return nothing. This is discussed in [this commit](facebook/mysql-5.6@70f2bcd), and [issue 624](facebook/mysql-5.6#624) and [PR 626](facebook/mysql-5.6#626). So the workaround we had at that time is to simply treat empty map as implicitly everything, and the side effect is massively slowed down count(*).
We have a few options to address this:
1. Fix index merge optimizer - looking at the code in QUICK_RANGE_SELECT::init_ror_merged_scan, it actually fixes up the column_bitmap properly, but after init/reset, so the fix would simply be moving the bitmap set code up. For secondary keys, prepare_for_position will automatically call `mark_columns_used_by_index_no_reset(s->primary_key, read_set)` if HA_PRIMARY_KEY_REQUIRED_FOR_POSITION is set (true for both InnoDB and MyRocks), so we would know correctly that we need to unpack PK when walking SK during index merge.
2. Overriding `column_bitmaps_signal` and setup decoders whenever the bitmap changes - however this doesn't work by itself. Because no storage engine today actually use handler::column_bitmaps_signal this path haven't been tested properly in index merge. In this case, QUICK_RANGE_SELECT::init_ror_merged_scan should call set_column_bitmaps_no_signal to avoid resetting the correct read/write set of head since head is used as first handler (reuses_handler=true) and subsequent place holders for read/write set updates (reuse_handler=false).
3. Follow InnoDB's solution - InnoDB delays it actually initialize its template again in index_read for the 2nd time (relying on `prebuilt->sql_stat_start`), and during index_read `QUICK_RANGE_SELECT::column_bitmap` is already fixed up and the table read/write set is switched to it, so the new template would be built correctly.
In order to make it easier to maintain and port, after discussing with Manuel, I'm going with a simplified version of #3 that delays decoder creation until the first read operation (index_*, rnd_*, range_read_*, multi_range_read_*), and setting the delay flag in index_init / rnd_init / multi_range_read_init.
Also, I ran into a bug with truncation_partition where Rdb_converter's tbl_def is stale (we only update ha_rocksdb::m_tbl_def), but it is fine because it is not being used after table open. But my change moves the lookup_bitmap initialization into Rdb_converter which takes a dependency on Rdb_converter::m_tbl_def so now we need to reset it properly.
Reference Patch: facebook/mysql-5.6@44d6a8d
---------
Porting Note: Due to 8.0's new counting infra (handler::record & handler::record_with_index), this only helps PK counting. Will send out a better fix that works better with 8.0 new counting infra.
Reviewed By: Pushapgl
Differential Revision: D26265470
fbshipit-source-id: f142be681ab
inikep
added a commit
that referenced
this pull request
Jan 26, 2023
PS-5741: Incorrect use of memset_s in keyring_vault.
Fixed the usage of memset_s. The arguments should be:
void memset_s(void *dest, size_t dest_max, int c, size_t n)
where the 2nd argument is size of buffer and the 3rd is
argument is character to fill.
---------------------------------------------------------------------------
PS-7769 - Fix use-after-return error in audit_log_exclude_accounts_validate
---
*Problem:*
`st_mysql_value::val_str` might return a pointer to `buf` which after
the function called is deleted. Therefore the value in `save`, after
reuturnin from the function, is invalid.
In this particular case, the error is not manifesting as val_str`
returns memory allocated with `thd_strmake` and it does not use `buf`.
*Solution:*
Allocate memory with `thd_strmake` so the memory in `save` is not local.
---------------------------------------------------------------------------
Fix test main.bug12969156 when WITH_ASAN=ON
*Problem:*
ASAN complains about stack-buffer-overflow on function `mysql_heartbeat`:
```
==90890==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fe746d06d14 at pc 0x7fe760f5b017 bp 0x7fe746d06cd0 sp 0x7fe746d06478
WRITE of size 24 at 0x7fe746d06d14 thread T16777215
Address 0x7fe746d06d14 is located in stack of thread T26 at offset 340 in frame
#0 0x7fe746d0a55c in mysql_heartbeat(void*) /home/yura/ws/percona-server/plugin/daemon_example/daemon_example.cc:62
This frame has 4 object(s):
[48, 56) 'result' (line 66)
[80, 112) '_db_stack_frame_' (line 63)
[144, 200) 'tm_tmp' (line 67)
[240, 340) 'buffer' (line 65) <== Memory access at offset 340 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
(longjmp and C++ exceptions *are* supported)
Thread T26 created by T25 here:
#0 0x7fe760f5f6d5 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:216
#1 0x557ccbbcb857 in my_thread_create /home/yura/ws/percona-server/mysys/my_thread.c:104
#2 0x7fe746d0b21a in daemon_example_plugin_init /home/yura/ws/percona-server/plugin/daemon_example/daemon_example.cc:148
#3 0x557ccb4c69c7 in plugin_initialize /home/yura/ws/percona-server/sql/sql_plugin.cc:1279
#4 0x557ccb4d19cd in mysql_install_plugin /home/yura/ws/percona-server/sql/sql_plugin.cc:2279
#5 0x557ccb4d218f in Sql_cmd_install_plugin::execute(THD*) /home/yura/ws/percona-server/sql/sql_plugin.cc:4664
#6 0x557ccb47695e in mysql_execute_command(THD*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5160
#7 0x557ccb47977c in mysql_parse(THD*, Parser_state*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5952
#8 0x557ccb47b6c2 in dispatch_command(THD*, COM_DATA const*, enum_server_command) /home/yura/ws/percona-server/sql/sql_parse.cc:1544
#9 0x557ccb47de1d in do_command(THD*) /home/yura/ws/percona-server/sql/sql_parse.cc:1065
#10 0x557ccb6ac294 in handle_connection /home/yura/ws/percona-server/sql/conn_handler/connection_handler_per_thread.cc:325
#11 0x557ccbbfabb0 in pfs_spawn_thread /home/yura/ws/percona-server/storage/perfschema/pfs.cc:2198
#12 0x7fe760ab544f in start_thread nptl/pthread_create.c:473
```
The reason is that `my_thread_cancel` is used to finish the daemon thread. This is not and orderly way of finishing the thread. ASAN does not register the stack variables are not used anymore which generates the error above.
This is a benign error as all the variables are on the stack.
*Solution*:
Finish the thread in orderly way by using a signalling variable.
---------------------------------------------------------------------------
PS-8204: Fix XML escape rules for audit plugin
https://jira.percona.com/browse/PS-8204
There was a wrong length specified for some XML
escape rules. As a result of this terminating null symbol from
replacement rule was copied into resulting string. This lead to
quer text truncation in audit log file.
In addition added empty replacement rules for '\b' and 'f' symbols
which just remove them from resulting string. These symboles are
not supported in XML 1.0.
inikep
pushed a commit
that referenced
this pull request
Apr 14, 2023
Upstream commit ID : fb-mysql-5.6.35/8cb1dc836b68f1f13e8b2655b2b8cb2d57f400b3 PS-5217 : Merge fb-prod201803 Summary: Original report: https://jira.mariadb.org/browse/MDEV-15816 To reproduce this bug just following below steps, client 1: USE test; CREATE TABLE t1 (i INT) ENGINE=MyISAM; HANDLER t1 OPEN h; CREATE TABLE t2 (i INT) ENGINE=RocksDB; LOCK TABLES t2 WRITE; client 2: FLUSH TABLES WITH READ LOCK; client 1: INSERT INTO t2 VALUES (1); So client 1 acquired the lock and set m_lock_rows = RDB_LOCK_WRITE. Then client 2 calls store_lock(TL_IGNORE) and m_lock_rows was wrongly set to RDB_LOCK_NONE, as below ``` #0 myrocks::ha_rocksdb::store_lock (this=0x7fffbc03c7c8, thd=0x7fffc0000ba0, to=0x7fffc0011220, lock_type=TL_IGNORE) #1 get_lock_data (thd=0x7fffc0000ba0, table_ptr=0x7fffe84b7d20, count=1, flags=2) #2 mysql_lock_abort_for_thread (thd=0x7fffc0000ba0, table=0x7fffbc03bbc0) #3 THD::notify_shared_lock (this=0x7fffc0000ba0, ctx_in_use=0x7fffbc000bd8, needs_thr_lock_abort=true) #4 MDL_lock::notify_conflicting_locks (this=0x555557a82380, ctx=0x7fffc0000cc8) #5 MDL_context::acquire_lock (this=0x7fffc0000cc8, mdl_request=0x7fffe84b8350, lock_wait_timeout=2) #6 Global_read_lock::lock_global_read_lock (this=0x7fffc0003fe0, thd=0x7fffc0000ba0) ``` Finally, client 1 "INSERT INTO..." hits the Assertion 'm_lock_rows == RDB_LOCK_WRITE' failed in myrocks::ha_rocksdb::write_row() Fix this bug by not setting m_locks_rows if lock_type == TL_IGNORE. Closes facebook/mysql-5.6#838 Pull Request resolved: facebook/mysql-5.6#871 Differential Revision: D9417382 Pulled By: lth fbshipit-source-id: c36c164e06c
inikep
pushed a commit
that referenced
this pull request
Sep 25, 2024
…ocal DDL
executed
https://perconadev.atlassian.net/browse/PS-9018
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
Oct 28, 2024
Upstream commit ID : fb-mysql-5.6.35/8cb1dc836b68f1f13e8b2655b2b8cb2d57f400b3 PS-5217 : Merge fb-prod201803 Summary: Original report: https://jira.mariadb.org/browse/MDEV-15816 To reproduce this bug just following below steps, client 1: USE test; CREATE TABLE t1 (i INT) ENGINE=MyISAM; HANDLER t1 OPEN h; CREATE TABLE t2 (i INT) ENGINE=RocksDB; LOCK TABLES t2 WRITE; client 2: FLUSH TABLES WITH READ LOCK; client 1: INSERT INTO t2 VALUES (1); So client 1 acquired the lock and set m_lock_rows = RDB_LOCK_WRITE. Then client 2 calls store_lock(TL_IGNORE) and m_lock_rows was wrongly set to RDB_LOCK_NONE, as below ``` #0 myrocks::ha_rocksdb::store_lock (this=0x7fffbc03c7c8, thd=0x7fffc0000ba0, to=0x7fffc0011220, lock_type=TL_IGNORE) #1 get_lock_data (thd=0x7fffc0000ba0, table_ptr=0x7fffe84b7d20, count=1, flags=2) #2 mysql_lock_abort_for_thread (thd=0x7fffc0000ba0, table=0x7fffbc03bbc0) #3 THD::notify_shared_lock (this=0x7fffc0000ba0, ctx_in_use=0x7fffbc000bd8, needs_thr_lock_abort=true) #4 MDL_lock::notify_conflicting_locks (this=0x555557a82380, ctx=0x7fffc0000cc8) #5 MDL_context::acquire_lock (this=0x7fffc0000cc8, mdl_request=0x7fffe84b8350, lock_wait_timeout=2) #6 Global_read_lock::lock_global_read_lock (this=0x7fffc0003fe0, thd=0x7fffc0000ba0) ``` Finally, client 1 "INSERT INTO..." hits the Assertion 'm_lock_rows == RDB_LOCK_WRITE' failed in myrocks::ha_rocksdb::write_row() Fix this bug by not setting m_locks_rows if lock_type == TL_IGNORE. Closes facebook/mysql-5.6#838 Pull Request resolved: facebook/mysql-5.6#871 Differential Revision: D9417382 Pulled By: lth fbshipit-source-id: c36c164e06c
inikep
pushed a commit
that referenced
this pull request
Oct 28, 2024
Upstream commit ID : fb-mysql-5.6.35/77032004ad23d21a4c386f8136ecfbb071ea42d6 PS-6865 : Merge fb-prod201903 Summary: Currently during primary key's value encode, its ttl value can be from either one of these 3 cases 1. ttl column in primary key 2. non-ttl column a. old record(update case) b. current timestamp 3. ttl column in non-key field Workflow #1: first in Rdb_key_def::pack_record() find and store pk_offset, then in value encode try to parse key slice to fetch ttl value by using pk_offset. Workflow #3: fetch ttl value from ttl column The change is to merge #1 and #3 by always fetching TTL value from ttl column, not matter whether the ttl column is in primary key or not. Of course, remove pk_offset, since it isn't used. BTW, for secondary keys, its ttl value is always from m_ttl_bytes, which is stored by primary value encoding. Reviewed By: yizhang82 Differential Revision: D14662716 fbshipit-source-id: 6b4e5f044fd
inikep
pushed a commit
that referenced
this pull request
Oct 28, 2024
Upstream commit ID : fb-mysql-5.6.35/e025cf1c47e63aada985d78e4083f2e02fba434f
PS-7731 : Merge percona-202102
Summary:
Today in `SELECT count(*)` MyRocks would still decode every single column due to this check, despite the readset being empty:
```
// bitmap is cleared on index merge, but it still needs to decode columns
bool field_requested =
decode_all_fields || m_verify_row_debug_checksums ||
bitmap_is_set(field_map, m_table->field[i]->field_index);
```
As a result MyRocks is significantly slower than InnoDB in this particular scenario.
Turns out in index merge, when it tries to reset, it calls ha_index_init with an empty column_bitmap, so our field decoders didn't know it needs to decode anything, so the entire query would return nothing. This is discussed in [this commit](facebook/mysql-5.6@70f2bcd), and [issue 624](facebook/mysql-5.6#624) and [PR 626](facebook/mysql-5.6#626). So the workaround we had at that time is to simply treat empty map as implicitly everything, and the side effect is massively slowed down count(*).
We have a few options to address this:
1. Fix index merge optimizer - looking at the code in QUICK_RANGE_SELECT::init_ror_merged_scan, it actually fixes up the column_bitmap properly, but after init/reset, so the fix would simply be moving the bitmap set code up. For secondary keys, prepare_for_position will automatically call `mark_columns_used_by_index_no_reset(s->primary_key, read_set)` if HA_PRIMARY_KEY_REQUIRED_FOR_POSITION is set (true for both InnoDB and MyRocks), so we would know correctly that we need to unpack PK when walking SK during index merge.
2. Overriding `column_bitmaps_signal` and setup decoders whenever the bitmap changes - however this doesn't work by itself. Because no storage engine today actually use handler::column_bitmaps_signal this path haven't been tested properly in index merge. In this case, QUICK_RANGE_SELECT::init_ror_merged_scan should call set_column_bitmaps_no_signal to avoid resetting the correct read/write set of head since head is used as first handler (reuses_handler=true) and subsequent place holders for read/write set updates (reuse_handler=false).
3. Follow InnoDB's solution - InnoDB delays it actually initialize its template again in index_read for the 2nd time (relying on `prebuilt->sql_stat_start`), and during index_read `QUICK_RANGE_SELECT::column_bitmap` is already fixed up and the table read/write set is switched to it, so the new template would be built correctly.
In order to make it easier to maintain and port, after discussing with Manuel, I'm going with a simplified version of #3 that delays decoder creation until the first read operation (index_*, rnd_*, range_read_*, multi_range_read_*), and setting the delay flag in index_init / rnd_init / multi_range_read_init.
Also, I ran into a bug with truncation_partition where Rdb_converter's tbl_def is stale (we only update ha_rocksdb::m_tbl_def), but it is fine because it is not being used after table open. But my change moves the lookup_bitmap initialization into Rdb_converter which takes a dependency on Rdb_converter::m_tbl_def so now we need to reset it properly.
Reference Patch: facebook/mysql-5.6@44d6a8d
---------
Porting Note: Due to 8.0's new counting infra (handler::record & handler::record_with_index), this only helps PK counting. Will send out a better fix that works better with 8.0 new counting infra.
Reviewed By: Pushapgl
Differential Revision: D26265470
fbshipit-source-id: f142be681ab
inikep
added a commit
that referenced
this pull request
Oct 28, 2024
PS-5741: Incorrect use of memset_s in keyring_vault.
Fixed the usage of memset_s. The arguments should be:
void memset_s(void *dest, size_t dest_max, int c, size_t n)
where the 2nd argument is size of buffer and the 3rd is
argument is character to fill.
---------------------------------------------------------------------------
PS-7769 - Fix use-after-return error in audit_log_exclude_accounts_validate
---
*Problem:*
`st_mysql_value::val_str` might return a pointer to `buf` which after
the function called is deleted. Therefore the value in `save`, after
reuturnin from the function, is invalid.
In this particular case, the error is not manifesting as val_str`
returns memory allocated with `thd_strmake` and it does not use `buf`.
*Solution:*
Allocate memory with `thd_strmake` so the memory in `save` is not local.
---------------------------------------------------------------------------
Fix test main.bug12969156 when WITH_ASAN=ON
*Problem:*
ASAN complains about stack-buffer-overflow on function `mysql_heartbeat`:
```
==90890==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fe746d06d14 at pc 0x7fe760f5b017 bp 0x7fe746d06cd0 sp 0x7fe746d06478
WRITE of size 24 at 0x7fe746d06d14 thread T16777215
Address 0x7fe746d06d14 is located in stack of thread T26 at offset 340 in frame
#0 0x7fe746d0a55c in mysql_heartbeat(void*) /home/yura/ws/percona-server/plugin/daemon_example/daemon_example.cc:62
This frame has 4 object(s):
[48, 56) 'result' (line 66)
[80, 112) '_db_stack_frame_' (line 63)
[144, 200) 'tm_tmp' (line 67)
[240, 340) 'buffer' (line 65) <== Memory access at offset 340 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
(longjmp and C++ exceptions *are* supported)
Thread T26 created by T25 here:
#0 0x7fe760f5f6d5 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:216
#1 0x557ccbbcb857 in my_thread_create /home/yura/ws/percona-server/mysys/my_thread.c:104
#2 0x7fe746d0b21a in daemon_example_plugin_init /home/yura/ws/percona-server/plugin/daemon_example/daemon_example.cc:148
#3 0x557ccb4c69c7 in plugin_initialize /home/yura/ws/percona-server/sql/sql_plugin.cc:1279
#4 0x557ccb4d19cd in mysql_install_plugin /home/yura/ws/percona-server/sql/sql_plugin.cc:2279
#5 0x557ccb4d218f in Sql_cmd_install_plugin::execute(THD*) /home/yura/ws/percona-server/sql/sql_plugin.cc:4664
#6 0x557ccb47695e in mysql_execute_command(THD*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5160
#7 0x557ccb47977c in mysql_parse(THD*, Parser_state*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5952
#8 0x557ccb47b6c2 in dispatch_command(THD*, COM_DATA const*, enum_server_command) /home/yura/ws/percona-server/sql/sql_parse.cc:1544
#9 0x557ccb47de1d in do_command(THD*) /home/yura/ws/percona-server/sql/sql_parse.cc:1065
#10 0x557ccb6ac294 in handle_connection /home/yura/ws/percona-server/sql/conn_handler/connection_handler_per_thread.cc:325
#11 0x557ccbbfabb0 in pfs_spawn_thread /home/yura/ws/percona-server/storage/perfschema/pfs.cc:2198
#12 0x7fe760ab544f in start_thread nptl/pthread_create.c:473
```
The reason is that `my_thread_cancel` is used to finish the daemon thread. This is not and orderly way of finishing the thread. ASAN does not register the stack variables are not used anymore which generates the error above.
This is a benign error as all the variables are on the stack.
*Solution*:
Finish the thread in orderly way by using a signalling variable.
---------------------------------------------------------------------------
PS-8204: Fix XML escape rules for audit plugin
https://jira.percona.com/browse/PS-8204
There was a wrong length specified for some XML
escape rules. As a result of this terminating null symbol from
replacement rule was copied into resulting string. This lead to
quer text truncation in audit log file.
In addition added empty replacement rules for '\b' and 'f' symbols
which just remove them from resulting string. These symboles are
not supported in XML 1.0.
---------------------------------------------------------------------------
PS-8854: Add main.percona_udf MTR test
Add a test to check FNV1A_64, FNV_64, and MURMUR_HASH user-defined functions.
---------------------------------------------------------------------------
PS-9369: Fix currently processed query comparison in audit_log
https://perconadev.atlassian.net/browse/PS-9369
The audit_log uses stack to keep track of table access operations being
performed in scope of one query. It compares last known table access query
string stored on top of this stack with actual query in audit event being
processed at the moment to decide if new record should be pushed to stack
or it is time to clean records from the stack.
Currently audit_log simply compares char* variables to decide if this is
the same query string. This approach doesn't work. As a result plugin looses
control of the stack size and it starts growing with the time consuming
memory. This issue is not noticable on short term server connections
as memory is freed once connection is closed. At the same time this
leads to extra memory consumption for long running server connections.
The following is done to fix the issue:
- Query is sent along with audit event as MYSQL_LEX_CSTRING structure.
It is not correct to ignore MYSQL_LEX_CSTRING.length comparison as
sometimes MYSQL_LEX_CSTRING.str pointer may be not iniialised
properly. Added string length check to make sure structure contains
any valid string.
- Used strncmp to compare actual strings instead of comparing char*
variables.
inikep
pushed a commit
that referenced
this pull request
Oct 28, 2024
…n read() syscall over network https://jira.percona.com/browse/PS-8592 Description ----------- GR suffered from problems caused by the security probes and network scanner processes connecting to the group replication communication port. This usually is not a problem, but poses a serious threat when another member tries to join the cluster by initialting a connection to the member which is affected by external processes using the port dedicated for group communication for longer durations. On such activites by external processes, the SSL enabled server stalled forever on the SSL_accept() call waiting for handshake data. Below is the stacktrace: Thread 55 (Thread 0x7f7bb77ff700 (LWP 2198598)): #0 in read () #1 in sock_read () #2 in BIO_read () #3 in ssl23_read_bytes () #4 in ssl23_get_client_hello () #5 in ssl23_accept () #6 in xcom_tcp_server_startup(Xcom_network_provider*) () When the server stalled in the above path forever, it prohibited other members to join the cluster resulting in the following messages on the joiner server's logs. [ERROR] [MY-011640] [Repl] Plugin group_replication reported: 'Timeout on wait for view after joining group' [ERROR] [MY-011735] [Repl] Plugin group_replication reported: '[GCS] The member is already leaving or joining a group.' Solution -------- This patch adds two new variables 1. group_replication_xcom_ssl_socket_timeout It is a file-descriptor level timeout in seconds for both accept() and SSL_accept() calls when group replication is listening on the xcom port. When set to a valid value, say for example 5 seconds, both accept() and SSL_accept() return after 5 seconds. The default value has been set to 0 (waits infinitely) for backward compatibility. This variable is effective only when GR is configred with SSL. 2. group_replication_xcom_ssl_accept_retries It defines the number of retries to be performed before closing the socket. For each retry the server thread calls SSL_accept() with timeout defined by the group_replication_xcom_ssl_socket_timeout for the SSL handshake process once the connection has been accepted by the first accept() call. The default value has been set to 10. This variable is effective only when GR is configred with SSL. Note: - Both of the above variables are dynamically configurable, but will become effective only on START GROUP_REPLICATION. ------------------------------------------------------------------------------- PS-8844: Fix the failing main.mysqldump_gtid_purged https://jira.percona.com/browse/PS-8844 This patch fixes the test failure of main.mysqldump_gtid_purged that failed due to the uninitialized variable $redirect_stderr in the start_proc_in_background.inc.
inikep
pushed a commit
that referenced
this pull request
Oct 28, 2024
…ocal DDL
executed
https://perconadev.atlassian.net/browse/PS-9018
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
Nov 6, 2024
… for connection xxx'.
The new iterator based explains are not impacted.
The issue here is a race condition. More than one thread is using the
query term iterator at the same time (whoch is neithe threas safe nor
reantrant), and part of its state is in the query terms being visited
which leads to interference/race conditions.
a) the explain thread
uses an iterator here:
Sql_cmd_explain_other_thread::execute
is inspecting the Query_expression of the running query
calling master_query_expression()->find_blocks_query_term which uses
an iterator over the query terms in the query expression:
for (auto qt : query_terms<>()) {
if (qt->query_block() == qb) {
return qt;
}
}
the above search fails to find qb due to the interference of the
thread b), see below, and then tries to access a nullpointer:
* thread percona#36, name = ‘connection’, stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
frame #0: 0x000000010bb3cf0d mysqld`Query_block::type(this=0x00007f8f82719088) const at sql_lex.cc:4441:11
frame #1: 0x000000010b83763e mysqld`(anonymous namespace)::Explain::explain_select_type(this=0x00007000020611b8) at opt_explain.cc:792:50
frame #2: 0x000000010b83cc4d mysqld`(anonymous namespace)::Explain_join::explain_select_type(this=0x00007000020611b8) at opt_explain.cc:1487:21
frame #3: 0x000000010b837c34 mysqld`(anonymous namespace)::Explain::prepare_columns(this=0x00007000020611b8) at opt_explain.cc:744:26
frame #4: 0x000000010b83ea0e mysqld`(anonymous namespace)::Explain_join::explain_qep_tab(this=0x00007000020611b8, tabnum=0) at opt_explain.cc:1415:32
frame #5: 0x000000010b83ca0a mysqld`(anonymous namespace)::Explain_join::shallow_explain(this=0x00007000020611b8) at opt_explain.cc:1364:9
frame #6: 0x000000010b83379b mysqld`(anonymous namespace)::Explain::send(this=0x00007000020611b8) at opt_explain.cc:770:14
frame #7: 0x000000010b834147 mysqld`explain_query_specification(explain_thd=0x00007f8fbb111e00, query_thd=0x00007f8fbb919c00, query_term=0x00007f8f82719088, ctx=CTX_JOIN) at opt_explain.cc:2088:20
frame #8: 0x000000010bd36b91 mysqld`Query_expression::explain_query_term(this=0x00007f8f7a090360, explain_thd=0x00007f8fbb111e00, query_thd=0x00007f8fbb919c00, qt=0x00007f8f82719088) at sql_union.cc:1519:11
frame #9: 0x000000010bd36c68 mysqld`Query_expression::explain_query_term(this=0x00007f8f7a090360, explain_thd=0x00007f8fbb111e00, query_thd=0x00007f8fbb919c00, qt=0x00007f8f8271d748) at sql_union.cc:1526:13
frame #10: 0x000000010bd373f7 mysqld`Query_expression::explain(this=0x00007f8f7a090360, explain_thd=0x00007f8fbb111e00, query_thd=0x00007f8fbb919c00) at sql_union.cc:1591:7
frame #11: 0x000000010b835820 mysqld`mysql_explain_query_expression(explain_thd=0x00007f8fbb111e00, query_thd=0x00007f8fbb919c00, unit=0x00007f8f7a090360) at opt_explain.cc:2392:17
frame #12: 0x000000010b835400 mysqld`explain_query(explain_thd=0x00007f8fbb111e00, query_thd=0x00007f8fbb919c00, unit=0x00007f8f7a090360) at opt_explain.cc:2353:13
* frame percona#13: 0x000000010b8363e4 mysqld`Sql_cmd_explain_other_thread::execute(this=0x00007f8fba585b68, thd=0x00007f8fbb111e00) at opt_explain.cc:2531:11
frame percona#14: 0x000000010bba7d8b mysqld`mysql_execute_command(thd=0x00007f8fbb111e00, first_level=true) at sql_parse.cc:4648:29
frame percona#15: 0x000000010bb9e230 mysqld`dispatch_sql_command(thd=0x00007f8fbb111e00, parser_state=0x0000700002065de8) at sql_parse.cc:5303:19
frame percona#16: 0x000000010bb9a4cb mysqld`dispatch_command(thd=0x00007f8fbb111e00, com_data=0x0000700002066e38, command=COM_QUERY) at sql_parse.cc:2135:7
frame percona#17: 0x000000010bb9c846 mysqld`do_command(thd=0x00007f8fbb111e00) at sql_parse.cc:1464:18
frame percona#18: 0x000000010b2f2574 mysqld`handle_connection(arg=0x0000600000e34200) at connection_handler_per_thread.cc:304:13
frame percona#19: 0x000000010e072fc4 mysqld`pfs_spawn_thread(arg=0x00007f8fba8160b0) at pfs.cc:3051:3
frame percona#20: 0x00007ff806c2b202 libsystem_pthread.dylib`_pthread_start + 99
frame percona#21: 0x00007ff806c26bab libsystem_pthread.dylib`thread_start + 15
b) the query thread being explained is itself performing LEX::cleanup
and as part of the iterates over the query terms, but still allows
EXPLAIN of the query plan since
thd->query_plan.set_query_plan(SQLCOM_END, ...)
hasn't been called yet.
20:frame: Query_terms<(Visit_order)1, (Visit_leaves)0>::Query_term_iterator::operator++() (in mysqld) (query_term.h:613)
21:frame: Query_expression::cleanup(bool) (in mysqld) (sql_union.cc:1861)
22:frame: LEX::cleanup(bool) (in mysqld) (sql_lex.h:4286)
30:frame: Sql_cmd_dml::execute(THD*) (in mysqld) (sql_select.cc:799)
31:frame: mysql_execute_command(THD*, bool) (in mysqld) (sql_parse.cc:4648)
32:frame: dispatch_sql_command(THD*, Parser_state*) (in mysqld) (sql_parse.cc:5303)
33:frame: dispatch_command(THD*, COM_DATA const*, enum_server_command) (in mysqld) (sql_parse.cc:2135)
34:frame: do_command(THD*) (in mysqld) (sql_parse.cc:1464)
57:frame: handle_connection(void*) (in mysqld) (connection_handler_per_thread.cc:304)
58:frame: pfs_spawn_thread(void*) (in mysqld) (pfs.cc:3053)
65:frame: _pthread_start (in libsystem_pthread.dylib) + 99
66:frame: thread_start (in libsystem_pthread.dylib) + 15
Solution:
This patch solves the issue by removing iterator state from
Query_term, making the query_term iterators thread safe. This solution
labels every child query_term with its index in its parent's
m_children vector. The iterator can therefore easily compute the next
child to visit based on Query_term::m_sibling_idx.
A unit test case is added to check reentrancy.
One can also manually verify that we have no remaining race condition
by running two client connections files (with \. <file>) with a big
number of copies of the repro query in one connection and a big number
of EXPLAIN format=json FOR <connection>, e.g.
EXPLAIN FORMAT=json FOR CONNECTION 8\G
in the other. The actual connection number would need to verified
in connection one, of course.
Change-Id: Ie7d56610914738ccbbecf399ccc4f465f7d26ea7
inikep
pushed a commit
that referenced
this pull request
Nov 6, 2024
This is a combination of 5 commits.
This is the 1st commit message:
WL#15746: TLS Enhancements for HeatWave-AutoML & Dask Comm. Upgrade
Problem:
--------
- HeatWave-AutoML communication was unauthenticated, unauthorized,
and unencrypted.
- Dask communication utilized TCP, not aligning with FedRamp
guidelines.
Solution:
---------
- Introduced TLS and mTLS in HeatWave-AutoML's plugin and driver for
authentication, authorization, and encryption.
- Applied TLS to Dask to ensure authentication, encryption, and
authorization.
Dask Authorization (OCID-based):
--------------------------------
1. For each DBsystem:
- MySQL node sends OCIDs of authorized nodes to the head driver
via:
a. rapid_net_nodes
b. rapid_net_allowed_ocids (older API, mainly for MTR tests)
- Scenarios:
a. All OCIDs provided: Dask authorizes.
b. Any OCID absent: ML call fails with message.
2. During Dask worker registration to the Dask scheduler, a script is
dispatched to the Dask worker for execution, retrieving the worker's
OCID for authorization purposes.
- If the OCID isn't approved, the connection is denied, terminating
the worker and causing the ML query to fail.
3. For every Dask worker (both as listener and connector), an OCID-
based authorization is performed post SSL/TLS connection handshake.
The process compares the OCID from the peer's certificate against
the allowed_ocids received from the HeatWave-AutoML MySQL plugin.
HWAML Plugin Changes:
---------------------
- Sourced certificate data and SSL setup from disk, incorporating
SSL/TLS for HWAML.
- Reused "keystore" variable to specify disk location for
certificate retrieval.
- Certificates and keys expected in PKCS12 format.
- Introduced "have_ml_encryption" variable (default=0).
> Acts as a switch to explicitly deactivate HWAML network
encryption, akin to "disable_net_encryption" affecting
network encryption for HeatWave. Set to 1 to enable.
- Introduced a customized verifier function for verify_callback to
be set in SSL_CTX_set_verify and used in the handshake process
of SSL/TLS. The customized verifier function will perform
instance id (OCID) based authorization on the plugin side during
standard SSL/TLS handshake process.
- CRL (Certificate Revocation List) checks are also conducted if CRL
Distribution Points are present and accessible in the provided
certificate.
HWAML Driver Changes & OCID-based Authorization:
------------------------------------------------
- Introduced "enable_encryption" (default=0).
> Set to 1 to enable encryption.
- When receiving a new connection request and encryption is on, the
driver performs OCID-based self-checking, comparing OCID retrieved
from its own instance principal with the OCID in the
provided certificate on disk.
- The driver compares OCID from "mysql_compute_id" and extracted OCID
from mTLS certificate during connection.
- Introduced "cert_dir" argument for certificate directory
specification.
- Expected files: cert_chain.pem, certificate.pem, private_key.pem.
> OCID should be in the userID (UID) or CN field of the
certificate.pem subject.
- CRL (Certificate Revocation List) checks are also conducted post
handshake, if CRL Distribution Points are present and accessible in
the provided certificate, alongside OCID authorization.
Encryption Behavior:
--------------------
- If encryption is deactivated on both plugin and driver side, HWAML
will work without encryption as it was before this commit.
Enabling Encryption:
--------------------
- By default, "have_ml_encryption" and "enable_encryption" are set to 0
> Encryption is disabled by default.
- For the HWAML plugin:
> "have_ml_encryption" set to 1 (default is 0).
> Specify the .pfx file's path using the "keystore".
- For the HWAML Driver:
> "enable_encryption" set to 1 (default is 0)
> Specify "mysql_instance_id" and "cert_dir".
Testing:
--------
- MTR has been modified for the encryption setup.
> Runs with encryption if "OCI_INSTANCE_ID" is set to a valid
value.
- On OCI (when "OLRAPID_KEYSTORE" is not set):
> Certificates and keys are generated; PEMs for driver and PKCS12
for plugin.
- On AWS (when "OLRAPID_KEYSTORE" is set as the path to PKCS12
keystore files):
> PEM files are extracted from the provided PKCS12 and used for
the driver. The plugin uses the provided PKCS12 keystore file.
Change-Id: I553ca135241e03484db6debbe186e6d34d582bf4
This is the commit message #2:
WL#15746 - Adding ML encryption support to BM
Enabling ML encryption on Baumeister:
- Certificates are generated on MySQLd during initialization
- Needed certicates for workers are packaged and sent to worker nodes
- Workers use packaged files to generate their certificates
- Arguments are added to driver.py invoke
- Keystore path is added to mysql config
Change-Id: I11a5cc5926488ff4fbf91bb6c10a091358db7dc9
This is the commit message #3:
WL#15746: Enhanced CRL Daemon Checker
Issue
=====
The previous design assumed a plain HTTPS link for the CRL distribution
point, accessible to all. This assumption no longer holds, as public
accessibility for CRL distribution contradicts OCI guidelines. Now, the
CRL distribution point in certificates provided by the control plane is
expected to be protected by OCI Instance Principal Authentication.
However, using this authentication method introduces a delay of several
seconds, which is impractical for HeatWave-AutoML.
Solution
========
The CRL fetching code now uses OCI Instance Principal Authentication.
To mitigate performance issues, the CRL checking process has been
redesigned. Instead of enforcing CRL checks per connection in MySQL
Plugin and HeatWave-AutoML Driver communications, a daemon thread in
HeatWave-AutoML Driver, Dask scheduler, and Dask Worker now periodically
fetches and verifies the CRL against all active connections. This
separation minimizes performance impacts. Consequently, MySQL Plugin's
CRL checks have been removed, as checks in the Driver, Scheduler, and
Worker sufficiently cover all cluster nodes.
Changes
=======
- Implemented CRL checker as a daemon thread in Driver, Scheduler, and
Worker.
- Each connection/socket has an associated CRL checker.
- CRL checks occur periodically at set intervals.
- Skips CRL check if the CRL is temporarily unavailable.
- Failing a CRL check results in the associated connection/socket being
closed. On the Driver, a stop event is triggered (akin to CTRL-C).
Change-Id: Id998cfe9e15d9236291b0ae420d65c2197837966
This is the commit message #4:
WL#15746: Fix Dask workers being shutdown without releasing address
Issue
=====
Dask workers getting shutting but not releasing the address used
properly sometimes.
Solution
========
Reverted some changes in heatwave_cluster.py in dask worker shutdown
function. Hopefully this will fix the address issue
Change-Id: I5a6749b5a25b0ccb73ba7369e545bc010da1b84f
This is the commit message #5:
WL#15746: Implement Dask Worker Join Timeout for Head Node
Issue:
======
In the cluster_shutdown method, the join operation on the head node's
worker process lacked a timeout. This led to potential indefinite
waiting and eventual hanging of the head node.
Solution:
=========
A timeout has been introduced for the worker process join on the head
node. Unlike non-head nodes, which rely on worker join to complete Dask
tasks and cannot have a timeout, the head node can safely implement
this. Now, if the worker process on the head node fails to shut down
post-join, indicating a timeout, it will be manually terminated. This
ensures proper release of associated resources and prevents hanging of
the head node.
Additional Change:
==================
Added Cert Rotation Guard for DASK clusters. This feature initiates on
the first plugin-driver connection when the DASK cluster is off,
recording the certificate's expiry date. During driver idle times,
it checks the current cert's expiry against this date. If it detects a
change, indicating a certificate rotation, it shuts down the DASK
cluster. The cluster restarts on the next connection request, ensuring
the use of the latest certificate.
Change-Id: Ie63a2e2b7664e05e1622d8bd6503663e13fa73cb
inikep
pushed a commit
that referenced
this pull request
Nov 11, 2024
Upstream commit ID : fb-mysql-5.6.35/8cb1dc836b68f1f13e8b2655b2b8cb2d57f400b3 PS-5217 : Merge fb-prod201803 Summary: Original report: https://jira.mariadb.org/browse/MDEV-15816 To reproduce this bug just following below steps, client 1: USE test; CREATE TABLE t1 (i INT) ENGINE=MyISAM; HANDLER t1 OPEN h; CREATE TABLE t2 (i INT) ENGINE=RocksDB; LOCK TABLES t2 WRITE; client 2: FLUSH TABLES WITH READ LOCK; client 1: INSERT INTO t2 VALUES (1); So client 1 acquired the lock and set m_lock_rows = RDB_LOCK_WRITE. Then client 2 calls store_lock(TL_IGNORE) and m_lock_rows was wrongly set to RDB_LOCK_NONE, as below ``` #0 myrocks::ha_rocksdb::store_lock (this=0x7fffbc03c7c8, thd=0x7fffc0000ba0, to=0x7fffc0011220, lock_type=TL_IGNORE) #1 get_lock_data (thd=0x7fffc0000ba0, table_ptr=0x7fffe84b7d20, count=1, flags=2) #2 mysql_lock_abort_for_thread (thd=0x7fffc0000ba0, table=0x7fffbc03bbc0) #3 THD::notify_shared_lock (this=0x7fffc0000ba0, ctx_in_use=0x7fffbc000bd8, needs_thr_lock_abort=true) #4 MDL_lock::notify_conflicting_locks (this=0x555557a82380, ctx=0x7fffc0000cc8) #5 MDL_context::acquire_lock (this=0x7fffc0000cc8, mdl_request=0x7fffe84b8350, lock_wait_timeout=2) #6 Global_read_lock::lock_global_read_lock (this=0x7fffc0003fe0, thd=0x7fffc0000ba0) ``` Finally, client 1 "INSERT INTO..." hits the Assertion 'm_lock_rows == RDB_LOCK_WRITE' failed in myrocks::ha_rocksdb::write_row() Fix this bug by not setting m_locks_rows if lock_type == TL_IGNORE. Closes facebook/mysql-5.6#838 Pull Request resolved: facebook/mysql-5.6#871 Differential Revision: D9417382 Pulled By: lth fbshipit-source-id: c36c164e06c
inikep
pushed a commit
that referenced
this pull request
Nov 11, 2024
Upstream commit ID : fb-mysql-5.6.35/77032004ad23d21a4c386f8136ecfbb071ea42d6 PS-6865 : Merge fb-prod201903 Summary: Currently during primary key's value encode, its ttl value can be from either one of these 3 cases 1. ttl column in primary key 2. non-ttl column a. old record(update case) b. current timestamp 3. ttl column in non-key field Workflow #1: first in Rdb_key_def::pack_record() find and store pk_offset, then in value encode try to parse key slice to fetch ttl value by using pk_offset. Workflow #3: fetch ttl value from ttl column The change is to merge #1 and #3 by always fetching TTL value from ttl column, not matter whether the ttl column is in primary key or not. Of course, remove pk_offset, since it isn't used. BTW, for secondary keys, its ttl value is always from m_ttl_bytes, which is stored by primary value encoding. Reviewed By: yizhang82 Differential Revision: D14662716 fbshipit-source-id: 6b4e5f044fd
inikep
pushed a commit
that referenced
this pull request
Nov 11, 2024
Upstream commit ID : fb-mysql-5.6.35/e025cf1c47e63aada985d78e4083f2e02fba434f
PS-7731 : Merge percona-202102
Summary:
Today in `SELECT count(*)` MyRocks would still decode every single column due to this check, despite the readset being empty:
```
// bitmap is cleared on index merge, but it still needs to decode columns
bool field_requested =
decode_all_fields || m_verify_row_debug_checksums ||
bitmap_is_set(field_map, m_table->field[i]->field_index);
```
As a result MyRocks is significantly slower than InnoDB in this particular scenario.
Turns out in index merge, when it tries to reset, it calls ha_index_init with an empty column_bitmap, so our field decoders didn't know it needs to decode anything, so the entire query would return nothing. This is discussed in [this commit](facebook/mysql-5.6@70f2bcd), and [issue 624](facebook/mysql-5.6#624) and [PR 626](facebook/mysql-5.6#626). So the workaround we had at that time is to simply treat empty map as implicitly everything, and the side effect is massively slowed down count(*).
We have a few options to address this:
1. Fix index merge optimizer - looking at the code in QUICK_RANGE_SELECT::init_ror_merged_scan, it actually fixes up the column_bitmap properly, but after init/reset, so the fix would simply be moving the bitmap set code up. For secondary keys, prepare_for_position will automatically call `mark_columns_used_by_index_no_reset(s->primary_key, read_set)` if HA_PRIMARY_KEY_REQUIRED_FOR_POSITION is set (true for both InnoDB and MyRocks), so we would know correctly that we need to unpack PK when walking SK during index merge.
2. Overriding `column_bitmaps_signal` and setup decoders whenever the bitmap changes - however this doesn't work by itself. Because no storage engine today actually use handler::column_bitmaps_signal this path haven't been tested properly in index merge. In this case, QUICK_RANGE_SELECT::init_ror_merged_scan should call set_column_bitmaps_no_signal to avoid resetting the correct read/write set of head since head is used as first handler (reuses_handler=true) and subsequent place holders for read/write set updates (reuse_handler=false).
3. Follow InnoDB's solution - InnoDB delays it actually initialize its template again in index_read for the 2nd time (relying on `prebuilt->sql_stat_start`), and during index_read `QUICK_RANGE_SELECT::column_bitmap` is already fixed up and the table read/write set is switched to it, so the new template would be built correctly.
In order to make it easier to maintain and port, after discussing with Manuel, I'm going with a simplified version of #3 that delays decoder creation until the first read operation (index_*, rnd_*, range_read_*, multi_range_read_*), and setting the delay flag in index_init / rnd_init / multi_range_read_init.
Also, I ran into a bug with truncation_partition where Rdb_converter's tbl_def is stale (we only update ha_rocksdb::m_tbl_def), but it is fine because it is not being used after table open. But my change moves the lookup_bitmap initialization into Rdb_converter which takes a dependency on Rdb_converter::m_tbl_def so now we need to reset it properly.
Reference Patch: facebook/mysql-5.6@44d6a8d
---------
Porting Note: Due to 8.0's new counting infra (handler::record & handler::record_with_index), this only helps PK counting. Will send out a better fix that works better with 8.0 new counting infra.
Reviewed By: Pushapgl
Differential Revision: D26265470
fbshipit-source-id: f142be681ab
inikep
pushed a commit
that referenced
this pull request
Nov 11, 2024
…s=0 and a local DDL
executed
https://perconadev.atlassian.net/browse/PS-9018
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
added a commit
that referenced
this pull request
Nov 11, 2024
PS-5741: Incorrect use of memset_s in keyring_vault.
Fixed the usage of memset_s. The arguments should be:
void memset_s(void *dest, size_t dest_max, int c, size_t n)
where the 2nd argument is size of buffer and the 3rd is
argument is character to fill.
---------------------------------------------------------------------------
PS-7769 - Fix use-after-return error in audit_log_exclude_accounts_validate
---
*Problem:*
`st_mysql_value::val_str` might return a pointer to `buf` which after
the function called is deleted. Therefore the value in `save`, after
reuturnin from the function, is invalid.
In this particular case, the error is not manifesting as val_str`
returns memory allocated with `thd_strmake` and it does not use `buf`.
*Solution:*
Allocate memory with `thd_strmake` so the memory in `save` is not local.
---------------------------------------------------------------------------
Fix test main.bug12969156 when WITH_ASAN=ON
*Problem:*
ASAN complains about stack-buffer-overflow on function `mysql_heartbeat`:
```
==90890==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fe746d06d14 at pc 0x7fe760f5b017 bp 0x7fe746d06cd0 sp 0x7fe746d06478
WRITE of size 24 at 0x7fe746d06d14 thread T16777215
Address 0x7fe746d06d14 is located in stack of thread T26 at offset 340 in frame
#0 0x7fe746d0a55c in mysql_heartbeat(void*) /home/yura/ws/percona-server/plugin/daemon_example/daemon_example.cc:62
This frame has 4 object(s):
[48, 56) 'result' (line 66)
[80, 112) '_db_stack_frame_' (line 63)
[144, 200) 'tm_tmp' (line 67)
[240, 340) 'buffer' (line 65) <== Memory access at offset 340 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
(longjmp and C++ exceptions *are* supported)
Thread T26 created by T25 here:
#0 0x7fe760f5f6d5 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:216
#1 0x557ccbbcb857 in my_thread_create /home/yura/ws/percona-server/mysys/my_thread.c:104
#2 0x7fe746d0b21a in daemon_example_plugin_init /home/yura/ws/percona-server/plugin/daemon_example/daemon_example.cc:148
#3 0x557ccb4c69c7 in plugin_initialize /home/yura/ws/percona-server/sql/sql_plugin.cc:1279
#4 0x557ccb4d19cd in mysql_install_plugin /home/yura/ws/percona-server/sql/sql_plugin.cc:2279
#5 0x557ccb4d218f in Sql_cmd_install_plugin::execute(THD*) /home/yura/ws/percona-server/sql/sql_plugin.cc:4664
#6 0x557ccb47695e in mysql_execute_command(THD*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5160
#7 0x557ccb47977c in mysql_parse(THD*, Parser_state*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5952
#8 0x557ccb47b6c2 in dispatch_command(THD*, COM_DATA const*, enum_server_command) /home/yura/ws/percona-server/sql/sql_parse.cc:1544
#9 0x557ccb47de1d in do_command(THD*) /home/yura/ws/percona-server/sql/sql_parse.cc:1065
#10 0x557ccb6ac294 in handle_connection /home/yura/ws/percona-server/sql/conn_handler/connection_handler_per_thread.cc:325
#11 0x557ccbbfabb0 in pfs_spawn_thread /home/yura/ws/percona-server/storage/perfschema/pfs.cc:2198
#12 0x7fe760ab544f in start_thread nptl/pthread_create.c:473
```
The reason is that `my_thread_cancel` is used to finish the daemon thread. This is not and orderly way of finishing the thread. ASAN does not register the stack variables are not used anymore which generates the error above.
This is a benign error as all the variables are on the stack.
*Solution*:
Finish the thread in orderly way by using a signalling variable.
---------------------------------------------------------------------------
PS-8204: Fix XML escape rules for audit plugin
https://jira.percona.com/browse/PS-8204
There was a wrong length specified for some XML
escape rules. As a result of this terminating null symbol from
replacement rule was copied into resulting string. This lead to
quer text truncation in audit log file.
In addition added empty replacement rules for '\b' and 'f' symbols
which just remove them from resulting string. These symboles are
not supported in XML 1.0.
---------------------------------------------------------------------------
PS-8854: Add main.percona_udf MTR test
Add a test to check FNV1A_64, FNV_64, and MURMUR_HASH user-defined functions.
---------------------------------------------------------------------------
PS-9218: Merge MySQL 8.4.0 (fix gcc-14 build)
https://perconadev.atlassian.net/browse/PS-9218
inikep
pushed a commit
that referenced
this pull request
Nov 11, 2024
…n read() syscall over network https://jira.percona.com/browse/PS-8592 Description ----------- GR suffered from problems caused by the security probes and network scanner processes connecting to the group replication communication port. This usually is not a problem, but poses a serious threat when another member tries to join the cluster by initialting a connection to the member which is affected by external processes using the port dedicated for group communication for longer durations. On such activites by external processes, the SSL enabled server stalled forever on the SSL_accept() call waiting for handshake data. Below is the stacktrace: Thread 55 (Thread 0x7f7bb77ff700 (LWP 2198598)): #0 in read () #1 in sock_read () #2 in BIO_read () #3 in ssl23_read_bytes () #4 in ssl23_get_client_hello () #5 in ssl23_accept () #6 in xcom_tcp_server_startup(Xcom_network_provider*) () When the server stalled in the above path forever, it prohibited other members to join the cluster resulting in the following messages on the joiner server's logs. [ERROR] [MY-011640] [Repl] Plugin group_replication reported: 'Timeout on wait for view after joining group' [ERROR] [MY-011735] [Repl] Plugin group_replication reported: '[GCS] The member is already leaving or joining a group.' Solution -------- This patch adds two new variables 1. group_replication_xcom_ssl_socket_timeout It is a file-descriptor level timeout in seconds for both accept() and SSL_accept() calls when group replication is listening on the xcom port. When set to a valid value, say for example 5 seconds, both accept() and SSL_accept() return after 5 seconds. The default value has been set to 0 (waits infinitely) for backward compatibility. This variable is effective only when GR is configred with SSL. 2. group_replication_xcom_ssl_accept_retries It defines the number of retries to be performed before closing the socket. For each retry the server thread calls SSL_accept() with timeout defined by the group_replication_xcom_ssl_socket_timeout for the SSL handshake process once the connection has been accepted by the first accept() call. The default value has been set to 10. This variable is effective only when GR is configred with SSL. Note: - Both of the above variables are dynamically configurable, but will become effective only on START GROUP_REPLICATION. ------------------------------------------------------------------------- PS-8844: Fix the failing main.mysqldump_gtid_purged https://jira.percona.com/browse/PS-8844 This patch fixes the test failure of main.mysqldump_gtid_purged that failed due to the uninitialized variable $redirect_stderr in the start_proc_in_background.inc. ---------------------------------------------------------------------- PS-9218: Merge MySQL 8.4.0 (fix terminology in replication tests) https://perconadev.atlassian.net/browse/PS-9218 mysql/mysql-server@44a77b5
inikep
pushed a commit
that referenced
this pull request
Nov 14, 2024
Upstream commit ID : fb-mysql-5.6.35/8cb1dc836b68f1f13e8b2655b2b8cb2d57f400b3 PS-5217 : Merge fb-prod201803 Summary: Original report: https://jira.mariadb.org/browse/MDEV-15816 To reproduce this bug just following below steps, client 1: USE test; CREATE TABLE t1 (i INT) ENGINE=MyISAM; HANDLER t1 OPEN h; CREATE TABLE t2 (i INT) ENGINE=RocksDB; LOCK TABLES t2 WRITE; client 2: FLUSH TABLES WITH READ LOCK; client 1: INSERT INTO t2 VALUES (1); So client 1 acquired the lock and set m_lock_rows = RDB_LOCK_WRITE. Then client 2 calls store_lock(TL_IGNORE) and m_lock_rows was wrongly set to RDB_LOCK_NONE, as below ``` #0 myrocks::ha_rocksdb::store_lock (this=0x7fffbc03c7c8, thd=0x7fffc0000ba0, to=0x7fffc0011220, lock_type=TL_IGNORE) #1 get_lock_data (thd=0x7fffc0000ba0, table_ptr=0x7fffe84b7d20, count=1, flags=2) #2 mysql_lock_abort_for_thread (thd=0x7fffc0000ba0, table=0x7fffbc03bbc0) #3 THD::notify_shared_lock (this=0x7fffc0000ba0, ctx_in_use=0x7fffbc000bd8, needs_thr_lock_abort=true) #4 MDL_lock::notify_conflicting_locks (this=0x555557a82380, ctx=0x7fffc0000cc8) #5 MDL_context::acquire_lock (this=0x7fffc0000cc8, mdl_request=0x7fffe84b8350, lock_wait_timeout=2) #6 Global_read_lock::lock_global_read_lock (this=0x7fffc0003fe0, thd=0x7fffc0000ba0) ``` Finally, client 1 "INSERT INTO..." hits the Assertion 'm_lock_rows == RDB_LOCK_WRITE' failed in myrocks::ha_rocksdb::write_row() Fix this bug by not setting m_locks_rows if lock_type == TL_IGNORE. Closes facebook/mysql-5.6#838 Pull Request resolved: facebook/mysql-5.6#871 Differential Revision: D9417382 Pulled By: lth fbshipit-source-id: c36c164e06c
inikep
pushed a commit
that referenced
this pull request
Nov 14, 2024
Upstream commit ID : fb-mysql-5.6.35/77032004ad23d21a4c386f8136ecfbb071ea42d6 PS-6865 : Merge fb-prod201903 Summary: Currently during primary key's value encode, its ttl value can be from either one of these 3 cases 1. ttl column in primary key 2. non-ttl column a. old record(update case) b. current timestamp 3. ttl column in non-key field Workflow #1: first in Rdb_key_def::pack_record() find and store pk_offset, then in value encode try to parse key slice to fetch ttl value by using pk_offset. Workflow #3: fetch ttl value from ttl column The change is to merge #1 and #3 by always fetching TTL value from ttl column, not matter whether the ttl column is in primary key or not. Of course, remove pk_offset, since it isn't used. BTW, for secondary keys, its ttl value is always from m_ttl_bytes, which is stored by primary value encoding. Reviewed By: yizhang82 Differential Revision: D14662716 fbshipit-source-id: 6b4e5f044fd
inikep
pushed a commit
that referenced
this pull request
Nov 14, 2024
Upstream commit ID : fb-mysql-5.6.35/e025cf1c47e63aada985d78e4083f2e02fba434f
PS-7731 : Merge percona-202102
Summary:
Today in `SELECT count(*)` MyRocks would still decode every single column due to this check, despite the readset being empty:
```
// bitmap is cleared on index merge, but it still needs to decode columns
bool field_requested =
decode_all_fields || m_verify_row_debug_checksums ||
bitmap_is_set(field_map, m_table->field[i]->field_index);
```
As a result MyRocks is significantly slower than InnoDB in this particular scenario.
Turns out in index merge, when it tries to reset, it calls ha_index_init with an empty column_bitmap, so our field decoders didn't know it needs to decode anything, so the entire query would return nothing. This is discussed in [this commit](facebook/mysql-5.6@70f2bcd), and [issue 624](facebook/mysql-5.6#624) and [PR 626](facebook/mysql-5.6#626). So the workaround we had at that time is to simply treat empty map as implicitly everything, and the side effect is massively slowed down count(*).
We have a few options to address this:
1. Fix index merge optimizer - looking at the code in QUICK_RANGE_SELECT::init_ror_merged_scan, it actually fixes up the column_bitmap properly, but after init/reset, so the fix would simply be moving the bitmap set code up. For secondary keys, prepare_for_position will automatically call `mark_columns_used_by_index_no_reset(s->primary_key, read_set)` if HA_PRIMARY_KEY_REQUIRED_FOR_POSITION is set (true for both InnoDB and MyRocks), so we would know correctly that we need to unpack PK when walking SK during index merge.
2. Overriding `column_bitmaps_signal` and setup decoders whenever the bitmap changes - however this doesn't work by itself. Because no storage engine today actually use handler::column_bitmaps_signal this path haven't been tested properly in index merge. In this case, QUICK_RANGE_SELECT::init_ror_merged_scan should call set_column_bitmaps_no_signal to avoid resetting the correct read/write set of head since head is used as first handler (reuses_handler=true) and subsequent place holders for read/write set updates (reuse_handler=false).
3. Follow InnoDB's solution - InnoDB delays it actually initialize its template again in index_read for the 2nd time (relying on `prebuilt->sql_stat_start`), and during index_read `QUICK_RANGE_SELECT::column_bitmap` is already fixed up and the table read/write set is switched to it, so the new template would be built correctly.
In order to make it easier to maintain and port, after discussing with Manuel, I'm going with a simplified version of #3 that delays decoder creation until the first read operation (index_*, rnd_*, range_read_*, multi_range_read_*), and setting the delay flag in index_init / rnd_init / multi_range_read_init.
Also, I ran into a bug with truncation_partition where Rdb_converter's tbl_def is stale (we only update ha_rocksdb::m_tbl_def), but it is fine because it is not being used after table open. But my change moves the lookup_bitmap initialization into Rdb_converter which takes a dependency on Rdb_converter::m_tbl_def so now we need to reset it properly.
Reference Patch: facebook/mysql-5.6@44d6a8d
---------
Porting Note: Due to 8.0's new counting infra (handler::record & handler::record_with_index), this only helps PK counting. Will send out a better fix that works better with 8.0 new counting infra.
Reviewed By: Pushapgl
Differential Revision: D26265470
fbshipit-source-id: f142be681ab
inikep
pushed a commit
that referenced
this pull request
Nov 14, 2024
…s=0 and a local DDL
executed
https://perconadev.atlassian.net/browse/PS-9018
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
added a commit
that referenced
this pull request
Nov 14, 2024
PS-5741: Incorrect use of memset_s in keyring_vault.
Fixed the usage of memset_s. The arguments should be:
void memset_s(void *dest, size_t dest_max, int c, size_t n)
where the 2nd argument is size of buffer and the 3rd is
argument is character to fill.
---------------------------------------------------------------------------
PS-7769 - Fix use-after-return error in audit_log_exclude_accounts_validate
---
*Problem:*
`st_mysql_value::val_str` might return a pointer to `buf` which after
the function called is deleted. Therefore the value in `save`, after
reuturnin from the function, is invalid.
In this particular case, the error is not manifesting as val_str`
returns memory allocated with `thd_strmake` and it does not use `buf`.
*Solution:*
Allocate memory with `thd_strmake` so the memory in `save` is not local.
---------------------------------------------------------------------------
Fix test main.bug12969156 when WITH_ASAN=ON
*Problem:*
ASAN complains about stack-buffer-overflow on function `mysql_heartbeat`:
```
==90890==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fe746d06d14 at pc 0x7fe760f5b017 bp 0x7fe746d06cd0 sp 0x7fe746d06478
WRITE of size 24 at 0x7fe746d06d14 thread T16777215
Address 0x7fe746d06d14 is located in stack of thread T26 at offset 340 in frame
#0 0x7fe746d0a55c in mysql_heartbeat(void*) /home/yura/ws/percona-server/plugin/daemon_example/daemon_example.cc:62
This frame has 4 object(s):
[48, 56) 'result' (line 66)
[80, 112) '_db_stack_frame_' (line 63)
[144, 200) 'tm_tmp' (line 67)
[240, 340) 'buffer' (line 65) <== Memory access at offset 340 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
(longjmp and C++ exceptions *are* supported)
Thread T26 created by T25 here:
#0 0x7fe760f5f6d5 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:216
#1 0x557ccbbcb857 in my_thread_create /home/yura/ws/percona-server/mysys/my_thread.c:104
#2 0x7fe746d0b21a in daemon_example_plugin_init /home/yura/ws/percona-server/plugin/daemon_example/daemon_example.cc:148
#3 0x557ccb4c69c7 in plugin_initialize /home/yura/ws/percona-server/sql/sql_plugin.cc:1279
#4 0x557ccb4d19cd in mysql_install_plugin /home/yura/ws/percona-server/sql/sql_plugin.cc:2279
#5 0x557ccb4d218f in Sql_cmd_install_plugin::execute(THD*) /home/yura/ws/percona-server/sql/sql_plugin.cc:4664
#6 0x557ccb47695e in mysql_execute_command(THD*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5160
#7 0x557ccb47977c in mysql_parse(THD*, Parser_state*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5952
#8 0x557ccb47b6c2 in dispatch_command(THD*, COM_DATA const*, enum_server_command) /home/yura/ws/percona-server/sql/sql_parse.cc:1544
#9 0x557ccb47de1d in do_command(THD*) /home/yura/ws/percona-server/sql/sql_parse.cc:1065
#10 0x557ccb6ac294 in handle_connection /home/yura/ws/percona-server/sql/conn_handler/connection_handler_per_thread.cc:325
#11 0x557ccbbfabb0 in pfs_spawn_thread /home/yura/ws/percona-server/storage/perfschema/pfs.cc:2198
#12 0x7fe760ab544f in start_thread nptl/pthread_create.c:473
```
The reason is that `my_thread_cancel` is used to finish the daemon thread. This is not and orderly way of finishing the thread. ASAN does not register the stack variables are not used anymore which generates the error above.
This is a benign error as all the variables are on the stack.
*Solution*:
Finish the thread in orderly way by using a signalling variable.
---------------------------------------------------------------------------
PS-8204: Fix XML escape rules for audit plugin
https://jira.percona.com/browse/PS-8204
There was a wrong length specified for some XML
escape rules. As a result of this terminating null symbol from
replacement rule was copied into resulting string. This lead to
quer text truncation in audit log file.
In addition added empty replacement rules for '\b' and 'f' symbols
which just remove them from resulting string. These symboles are
not supported in XML 1.0.
---------------------------------------------------------------------------
PS-8854: Add main.percona_udf MTR test
Add a test to check FNV1A_64, FNV_64, and MURMUR_HASH user-defined functions.
---------------------------------------------------------------------------
PS-9218: Merge MySQL 8.4.0 (fix gcc-14 build)
https://perconadev.atlassian.net/browse/PS-9218
inikep
pushed a commit
that referenced
this pull request
Nov 14, 2024
…n read() syscall over network https://jira.percona.com/browse/PS-8592 Description ----------- GR suffered from problems caused by the security probes and network scanner processes connecting to the group replication communication port. This usually is not a problem, but poses a serious threat when another member tries to join the cluster by initialting a connection to the member which is affected by external processes using the port dedicated for group communication for longer durations. On such activites by external processes, the SSL enabled server stalled forever on the SSL_accept() call waiting for handshake data. Below is the stacktrace: Thread 55 (Thread 0x7f7bb77ff700 (LWP 2198598)): #0 in read () #1 in sock_read () #2 in BIO_read () #3 in ssl23_read_bytes () #4 in ssl23_get_client_hello () #5 in ssl23_accept () #6 in xcom_tcp_server_startup(Xcom_network_provider*) () When the server stalled in the above path forever, it prohibited other members to join the cluster resulting in the following messages on the joiner server's logs. [ERROR] [MY-011640] [Repl] Plugin group_replication reported: 'Timeout on wait for view after joining group' [ERROR] [MY-011735] [Repl] Plugin group_replication reported: '[GCS] The member is already leaving or joining a group.' Solution -------- This patch adds two new variables 1. group_replication_xcom_ssl_socket_timeout It is a file-descriptor level timeout in seconds for both accept() and SSL_accept() calls when group replication is listening on the xcom port. When set to a valid value, say for example 5 seconds, both accept() and SSL_accept() return after 5 seconds. The default value has been set to 0 (waits infinitely) for backward compatibility. This variable is effective only when GR is configred with SSL. 2. group_replication_xcom_ssl_accept_retries It defines the number of retries to be performed before closing the socket. For each retry the server thread calls SSL_accept() with timeout defined by the group_replication_xcom_ssl_socket_timeout for the SSL handshake process once the connection has been accepted by the first accept() call. The default value has been set to 10. This variable is effective only when GR is configred with SSL. Note: - Both of the above variables are dynamically configurable, but will become effective only on START GROUP_REPLICATION. ------------------------------------------------------------------------- PS-8844: Fix the failing main.mysqldump_gtid_purged https://jira.percona.com/browse/PS-8844 This patch fixes the test failure of main.mysqldump_gtid_purged that failed due to the uninitialized variable $redirect_stderr in the start_proc_in_background.inc. ---------------------------------------------------------------------- PS-9218: Merge MySQL 8.4.0 (fix terminology in replication tests) https://perconadev.atlassian.net/browse/PS-9218 mysql/mysql-server@44a77b5
inikep
pushed a commit
that referenced
this pull request
Nov 14, 2024
Upstream commit ID : fb-mysql-5.6.35/8cb1dc836b68f1f13e8b2655b2b8cb2d57f400b3 PS-5217 : Merge fb-prod201803 Summary: Original report: https://jira.mariadb.org/browse/MDEV-15816 To reproduce this bug just following below steps, client 1: USE test; CREATE TABLE t1 (i INT) ENGINE=MyISAM; HANDLER t1 OPEN h; CREATE TABLE t2 (i INT) ENGINE=RocksDB; LOCK TABLES t2 WRITE; client 2: FLUSH TABLES WITH READ LOCK; client 1: INSERT INTO t2 VALUES (1); So client 1 acquired the lock and set m_lock_rows = RDB_LOCK_WRITE. Then client 2 calls store_lock(TL_IGNORE) and m_lock_rows was wrongly set to RDB_LOCK_NONE, as below ``` #0 myrocks::ha_rocksdb::store_lock (this=0x7fffbc03c7c8, thd=0x7fffc0000ba0, to=0x7fffc0011220, lock_type=TL_IGNORE) #1 get_lock_data (thd=0x7fffc0000ba0, table_ptr=0x7fffe84b7d20, count=1, flags=2) #2 mysql_lock_abort_for_thread (thd=0x7fffc0000ba0, table=0x7fffbc03bbc0) #3 THD::notify_shared_lock (this=0x7fffc0000ba0, ctx_in_use=0x7fffbc000bd8, needs_thr_lock_abort=true) #4 MDL_lock::notify_conflicting_locks (this=0x555557a82380, ctx=0x7fffc0000cc8) #5 MDL_context::acquire_lock (this=0x7fffc0000cc8, mdl_request=0x7fffe84b8350, lock_wait_timeout=2) #6 Global_read_lock::lock_global_read_lock (this=0x7fffc0003fe0, thd=0x7fffc0000ba0) ``` Finally, client 1 "INSERT INTO..." hits the Assertion 'm_lock_rows == RDB_LOCK_WRITE' failed in myrocks::ha_rocksdb::write_row() Fix this bug by not setting m_locks_rows if lock_type == TL_IGNORE. Closes facebook/mysql-5.6#838 Pull Request resolved: facebook/mysql-5.6#871 Differential Revision: D9417382 Pulled By: lth fbshipit-source-id: c36c164e06c
inikep
pushed a commit
that referenced
this pull request
Nov 14, 2024
Upstream commit ID : fb-mysql-5.6.35/77032004ad23d21a4c386f8136ecfbb071ea42d6 PS-6865 : Merge fb-prod201903 Summary: Currently during primary key's value encode, its ttl value can be from either one of these 3 cases 1. ttl column in primary key 2. non-ttl column a. old record(update case) b. current timestamp 3. ttl column in non-key field Workflow #1: first in Rdb_key_def::pack_record() find and store pk_offset, then in value encode try to parse key slice to fetch ttl value by using pk_offset. Workflow #3: fetch ttl value from ttl column The change is to merge #1 and #3 by always fetching TTL value from ttl column, not matter whether the ttl column is in primary key or not. Of course, remove pk_offset, since it isn't used. BTW, for secondary keys, its ttl value is always from m_ttl_bytes, which is stored by primary value encoding. Reviewed By: yizhang82 Differential Revision: D14662716 fbshipit-source-id: 6b4e5f044fd
inikep
pushed a commit
that referenced
this pull request
Nov 14, 2024
Upstream commit ID : fb-mysql-5.6.35/e025cf1c47e63aada985d78e4083f2e02fba434f
PS-7731 : Merge percona-202102
Summary:
Today in `SELECT count(*)` MyRocks would still decode every single column due to this check, despite the readset being empty:
```
// bitmap is cleared on index merge, but it still needs to decode columns
bool field_requested =
decode_all_fields || m_verify_row_debug_checksums ||
bitmap_is_set(field_map, m_table->field[i]->field_index);
```
As a result MyRocks is significantly slower than InnoDB in this particular scenario.
Turns out in index merge, when it tries to reset, it calls ha_index_init with an empty column_bitmap, so our field decoders didn't know it needs to decode anything, so the entire query would return nothing. This is discussed in [this commit](facebook/mysql-5.6@70f2bcd), and [issue 624](facebook/mysql-5.6#624) and [PR 626](facebook/mysql-5.6#626). So the workaround we had at that time is to simply treat empty map as implicitly everything, and the side effect is massively slowed down count(*).
We have a few options to address this:
1. Fix index merge optimizer - looking at the code in QUICK_RANGE_SELECT::init_ror_merged_scan, it actually fixes up the column_bitmap properly, but after init/reset, so the fix would simply be moving the bitmap set code up. For secondary keys, prepare_for_position will automatically call `mark_columns_used_by_index_no_reset(s->primary_key, read_set)` if HA_PRIMARY_KEY_REQUIRED_FOR_POSITION is set (true for both InnoDB and MyRocks), so we would know correctly that we need to unpack PK when walking SK during index merge.
2. Overriding `column_bitmaps_signal` and setup decoders whenever the bitmap changes - however this doesn't work by itself. Because no storage engine today actually use handler::column_bitmaps_signal this path haven't been tested properly in index merge. In this case, QUICK_RANGE_SELECT::init_ror_merged_scan should call set_column_bitmaps_no_signal to avoid resetting the correct read/write set of head since head is used as first handler (reuses_handler=true) and subsequent place holders for read/write set updates (reuse_handler=false).
3. Follow InnoDB's solution - InnoDB delays it actually initialize its template again in index_read for the 2nd time (relying on `prebuilt->sql_stat_start`), and during index_read `QUICK_RANGE_SELECT::column_bitmap` is already fixed up and the table read/write set is switched to it, so the new template would be built correctly.
In order to make it easier to maintain and port, after discussing with Manuel, I'm going with a simplified version of #3 that delays decoder creation until the first read operation (index_*, rnd_*, range_read_*, multi_range_read_*), and setting the delay flag in index_init / rnd_init / multi_range_read_init.
Also, I ran into a bug with truncation_partition where Rdb_converter's tbl_def is stale (we only update ha_rocksdb::m_tbl_def), but it is fine because it is not being used after table open. But my change moves the lookup_bitmap initialization into Rdb_converter which takes a dependency on Rdb_converter::m_tbl_def so now we need to reset it properly.
Reference Patch: facebook/mysql-5.6@44d6a8d
---------
Porting Note: Due to 8.0's new counting infra (handler::record & handler::record_with_index), this only helps PK counting. Will send out a better fix that works better with 8.0 new counting infra.
Reviewed By: Pushapgl
Differential Revision: D26265470
fbshipit-source-id: f142be681ab
inikep
pushed a commit
that referenced
this pull request
Nov 14, 2024
…s=0 and a local DDL
executed
https://perconadev.atlassian.net/browse/PS-9018
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
added a commit
that referenced
this pull request
Nov 14, 2024
PS-5741: Incorrect use of memset_s in keyring_vault.
Fixed the usage of memset_s. The arguments should be:
void memset_s(void *dest, size_t dest_max, int c, size_t n)
where the 2nd argument is size of buffer and the 3rd is
argument is character to fill.
---------------------------------------------------------------------------
PS-7769 - Fix use-after-return error in audit_log_exclude_accounts_validate
---
*Problem:*
`st_mysql_value::val_str` might return a pointer to `buf` which after
the function called is deleted. Therefore the value in `save`, after
reuturnin from the function, is invalid.
In this particular case, the error is not manifesting as val_str`
returns memory allocated with `thd_strmake` and it does not use `buf`.
*Solution:*
Allocate memory with `thd_strmake` so the memory in `save` is not local.
---------------------------------------------------------------------------
Fix test main.bug12969156 when WITH_ASAN=ON
*Problem:*
ASAN complains about stack-buffer-overflow on function `mysql_heartbeat`:
```
==90890==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fe746d06d14 at pc 0x7fe760f5b017 bp 0x7fe746d06cd0 sp 0x7fe746d06478
WRITE of size 24 at 0x7fe746d06d14 thread T16777215
Address 0x7fe746d06d14 is located in stack of thread T26 at offset 340 in frame
#0 0x7fe746d0a55c in mysql_heartbeat(void*) /home/yura/ws/percona-server/plugin/daemon_example/daemon_example.cc:62
This frame has 4 object(s):
[48, 56) 'result' (line 66)
[80, 112) '_db_stack_frame_' (line 63)
[144, 200) 'tm_tmp' (line 67)
[240, 340) 'buffer' (line 65) <== Memory access at offset 340 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
(longjmp and C++ exceptions *are* supported)
Thread T26 created by T25 here:
#0 0x7fe760f5f6d5 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:216
#1 0x557ccbbcb857 in my_thread_create /home/yura/ws/percona-server/mysys/my_thread.c:104
#2 0x7fe746d0b21a in daemon_example_plugin_init /home/yura/ws/percona-server/plugin/daemon_example/daemon_example.cc:148
#3 0x557ccb4c69c7 in plugin_initialize /home/yura/ws/percona-server/sql/sql_plugin.cc:1279
#4 0x557ccb4d19cd in mysql_install_plugin /home/yura/ws/percona-server/sql/sql_plugin.cc:2279
#5 0x557ccb4d218f in Sql_cmd_install_plugin::execute(THD*) /home/yura/ws/percona-server/sql/sql_plugin.cc:4664
#6 0x557ccb47695e in mysql_execute_command(THD*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5160
#7 0x557ccb47977c in mysql_parse(THD*, Parser_state*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5952
#8 0x557ccb47b6c2 in dispatch_command(THD*, COM_DATA const*, enum_server_command) /home/yura/ws/percona-server/sql/sql_parse.cc:1544
#9 0x557ccb47de1d in do_command(THD*) /home/yura/ws/percona-server/sql/sql_parse.cc:1065
#10 0x557ccb6ac294 in handle_connection /home/yura/ws/percona-server/sql/conn_handler/connection_handler_per_thread.cc:325
#11 0x557ccbbfabb0 in pfs_spawn_thread /home/yura/ws/percona-server/storage/perfschema/pfs.cc:2198
#12 0x7fe760ab544f in start_thread nptl/pthread_create.c:473
```
The reason is that `my_thread_cancel` is used to finish the daemon thread. This is not and orderly way of finishing the thread. ASAN does not register the stack variables are not used anymore which generates the error above.
This is a benign error as all the variables are on the stack.
*Solution*:
Finish the thread in orderly way by using a signalling variable.
---------------------------------------------------------------------------
PS-8204: Fix XML escape rules for audit plugin
https://jira.percona.com/browse/PS-8204
There was a wrong length specified for some XML
escape rules. As a result of this terminating null symbol from
replacement rule was copied into resulting string. This lead to
quer text truncation in audit log file.
In addition added empty replacement rules for '\b' and 'f' symbols
which just remove them from resulting string. These symboles are
not supported in XML 1.0.
---------------------------------------------------------------------------
PS-8854: Add main.percona_udf MTR test
Add a test to check FNV1A_64, FNV_64, and MURMUR_HASH user-defined functions.
---------------------------------------------------------------------------
PS-9218: Merge MySQL 8.4.0 (fix gcc-14 build)
https://perconadev.atlassian.net/browse/PS-9218
inikep
pushed a commit
that referenced
this pull request
Nov 14, 2024
…n read() syscall over network https://jira.percona.com/browse/PS-8592 Description ----------- GR suffered from problems caused by the security probes and network scanner processes connecting to the group replication communication port. This usually is not a problem, but poses a serious threat when another member tries to join the cluster by initialting a connection to the member which is affected by external processes using the port dedicated for group communication for longer durations. On such activites by external processes, the SSL enabled server stalled forever on the SSL_accept() call waiting for handshake data. Below is the stacktrace: Thread 55 (Thread 0x7f7bb77ff700 (LWP 2198598)): #0 in read () #1 in sock_read () #2 in BIO_read () #3 in ssl23_read_bytes () #4 in ssl23_get_client_hello () #5 in ssl23_accept () #6 in xcom_tcp_server_startup(Xcom_network_provider*) () When the server stalled in the above path forever, it prohibited other members to join the cluster resulting in the following messages on the joiner server's logs. [ERROR] [MY-011640] [Repl] Plugin group_replication reported: 'Timeout on wait for view after joining group' [ERROR] [MY-011735] [Repl] Plugin group_replication reported: '[GCS] The member is already leaving or joining a group.' Solution -------- This patch adds two new variables 1. group_replication_xcom_ssl_socket_timeout It is a file-descriptor level timeout in seconds for both accept() and SSL_accept() calls when group replication is listening on the xcom port. When set to a valid value, say for example 5 seconds, both accept() and SSL_accept() return after 5 seconds. The default value has been set to 0 (waits infinitely) for backward compatibility. This variable is effective only when GR is configred with SSL. 2. group_replication_xcom_ssl_accept_retries It defines the number of retries to be performed before closing the socket. For each retry the server thread calls SSL_accept() with timeout defined by the group_replication_xcom_ssl_socket_timeout for the SSL handshake process once the connection has been accepted by the first accept() call. The default value has been set to 10. This variable is effective only when GR is configred with SSL. Note: - Both of the above variables are dynamically configurable, but will become effective only on START GROUP_REPLICATION. ------------------------------------------------------------------------- PS-8844: Fix the failing main.mysqldump_gtid_purged https://jira.percona.com/browse/PS-8844 This patch fixes the test failure of main.mysqldump_gtid_purged that failed due to the uninitialized variable $redirect_stderr in the start_proc_in_background.inc. ---------------------------------------------------------------------- PS-9218: Merge MySQL 8.4.0 (fix terminology in replication tests) https://perconadev.atlassian.net/browse/PS-9218 mysql/mysql-server@44a77b5
inikep
pushed a commit
that referenced
this pull request
Nov 18, 2024
…s=0 and a local DDL
executed
https://perconadev.atlassian.net/browse/PS-9018
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
added a commit
that referenced
this pull request
Nov 18, 2024
PS-5741: Incorrect use of memset_s in keyring_vault.
Fixed the usage of memset_s. The arguments should be:
void memset_s(void *dest, size_t dest_max, int c, size_t n)
where the 2nd argument is size of buffer and the 3rd is
argument is character to fill.
---------------------------------------------------------------------------
PS-7769 - Fix use-after-return error in audit_log_exclude_accounts_validate
---
*Problem:*
`st_mysql_value::val_str` might return a pointer to `buf` which after
the function called is deleted. Therefore the value in `save`, after
reuturnin from the function, is invalid.
In this particular case, the error is not manifesting as val_str`
returns memory allocated with `thd_strmake` and it does not use `buf`.
*Solution:*
Allocate memory with `thd_strmake` so the memory in `save` is not local.
---------------------------------------------------------------------------
Fix test main.bug12969156 when WITH_ASAN=ON
*Problem:*
ASAN complains about stack-buffer-overflow on function `mysql_heartbeat`:
```
==90890==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fe746d06d14 at pc 0x7fe760f5b017 bp 0x7fe746d06cd0 sp 0x7fe746d06478
WRITE of size 24 at 0x7fe746d06d14 thread T16777215
Address 0x7fe746d06d14 is located in stack of thread T26 at offset 340 in frame
#0 0x7fe746d0a55c in mysql_heartbeat(void*) /home/yura/ws/percona-server/plugin/daemon_example/daemon_example.cc:62
This frame has 4 object(s):
[48, 56) 'result' (line 66)
[80, 112) '_db_stack_frame_' (line 63)
[144, 200) 'tm_tmp' (line 67)
[240, 340) 'buffer' (line 65) <== Memory access at offset 340 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
(longjmp and C++ exceptions *are* supported)
Thread T26 created by T25 here:
#0 0x7fe760f5f6d5 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:216
#1 0x557ccbbcb857 in my_thread_create /home/yura/ws/percona-server/mysys/my_thread.c:104
#2 0x7fe746d0b21a in daemon_example_plugin_init /home/yura/ws/percona-server/plugin/daemon_example/daemon_example.cc:148
#3 0x557ccb4c69c7 in plugin_initialize /home/yura/ws/percona-server/sql/sql_plugin.cc:1279
#4 0x557ccb4d19cd in mysql_install_plugin /home/yura/ws/percona-server/sql/sql_plugin.cc:2279
#5 0x557ccb4d218f in Sql_cmd_install_plugin::execute(THD*) /home/yura/ws/percona-server/sql/sql_plugin.cc:4664
#6 0x557ccb47695e in mysql_execute_command(THD*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5160
#7 0x557ccb47977c in mysql_parse(THD*, Parser_state*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5952
#8 0x557ccb47b6c2 in dispatch_command(THD*, COM_DATA const*, enum_server_command) /home/yura/ws/percona-server/sql/sql_parse.cc:1544
#9 0x557ccb47de1d in do_command(THD*) /home/yura/ws/percona-server/sql/sql_parse.cc:1065
#10 0x557ccb6ac294 in handle_connection /home/yura/ws/percona-server/sql/conn_handler/connection_handler_per_thread.cc:325
#11 0x557ccbbfabb0 in pfs_spawn_thread /home/yura/ws/percona-server/storage/perfschema/pfs.cc:2198
#12 0x7fe760ab544f in start_thread nptl/pthread_create.c:473
```
The reason is that `my_thread_cancel` is used to finish the daemon thread. This is not and orderly way of finishing the thread. ASAN does not register the stack variables are not used anymore which generates the error above.
This is a benign error as all the variables are on the stack.
*Solution*:
Finish the thread in orderly way by using a signalling variable.
---------------------------------------------------------------------------
PS-8204: Fix XML escape rules for audit plugin
https://jira.percona.com/browse/PS-8204
There was a wrong length specified for some XML
escape rules. As a result of this terminating null symbol from
replacement rule was copied into resulting string. This lead to
quer text truncation in audit log file.
In addition added empty replacement rules for '\b' and 'f' symbols
which just remove them from resulting string. These symboles are
not supported in XML 1.0.
---------------------------------------------------------------------------
PS-8854: Add main.percona_udf MTR test
Add a test to check FNV1A_64, FNV_64, and MURMUR_HASH user-defined functions.
---------------------------------------------------------------------------
PS-9218: Merge MySQL 8.4.0 (fix gcc-14 build)
https://perconadev.atlassian.net/browse/PS-9218
inikep
pushed a commit
that referenced
this pull request
Nov 18, 2024
…n read() syscall over network https://jira.percona.com/browse/PS-8592 Description ----------- GR suffered from problems caused by the security probes and network scanner processes connecting to the group replication communication port. This usually is not a problem, but poses a serious threat when another member tries to join the cluster by initialting a connection to the member which is affected by external processes using the port dedicated for group communication for longer durations. On such activites by external processes, the SSL enabled server stalled forever on the SSL_accept() call waiting for handshake data. Below is the stacktrace: Thread 55 (Thread 0x7f7bb77ff700 (LWP 2198598)): #0 in read () #1 in sock_read () #2 in BIO_read () #3 in ssl23_read_bytes () #4 in ssl23_get_client_hello () #5 in ssl23_accept () #6 in xcom_tcp_server_startup(Xcom_network_provider*) () When the server stalled in the above path forever, it prohibited other members to join the cluster resulting in the following messages on the joiner server's logs. [ERROR] [MY-011640] [Repl] Plugin group_replication reported: 'Timeout on wait for view after joining group' [ERROR] [MY-011735] [Repl] Plugin group_replication reported: '[GCS] The member is already leaving or joining a group.' Solution -------- This patch adds two new variables 1. group_replication_xcom_ssl_socket_timeout It is a file-descriptor level timeout in seconds for both accept() and SSL_accept() calls when group replication is listening on the xcom port. When set to a valid value, say for example 5 seconds, both accept() and SSL_accept() return after 5 seconds. The default value has been set to 0 (waits infinitely) for backward compatibility. This variable is effective only when GR is configred with SSL. 2. group_replication_xcom_ssl_accept_retries It defines the number of retries to be performed before closing the socket. For each retry the server thread calls SSL_accept() with timeout defined by the group_replication_xcom_ssl_socket_timeout for the SSL handshake process once the connection has been accepted by the first accept() call. The default value has been set to 10. This variable is effective only when GR is configred with SSL. Note: - Both of the above variables are dynamically configurable, but will become effective only on START GROUP_REPLICATION. ------------------------------------------------------------------------- PS-8844: Fix the failing main.mysqldump_gtid_purged https://jira.percona.com/browse/PS-8844 This patch fixes the test failure of main.mysqldump_gtid_purged that failed due to the uninitialized variable $redirect_stderr in the start_proc_in_background.inc. ---------------------------------------------------------------------- PS-9218: Merge MySQL 8.4.0 (fix terminology in replication tests) https://perconadev.atlassian.net/browse/PS-9218 mysql/mysql-server@44a77b5
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.