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

MDEV-23630: mysqldump logically dump system table information #2

Open
wants to merge 2 commits into
base: 10.2
Choose a base branch
from

Conversation

grooverdan
Copy link
Owner

Add --system={all, users, plugins, udf, servers, stats, timezones}

This will dump system information from the server in
a logical form like

  • CREATE USER
  • GRANT
  • CREATE ROLE
  • CREATE SERVER
  • INSTALL PLUGIN
  • CREATE FUNCTION

"stats" is the innodb statistics tables or EITS and
these are dumped as INSERT/REPLACE INTO statements
without recreating the table.

"timezones" is the collection of timezone tables
which are important to transfer to generate identical
results on restoration.

Two other options have an effect on the SQL generated by
--system=all. These are mutually exclusive of each other.

  • --replace
  • --insert-ignore

--replace will include "OR REPLACE" into the logical form
like 'CREATE OR REPLACE USER ...'

--insert-ignore uses the construct " IF NOT EXISTS" where
supported in the logical syntax.

@grooverdan grooverdan force-pushed the bb-10.2-danielblack-MDEV-23630-mysqldump-native-sql branch 6 times, most recently from 9bd8d52 to a39a4ce Compare October 2, 2020 12:01
@grooverdan grooverdan force-pushed the bb-10.2-danielblack-MDEV-23630-mysqldump-native-sql branch from a39a4ce to 51b8208 Compare October 8, 2020 02:40
grooverdan pushed a commit that referenced this pull request Oct 8, 2020
For the sake of completeness, call sync_clones in reset_stmt_params, too.
grooverdan pushed a commit that referenced this pull request Oct 9, 2020
…USER

Part #2:
- row_search_mvcc() should return DB_INTERRUPTED when it got
- Move the sync point from innodb internals to
  handler_rowid_filter_check() where other storage engines can use
  it too
- Add a similar syncpoint for the ICP check.
- Add a bigger test and test coverage for Rowid Filter with MyISAM
- Add test coverage for killed-during-ICP-check scenario
@grooverdan grooverdan changed the base branch from 10.2 to 10.1-MDEV-7811 October 12, 2020 01:51
@grooverdan grooverdan changed the base branch from 10.1-MDEV-7811 to 10.2 October 12, 2020 01:51
Copy link

@an3l an3l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @grooverdan ,
Additional to this I would update help:

Dump system tables as portable SQL. Any combination of:.. [to name is allowed]

And if you see the help for

character-sets-dir                (No default value)

Your system is empty -> should write (No default value ) too.
Also noted that for 10.2 default anonymous user we get this:

user - anonymous:

mysqldump: Couldn't execute 'SELECT CONCAT(QUOTE(u.user), '@', QUOTE(u.Host)) AS u FROM mysql.user u  /*!80001 LEFT JOIN mysql.role_edges e ON u.user=e.from_user AND u.host=e.from_host          WHERE e.from_user IS NULL */ /*M!100005 WHERE is_role='N' */': SELECT command denied to user ''@'localhost' for table 'user' (1142)

MySQL error code 1142 (ER_TABLEACCESS_DENIED_ERROR): %-.100T command denied to user '%s'@'%s' for table '%-.192s'

#define OPT_SYSTEM_STATS 32
#define OPT_SYSTEM_TIMEZONES 64
static const char *opt_system_type_values[]=
{"all", "users", "plugins", "udf", "servers", "stats", "timezones"};
Copy link

@an3l an3l Oct 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not having udfs in plural too? Indentation 80 chars + 2 spaces (seems here are 4).

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've brought back a lot of the strings to 80 chars or less. There's a few exceptions on longer SQL that I've just tried to keep readable while following indenting rules.

@@ -578,6 +595,12 @@ static int init_dumping_tables(char *);
static int init_dumping(char *, int init_func(char*));
static int dump_databases(char **);
static int dump_all_databases();
static int dump_all_users();
static int dump_all_plugins();
static int dump_all_udf();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all_udfs

(my_hash_insert(&ignore_table,
(uchar*) my_strdup("mysql.db", MYF(MY_WME))) ||
my_hash_insert(&ignore_table,
(uchar*) my_strdup("mysql.global_priv", MYF(MY_WME))) ||
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So here for specific options you are ignoring some tables.
https://mariadb.com/kb/en/mysqlglobal_priv-table/ in 10.4 started, it shouldn't be there for 10.2, right?
Also it is replacing user table, user is still there but is a view.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its there so a), the merge up branches works. b) so if a user happens to use a mysqldump-10.2 on mariadb-server-10.4 they don't get the global_privs table dumped along with all the create user,role; grant that overwrites the same information.

Whether user is a table or view, it needs to be ignored from the dump.

This is important when mysqldump --system=all --all-databases is used, or if mysql database is explicitly listed as the dump database,

my_hash_insert(&ignore_table,
(uchar*) my_strdup("mysql.global_priv", MYF(MY_WME))) ||
my_hash_insert(&ignore_table,
(uchar*) my_strdup("mysql.table_priv", MYF(MY_WME))) ||
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my_hash_insert(&ignore_table,
(uchar*) my_strdup("mysql.table_priv", MYF(MY_WME))) ||
my_hash_insert(&ignore_table,
(uchar*) my_strdup("mysql.column_priv", MYF(MY_WME))) ||
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

columns_priv

client/mysqldump.c Show resolved Hide resolved
client/mysqldump.c Show resolved Hide resolved
client/mysqldump.c Show resolved Hide resolved
client/mysqldump.c Show resolved Hide resolved
client/mysqldump.c Show resolved Hide resolved
@grooverdan
Copy link
Owner Author

Thanks @grooverdan ,
Additional to this I would update help:

Dump system tables as portable SQL. Any combination of:.. [to name is allowed]

I see:

  --system=name       Dump system tables as portable SQL. Any combination of: 
                      all, users, plugins, udfs, servers, stats, timezones

With the below patch I could get:

  --ssl-crl=name      Certificate revocation list (implies --ssl).
  --ssl-crlpath=name  Certificate revocation list path (implies --ssl).
  --ssl-verify-server-cert 
                      Verify server's "Common Name" in its cert against
                      hostname used when connecting. This option is disabled by
                      default.
  --system=name       Dump system tables as portable SQL. Any combination of
                      [for name]: all, users, plugins, udfs, servers, stats, 
                      timezones

It looks odd because all the other =name values make no reference to name in the help.

diff --git a/mysys/my_getopt.c b/mysys/my_getopt.c
index 13d0d7c240e..85a34a77b62 100644
--- a/mysys/my_getopt.c
+++ b/mysys/my_getopt.c
@@ -1522,7 +1522,7 @@ void my_print_help(const struct my_option *options)
         count= optp->typelib->count;
         break;
       case GET_SET: 
-        typelib_help= ". Any combination of: ";
+        typelib_help= ". Any combination of [for name]: ";
         count= optp->typelib->count;
         break;
       case GET_FLAGSET:

And if you see the help for

character-sets-dir                (No default value)

This would be a global change and would affect every other client tool and possibly some of the server parsing. I'm not sure I want to touch this.

Your system is empty -> should write (No default value ) too.

Is Empty set ok?

ssl-verify-server-cert            FALSE
system                            (Empty set)

Also noted that for 10.2 default anonymous user we get this:

user - anonymous:

