Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Implemented PS-7621 (Merge MySQL 5.7.34) #5

Closed
wants to merge 74 commits into from

Conversation

ldonoso
Copy link
Owner

@ldonoso ldonoso commented Apr 21, 2021

bkandasa and others added 30 commits November 30, 2020 14:38
…UE TO MUTEX SPINNING [POST-PUSH FIX]

Description
-----------
Failling test due to removed trailing white-spaces in result file.

Fix
---
Re-recorded result file for `main.mysqld--help-win`. In 8.0, also
re-recorded the result file for `main.all_persisted_variables`.

Reviewed-by: Sven Sandberg <[email protected]>
RB: 25569
…GHT HEADER IN USER VISIBLE TEXT

Patch for 5.6.

Change-Id: Ib90027051452599559cbc50eff102dcbb1480fed
…GHT HEADER IN USER VISIBLE TEXT

Patch for 5.7.

Change-Id: I6f9c504d9760afe63461b7a1f59df695f51f1c53
When collecting write sets for transactions, the server might run out
of memory. This failure shall be handled gracefully and the
transaction shall fail for lack of resources.

ReviewBoard: 25572
When the option is active, each server transaction will collect write
sets for each row, allowing to identify what changes the transaction
made, important for replication parallelization in general and Group
Replication conflict detection.

This patch improves the handling of write set collection for each
transaction in 2 ways.

First is that write sets are discarded if their size grow above
binlog_transaction_dependency_history_size. As a consequence the
transaction will no longer be marked as concurrent with previous ones
even if they touch different data.

The second one is that components like Group Replication can set
memory limits for the collection of write sets. Components like GR are
dependent on write sets being present, so they will never be
discarded, but breaking this memory limit will cause transactions to
abort.

ReviewBoard: 25520
ReviewBoard: 25592
[Post Fix]
Fixing minor issue on a method parameter name documentation.

Reviewed-by: Erlend Dahl <[email protected]>
Oracle Linux 8 comes with DTrace 2.0.0 [Pre-Release with limited functionality]
dtrace: Oracle D 2.0

The dtrace probes in mysql are not compatible with this version, and
the build fails.  Disable this DTrace version, unless ENABLE_DTRACE is
explicitly set to TRUE on the cmake command line.

The /usr/bin/dtrace script/wrapper works, and should be installed
rather than /usr/sbin/dtrace

Change-Id: I024fe963a092a1943db8f4e648e55c30edebc2fb
RB: 25507
Reviewed by Georgi Kodinov <[email protected]>
Reviewed by Ivo Roylev <[email protected]>
RB: 25507
Reviewed by Georgi Kodinov <[email protected]>
Reviewed by Ivo Roylev <[email protected]>
…UE TO MUTEX SPINNING

Post-push fix for 5.7: unbreak the build on macOS
sql/locks/shared_spin_lock.cc:242:3: error: no matching
      function for call to 'my_atomic_store64'
  my_atomic_store64(&this->m_exclusive_owner, self);

Change-Id: I374d9893d0c37d58ef5786e1ccad34b5c85d1383
…CACHE EFFICIENCY

In the term of CPU cache efficiency, often used shard counter "rw_lock_stats"
is better to be sharded by "os_thread_get_curr_id() based" index (thread bonded)
than "my_timer_cycles()" index (random).

Because of the changing about shard index, regression seems to be caused from 5.7.5.

(for 5.7): Change sharding method to os_thread_get_curr_id() based and optimize, for compatibility

(for 8.0): Remove rw_lock_stats (the best for performance). And deprecated outputs as always 0.

RB: 25646
RB: 25647
Reviewed-by: Jakub Lopuszanski <[email protected]>
Reviewed-by: Marcin Babij <[email protected]>
…CMAKE_BUILD_TYPE=RELEASE

Backport the patch for
Bug #27874068 ADD SUPPORT FOR -DCMAKE_BUILD_TYPE=RELEASE
to 5.7

This is a cleanup of our usage of CMAKE_<LANG>_FLAGS_<CONFIG> and
CMAKE_<LANG>_FLAGS.  It enables a more standardized way of doing
RelWithDebInfo, Release and MinSizeRel builds.

It is also a prerequisite for backporting the patch for
Bug #32216281 REMOVE DBUG_OFF AND DBUG_ASSERT

Original commit message:
=======================
Move all compiler/language features, like -std=c++11 or
-fsanitize=address for ASAN builds,
from CMAKE_<LANG>_FLAGS_<CONFIG> to CMAKE_<LANG>_FLAGS

With this patch, CMAKE_<LANG>_FLAGS_DEBUG contains only debug related flags:
-- CMAKE_CXX_FLAGS_DEBUG: -DSAFE_MUTEX -DENABLED_DEBUG_SYNC -g

