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.

Sign up for GitHub

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

Jump to bottom

MDEV-28452 wsrep_ready: OFF after MDL BF-BF conflict #426

Open
wants to merge 6 commits into
base: 10.6
Choose a base branch
from
Open

MDEV-28452 wsrep_ready: OFF after MDL BF-BF conflict #426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3aa83f7
MDEV-28452 wsrep_ready: OFF after MDL BF-BF conflict.
plampio May 6, 2024
d456e13
Code cleanup.
plampio May 7, 2024
4e785bd
Fixes for issues raised by the reviewers.
plampio Jun 3, 2024
827d90d
Improved MTR test MDEV-28452 by removing a sleep.
plampio Jun 10, 2024
d20fcad
Improved synchronization in the MTR test MDEV-28452.
plampio Jun 17, 2024
a9adda4
Improve Sql_cmd_optimize_table::execute().
plampio Jun 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions mysql-test/suite/galera/r/MDEV-28452.result
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
connection node_2;
connection node_1;
connect node_2a, 127.0.0.1, root, , test, $NODE_MYPORT_2;
connection node_1;
CREATE TABLE `user` (
`id` char(36) COLLATE utf8mb4_unicode_ci NOT NULL,
`environment_id` char(36) COLLATE utf8mb4_unicode_ci NOT NULL,
`username` varchar(64) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL,
`password` char(60) COLLATE utf8mb4_unicode_ci NOT NULL,
`status` enum('sign_up','invited','active','archived')
COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT 'invited',
`last_login` datetime DEFAULT NULL,
PRIMARY KEY (`id`),
UNIQUE KEY `user_username_unique` (`username`),
KEY `user_environment_id_index` (`environment_id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
CREATE TABLE `log_history_daily` (
`type` enum('unknown','user','developer','api_key','api_env',
'mail_token','device') COLLATE utf8mb4_unicode_ci NOT NULL,
`status` enum('valid','invalid') COLLATE utf8mb4_unicode_ci NOT NULL,
`origin` enum('unknown','browser','go','android','ios','third_party')
COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT 'unknown',
`ip` varchar(45) COLLATE utf8mb4_unicode_ci NOT NULL,
`value` varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL,
`user_id` char(36) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
`date` date NOT NULL,
PRIMARY KEY (`type`,`status`,`origin`,`ip`,`value`,`date`),
KEY `log_history_daily_user_id_foreign` (`user_id`),
CONSTRAINT `log_history_daily_user_id_foreign` FOREIGN KEY (`user_id`)
REFERENCES `user` (`id`) ON DELETE SET NULL ON UPDATE CASCADE
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
SET SESSION autocommit = 0;
INSERT INTO `user` VALUES ("id_1", "abc", "user 1", "secret", "active", NULL);
INSERT INTO `log_history_daily` VALUES ("user", "valid", "unknown",
"192.168.0.1", "Sample", "id_1", "2024-04-22");
COMMIT;
connection node_2;
SET GLOBAL wsrep_slave_threads = 2;
SELECT @@wsrep_slave_threads;
@@wsrep_slave_threads
2
SET GLOBAL debug_dbug = "+d,sync.mdev_28452";
connection node_1;
OPTIMIZE TABLE `log_history_daily`;
Table Op Msg_type Msg_text
test.log_history_daily optimize note Table does not support optimize, doing recreate + analyze instead
test.log_history_daily optimize status OK
COMMIT;
UPDATE `user` SET `last_login` = '2024-04-23' WHERE `id`="id_1";
COMMIT;
connection node_2a;
SET DEBUG_SYNC='NOW WAIT_FOR mdev_28452_reached';
SET GLOBAL debug_dbug = 'RESET';
SET DEBUG_SYNC = 'now SIGNAL signal.mdev_28452';
SET DEBUG_SYNC = 'RESET';
connection node_1;
DROP TABLE `log_history_daily`;
DROP TABLE `user`;
connection node_2;
SET GLOBAL wsrep_slave_threads = DEFAULT;
93 changes: 93 additions & 0 deletions mysql-test/suite/galera/t/MDEV-28452.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
#
# Test for MDEV-28452: wsrep_ready: OFF after MDL BF-BF conflict
#

--source include/galera_cluster.inc
--source include/have_debug.inc
--source include/have_debug_sync.inc

--connect node_2a, 127.0.0.1, root, , test, $NODE_MYPORT_2

--connection node_1

# create two tables
CREATE TABLE `user` (
`id` char(36) COLLATE utf8mb4_unicode_ci NOT NULL,
Copy link

Choose a reason for hiding this comment

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

Is these table column names directly from customer and do you really need all columns?

Copy link
Author

Choose a reason for hiding this comment

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

These column names are indeed copied exactly from the customer case.
I have now removed all those columns that are not part of the keys and, thus, do not affect the test in any way.
But it would be possible to create a minimal test case with table names t1 and t2 and with just two columns per table.

`environment_id` char(36) COLLATE utf8mb4_unicode_ci NOT NULL,
`username` varchar(64) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL,
`password` char(60) COLLATE utf8mb4_unicode_ci NOT NULL,
`status` enum('sign_up','invited','active','archived')
COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT 'invited',
`last_login` datetime DEFAULT NULL,
PRIMARY KEY (`id`),
UNIQUE KEY `user_username_unique` (`username`),
KEY `user_environment_id_index` (`environment_id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;


CREATE TABLE `log_history_daily` (
`type` enum('unknown','user','developer','api_key','api_env',
'mail_token','device') COLLATE utf8mb4_unicode_ci NOT NULL,
`status` enum('valid','invalid') COLLATE utf8mb4_unicode_ci NOT NULL,
`origin` enum('unknown','browser','go','android','ios','third_party')
COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT 'unknown',
`ip` varchar(45) COLLATE utf8mb4_unicode_ci NOT NULL,
`value` varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL,
`user_id` char(36) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
`date` date NOT NULL,
PRIMARY KEY (`type`,`status`,`origin`,`ip`,`value`,`date`),
KEY `log_history_daily_user_id_foreign` (`user_id`),
CONSTRAINT `log_history_daily_user_id_foreign` FOREIGN KEY (`user_id`)
REFERENCES `user` (`id`) ON DELETE SET NULL ON UPDATE CASCADE
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;


SET SESSION autocommit = 0;

# insert one row in each table
INSERT INTO `user` VALUES ("id_1", "abc", "user 1", "secret", "active", NULL);
INSERT INTO `log_history_daily` VALUES ("user", "valid", "unknown",
"192.168.0.1", "Sample", "id_1", "2024-04-22");
COMMIT;

# Start two slave threads in node 2.
# Set up a synchronizaton point (sync.mdev_28452) for slave thread 1
# where the thread stops when it holds an exclusive metadata lock
# (MDL_EXCLUSIVE) for the OPTIMIZE TABLE statement (see below).
--connection node_2
SET GLOBAL wsrep_slave_threads = 2;
SELECT @@wsrep_slave_threads;
SET GLOBAL debug_dbug = "+d,sync.mdev_28452";

# Run two transactions sequentially on node 1:
# 1) OPTIMIZE TABLE on the child table of the foreign key constraint,
--connection node_1
OPTIMIZE TABLE `log_history_daily`;
COMMIT;

# 2) and UPDATE on the parent table of the foreign key constraint.
# update the row in "user" so that it affects the foreign key in
# "log_history_daily"
UPDATE `user` SET `last_login` = '2024-04-23' WHERE `id`="id_1";
COMMIT;

# allow the stopped OPTIMIZE TABLE transaction to proceed
--connection node_2a

# wait till debug sync point has been set on node 2
SET DEBUG_SYNC='NOW WAIT_FOR mdev_28452_reached';

SET GLOBAL debug_dbug = 'RESET';
SET DEBUG_SYNC = 'now SIGNAL signal.mdev_28452';
SET DEBUG_SYNC = 'RESET';

# wait till the UPDATE has finished on node 2
--let $wait_condition = SELECT COUNT(*) > 0 FROM `user` WHERE `last_login` = '2024-04-23'
--source include/wait_condition.inc

# cleanup
--connection node_1
DROP TABLE `log_history_daily`;
DROP TABLE `user`;
--connection node_2
SET GLOBAL wsrep_slave_threads = DEFAULT;
15 changes: 15 additions & 0 deletions sql/mdl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2308,6 +2308,21 @@ MDL_context::acquire_lock(MDL_request *mdl_request, double lock_wait_timeout)
*/
DBUG_PRINT("info", ("Got lock without waiting"));
DBUG_PRINT("mdl", ("Seized: %s", dbug_print_mdl(mdl_request->ticket)));
#ifdef WITH_WSREP
#ifdef ENABLED_DEBUG_SYNC
if (0 == strcmp(dbug_print_mdl(mdl_request->ticket),
"test/log_history_daily (MDL_EXCLUSIVE)")) {
DBUG_EXECUTE_IF("sync.mdev_28452",
{
const char act[]=
"NOW SIGNAL mdev_28452_reached "
"WAIT_FOR signal.mdev_28452";
DBUG_ASSERT(!debug_sync_set_action(get_thd(),
STRING_WITH_LEN(act)));
};);
}
#endif /* ENABLED_DEBUG_SYNC */
#endif /* WITH_WSREP! */
DBUG_RETURN(FALSE);
}

Expand Down
37 changes: 32 additions & 5 deletions sql/sql_admin.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1608,18 +1608,45 @@ bool Sql_cmd_optimize_table::execute(THD *thd)
FALSE, UINT_MAX, FALSE))
goto error; /* purecov: inspected */