mysqldump: Couldn't execute 'SELECT CONCAT(QUOTE(u.user), '@', QUOTE(u.Host)) AS u FROM mysql.user u  /*!80001 LEFT JOIN mysql.role_edges e ON u.user=e.from_user AND u.host=e.from_host          WHERE e.from_user IS NULL */ /*M!100005 WHERE is_role='N' */': SELECT command denied to user ''@'localhost' for table 'user' (1142)

MySQL error code 1142 (ER_TABLEACCESS_DENIED_ERROR): %-.100T command denied to user '%s'@'%s' for table '%-.192s'

There's no other privilege checks currently. To address this a generic error saying what privs are require would be needed and we'd need to do this for all the existing options.

@grooverdan grooverdan force-pushed the bb-10.2-danielblack-MDEV-23630-mysqldump-native-sql branch from 3a710a4 to 20a3cec Compare October 22, 2020 01:56
grooverdan pushed a commit that referenced this pull request Oct 22, 2020
…USER

Part #2:
- row_search_mvcc() should return DB_INTERRUPTED when it got
- Move the sync point from innodb internals to
  handler_rowid_filter_check() where other storage engines can use
  it too
- Add a similar syncpoint for the ICP check.
- Add a bigger test and test coverage for Rowid Filter with MyISAM
- Add test coverage for killed-during-ICP-check scenario
@grooverdan grooverdan force-pushed the bb-10.2-danielblack-MDEV-23630-mysqldump-native-sql branch from 20a3cec to 960a548 Compare October 23, 2020 23:16
grooverdan pushed a commit that referenced this pull request Oct 23, 2020
There are 2 issues here:

Issue #1: memory allocation.
An IO_CACHE that uses encryption uses a larger buffer (it needs space for the encrypted data,
decrypted data, IO_CACHE_CRYPT struct to describe encryption parameters etc).

Issue #2: IO_CACHE::seek_not_done
When IO_CACHE objects are cloned, they still share the file descriptor.
This means, operation on one IO_CACHE may change the file read position
which will confuse other IO_CACHEs using it.

The fix of these issues would be:
Allocate the buffer to also include the extra size needed for encryption.
Perform seek again after one IO_CACHE reads the file.
@grooverdan grooverdan force-pushed the bb-10.2-danielblack-MDEV-23630-mysqldump-native-sql branch from 960a548 to c932df0 Compare October 23, 2020 23:21
@grooverdan grooverdan changed the base branch from 10.2 to bb-10.1-mdev7572 October 23, 2020 23:25
@grooverdan grooverdan changed the base branch from bb-10.1-mdev7572 to 10.2 October 23, 2020 23:25
@grooverdan grooverdan force-pushed the bb-10.2-danielblack-MDEV-23630-mysqldump-native-sql branch from c932df0 to 27b12ab Compare October 24, 2020 03:41
Add --system={all, users, plugins, udfs, servers, stats, timezones}

This will dump system information from the server in
a logical form like:
* CREATE USER
* GRANT
* SET DEFAULT ROLE
* CREATE ROLE
* CREATE SERVER
* INSTALL PLUGIN
* CREATE FUNCTION

"stats" is the innodb statistics tables or EITS and
these are dumped as INSERT/REPLACE INTO statements
without recreating the table.

"timezones" is the collection of timezone tables
which are important to transfer to generate identical
results on restoration.

Two other options have an effect on the SQL generated by
--system=all. These are mutually exclusive of each other.
* --replace
* --insert-ignore

--replace will include "OR REPLACE" into the logical form
like:
* CREATE OR REPLACE USER ...
* DROP ROLE IF EXISTS (MySQL-8.0+)
* CREATE OR REPLACE ROLE ...
* UNINSTALL PLUGIN IF EXISTS (10.4+) ... (before INSTALL PLUGIN)
* DROP FUNCTION IF EXISTS (MySQL-5.7+)
* CREATE OR REPLACE [AGGREGATE] FUNCTION
* CREATE OR REPLACE SERVER

--insert-ignore uses the construct " IF NOT EXISTS" where
supported in the logical syntax.

'CREATE OR REPLACE USER' includes protection against
being run as the same user that is importing the mysqldump.

Includes experimental support for dumping mysql-5.7/8.0
system tables and exporting logical SQL compatible with MySQL.

Updates mysqldump man page, including this information and
(removing obsolute bug reference)
@grooverdan grooverdan force-pushed the bb-10.2-danielblack-MDEV-23630-mysqldump-native-sql branch from 27b12ab to a7fe616 Compare October 24, 2020 06:09
grooverdan pushed a commit that referenced this pull request Mar 24, 2021
…elete #2

Note they key_or() may call tree_delete(), which will cause the weight
asserts to be checked. In order to avoid them from firing, update key1
tree's weight after we've changed key1->some_local_child->next_key_part.

Having done that, do we still need this at the function end:

  /* Re-compute the result tree's weight. */
  key1->update_weight_locally();
?
grooverdan pushed a commit that referenced this pull request Apr 16, 2021
…warning in case it is executed in PS (prepared statement) mode

The EXPLAIN EXTENDED statement run as a prepared statement can produce extra
warning comparing with a case when EXPLAIN EXTENDED statement is run as
a regular statement. For example, the following test case
  CREATE TABLE t1 (c int);
  CREATE TABLE t2 (d int);
  EXPLAIN EXTENDED SELECT (SELECT 1 FROM t2 WHERE d = c) FROM t1;

produces the extra warning
  "Field or reference 'c' of SELECT #2 was resolved in SELECT #1"
in case the above mentioned "EXPLAIN EXTENDED" statement is executed
in PS mode, that is by submitting the following statements:
   PREPARE stmt FROM "EXPLAIN EXTENDED SELECT (SELECT 1 FROM t2 WHERE d = c) FROM t1";
   EXECUTE stmt;

The reason of the extra warning emittion is in a way items
are handled (being fixed) during execution of the JOIN::prepare() method.
The method Item_field::fix_fields() calls the find_field_in_tables()
function in case a field hasn't been associated yet with the item.
Implementation of the find_field_in_tables() function first checks whether
a table containing the required field was already opened and cached.
It is done by checking the data member item->cached_table. This data member
is set on handling the PRERARE FROM statement and checked on executing
the EXECUTE statement. If the data member item->cached_table is set
the find_field_in_tables() function invoked and the
mark_select_range_as_dependent() function called if the field
is an outer referencee. The mark_select_range_as_dependent() function
calls the mark_as_dependent() function that finally invokes
the push_warning_printf() function that produces extra warning.

To fix the issue, calling of push_warning_printf() is elimited in case
it was run indirectly in result of hanlding already opened table from
the Item_field::fix_fields() method.
grooverdan pushed a commit that referenced this pull request Apr 16, 2021
…::make_sort_key_part (#2)

IFNULL(inet6_not_null_expr, 'foo') erroneously set its nullability to NOT NULL.

Fix:
- Moving the line "maybe_null= args[1]->maybe_null" before the call
  of fix_length_and_dec2(), so the call of Type_handler method
  Item_hybrid_func_fix_attributes() can reset it when desired.

- Fixing Type_handler_inet6::Item_hybrid_func_fix_attributes()
  to ignore args[0] when detecting nullability of IFNULL().
grooverdan pushed a commit that referenced this pull request Apr 16, 2021
…the test

Part #2, specifically for the 10.5+ branch:

The auto-increment parameters can change sporadically during the
execution of the mtr test "galera_vote_rejoin_ddl", causing it to
fail. This patch creates an environment where unpredictable changes
to these auto-increment settings do not occur during the test.
grooverdan pushed a commit that referenced this pull request Nov 17, 2021
  Read of size 8 at 0x7fecf2e75fc8 by thread T2 (mutexes: write M1318):
    #0 tpool::thread_pool_generic::submit_task(tpool::task*) /tpool/tpool_generic.cc:823:9 (mariadbd+0x25fd2d2)
    #1 (anonymous namespace)::aio_uring::thread_routine((anonymous namespace)::aio_uring*) /tpool/aio_liburing.cc:173:20 (mariadbd+0x260b21b)
    #2 void std::__invoke_impl<void, void (*)((anonymous namespace)::aio_uring*), (anonymous namespace)::aio_uring*>(std::__invoke_other, void (*&&)((anonymous namespace)::aio_uring*), (anonymous namespace)::aio_uring*&&) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:61:14 (mariadbd+0x260c62a)
    #3 std::__invoke_result<void (*)((anonymous namespace)::aio_uring*), (anonymous namespace)::aio_uring*>::type std::__invoke<void (*)((anonymous namespace)::aio_uring*), (anonymous namespace)::aio_uring*>(void (*&&)((anonymous namespace)::aio_uring*), (anonymous namespace)::aio_uring*&&) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:96:14 (mariadbd+0x260c4ba)
    #4 void std::thread::_Invoker<std::tuple<void (*)((anonymous namespace)::aio_uring*), (anonymous namespace)::aio_uring*> >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_thread.h:253:13 (mariadbd+0x260c442)
    #5 std::thread::_Invoker<std::tuple<void (*)((anonymous namespace)::aio_uring*), (anonymous namespace)::aio_uring*> >::operator()() /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_thread.h:260:11 (mariadbd+0x260c3c5)
    #6 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)((anonymous namespace)::aio_uring*), (anonymous namespace)::aio_uring*> > >::_M_run() /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_thread.h:211:13 (mariadbd+0x260c189)
    #7 <null> <null> (libstdc++.so.6+0xd230f)

  Previous write of size 8 at 0x7fecf2e75fc8 by main thread:
    #0 tpool::task::task(void (*)(void*), void*, tpool::task_group*) /tpool/task.cc:40:46 (mariadbd+0x260a138)
    #1 tpool::aiocb::aiocb() /tpool/tpool.h:147:13 (mariadbd+0x2355943)
    #2 void std::_Construct<tpool::aiocb>(tpool::aiocb*) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_construct.h:109:38 (mariadbd+0x2355845)
    #3 tpool::aiocb* std::__uninitialized_default_n_1<false>::__uninit_default_n<tpool::aiocb*, unsigned long>(tpool::aiocb*, unsigned long) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_uninitialized.h:579:3 (mariadbd+0x235576c)
    #4 tpool::aiocb* std::__uninitialized_default_n<tpool::aiocb*, unsigned long>(tpool::aiocb*, unsigned long) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_uninitialized.h:638:14 (mariadbd+0x23556e9)
    #5 tpool::aiocb* std::__uninitialized_default_n_a<tpool::aiocb*, unsigned long, tpool::aiocb>(tpool::aiocb*, unsigned long, std::allocator<tpool::aiocb>&) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_uninitialized.h:704:14 (mariadbd+0x2355641)
    #6 std::vector<tpool::aiocb, std::allocator<tpool::aiocb> >::_M_default_initialize(unsigned long) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_vector.h:1606:4 (mariadbd+0x2354f3d)
    #7 std::vector<tpool::aiocb, std::allocator<tpool::aiocb> >::vector(unsigned long, std::allocator<tpool::aiocb> const&) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_vector.h:512:9 (mariadbd+0x2354a19)
    MariaDB#8 tpool::cache<tpool::aiocb>::cache(unsigned long, tpool::cache_notification_mode) /tpool/tpool_structs.h:73:20 (mariadbd+0x2354784)
    MariaDB#9 io_slots::io_slots(int, int) /storage/innobase/os/os0file.cc:93:3 (mariadbd+0x235343b)
    MariaDB#10 os_aio_init() /storage/innobase/os/os0file.cc:3780:22 (mariadbd+0x234ebce)
    MariaDB#11 srv_start(bool) /storage/innobase/srv/srv0start.cc:1190:6 (mariadbd+0x256720c)
    MariaDB#12 innodb_init(void*) /storage/innobase/handler/ha_innodb.cc:4188:8 (mariadbd+0x1ed3bda)
    MariaDB#13 ha_initialize_handlerton(st_plugin_int*) /sql/handler.cc:659:31 (mariadbd+0xf7be06)
    MariaDB#14 plugin_initialize(st_mem_root*, st_plugin_int*, int*, char**, bool) /sql/sql_plugin.cc:1463:9 (mariadbd+0x160fa1b)
    MariaDB#15 plugin_init(int*, char**, int) /sql/sql_plugin.cc:1756:15 (mariadbd+0x160f07f)
    MariaDB#16 init_server_components() /sql/mysqld.cc:5043:7 (mariadbd+0xd70fb2)
    MariaDB#17 mysqld_main(int, char**) /sql/mysqld.cc:5655:7 (mariadbd+0xd6a9d7)
    MariaDB#18 main /sql/main.cc:34:10 (mariadbd+0xd65d18)

I think the report is incorrect: it's not possible to have such a race
condition. I've checked it by reading the code and putting assertions.
Namely, no aio I/O is possible before the end of os_aio_init().
Most probably it's some bug in TSAN. But the patch fixes around 5 related
reports and this is a step toward TSAN usefullness. Currently it reports too
much noise.

std::unique_ptr is a step toward https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r11-avoid-calling-new-and-delete-explicitly
There is no std::make_unique() in C++11, however.
grooverdan pushed a commit that referenced this pull request Nov 17, 2021
WARNING: ThreadSanitizer: data race (pid=1503350)
  Write of size 8 at 0x0000067b1f20 by thread T3:
    #0 os_file_sync_posix(int) /storage/innobase/os/os0file.cc:895:5 (mariadbd+0x23493f6)
    #1 os_file_flush_func(int) /storage/innobase/os/os0file.cc:983:8 (mariadbd+0x2349204)
    #2 file_os_io::flush() /storage/innobase/log/log0log.cc:326:10 (mariadbd+0x22eaaa9)
    #3 log_file_t::flush() /storage/innobase/log/log0log.cc:440:18 (mariadbd+0x22eb2d0)
    #4 log_t::file::flush() /storage/innobase/log/log0log.cc:507:29 (mariadbd+0x22ebe69)
    #5 log_write_flush_to_disk_low(unsigned long) /storage/innobase/log/log0log.cc:629:17 (mariadbd+0x22ed3f3)
    #6 log_write_up_to(unsigned long, bool, bool, completion_callback const*) /storage/innobase/log/log0log.cc:829:3 (mariadbd+0x22ecb04)
    #7 log_checkpoint_low(unsigned long, unsigned long) /storage/innobase/buf/buf0flu.cc:1734:5 (mariadbd+0x20d37f1)
    MariaDB#8 buf_flush_sync_for_checkpoint(unsigned long) /storage/innobase/buf/buf0flu.cc:1947:7 (mariadbd+0x20d4193)
    MariaDB#9 buf_flush_page_cleaner() /storage/innobase/buf/buf0flu.cc:2186:9 (mariadbd+0x20cdad7)
    MariaDB#10 void std::__invoke_impl<void, void (*)()>(std::__invoke_other, void (*&&)()) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:61:14 (mariadbd+0x20c3aaa)
    MariaDB#11 std::__invoke_result<void (*)()>::type std::__invoke<void (*)()>(void (*&&)()) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:96:14 (mariadbd+0x20c39bd)
    MariaDB#12 void std::thread::_Invoker<std::tuple<void (*)()> >::_M_invoke<0ul>(std::_Index_tuple<0ul>) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_thread.h:253:13 (mariadbd+0x20c3965)
    MariaDB#13 std::thread::_Invoker<std::tuple<void (*)()> >::operator()() /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_thread.h:260:11 (mariadbd+0x20c3905)
    MariaDB#14 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)()> > >::_M_run() /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_thread.h:211:13 (mariadbd+0x20c37f9)
    MariaDB#15 <null> <null> (libstdc++.so.6+0xd230f)

  Previous write of size 8 at 0x0000067b1f20 by main thread:
    #0 os_file_sync_posix(int) /storage/innobase/os/os0file.cc:895:5 (mariadbd+0x23493f6)
    #1 os_file_flush_func(int) /storage/innobase/os/os0file.cc:983:8 (mariadbd+0x2349204)
    #2 fil_space_t::flush_low() /storage/innobase/fil/fil0fil.cc:504:5 (mariadbd+0x205cad5)
    #3 fil_flush_file_spaces() /storage/innobase/fil/fil0fil.cc:2947:13 (mariadbd+0x206523f)
    #4 log_checkpoint() /storage/innobase/buf/buf0flu.cc:1777:5 (mariadbd+0x20cd069)
    #5 buf_flush_wait_flushed(unsigned long) /storage/innobase/buf/buf0flu.cc:1867:5 (mariadbd+0x20ccf95)
    #6 log_make_checkpoint() /storage/innobase/buf/buf0flu.cc:1793:3 (mariadbd+0x20cc4c9)
    #7 buf_dblwr_t::create() /storage/innobase/buf/buf0dblwr.cc:216:3 (mariadbd+0x209076a)
    MariaDB#8 srv_start(bool) /storage/innobase/srv/srv0start.cc:1685:20 (mariadbd+0x256b514)
    MariaDB#9 innodb_init(void*) /storage/innobase/handler/ha_innodb.cc:4188:8 (mariadbd+0x1ed406a)
    MariaDB#10 ha_initialize_handlerton(st_plugin_int*) /sql/handler.cc:659:31 (mariadbd+0xf7c246)
    MariaDB#11 plugin_initialize(st_mem_root*, st_plugin_int*, int*, char**, bool) /sql/sql_plugin.cc:1463:9 (mariadbd+0x160fe6b)
    MariaDB#12 plugin_init(int*, char**, int) /sql/sql_plugin.cc:1756:15 (mariadbd+0x160f4cf)
    MariaDB#13 init_server_components() /sql/mysqld.cc:5043:7 (mariadbd+0xd713f2)
    MariaDB#14 mysqld_main(int, char**) /sql/mysqld.cc:5655:7 (mariadbd+0xd6ae17)
    MariaDB#15 main /sql/main.cc:34:10 (mariadbd+0xd66158)

This is a correct report by TSAN for an obvious case: unprotected global
counter. Fix it by making counter std::atomic.
grooverdan pushed a commit that referenced this pull request Nov 17, 2021
WARNING: ThreadSanitizer: data race (pid=1506937)
  Write of size 8 at 0x0000067ab740 by thread T6:
    #0 buf_page_get_low(page_id_t, unsigned long, unsigned long, buf_block_t*, unsigned long, mtr_t*, dberr_t*, bool) /storage/innobase/buf/buf0buf.cc:2946:8 (mariadbd+0x2014c7f)
    #1 buf_page_get_gen(page_id_t, unsigned long, unsigned long, buf_block_t*, unsigned long, mtr_t*, dberr_t*, bool) /storage/innobase/buf/buf0buf.cc:3047:10 (mariadbd+0x2016216)
    #2 btr_cur_search_to_nth_level_func(dict_index_t*, unsigned long, dtuple_t const*, page_cur_mode_t, unsigned long, btr_cur_t*, ssux_lock_impl<true>*, mtr_t*, unsigned long) /storage/innobase/btr/btr0cur.cc:1613:10 (mariadbd+0x1fb5bff)
    #3 btr_pcur_open_low(dict_index_t*, unsigned long, dtuple_t const*, page_cur_mode_t, unsigned long, btr_pcur_t*, unsigned long, mtr_t*) /storage/innobase/include/btr0pcur.ic:439:8 (mariadbd+0x24ddead)
    #4 row_search_on_row_ref(btr_pcur_t*, unsigned long, dict_table_t const*, dtuple_t const*, mtr_t*) /storage/innobase/row/row0row.cc:1215:7 (mariadbd+0x24dd537)
    #5 row_purge_reposition_pcur(unsigned long, purge_node_t*, mtr_t*) /storage/innobase/row/row0purge.cc:81:23 (mariadbd+0x24c5369)
    #6 row_purge_reset_trx_id(purge_node_t*, mtr_t*) /storage/innobase/row/row0purge.cc:748:6 (mariadbd+0x24c90c7)
    #7 row_purge_record_func(purge_node_t*, unsigned char*, que_thr_t const*, bool) /storage/innobase/row/row0purge.cc:1174:4 (mariadbd+0x24c8262)
    MariaDB#8 row_purge(purge_node_t*, unsigned char*, que_thr_t*) /storage/innobase/row/row0purge.cc:1218:18 (mariadbd+0x24c5af3)
    MariaDB#9 row_purge_step(que_thr_t*) /storage/innobase/row/row0purge.cc:1267:3 (mariadbd+0x24c5996)
    MariaDB#10 que_thr_step(que_thr_t*) /storage/innobase/que/que0que.cc:653:9 (mariadbd+0x23d5298)
    MariaDB#11 que_run_threads_low(que_thr_t*) /storage/innobase/que/que0que.cc:709:25 (mariadbd+0x23d3f29)
    MariaDB#12 que_run_threads(que_thr_t*) /storage/innobase/que/que0que.cc:729:2 (mariadbd+0x23d3bdf)
    MariaDB#13 srv_task_execute() /storage/innobase/srv/srv0srv.cc:1692:3 (mariadbd+0x2562841)
    MariaDB#14 purge_worker_callback(void*) /storage/innobase/srv/srv0srv.cc:1864:10 (mariadbd+0x255f361)
    MariaDB#15 tpool::task_group::execute(tpool::task*) /tpool/task_group.cc:55:9 (mariadbd+0x260a5ca)
    MariaDB#16 tpool::task::execute() /tpool/task.cc:47:16 (mariadbd+0x260adf6)
    MariaDB#17 tpool::thread_pool_generic::worker_main(tpool::worker_data*) /tpool/tpool_generic.cc:550:11 (mariadbd+0x25fc590)
    MariaDB#18 void std::__invoke_impl<void, void (tpool::thread_pool_generic::*)(tpool::worker_data*), tpool::thread_pool_generic*, tpool::worker_data*>(std::__invoke_memfun_deref, void (tpool::thread_pool_generic::*&&)(tpool::worker_data*), tpool::thread_pool_generic*&&, tpool::worker_data*&&) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:74:14 (mariadbd+0x26061b5)
    MariaDB#19 std::__invoke_result<void (tpool::thread_pool_generic::*)(tpool::worker_data*), tpool::thread_pool_generic*, tpool::worker_data*>::type std::__invoke<void (tpool::thread_pool_generic::*)(tpool::worker_data*), tpool::thread_pool_generic*, tpool::worker_data*>(void (tpool::thread_pool_generic::*&&)(tpool::worker_data*), tpool::thread_pool_generic*&&, tpool::worker_data*&&) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:96:14 (mariadbd+0x2605f57)
    MariaDB#20 void std::thread::_Invoker<std::tuple<void (tpool::thread_pool_generic::*)(tpool::worker_data*), tpool::thread_pool_generic*, tpool::worker_data*> >::_M_invoke<0ul, 1ul, 2ul>(std::_Index_tuple<0ul, 1ul, 2ul>) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_thread.h:253:13 (mariadbd+0x2605ecb)
    MariaDB#21 std::thread::_Invoker<std::tuple<void (tpool::thread_pool_generic::*)(tpool::worker_data*), tpool::thread_pool_generic*, tpool::worker_data*> >::operator()() /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_thread.h:260:11 (mariadbd+0x2605e35)
    MariaDB#22 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (tpool::thread_pool_generic::*)(tpool::worker_data*), tpool::thread_pool_generic*, tpool::worker_data*> > >::_M_run() /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_thread.h:211:13 (mariadbd+0x2605ac9)
    MariaDB#23 <null> <null> (libstdc++.so.6+0xd230f)

  Previous write of size 8 at 0x0000067ab740 by thread T8:
    #0 buf_page_get_low(page_id_t, unsigned long, unsigned long, buf_block_t*, unsigned long, mtr_t*, dberr_t*, bool) /storage/innobase/buf/buf0buf.cc:2946:8 (mariadbd+0x2014c7f)
    #1 buf_page_get_gen(page_id_t, unsigned long, unsigned long, buf_block_t*, unsigned long, mtr_t*, dberr_t*, bool) /storage/innobase/buf/buf0buf.cc:3047:10 (mariadbd+0x2016216)
    #2 btr_cur_search_to_nth_level_func(dict_index_t*, unsigned long, dtuple_t const*, page_cur_mode_t, unsigned long, btr_cur_t*, ssux_lock_impl<true>*, mtr_t*, unsigned long) /storage/innobase/btr/btr0cur.cc:1613:10 (mariadbd+0x1fb5bff)
    #3 btr_pcur_open_low(dict_index_t*, unsigned long, dtuple_t const*, page_cur_mode_t, unsigned long, btr_pcur_t*, unsigned long, mtr_t*) /storage/innobase/include/btr0pcur.ic:439:8 (mariadbd+0x24ddead)
    #4 row_search_on_row_ref(btr_pcur_t*, unsigned long, dict_table_t const*, dtuple_t const*, mtr_t*) /storage/innobase/row/row0row.cc:1215:7 (mariadbd+0x24dd537)
    #5 row_purge_reposition_pcur(unsigned long, purge_node_t*, mtr_t*) /storage/innobase/row/row0purge.cc:81:23 (mariadbd+0x24c5369)
    #6 row_purge_reset_trx_id(purge_node_t*, mtr_t*) /storage/innobase/row/row0purge.cc:748:6 (mariadbd+0x24c90c7)
    #7 row_purge_record_func(purge_node_t*, unsigned char*, que_thr_t const*, bool) /storage/innobase/row/row0purge.cc:1174:4 (mariadbd+0x24c8262)
    MariaDB#8 row_purge(purge_node_t*, unsigned char*, que_thr_t*) /storage/innobase/row/row0purge.cc:1218:18 (mariadbd+0x24c5af3)
    MariaDB#9 row_purge_step(que_thr_t*) /storage/innobase/row/row0purge.cc:1267:3 (mariadbd+0x24c5996)
    MariaDB#10 que_thr_step(que_thr_t*) /storage/innobase/que/que0que.cc:653:9 (mariadbd+0x23d5298)
    MariaDB#11 que_run_threads_low(que_thr_t*) /storage/innobase/que/que0que.cc:709:25 (mariadbd+0x23d3f29)
    MariaDB#12 que_run_threads(que_thr_t*) /storage/innobase/que/que0que.cc:729:2 (mariadbd+0x23d3bdf)
    MariaDB#13 trx_purge(unsigned long, bool) /storage/innobase/trx/trx0purge.cc:1271:2 (mariadbd+0x25841b4)
    MariaDB#14 srv_do_purge(unsigned long*) /storage/innobase/srv/srv0srv.cc:1784:20 (mariadbd+0x2563224)
    MariaDB#15 purge_coordinator_callback_low() /storage/innobase/srv/srv0srv.cc:1881:35 (mariadbd+0x2562b3b)
    MariaDB#16 purge_coordinator_callback(void*) /storage/innobase/srv/srv0srv.cc:1910:3 (mariadbd+0x255f4ab)
    MariaDB#17 tpool::task_group::execute(tpool::task*) /tpool/task_group.cc:55:9 (mariadbd+0x260a5ca)
    MariaDB#18 tpool::task::execute() /tpool/task.cc:47:16 (mariadbd+0x260adf6)
    MariaDB#19 tpool::thread_pool_generic::worker_main(tpool::worker_data*) /tpool/tpool_generic.cc:550:11 (mariadbd+0x25fc590)
    MariaDB#20 void std::__invoke_impl<void, void (tpool::thread_pool_generic::*)(tpool::worker_data*), tpool::thread_pool_generic*, tpool::worker_data*>(std::__invoke_memfun_deref, void (tpool::thread_pool_generic::*&&)(tpool::worker_data*), tpool::thread_pool_generic*&&, tpool::worker_data*&&) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:74:14 (mariadbd+0x26061b5)
    MariaDB#21 std::__invoke_result<void (tpool::thread_pool_generic::*)(tpool::worker_data*), tpool::thread_pool_generic*, tpool::worker_data*>::type std::__invoke<void (tpool::thread_pool_generic::*)(tpool::worker_data*), tpool::thread_pool_generic*, tpool::worker_data*>(void (tpool::thread_pool_generic::*&&)(tpool::worker_data*), tpool::thread_pool_generic*&&, tpool::worker_data*&&) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:96:14 (mariadbd+0x2605f57)
    MariaDB#22 void std::thread::_Invoker<std::tuple<void (tpool::thread_pool_generic::*)(tpool::worker_data*), tpool::thread_pool_generic*, tpool::worker_data*> >::_M_invoke<0ul, 1ul, 2ul>(std::_Index_tuple<0ul, 1ul, 2ul>) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_thread.h:253:13 (mariadbd+0x2605ecb)
    MariaDB#23 std::thread::_Invoker<std::tuple<void (tpool::thread_pool_generic::*)(tpool::worker_data*), tpool::thread_pool_generic*, tpool::worker_data*> >::operator()() /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_thread.h:260:11 (mariadbd+0x2605e35)
    MariaDB#24 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (tpool::thread_pool_generic::*)(tpool::worker_data*), tpool::thread_pool_generic*, tpool::worker_data*> > >::_M_run() /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_thread.h:211:13 (mariadbd+0x2605ac9)
    MariaDB#25 <null> <null> (libstdc++.so.6+0xd230f)

  Location is global 'buf_dbg_counter' of size 8 at 0x0000067ab740 (mariadbd+0x67ab740)

  The obvious fix is to make counter atomic.
grooverdan pushed a commit that referenced this pull request Nov 17, 2021
  Write of size 1 at 0x0000067abe08 by thread T3 (mutexes: write M1372):
    #0 buf_flush_page_cleaner() /storage/innobase/buf/buf0flu.cc:2366:29 (mariadbd+0x20cea7c)
    #1 void std::__invoke_impl<void, void (*)()>(std::__invoke_other, void (*&&)()) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:61:14 (mariadbd+0x20c3a8a)
    #2 std::__invoke_result<void (*)()>::type std::__invoke<void (*)()>(void (*&&)()) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:96:14 (mariadbd+0x20c399d)
    #3 void std::thread::_Invoker<std::tuple<void (*)()> >::_M_invoke<0ul>(std::_Index_tuple<0ul>) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_thread.h:253:13 (mariadbd+0x20c3945)
    #4 std::thread::_Invoker<std::tuple<void (*)()> >::operator()() /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_thread.h:260:11 (mariadbd+0x20c38e5)
    #5 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)()> > >::_M_run() /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_thread.h:211:13 (mariadbd+0x20c37d9)
    #6 <null> <null> (libstdc++.so.6+0xd230f)

  Previous read of size 1 at 0x0000067abe08 by main thread:
    #0 logs_empty_and_mark_files_at_shutdown() /storage/innobase/log/log0log.cc:1094:6 (mariadbd+0x22eeff3)
    #1 innodb_shutdown() /storage/innobase/srv/srv0start.cc:1970:3 (mariadbd+0x256ffd6)
    #2 innobase_end(handlerton*, ha_panic_function) /storage/innobase/handler/ha_innodb.cc:4265:3 (mariadbd+0x1ee3fc4)
    #3 ha_finalize_handlerton(st_plugin_int*) /sql/handler.cc:595:5 (mariadbd+0xf7bac9)
    #4 plugin_deinitialize(st_plugin_int*, bool) /sql/sql_plugin.cc:1266:9 (mariadbd+0x1611789)
    #5 reap_plugins() /sql/sql_plugin.cc:1342:7 (mariadbd+0x160e17d)
    #6 plugin_shutdown() /sql/sql_plugin.cc:2050:7 (mariadbd+0x1611f42)
    #7 clean_up(bool) /sql/mysqld.cc:1923:3 (mariadbd+0xd67a4c)
    MariaDB#8 unireg_abort /sql/mysqld.cc:1835:3 (mariadbd+0xd67605)
    MariaDB#9 mysqld_main(int, char**) /sql/mysqld.cc:5741:7 (mariadbd+0xd6b36a)
    MariaDB#10 main /sql/main.cc:34:10 (mariadbd+0xd661a8)

  Location is global 'buf_page_cleaner_is_active' of size 1 at 0x0000067abe08 (mariadbd+0x67abe08)
grooverdan pushed a commit that referenced this pull request Nov 17, 2021
WARNING: ThreadSanitizer: data race (pid=1510842)
  Write of size 8 at 0x0000067b1e98 by main thread:
    #0 os_file_pwrite(IORequest const&, int, unsigned char const*, unsigned long, unsigned long, dberr_t*) /storage/innobase/os/os0file.cc:2928:2 (mariadbd+0x234c5ac)
    #1 os_file_write_func(IORequest const&, char const*, int, void const*, unsigned long, unsigned long) /storage/innobase/os/os0file.cc:2963:20 (mariadbd+0x234c019)
    #2 file_os_io::write(char const*, unsigned long, st_::span<unsigned char const>) /storage/innobase/log/log0log.cc:320:10 (mariadbd+0x22eaa50)
    #3 log_file_t::write(unsigned long, st_::span<unsigned char const>) /storage/innobase/log/log0log.cc:434:18 (mariadbd+0x22eb1d8)
    #4 log_t::file::write(unsigned long, st_::span<unsigned char>) /storage/innobase/log/log0log.cc:496:29 (mariadbd+0x22ebb55)
    #5 log_write_buf(unsigned char*, unsigned long, unsigned long, unsigned long, unsigned long) /storage/innobase/log/log0log.cc:614:14 (mariadbd+0x22f1b51)
    #6 log_write(bool) /storage/innobase/log/log0log.cc:755:2 (mariadbd+0x22ed2ec)
    #7 log_write_up_to(unsigned long, bool, bool, completion_callback const*) /storage/innobase/log/log0log.cc:817:5 (mariadbd+0x22eca44)
    MariaDB#8 log_checkpoint_low(unsigned long, unsigned long) /storage/innobase/buf/buf0flu.cc:1734:5 (mariadbd+0x20d37c1)
    MariaDB#9 log_checkpoint() /storage/innobase/buf/buf0flu.cc:1787:10 (mariadbd+0x20cd155)
    MariaDB#10 buf_flush_wait_flushed(unsigned long) /storage/innobase/buf/buf0flu.cc:1867:5 (mariadbd+0x20ccf8f)
    MariaDB#11 log_make_checkpoint() /storage/innobase/buf/buf0flu.cc:1793:3 (mariadbd+0x20cc4c9)
    MariaDB#12 buf_dblwr_t::create() /storage/innobase/buf/buf0dblwr.cc:216:3 (mariadbd+0x209076a)
    MariaDB#13 srv_start(bool) /storage/innobase/srv/srv0start.cc:1685:20 (mariadbd+0x256b4aa)
    MariaDB#14 innodb_init(void*) /storage/innobase/handler/ha_innodb.cc:4188:8 (mariadbd+0x1ed40da)
    MariaDB#15 ha_initialize_handlerton(st_plugin_int*) /sql/handler.cc:659:31 (mariadbd+0xf7c2b6)
    MariaDB#16 plugin_initialize(st_mem_root*, st_plugin_int*, int*, char**, bool) /sql/sql_plugin.cc:1463:9 (mariadbd+0x160fedb)
    MariaDB#17 plugin_init(int*, char**, int) /sql/sql_plugin.cc:1756:15 (mariadbd+0x160f53f)
    MariaDB#18 init_server_components() /sql/mysqld.cc:5043:7 (mariadbd+0xd71462)
    MariaDB#19 mysqld_main(int, char**) /sql/mysqld.cc:5655:7 (mariadbd+0xd6ae87)
    MariaDB#20 main /sql/main.cc:34:10 (mariadbd+0xd661c8)

  Previous write of size 8 at 0x0000067b1e98 by thread T3:
    #0 os_file_pwrite(IORequest const&, int, unsigned char const*, unsigned long, unsigned long, dberr_t*) /storage/innobase/os/os0file.cc:2928:2 (mariadbd+0x234c5ac)
    #1 os_file_write_func(IORequest const&, char const*, int, void const*, unsigned long, unsigned long) /storage/innobase/os/os0file.cc:2963:20 (mariadbd+0x234c019)
    #2 file_os_io::write(char const*, unsigned long, st_::span<unsigned char const>) /storage/innobase/log/log0log.cc:320:10 (mariadbd+0x22eaa50)
    #3 log_file_t::write(unsigned long, st_::span<unsigned char const>) /storage/innobase/log/log0log.cc:434:18 (mariadbd+0x22eb1d8)
    #4 log_t::file::write(unsigned long, st_::span<unsigned char>) /storage/innobase/log/log0log.cc:496:29 (mariadbd+0x22ebb55)
    #5 log_write_checkpoint_info(unsigned long) /storage/innobase/log/log0log.cc:911:14 (mariadbd+0x22edd4e)
    #6 log_checkpoint_low(unsigned long, unsigned long) /storage/innobase/buf/buf0flu.cc:1755:3 (mariadbd+0x20d3a3d)
    #7 buf_flush_sync_for_checkpoint(unsigned long) /storage/innobase/buf/buf0flu.cc:1947:7 (mariadbd+0x20d4163)
    MariaDB#8 buf_flush_page_cleaner() /storage/innobase/buf/buf0flu.cc:2186:9 (mariadbd+0x20cdab1)
    MariaDB#9 void std::__invoke_impl<void, void (*)()>(std::__invoke_other, void (*&&)()) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:61:14 (mariadbd+0x20c3aaa)
    MariaDB#10 std::__invoke_result<void (*)()>::type std::__invoke<void (*)()>(void (*&&)()) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:96:14 (mariadbd+0x20c39bd)
    MariaDB#11 void std::thread::_Invoker<std::tuple<void (*)()> >::_M_invoke<0ul>(std::_Index_tuple<0ul>) /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_thread.h:253:13 (mariadbd+0x20c3965)
    MariaDB#12 std::thread::_Invoker<std::tuple<void (*)()> >::operator()() /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_thread.h:260:11 (mariadbd+0x20c3905)
    MariaDB#13 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)()> > >::_M_run() /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_thread.h:211:13 (mariadbd+0x20c37f9)
    MariaDB#14 <null> <null> (libstdc++.so.6+0xd230f)

  Location is global 'os_n_file_writes' of size 8 at 0x0000067b1e98 (mariadbd+0x67b1e98)

  Make variable atomic.
grooverdan pushed a commit that referenced this pull request Dec 16, 2021
…nd execution of PS

Consider the following use case:
MariaDB [test]> CREATE TABLE t1 (field1 BIGINT DEFAULT -1);
MariaDB [test]> CREATE VIEW v1 AS SELECT DISTINCT field1 FROM t1;

Repeated execution of the following query as a Prepared Statement

MariaDB [test]> PREPARE stmt FROM 'SELECT * FROM v1 WHERE field1 <=> NULL';
MariaDB [test]> EXECUTE stmt;

results in a crash for a server built with DEBUG.

MariaDB [test]> EXECUTE stmt;
ERROR 2013 (HY000): Lost connection to MySQL server during query

Assertion failed: (!result), function convert_const_to_int, file item_cmpfunc.cc, line 476.
Abort trap: 6 (core dumped)

The crash inside the function convert_const_to_int() happens by the reason
that the value -1 is stored in an instance of the class Field_longlong
on restoring its original value in the statement
  result= field->store(orig_field_val, TRUE);
that leads to assigning the value 1 to the variable 'result' with subsequent
crash in the DBUG_ASSERT statement following it
  DBUG_ASSERT(!result);

The main matter here is why this assertion failure happens on the second
execution of the prepared statement and doens't on the first one.
On first handling of the statement
  'EXECUTE stmt;'
a temporary table is created for serving the query involving the view 'v1'.
The table is created by the function create_tmp_table() in the following
calls trace: (trace #1)
  JOIN::prepare (at sql_select.cc:725)
    st_select_lex::handle_derived
      LEX::handle_list_of_derived
        TABLE_LIST::handle_derived
          mysql_handle_single_derived
            mysql_derived_prepare
              select_union::create_result_table
                create_tmp_table

Note, that the data member TABLE::status of a TABLE instance returned by the
function create_tmp_table() has the value 0.

Later the function setup_table_map() is called on the TABLE instance just
created for the sake of the temporary table (calls trace #2 is below):
  JOIN::prepare (at sql_select.cc:737)
    setup_tables_and_check_access
      setup_tables
        setup_table_map
where the data member TABLE::status is set to the value STATUS_NO_RECORD.

After that when execution of the method JOIN::prepare reaches calling of
the function setup_without_group() the following calls trace is invoked
  JOIN::prepare
    setup_without_group
      setup_conds
        Item_func::fix_fields
          Item_func_equal::fix_length_and_dec
            Item_bool_rowready_func2::fix_length_and_dec
              Item_func::setup_args_and_comparator
                Item_func::convert_const_compared_to_int_field
                  convert_const_to_int

There is the following code snippet in the function convert_const_to_int()
at the line item_cmpfunc.cc:448
    bool save_field_value= (field_item->const_item() ||
                            !(field->table->status & STATUS_NO_RECORD));
Since field->table->status has bits STATUS_NO_RECORD set the variable
save_field_value is false and therefore neither the method
Field_longlong::val_int() nor the method Field_longlong::store is called
on the Field instance that has the numeric value -1.
That is the reason why first execution of the Prepared Statement for the query
  'SELECT * FROM v1 WHERE field1 <=> NULL'
is successful.

On second running of the statement 'EXECUTE stmt' a new temporary tables
is also created by running the calls trace #1 but the trace #2 is not executed
by the reason that data member SELECT_LEX::first_cond_optimization has been set
to false on first execution of the prepared statemet (in the method
JOIN::optimize_inner()). As a consequence, the data member TABLE::status for
a temporary table just created doesn't have the flags STATUS_NO_RECORD set and
therefore on re-execution of the prepared statement the methods
Field_longlong::val_int() and Field_longlong::store() are called for the field
having the value -1 and the DBUG_ASSERT(!result) is fired.

To fix the issue the data member TABLE::status has to be assigned the value
STATUS_NO_RECORD in every place where the macros empty_record() is called
to emptify a record for just instantiated TABLE object created on behalf
the new temporary table.
grooverdan pushed a commit that referenced this pull request Feb 22, 2022
grooverdan pushed a commit that referenced this pull request Feb 22, 2022
…ow query log

The problem affected queries in form:

  SELECT FROM (SELECT where Split Materialized is applicable) WHERE 1=0

The problem was caused by this:
- The select in derived table uses two-phase optimization (due to a
  possible Split Materialized).
- The primary select has "Impossible where" and so it short-cuts its
  optimization.
- The optimization for the SELECT in the derived table is never finished,
  and EXPLAIN data structure has a dangling pointer to select #2.

Fixed with this: make JOIN::optimize_stage2() invoke optimization of
derived tables when it is handing a degenerate JOIN with zero tables.
We will not execute the derived tables but we need their query plans
for [SHOW]EXPLAIN.
grooverdan pushed a commit that referenced this pull request Feb 22, 2022
ROR-index_merge relies on Rowid-ordered-retrieval property: a ROR scan,
e.g. a scan on equality range

  tbl.key=const

should return rows ordered by their Rowid. Also, handler->cmp_ref() should
compare rowids according to the Rowid ordering.

When the table's primary key uses DESC keyparts, ROR scans return rows
according to the PK's ordering.

But ha_innobase::cmp_ref() compared rowids as if PK used ASC keyparts.
This caused wrong query results with index_merge.

Fixed this by making ha_innobase::cmp_ref() compare according to the PK
defintion, including keypart's DESC property.
grooverdan pushed a commit that referenced this pull request Feb 22, 2022
…and UNION

(Edits by SergeiP: fix encryption.tempfiles_encrypted, re-word comment)

Global ORDER BY clause of a UNION may not refer to 1) aggregate functions
or 2) window functions.  setup_order() checked for #1 but not for #2.
grooverdan pushed a commit that referenced this pull request Jun 8, 2022
In SELECT_LEX::update_used_tables(),
do not run the loop setting tl->table->maybe_null
when tl is an eliminated table

(Rationale: First, with current table elimination, tl already
 has maybe_null=1. Second, one should not care what flags
 eliminated tables had)
grooverdan pushed a commit that referenced this pull request Jul 1, 2022
Histogram_json_hb::range_selectivity() may return small negative
numbers due to rounding errors in the histogram.

Make sure the returned value is non-negative.
Add an assert to catch negative values that are not small.

(attempt #2)
grooverdan pushed a commit that referenced this pull request Aug 9, 2022
Part #2: Extend heuristic pruning to use multiple tables as the
"Model tables".

Before the patch, heuristic pruning uses only one "Model table":
The table which had the best cost AND record became the "Model table".
After that, if a table's cost and record were both worse than
those of the Model Table, the table would be pruned away.

This didn't work well when the first table (the optimizer sorts them
by record_count) had low record_count but relatively high cost: nothing
could be pruned afterwards.

The patch adds the two additional "Model tables": one with the least
cost and the other with the least record_count.
(In both cases, a table can be pruned away if BOTH its cost and
record_count are worse than those of a Model table)

The new pruning is active when the number of tables to consider for
the prefix is higher than @@optimizer_extra_pruning_depth.

One can see the new pruning in the Optimizer Trace as
- "pruned_by_heuristic":"min_record_count", or
- "pruned_by_heuristic":"min_read_time".
Old heuristic pruning shows as "pruned_by_heuristic":1.
grooverdan pushed a commit that referenced this pull request Aug 9, 2022
Part #2: make sure we allocate space for two JOIN_TABs that
use temporary tables.
The dbug_join_tab_array_size is still set to catch cases where
we try to access more JOIN_TAB object than we thought we would have.
grooverdan pushed a commit that referenced this pull request Mar 2, 2023
Part #2: fix the case where table->stat_records()=1 (due to EITS
statistics), but the range returns rows=0.
grooverdan pushed a commit that referenced this pull request May 14, 2023
…ogram

Variant #2.

When Histogram::point_selectivity() sees that the point value of interest
falls into one bucket, it tries to guess whether the bucket has many
different (unpopular) values or a few popular values. (The number of
rows is fixed, as it's a Height-balanced histogram).
The basis for this guess is the "width" of the value range the bucket
covers. Buckets covering wider value ranges are assumed to contain
values with proportionally lower frequencies.

This is just a [brave] guesswork. For a very narrow bucket, it may
produce an estimate that's larger than total #rows in the bucket
or even in the whole table.

Remove the guesswork and replace it with basic logic: return
either the per-table average selectivity of col=const, or selectivity
of one bucket, whichever is lower.
grooverdan pushed a commit that referenced this pull request May 28, 2024
(Based on original patch by Oleksandr Byelkin)

Multi-table DELETE can execute via "buffered" mode: at phase #1 it collects
rowids of rows to be deleted, then at phase #2 in multi_delete::do_deletes()
it calls handler->rnd_pos() to read rows to be deleted and deletes them.

The problem occurred when phase #1 used Rowid Filter on the table that
phase #2 would be deleting from.
In InnoDB, h->rnd_init(scan=false) and h->rnd_pos() is an index scan over PK
under the hood. So, at phase #2 ha_innobase::rnd_init() would try to use the
Rowid Filter and hit an assertion inside ha_innobase::rnd_init().

Note that multi-table UPDATE works similarly but was not affected, because
patch for MDEV-7487 added code to disable rowid filter for phase #2 in
multi_update::do_updates().

This patch changes the approach:
- It makes InnoDB not use Rowid Filter in rnd_pos() scans: it is disabled in
  ha_innobase::rnd_init() and enabled back in ha_innobase::rnd_end().
- multi_update::do_updates() no longer disables Rowid Filter for phase#2 as
  it is no longer necessary.
grooverdan pushed a commit that referenced this pull request Jul 4, 2024
Fixing applying the COLLATE clause to a parameter caused an error error:
  COLLATION '...' is not valid for CHARACTER SET 'binary'

Fix:

- Changing the collation derivation for a non-prepared Item_param
  to DERIVATION_IGNORABLE.

- Allowing to apply any COLLATE clause to expressions with DERIVATION_IGNORABLE.
  This includes:
    1. A non-prepared Item_param
    2. An explicit NULL
    3. Expressions derived from #1 and #2

  For example:
    SELECT ? COLLATE utf8mb_unicode_ci;
    SELECT NULL COLLATE utf8mb_unicode_ci;
    SELECT CONCAT(?) COLLATE utf8mb_unicode_ci;
    SELECT CONCAT(NULL) COLLATE utf8mb_unicode_ci

- Additional change: preserving the collation of an expression when
  the expression gets assigned to a PS parameter and evaluates to SQL NULL.
  Before this change, the collation of the parameter was erroneously set
  to &my_charset_binary.

- Additional change: removing the multiplication to mbmaxlen from the
  fix_char_length_ulonglong() argument, because the multiplication already
  happens inside fix_char_length_ulonglong().
  This fixes a too large column size created for a COLLATE clause.
grooverdan pushed a commit that referenced this pull request Sep 17, 2024
Changing the return type of the following functions:
  - CURRENT_TIMESTAMP, CURRENT_TIMESTAMP(), NOW()
  - SYSDATE()
  - FROM_UNIXTIME()
from DATETIME to TIMESTAMP.

Note, the old function NOW() returning DATETIME is still available
as LOCALTIMESTAMP or LOCALTIMESTAMP(), e.g.:

  SELECT
    LOCALTIMESTAMP,     -- DATETIME
    CURRENT_TIMESTAMP;  -- TIMESTAMP

The change in the functions return data type fixes some problems
that occurred near a DST change:

- Problem #1

INSERT INTO t1 (timestamp_field) VALUES (CURRENT_TIMESTAMP);
INSERT INTO t1 (timestamp_field) VALUES (COALESCE(CURRENT_TIMESTAMP));

could result into two different values inserted.

- Problem #2

INSERT INTO t1 (timestamp_field) VALUES (FROM_UNIXTIME(1288477526));
INSERT INTO t1 (timestamp_field) VALUES (FROM_UNIXTIME(1288477526+3600));

could result into two equal TIMESTAMP values near a DST change.

Additional changes:

- FROM_UNIXTIME(0) now returns SQL NULL instead of '1970-01-01 00:00:00'
  (assuming time_zone='+00:00')

- UNIX_TIMESTAMP('1970-01-01 00:00:00') now returns SQL NULL instead of 0
  (assuming time_zone='+00:00'

These additional changes are needed for consistency with TIMESTAMP fields,
which cannot store '1970-01-01 00:00:00 +00:00'.
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.

2 participants