The other CMAKE_<LANG>_FLAGS_<CONFIG> flags will switch off debugging,
and add our default optimization flags:
-- CMAKE_CXX_FLAGS_MINSIZEREL: -DDBUG_OFF -ffunction-sections -fdata-sections -Os -DNDEBUG

Always PREPEND MySQL default flag values to cmake compiler flags,
so that they can easily be overriden by command line options.
This fixes
Bug#28208842 COMPILER OPTIMIZATION HARDWIRED TO O2

To override MySQL default flag values, for e.g. RELEASE builds, do:
cmake . -DCMAKE_CXX_FLAGS_RELEASE="-O3 -DNDEBUG -fno-function-sections -fno-data-sections"

Change-Id: I325a94909a201425dcf7cc286ac13eef9e3975df
…CMAKE_BUILD_TYPE=RELEASE

Additional patch, fixing new warnings when building optimized -Werror.

Change-Id: I0542449a6768eb14367925e1d64402078d04ba92
…CMAKE_BUILD_TYPE=RELEASE

Extra patch for Solaris: APPEND our own compiler flags to
CMAKE_C_FLAGS_<config> and CMAKE_CXX_FLAGS_<config> to ensure that
we keep the flags to minimize size of executables.

Change-Id: Ibe6a998e1df8b4af396a24868eeca3f4856b1d77
…_ASSERT