WSREP_TO_ISOLATION_BEGIN_WRTCHK(NULL, NULL, first_table);
#ifdef WITH_WSREP
if (WSREP(thd) &&
(!thd->is_current_stmt_binlog_format_row() ||
!thd->find_temporary_table(first_table)))
{
/*
It makes sense to set auto_increment_* to defaults in TOI operations.
Must be done before wsrep_TOI_begin() since Query_log_event encapsulating
TOI statement and auto inc variables for wsrep replication is constructed
there. Variables are reset back in THD::reset_for_next_command() before
processing of next command.
*/
if (wsrep_auto_increment_control)
Copy link

Choose a reason for hiding this comment

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

I don't understand why we need to change auto_increment variables here.
What happens if you don't?

Copy link
Author

Choose a reason for hiding this comment

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

I tested also without changing auto_increment_* variables and the MTR tests succeeded.
I don't understand what these variable do, but the code segment was copied from
Sql_cmd_alter_table::execute() (in sql/sql_admin.cc) and this fix is very similar to that.

Copy link

Choose a reason for hiding this comment

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

Optimize table could do table re-create, but I do not know if that has any effect on autoinc values (could be verified using show create table x; in both nodes).

{
thd->variables.auto_increment_offset = 1;
thd->variables.auto_increment_increment = 1;
}

wsrep::key_array keys;
if (wsrep_thd_is_local(thd) &&
!wsrep_append_fk_parent_table(thd, first_table, &keys))
Copy link

Choose a reason for hiding this comment

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

If wsrep_append_fk_parent_table fails, then we skip TOI, and but still perform the OPTIMIZE locally.
This is likely harmless. Still, I would prefer to perform OPTIMIZE only if we did enter TOI mode.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it is certainly possible to change the code so that wsrep_append_fk_parent_table() failure causes Sql_cmd_optimize_table::execute() to fail.
But the current code is now analogous to the implementation of Sql_cmd_alter_table::execute().

{
WSREP_TO_ISOLATION_BEGIN_ALTER(first_table->db.str,
first_table->table_name.str,
first_table, NULL, &keys, NULL)
{
WSREP_WARN("OPTIMIZE TABLE isolation failure");
DBUG_RETURN(TRUE);
}
}
}
#endif /* WITH_WSREP */
res= (specialflag & SPECIAL_NO_NEW_FUNC) ?
mysql_recreate_table(thd, first_table, &recreate_info, true) :
mysql_admin_table(thd, first_table, &m_lex->check_opt,
&msg_optimize, TL_WRITE, 1, 0, 0, 0,
&handler::ha_optimize, 0, true);
m_lex->first_select_lex()->table_list.first= first_table;
m_lex->query_tables= first_table;

#ifdef WITH_WSREP
wsrep_error_label:
#endif /* WITH_WSREP */
error:
DBUG_RETURN(res);
}
Expand Down