Replace DBUG_ASSERT() with assert(). These are semantically the same
(DBUG_ASSERT() has been #defined to assert() for a long time), but
nonstandard.

Manually edited:
dbug/user.r
include/my_dbug.h
libbinlogevents/include/wrapper_functions.h
libmysql/authentication_win/common.h

Everything else is automated: replace DBUG_ASSERT with assert, and
re-indent each region according to appropriate C style in each source
file.

Change-Id: Ib56a4e1f9677c04227ddf618e03c67dee05b98a4
…_ASSERT

Replace DBUG_OFF with NDEBUG

Manually edited: CMakeLists.txt
Remove -DDBUG_OFF from C/C++ definitions and flags.

Everything else is automated: replace DBUG_OFF with NDEBUG throughout
the codebase.

Change-Id: I56f3b92dd110ede2329f39d83682435d47e0447f
…_ASSERT

Additional patch, fixing -Werror=unused and -Werror=uninitialized.

Change-Id: Ifd3586ad74b7f3cb8b87ff9783dde980354ef8a6
Patch for 5.7

If the mysql client was started with option --default-character-set=utf8
the libedit library would reject input of multibyte characters.
The fix is to always call setlocale(LC_ALL, "") when initializing libedit.

Change-Id: Ie318e2d4bd86f3833593e2e234be518dba68b18d
getpeername is returning host ip which does not match any of the IP in
whitelist.

Resolution: Whitelist has been updated with hostname.

ReviewBoard: 25706
RB#25761

Backport to 5.7.
The lock wasn't taken when the queue elements were freed by the thread.
Now taking the lock while doing this.
Test added.
RB#25761
Added the windows failure code
@ldonoso ldonoso force-pushed the merge-mysql-5.7.34-part1 branch from 95f7011 to 33bcc3e Compare April 21, 2021 14:25
@ldonoso ldonoso changed the title Implemented PS-NNNN (Merge MySQL 5.7.34) (docs) Implemented PS-7621 (Merge MySQL 5.7.34) (docs) Apr 21, 2021
@ldonoso ldonoso changed the title Implemented PS-7621 (Merge MySQL 5.7.34) (docs) Implemented PS-7621 (Merge MySQL 5.7.34) Apr 23, 2021
@ldonoso ldonoso force-pushed the merge-mysql-5.7.34-part1 branch 6 times, most recently from 111ab2f to 5132123 Compare May 1, 2021 16:08
@ldonoso ldonoso force-pushed the merge-mysql-5.7.34-part1 branch from 5132123 to 3347b7d Compare May 3, 2021 06:33
Luis Donoso added 6 commits May 3, 2021 13:52
This version has been adapted to support MySql 5.7.34
https://jira.percona.com/browse/PS-7621

Move the check of the corruption of the `ib_table` to the place where
the `m_share` is initialized
https://jira.percona.com/browse/PS-7621

MySql introduced in
mysql/mysql-server@7e4fef6
a change in which it doesn't cache the read values of sys_var.

PS was expected a non NULL sys_var in two places:

- When processing `SET STATEMENT ... FOR`
- In `tokudb_backup.cc:source_dirs::find_plug_in_sys_var`

The solution is restoring the value but in a way that it is not cached
so it won't interfere with MySql fix.
MySql dropped the use of DBUG_OFF in
mysql/mysql-server@14054f5

This change doesn't affect any of the PS submodule or the
jenkins pipelines.
@ldonoso ldonoso force-pushed the merge-mysql-5.7.34-part1 branch from 3347b7d to 5f0d84e Compare May 3, 2021 11:54
@ldonoso ldonoso closed this May 3, 2021
ldonoso pushed a commit that referenced this pull request Nov 4, 2021
Rename "lock_count" to "external_lock_count", increment the variable
only when transaction has been registered sucessfully and thus
remove two unneccessary decrement calls. Improve description of the
"external_lock_count" variable.

Change-Id: I5bfa91fdca8fa2d377b170696b6086293d8cb743
ldonoso pushed a commit that referenced this pull request Mar 15, 2022
…?e=20to=20make=20all=20keys=20available=20to=20al=E2=80=A6=20(#5?= =?UTF-8?q?55)?= (percona#555)

Summary:
…l of

MyRocks code. As port to 5.7 and 8.0 will add memory keys and having global
access eliminates need to pass keys around across function/member barriers.
Closes facebook/mysql-5.6#555

Differential Revision: D4647988

Pulled By: jkedgar
ldonoso pushed a commit that referenced this pull request Mar 15, 2022
…ercona#871)

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
ldonoso pushed a commit that referenced this pull request Mar 15, 2022
Summary:
1. Account for explain format changes
2. Binary text isn't supported as MySQL has changed its internal make_sort_key implementation and we haven't yet account for that. Disabling the scenario with disable_testcase BUG#888003 for now. There is already a task for it.
3. Change regex for MySQL log format changes in rocksdb_checksums.test
4. Fix a bug where index stats calculation background threads are not started properly as the macro TARGET_OS_LINUX isn't defined.

Reviewed By: lloyd

Differential Revision: D17622870
ldonoso pushed a commit that referenced this pull request Mar 15, 2022
Summary:
In MySQL 8.0.17, sending data stage is gone. As a result, in testcase #5/#6 the test is waiting on sending data stage and timed out, and at that point the lock is already taken, so trying to take the same lock on the same row on another connection simply timed out, instead of getting a deadlock/snapshot conflict. For now I'm using an slightly earlier stage "executing" - this aligns what Percona has done and we can see if this works reasonably well. If not we can see if we can introduce the old stage back.

Note: The current implementation of the test can be flaky - it depends on the SELECT has already started the scan over some of the rows and taken snapshot, but before taking the lock, so that you can get snapshot conflict in another connection doing delete over the same row (instead of timeout with lock contention). Given that the test is intended to test taking snapshot before doing any get, this is fortunately the best the test can do at this point.

Reviewed By: lloyd

Differential Revision: D18716622
ldonoso pushed a commit that referenced this pull request Apr 4, 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
    percona#7 0x557ccb47977c in mysql_parse(THD*, Parser_state*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5952
    percona#8 0x557ccb47b6c2 in dispatch_command(THD*, COM_DATA const*, enum_server_command) /home/yura/ws/percona-server/sql/sql_parse.cc:1544
    percona#9 0x557ccb47de1d in do_command(THD*) /home/yura/ws/percona-server/sql/sql_parse.cc:1065
    percona#10 0x557ccb6ac294 in handle_connection /home/yura/ws/percona-server/sql/conn_handler/connection_handler_per_thread.cc:325
    percona#11 0x557ccbbfabb0 in pfs_spawn_thread /home/yura/ws/percona-server/storage/perfschema/pfs.cc:2198
    percona#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.
ldonoso pushed a commit that referenced this pull request Apr 12, 2022
… enabled

Summary:
For secondaries, when enable_super_log_bin_read_only is on and read_only is on, currently it will forbid to install/uninstall plugin during run time.

install/uninstall plugin doesn't generate event in binlog, although the thread thd contains OPTION_BIN_LOG flag due to log_slave_updates is on by default in secondaries. It should be safe to execute install/uninstall plugin.

the change is to call set_skip_readonly_check() before install/uninstall plugin and call reset_skip_readonly_check()(for completeness) after install/uninstall plugin.

BTW, mysql will always call reset_skip_readonly_check() for at the beginning of each statement. thus set_skip_readonly_check() won't affect other statement.
```
#0  THD::reset_skip_readonly_check (this=0x7fb5c1a0b000) at /home/luqun/mysql/mysql-8.0.20/sql/sql_class.h:1754
#1  0x0000000005a500e1 in THD::reset_for_next_command (this=0x7fb5c1a0b000) at /home/luqun/mysql/mysql-8.0.20/sql/sql_parse.cc:5892
#2  0x0000000005a517a5 in mysql_reset_thd_for_next_command (thd=0x7fb5c1a0b000) at /home/luqun/mysql/mysql-8.0.20/sql/sql_parse.cc:5817
#3  0x0000000005a50466 in mysql_parse (thd=0x7fb5c1a0b000, parser_state=0x7fb5f6bb4560, last_timer=0x7fb5f6bb39b0) at /home/luqun/mysql/mysql-8.0.20/sql/sql_parse.cc:6056
#4  0x0000000005a4c7c9 in dispatch_command (thd=0x7fb5c1a0b000, com_data=0x7fb5f6bb4d98, command=COM_QUERY) at /home/luqun/mysql/mysql-8.0.20/sql/sql_parse.cc:2222
#5  0x0000000005a4f991 in do_command (thd=0x7fb5c1a0b000) at /home/luqun/mysql/mysql-8.0.20/sql/sql_parse.cc:1556
#6  0x0000000005ccd4f1 in handle_connection (arg=0x7fb5cc85b740) at /home/luqun/mysql/mysql-8.0.20/sql/conn_handler/connection_handler_per_thread.cc:330
percona#7  0x00000000078eb95b in pfs_spawn_thread (arg=0x7fb5f8c89720) at /home/luqun/mysql/mysql-8.0.20/storage/perfschema/pfs.cc:2884
percona#8  0x00007fb5f957020c in start_thread (arg=0x7fb5f6bb6700) at pthread_create.c:479
percona#9  0x00007fb5f971881f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
```

Reviewed By: george-reynya

Differential Revision: D27213990
ldonoso pushed a commit that referenced this pull request Apr 12, 2022
recover raft logs by removing partial trxs

Summary:
Port D24628821

mysqld removes partial trxs in the tail of trx log (named binary-logs on
primaries and apply-logs on secondaries) during startup. However, relay logs
were not of much importance since it was anyways discarded and a new one would
be created.
However, with raft, this is not ideal. Relay logs are raft logs on secondaries
and have to be kept around (and kept sane and consistent). This diff adds the
ability to remove partial trxs from raft/relay logs.
Much of the code to open the last relay log (based on relay log index) and
identify partial trxs is borrowed from existing logic in
MYSQL_BIN_LOG::open_binlog() and binlog_recover()

Reviewed By: Pushapgl

Differential Revision: D26447448

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

Checking for inited bool to make sure global_init_info was successful

Summary:
Port D25584004

A master.info and relay.info file can be present
but needs to be properly inited for use. We were bypassing the inited
check which could lead to issues in Raft.
In case there is an error in global_init_info, Raft will do a
raft_reset_slave and make another attempt at it. If both recourses
fail, the init of the plugin would fail.

Reviewed By: Pushapgl

Differential Revision: D26447457

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

Support for dumping raft logs to vanilla async replicas

Summary:
[Porting Notes]
We want to dump raft logs to vanilla async replicas regardless
of whether it's the relay log or binlog. Effectively after this change
we'll dump relay logs on the followers and binlogs on the leader. When
the raft role changes, the logs to the dumped are also changed.
Dump_log class is introduced as a thin wrapper/continer around
mysql_bin_log or rli->relay_log and is inited with mysql_bin_log
to emulate vanilla mysql behavior. Dump threads use the global
dump_log object instead of mysql_bin_log directly. We switch the log
in dump log only when raft role changes (in binlog_change_to_binlog()
and binlog_change_to_apply_log()).
During raft role change we take all log releated locks (LOCK_log,
LOCK_index, LOCK_binlog_end_pos, and dump log lock) to serialize it with
other log operations like dumping logs.

Related doc - https://fb.quip.com/oTVAAdgEi4zY

This diff contains below 7 patches:
D23013977
D24766787
D24716539
D24900223
D24955284
D25174166
D25775525

Reviewed By: luqun

Differential Revision: D26141496

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

Fixed the flaky raft test suite

Summary:
First clean run of entire raft test suite :)

**Changes**
* Reset apply_logs on raft secondaries before the start of every test
* Increased the lock timeouts and changed isolation level in rpl_raft_slave_out_of_order_commit.

Reviewed By: luqun

Differential Revision: D26651257

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

return error from Raft_replication_delegate when plugin is not available

Summary:
Port D23065441 (facebook/mysql-5.6@b9067f7)

The new macro is used to call into raft plugin. If plugin gets unloaded
accidentally when enable_raft_plugin is ON, then this STRICT version returns
failure. This is to be called only by raft plugin currently

Reviewed By: Pushapgl

Differential Revision: D26447523

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

Adding timestamps for raft rotates which happen in the context of listener thread

Summary:
Port D25572614

The timestamp of a binlog event is picked up from the when field in
the event. In most cases of rotation, the when is left unpopulated
during rotation for the top 3 events (fd, pgtid, metadata). However
in such a situation, a normal rotate (flush binary logs) still manages
to get a valid timestamp, since the thread in which the flush binary
logs happens has a valid start time.
Now enter Raft relay log rotations. In those cases and in the case
of config change rotate, the rotations are happening in the context
of a raft listener queue thread. In that context, the when and start
time of the thread are both 0. The diff handles this case by populating
the when field appropriately.

Reviewed By: bhatvinay

Differential Revision: D26194612

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

Raft abrupt stepdown and trim binlog file / gtid test

Summary: binlog file should get trimmed for abrupt stepdown

Reviewed By: Pushapgl, bhatvinay

Differential Revision: D26169975

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

Port: Fixes around raft log truncation.

Summary:
**Notes**
* New functions to block and unblock dump threads for plugin to use
during raft log truncation.

Below check is already done in raft plugin as part of raft plugin in D26866429.
* Re-init rli->cur_log in Relay_log_info::rli_init_info() instead of
just seeking to the beginning to handle the case when raft log is
truncated before starting the applier

Reviewed By: luqun

Differential Revision: D26759813

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

rebase due to relay log Format_description_event event size difference

Summary:
WL#3549: Binlog Compression add extra 1 bytes data into Format_description_event

  3298 @@ -134,6 +137,7 @@ Format_description_event::Format_description_event(uint8_t binlog_ver,
  3299            VIEW_CHANGE_HEADER_LEN,
  3300            XA_PREPARE_HEADER_LEN,
  3301            ROWS_HEADER_LEN_V2,
  3302 +          TRANSACTION_PAYLOAD_EVENT,
  3303        };

Reviewed By: Pushapgl

Differential Revision: D27145095

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

fix raft change string metadata event

Summary:
Saw a bunch stage-1 replicaset instance failed due to config change string failure during add/remove instance
```
I0401 00:16:10.894434 1823842 IoCacheUtils.cpp:333] getConfigChangeString eventType = ^G
E0401 00:16:10.894451 1823842 IoCacheUtils.cpp:363] Failed to read metadata event body from cache
E0401 00:16:10.894456 1823842 MysqlRaft.cpp:659] [replicate] Failed to get config change string from iocache
2021-04-01T00:16:10.894464-07:00 8307 [ERROR] [MY-010207] [Repl] Run function 'before_flush' in plugin 'RPL_RAFT' failed
2021-04-01T00:16:10.894478-07:00 8307 [ERROR] [MY-000000] [Server] Failed to rotate binary log
```

After some investigation, the issue is caused is that calculate metadata event length with config change string but forgot write config change string into event body.

Reviewed By: Pushapgl

Differential Revision: D27504157

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

Tells raft mode in binlog

Summary:
WIn-Win: Tell whether this binlog is generated while this host is in raft mode.

We already has the bit in FD event, and this diff just speaks it out.

Reviewed By: mpercy

Differential Revision: D28664300

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

fix flaky raft testcase

Summary:
There are multiple issues for MTR:
1. in sql/rpl_binlog_sender.cc, if secondaries IO thread receives fatal_error,
   it will quit IO thread instead of reconnect. use unknown error so that
   secondary IO thread can try to reconnect
2. mtr.add_suppression() call: mtr.add_suppression() will execute an insert
   mysql statement into mtr.test_suppressions table and mtr.test_suppressions
   table doesn't contain primary key, thus during idempotent recovery, secondary
   will fail to execute mtr.add_suppression() due to missing PK. Try to move all
   mtr.add_suppression() at the end of testcase to workaround  idempotent recovery failure.
3. When promotion, use raft_promote_to_leader.inc instead of `set rpl_raft_new_leader_uuid`,
   since raft_promote_to_leader will wait the new primary state becomes writeable
4. pass specific warning instead of '.*' to mtr.add_supression
5. etc

Reviewed By: Pushapgl, bhatvinay

Differential Revision: D28774820

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

Using locks in Dump_log methods only when raft is enabled

Summary:
We don't need to take locks in non-raft mode since the
underlying MYSQL_BIN_LOG obj will never change. To handle race between
updation of enable_raft_plugin var and using its values in Dump_log we
kill and block all dump threads while updating the var and unblock them
once the var is updated.

Reviewed By: bhatvinay

Differential Revision: D28905522

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

leader election to be a sync point in the new leader

Summary:
Port of D27582002 (facebook/mysql-5.6@39c70ca) from 5.6.35 to 8.0

Newly elected raft leader makes sure that all trxs from the previous
leader is committed by sql appliers. It then switches the server's trx
logs from apply-log-* to binary-log-*. To other part of the system this
looks like a rotation, but the necessary sync calls are not made here.
So, if the server (or os) restarts, then the storage engine could lose
the commit markers of the last batch of trxs. This will result in silent
data drift.
This diff fixes the problem by making an explicit call to
ha_flush_logs() before switching the server's trx logs

Reviewed By: luqun

Differential Revision: D28880407

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

Error out SQL thread on out of order opids in raft logs

Summary:
OpIds should always be in order in the raft logs. Added a check
in SQL threads that thows an error and stops the applier when out of
order opids are detected.

Reviewed By: li-chi

Differential Revision: D28810840

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

add GET_COMMITTED_GTIDS for raft

Summary:
During recovery Raft Log::Init needs to check with server
what gtids have been committed. Before doing that it finds the entire
set of trxs in the last raft log. The difference between logged -
committed are the pending opids.

Reviewed By: Pushapgl

Differential Revision: D29622786

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

raft: skip mts_recovery_groups during start slave

Summary:
During MySQL8+Raft DMP, some instance fail to switch to Leader or start slave

```
2021-06-24T17:56:38.627423-07:00 431 [Note] [MY-010574] [Repl] Slave: MTS group recovery relay log info group_master_log_name /data/mysql/3127/bls-unittestdb658.frc2-3305-mysql.replicaset.180021/binary-logs-3727.000033, event_master_log_pos 1129.
2021-06-24T17:56:38.627473-07:00 431 [ERROR] [MY-010575] [Repl] Error looking for file after /binlogs/binary-logs-3307.000120.
2021-06-24T17:56:38.627516-07:00 431 [ERROR] [MY-000000] [Repl] load_mi_and_rli_from_repositories: rli_init_info returned error
```

similar to 5.6, we don't need to run mts_recovery_groups due to GTID_MODE is always enabled.

Reviewed By: Pushapgl

Differential Revision: D29520066

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

update RaftListenerCallbackArg struct

Summary: Due to contract between raft plugin and mysql change, update RaftListenerCallbackArg struct to add  master_uuid field

Reviewed By: Pushapgl, bhatvinay

Differential Revision: D29981349

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

always check mi during raft_reset_slave

Summary: add similar nullptr check for raft_reset_slave as raft_stop_sql_thread

Reviewed By: bhatvinay

Differential Revision: D29967915

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

raft mtr: update rpl_end_raft.inc for disable-raft

Summary:
rpl_end_raft.inc will unregister raft plugin which will call reset slaves when online disable-raft is enabled. move rpl_end_raft.inc after rpl_stop_slaves.inc to stop slave correctly first.

after stop slaves, call mtr.add_suppression("") won't replicate to slaves. just call mtr.add_suppression("") for all instances(replicas).

Reviewed By: yizhang82

Differential Revision: D30062236

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

fix master_uuid

Summary: fix master_uuid in raft mode.

Reviewed By: luqun

Differential Revision: D30261465

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

incorrect File_size value in show raft logs result

Summary:
in show raft logs result, the File_size for latest logs file isn't updated correctly.

such as show raft logs;

```
+-------------------------+------------
| Log_name                | File_size  |
| binary-logs-3304.000670 |       1529 |
```

in fact
```
-rw-r----- 1 mysql backup 1669180487 Aug  4 14:49 binary-logs-3304.000670
-rw-r----- 1 mysql backup 1723994154 Aug  4 14:49 binary-logs-3304.000670
```
the  file_size from show raft logs is always 1529 but the real file_size is 1669180487.

The issue is related when writing IO_CACHE directly, its wrapper ostream's position isn't updated.

Reviewed By: yizhang82

Differential Revision: D30116345

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

Add gtid_purged_for_tailing

Summary:
Add a new global variable gtid_purged_for_tailing.

It shows the purged GTID for binlog in non-raft mode, and purged GTID for raft log in raft mode.

Reviewed By: abhinav04sharma

Differential Revision: D29372776

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

disable skip_setup_replica_if_unnecessary

Summary: After sync with latest official raft plugin, most of MTR failed due to skip_setup_replica_if_unnecessary optimize.

Reviewed By: yizhang82

Differential Revision: D30821648

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

latency histograms for raft trx wait

Summary: Port of support added for the same in mysql 5.6. Should help monitor latencies in 8.0 + raft stage -1 replicasets.

Reviewed By: luqun

Differential Revision: D31064764

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

handle cases where clean shutdown in raft aborts trxs

Summary: This is a port of the feature in 5.6.

Reviewed By: anirbanr-fb

Differential Revision: D31070593

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

fix crash during binlog purging

Summary:
Any error returned by the plugin during binlog purging results in a
crash in mysql8 as the server tries to execute
binlog_error_action_abort. We need to differentiate explicitly between a
plugin error and other error (such as error related to doing disk IO
etc). In thsi particular case, the crash is because of trying to purge a
file that does not exist (i.e which is already purged previosuly) and
raft cannot find it in its index chunk (so it returns a error).

Reviewed By: anirbanr-fb

Differential Revision: D31149997

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

update flaky rpl_raft_purged_gtids_dump_threads

Summary:
rpl_raft_purged_gtids_dump_threads MTR failed due to "Cannot replicate because the master purged required binary logs" after server4 will tail server2.

Try to sync server4 before switch to tail server2.

Reviewed By: bhatvinay

Differential Revision: D30818078

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

fix various porting issues in mysql8 (raft) crash recovery

Summary:
1. Trimming of the binlog is left to raft plugin (based on the current
    leader's log). Server should skip this step as part of recovery. This
    essentially means setting 'valid_pos' to the last successfully parsed
    trx in the trx log (instead of the engine's view of the trx log) in
    MYSQL_BIN_LOG::recover()
  2. executed_gtid_set should be initialized based on the engine's view of
the trx log file cordinates. So, during startup appropriate flags need
to be passed into MYSQL_BIN_LOG::init_gtid_sets(). init_gtid_sets() is
already changed to handle this, but the flag was not set correctly
during server startup
3. Another fix is in MYSQL_BIN_LOG::init_gtid_sets()to corretly set the position
to read and calculate executed-gtid-set (based on the file name read from the
engine)

Reviewed By: anirbanr-fb

Differential Revision: D31284902

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

show_raft_status should take LOCK_status

Summary:
This is a straight port of a 5.6 patch to 8.0

SHOW RAFT STATUS and SHOW GLOBAL STATUS go to the
same functions in the plugin and access shared data structures.
These functions return internal char * pointers to plugin global
variables. They need to be serialized with LOCK_status otherwise
it leads to race conditions.

Reviewed By: li-chi

Differential Revision: D31318340

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

Disallowing reset master on raft replicasets

Summary:
This is a straight port of similar patch on 5.6 raft

reset master is inherently not raft compliant.
Its get rid of all binlogs and make an instance appear like
a fresh single instance database without consulting the ring.
We need to block it.

However under certain circumstances (in the future ) e.g. during
first time replicaset build, or when we can guarantee that instance
is single node raft ring, we can potentially do a reset master
followed by rebootstrap of raft. This can also be achieved by
disabling raft, reset master and then re-enabling raft, however
to keep open the door, I have left a force option and will thread
a mechanism from the plugin to call reset_master(force=true)

Reviewed By: li-chi

Differential Revision: D31299214

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

Setting topology config in MTR

Summary:
Setting topology config in MTR so that feature that use
topology config can be tested easily. Setting rpl_raft_skip_smc_updates
to avoid unnecessary calls to SMC (even though we supply a dummy
replicaset name).

Reviewed By: li-chi

Differential Revision: D31543877

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

Do not allow sql thread start when raft is doing a stop->{}->start transition

Summary:
This is a port of an existing patch made to 5.6 for Raft.

Raft will do a stop of the SQL thread during StopAllWrites.
Then it will repoint the binlog files and during that action, the
SQL threads have to remain stopped. We block it out in this diff by
keeping an atomic bool which can be checked from other functions.

This only applies to raft mode i.e. enable_raft_plugin = true.

Reviewed By: li-chi

Differential Revision: D31319499

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

Handle printing of FD event generated by slave SQL thread

Summary:
Early returning in the FD:print() function makes mysqlbinlog not be able to parse Raft logs on secondaries.

The original commit which added this is d048c0f (P173872135)

To comply with the intent of the original bug fix, we avoid printing the FD event of a relay log as a 'BINLOG'.

Reviewed By: anirbanr-fb

Differential Revision: D26359417

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

Add exponential backoff for smart restart

Summary:
RAFT Instance crash and Tx Logs filling up.

rocksdb can fill up txlogs.
we should stop restarting mysqld if we have restarted many times in a day

Reviewed By: anirbanr-fb

Differential Revision: D27372750

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

always release data_lock mutex to avoid deadlock

Summary: in stage-1 replicaset, when kill a secondary instance, sometime the instance will run into deadlock due to process_raft_queue thread forgot to release its acquired mutex in raft_change_master

Reviewed By: Pushapgl, bhatvinay

Differential Revision: D27602667

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

Gracefully exit mysqld_safe loop during backoff

Summary:
Currentt systemctl stop [email protected] can take 10 mins when mysqld_safe is in backoff period.

D28517599 adds a interrupt to sleep in mysql_stop, and mysqld_safe immediately break the retry loop if sleep is interruptted.

Reviewed By: anirbanr-fb

Differential Revision: D28606439

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

Fix heap overflow in group_relay_log_name handling

Summary:
We were accessing group_relay_log_name in
Query_log_event::do_apply_event_worker() but it's assigned only after
the coordinator thread encounters an end event (i.e. xid event or a
query event with "COMMIT" or "ROLLBACK" query). This was causing a race
between accessing group_relay_log_name in the worker thread and writing
it on the coordinator thread. We don't need to set transaction position
in events other than end event, so now we set transaction position in
query event only if it's an end event. The race is eliminated because
group_relay_log_name is set before enqueuing the event to the worker
thread (in both dep repl and vanilla mts).

Reviewed By: lth

Differential Revision: D28767430

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

fix memory during MYSQL_BIN_LOG::open_existing_binlog

Summary:
asandebug complain there are memory leaks during MYSQL_BIN_LOG open

Direct leak of 50 byte(s) in 1 object(s) allocated from:
    #0 0x67460ef in malloc
    #1 0x93f0777 in my_raw_malloc(unsigned long, int)
    #2 0x93f064a in my_malloc(unsigned int, unsigned long, int)
    #3 0x93f0eb0 in my_strdup(unsigned int, char const*, int)
    #4 0x8af01a6 in MYSQL_BIN_LOG::open(unsigned int, char const*, char const*, unsigned int)
    #5 0x8af8064 in MYSQL_BIN_LOG::open_binlog(char const*, char const*, unsigned long, bool, bool, bool, Format_description_log_event*, unsigned int, RaftRotateInfo*, bool)
    #6 0x8b00c00 in MYSQL_BIN_LOG::new_file_impl(bool, Format_description_log_event*, RaftRotateInfo*)
    percona#7 0x8d65e47 in rotate_relay_log(Master_info*, bool, bool, bool, RaftRotateInfo*)
    percona#8 0x8d661c0 in rotate_relay_log_for_raft(RaftRotateInfo*)
    percona#9 0x8c7696a in process_raft_queue
    percona#10 0xa0fa1fd in pfs_spawn_thread(void*)
    percona#11 0x7f8c9a12b20b in start_thread

release these memory before assign them

Reviewed By: Pushapgl

Differential Revision: D28819752

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

Reduce max backoff time from 24h to 30mins

Summary:
Over the recent SEV0, raft instances crash looping because of block skew. Even after the clock is normal, mysqld_safe failed to bring back mysqld because extended 24hours backoff.

This diff reduces the max backoff time from 24h to 30mins. So it will still keep trying, but not so aggressively that filling up the trx logs.

Reviewed By: bart2

Differential Revision: D31661172

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

Fix dump thread gtid_purged calculation in raft mode

Summary:
In raft mode, it is supposed to use gtid_purged_for_tailing, and lost_gtids should point to the it.

Because of a bug, lost_gtids pointer's reference never changes, and always points to gtid_state->get_lost_gtids(), which is gtid_purged, not gtid_purged_for_tailing.

Reviewed By: anirbanr-fb

Differential Revision: D34110214

fbshipit-source-id: 3f53a3a62f0
ldonoso pushed a commit that referenced this pull request Jul 12, 2022
**Problem:**

The test 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
    percona#7 0x55659655d579 in mysqld_main(int, char**) /tmp/ps/sql/mysqld.cc:8505
    percona#8 0x55659609c5b5 in main /tmp/ps/sql/main.cc:25
    percona#9 0x7fa46a6f6082 in __libc_start_main ../csu/libc-start.c:308
    percona#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
    percona#7 0x556599b8dcfc in shutdown_performance_schema() /tmp/ps/storage/perfschema/pfs_server.cc:374
    percona#8 0x556596531d96 in mysqld_exit /tmp/ps/sql/mysqld.cc:2500
    percona#9 0x55659655d579 in mysqld_main(int, char**) /tmp/ps/sql/mysqld.cc:8505
    percona#10 0x55659609c5b5 in main /tmp/ps/sql/main.cc:25
    percona#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
    percona#7 0x556599b9487b in inline_mysql_rwlock_init /tmp/ps/include/mysql/psi/mysql_rwlock.h:261
    percona#8 0x556599b94ba7 in init_pfs_tls_channels_instrumentation() /tmp/ps/storage/perfschema/pfs_tls_channel.cc:209
    percona#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
    percona#10 0x55659655a585 in mysqld_main(int, char**) /tmp/ps/sql/mysqld.cc:7497
    percona#11 0x55659609c5b5 in main /tmp/ps/sql/main.cc:25
    percona#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.
ldonoso pushed a commit that referenced this pull request Jul 13, 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
    percona#7 0x55f09937dd3c in rec_copy_prefix_to_buf_old /home/ldonoso/src/release-8.0.29-20/storage/innobase/rem/rem0rec.cc:1206
    percona#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
    percona#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
    percona#10 0x55f098c3d0ba in btr_pcur_t::store_position(mtr_t*) /home/ldonoso/src/release-8.0.29-20/storage/innobase/btr/btr0pcur.cc:141
    percona#11 0x55f098c027b6 in dict_getnext_system_low /home/ldonoso/src/release-8.0.29-20/storage/innobase/dict/dict0load.cc:256
    percona#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants