Skip to content

Commit 9c08a08

Browse files
author
Joao Gramacho
committed
BUG#24398760 GTIDS REDUCE PERFORMANCE ON WORKLOADS WITH MANY SMALL TRANSACTIONS
The approach this patch is using is to do the commit stage update of the GTIDs in a single "batch" without releasing the sidno_lock. After the refactoring on the previous commit, this patch introduces the update_commit_group function that works similarly to update_on_commit and update_on_rollback functions, but doing the update on commit or rollback in a set of transactions that were grouped to commit together. This patch also introduced two new group commit versions of some functions that were introduced on the refactoring: - update_gtids_impl_lock_sidnos: Lock all the sidnos of all GTIDs to be updated in a given commit group; - update_gtids_impl_broadcast_and_unlock_sidnos: Unlock all previously locked sidnos after broadcasting their changes;
1 parent c6f31d5 commit 9c08a08

10 files changed

+394
-25
lines changed

mysql-test/include/execute_at_sync_point.inc

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
# [--let $auxiliary_connection= CONNECTION_NAME]
1616
# [--let $quiet= [0|1|2]]
1717
# [--let $sync_point_timeout= N]
18-
# --source include/execute_statement_at_sync_point.inc
18+
# --source include/execute_at_sync_point.inc
1919
#
2020
# Parameters:
2121
#

mysql-test/include/execute_from_sync_point.inc

+14-6
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,16 @@
1010
# --let $statement_connection= CONNECTION_NAME
1111
# --let $statement= STATEMENT
1212
# --let $sync_point= SYNC_POINT_NAME
13+
# [--let $auxiliary_connection= CONNECTION_NAME]
1314
# [--let $quiet= [0|2]]
14-
# --source include/execute_statement_at_sync_point.inc
15+
# --source include/execute_from_sync_point.inc
1516
#
1617
# Parameters:
1718
# See execute_at_sync_point.inc
1819

1920

2021
--let $include_silent= 1
21-
--let $include_filename= execute_at_sync_point.inc
22+
--let $include_filename= execute_from_sync_point.inc
2223
--source include/begin_include_file.inc
2324
--let $include_silent= 0
2425

@@ -49,22 +50,29 @@ if (!$rpl_debug)
4950
{
5051
--disable_query_log
5152
}
53+
if ($rpl_debug)
54+
{
55+
--echo statement_connection=$statement_connection auxiliary_connection=$auxiliary_connection statement=$statement sync_point=$sync_point
56+
}
5257

5358
# Tell statement to continue on auxiliary connection.
54-
--connection $_esp_auxiliary_connection
59+
--let $rpl_connection_name= $_esp_auxiliary_connection
60+
--source include/rpl_connection.inc
5561
eval SET @@SESSION.DEBUG_SYNC = 'now SIGNAL _esp_go_$sync_point$underscore$statement_connection';
5662

5763
# Wait for statement to finish.
64+
--let $rpl_connection_name= $statement_connection
65+
--source include/rpl_connection.inc
5866
if ($_esp_quiet != 2)
5967
{
6068
--echo [END] $statement;
6169
}
62-
--connection $statement_connection
6370
if (!$skip_reap)
6471
{
6572
reap;
6673
}
67-
--connection $_esp_auxiliary_connection
74+
--let $rpl_connection_name= $_esp_auxiliary_connection
75+
--source include/rpl_connection.inc
6876

69-
--let $include_filename= execute_at_sync_point.inc
77+
--let $include_filename= execute_from_sync_point.inc
7078
--source include/end_include_file.inc

mysql-test/include/execute_to_sync_point.inc

+10-5
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,16 @@
99
# --let $statement_connection= CONNECTION_NAME
1010
# --let $statement= STATEMENT
1111
# --let $sync_point= SYNC_POINT_NAME
12+
# [--let $auxiliary_connection= CONNECTION_NAME]
1213
# [--let $quiet= [0|2]]
1314
# [--let $sync_point_timeout= N]
14-
# --source include/execute_statement_at_sync_point.inc
15+
# --source include/execute_to_sync_point.inc
1516
#
1617
# Parameters:
1718
# See execute_at_sync_point.inc.
1819

1920
--let $include_silent= 1
20-
--let $include_filename= execute_at_sync_point.inc
21+
--let $include_filename= execute_to_sync_point.inc
2122
--source include/begin_include_file.inc
2223
--let $include_silent= 0
2324

@@ -58,7 +59,9 @@ if ($sync_point_timeout)
5859
}
5960

6061
# On $statement_connection, setup sync point and begin execute statement.
61-
--connection $statement_connection
62+
--let $rpl_connection_name= $statement_connection
63+
--source include/rpl_connection.inc
64+
6265
eval SET @@SESSION.DEBUG_SYNC = '$sync_point SIGNAL _esp_parked_$sync_point$underscore$statement_connection WAIT_FOR _esp_go_$sync_point$underscore$statement_connection $_esp_timeout';
6366

6467
if ($quiet != 2)
@@ -74,9 +77,11 @@ if ($rpl_debug)
7477
{
7578
--echo waiting for debug sync point _esp_parked_$sync_point$underscore$statement_connection $_esp_timeout
7679
}
77-
--connection $_esp_auxiliary_connection
80+
--let $rpl_connection_name= $_esp_auxiliary_connection
81+
--source include/rpl_connection.inc
82+
7883
eval SET @@SESSION.DEBUG_SYNC = 'now WAIT_FOR _esp_parked_$sync_point$underscore$statement_connection $_esp_timeout';
7984

8085
--let $skip_restore_connection= 1
81-
--let $include_filename= execute_at_sync_point.inc
86+
--let $include_filename= execute_to_sync_point.inc
8287
--source include/end_include_file.inc
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
include/master-slave.inc
2+
Warnings:
3+
Note #### Sending passwords in plain text without SSL/TLS is extremely insecure.
4+
Note #### Storing MySQL user name or password information in the master info repository is not secure and is therefore not recommended. Please consider using the USER and PASSWORD connection options for START SLAVE; see the 'START SLAVE Syntax' in the MySQL Manual for more information.
5+
[connection master]
6+
CREATE TABLE t1 (a INT);
7+
TRUNCATE performance_schema.events_waits_summary_by_instance;
8+
[START] INSERT INTO t1 VALUES (1);
9+
[START] INSERT INTO t1 VALUES (2);
10+
[START] INSERT INTO t1 VALUES (3);
11+
[START] INSERT INTO t1 VALUES (4);
12+
[START] INSERT INTO t1 VALUES (5);
13+
[START] INSERT INTO t1 VALUES (6);
14+
[START] INSERT INTO t1 VALUES (7);
15+
[START] INSERT INTO t1 VALUES (8);
16+
[START] INSERT INTO t1 VALUES (9);
17+
[START] INSERT INTO t1 VALUES (10);
18+
[END] INSERT INTO t1 VALUES (1);
19+
[END] INSERT INTO t1 VALUES (2);
20+
[END] INSERT INTO t1 VALUES (3);
21+
[END] INSERT INTO t1 VALUES (4);
22+
[END] INSERT INTO t1 VALUES (5);
23+
[END] INSERT INTO t1 VALUES (6);
24+
[END] INSERT INTO t1 VALUES (7);
25+
[END] INSERT INTO t1 VALUES (8);
26+
[END] INSERT INTO t1 VALUES (9);
27+
[END] INSERT INTO t1 VALUES (10);
28+
[connection server_1_1]
29+
[connection server_1_2]
30+
[connection server_1_3]
31+
[connection server_1_4]
32+
[connection server_1_5]
33+
[connection server_1_6]
34+
[connection server_1_7]
35+
[connection server_1_8]
36+
[connection server_1_9]
37+
[connection server_1_10]
38+
include/assert.inc [The group commit should acquire sidno_lock for 11 times]
39+
DROP TABLE t1;
40+
include/rpl_end.inc
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
# ==== Purpose ====
2+
#
3+
# This test will make many transactions to be grouped when binary logged
4+
# and will assert that they used the lock_sidno less times than the amount
5+
# of transactions * 2.
6+
#
7+
# Previous to BUG#24398760 fix, every transaction being committed would
8+
# take the lock_sidno twice inside ordered_commit function:
9+
# a) At the flush stage, to be assigned with a new GTID and take the ownership
10+
# of the GTID;
11+
# b) At the commit stage, to release the ownership of the GTID and to add the
12+
# gtid to the GTID_EXECUTED set.
13+
#
14+
# So, a group committing 10 transactions would take the sidno_lock 20 times.
15+
#
16+
# According to the bug report, having about 300 threads committing small
17+
# transactions would lead to groups of up to 90 transactions on high performance
18+
# environments. This would mean that the system would have about 180 threads
19+
# trying to lock the sidno_lock (90 at the flush stage, 90 at the commit stage).
20+
#
21+
# Even knowing that the threads would acquire the lock one by one at each stage,
22+
# this might generate unnecessary overhead when the system is loaded.
23+
#
24+
# As the bug fix makes a group of transactions to acquire the sidno_lock only
25+
# once at the commit stage, this test case will create a group of 10
26+
# transactions to commit and will observe the performance schema data about the
27+
# sidno_lock, asserting that the amount of "hits" on COUNT_STAR is 11
28+
# (1 per transaction on FLUSH stage + 1 for the whole group on COMMIT stage).
29+
#
30+
# ==== Related Bugs and Worklogs ====
31+
#
32+
# BUG#24398760 GTIDS REDUCE PERFORMANCE ON WORKLOADS WITH MANY SMALL
33+
# TRANSACTIONS
34+
#
35+
36+
# This test case is binary log format agnostic
37+
--source include/have_binlog_format_mixed.inc
38+
--source include/have_binlog_order_commits.test
39+
--source include/have_perfschema.inc
40+
--source include/have_debug_sync.inc
41+
--source include/have_gtid.inc
42+
# The amount of transactions that will be part of the commit group
43+
--let $group_size= 10
44+
--let $rpl_extra_connections_per_server= $group_size
45+
--source include/master-slave.inc
46+
47+
--let $sync_point_timeout= 100
48+
49+
CREATE TABLE t1 (a INT);
50+
51+
# Cleanup performance schema table about locks
52+
TRUNCATE performance_schema.events_waits_summary_by_instance;
53+
54+
# Execute the transactions up to the FLUSH stage
55+
--let $i= 1
56+
while ($i <= $group_size)
57+
{
58+
--let $statement_connection= server_1_$i
59+
--let $sync_point= bgc_after_enrolling_for_flush_stage
60+
--let $statement= INSERT INTO t1 VALUES ($i)
61+
--let $auxiliary_connection= master
62+
--source include/execute_to_sync_point.inc
63+
64+
--inc $i
65+
}
66+
67+
# Let the transactions to execute from the FLUSH stage
68+
--let $skip_reap= 1
69+
70+
--let $i= 1
71+
while ($i <= $group_size)
72+
{
73+
--let $statement_connection= server_1_$i
74+
--let $sync_point= bgc_after_enrolling_for_flush_stage
75+
--let $statement= INSERT INTO t1 VALUES ($i)
76+
--let $auxiliary_connection= master
77+
--source include/execute_from_sync_point.inc
78+
79+
--inc $i
80+
}
81+
82+
# Reap all the transactions
83+
--let $i= 1
84+
while ($i <= $group_size)
85+
{
86+
--let $rpl_connection_name= server_1_$i
87+
--source include/rpl_connection.inc
88+
--reap
89+
90+
--inc $i
91+
}
92+
93+
# There will be one "hit" per transaction on FLUSH stage
94+
--let $sidno_lock_count_star= $group_size
95+
# There will be one "hit" per commit group on COMMIT stage
96+
--inc $sidno_lock_count_star
97+
98+
--let $assert_text= The group commit should acquire sidno_lock for $sidno_lock_count_star times
99+
--let $assert_cond= [ SELECT COUNT_STAR = $sidno_lock_count_star FROM performance_schema.events_waits_summary_by_instance WHERE EVENT_NAME = "wait/synch/mutex/sql/Gtid_state" AND COUNT_STAR > 0 ]
100+
--source include/assert.inc
101+
102+
# Cleanup
103+
DROP TABLE t1;
104+
--source include/rpl_end.inc

sql/binlog.cc

+31-11
Original file line numberDiff line numberDiff line change
@@ -1868,8 +1868,28 @@ Stage_manager::enroll_for(StageID stage, THD *thd, mysql_mutex_t *stage_mutex)
18681868
mysql_mutex_unlock(stage_mutex);
18691869

18701870
#ifndef DBUG_OFF
1871-
if (stage == Stage_manager::SYNC_STAGE)
1871+
DBUG_PRINT("info", ("This is a leader thread: %d (0=n 1=y)", leader));
1872+
1873+
DEBUG_SYNC(thd, "after_enrolling_for_stage");
1874+
1875+
switch (stage)
1876+
{
1877+
case Stage_manager::FLUSH_STAGE:
1878+
DEBUG_SYNC(thd, "bgc_after_enrolling_for_flush_stage");
1879+
break;
1880+
case Stage_manager::SYNC_STAGE:
18721881
DEBUG_SYNC(thd, "bgc_after_enrolling_for_sync_stage");
1882+
break;
1883+
case Stage_manager::COMMIT_STAGE:
1884+
DEBUG_SYNC(thd, "bgc_after_enrolling_for_commit_stage");
1885+
break;
1886+
default:
1887+
// not reached
1888+
DBUG_ASSERT(0);
1889+
}
1890+
1891+
DBUG_EXECUTE_IF("assert_leader", DBUG_ASSERT(leader););
1892+
DBUG_EXECUTE_IF("assert_follower", DBUG_ASSERT(!leader););
18731893
#endif
18741894

18751895
/*
@@ -1911,7 +1931,7 @@ THD *Stage_manager::Mutex_queue::fetch_and_empty()
19111931
DBUG_PRINT("info", ("m_first: 0x%llx, &m_first: 0x%llx, m_last: 0x%llx",
19121932
(ulonglong) m_first, (ulonglong) &m_first,
19131933
(ulonglong) m_last));
1914-
DBUG_ASSERT(m_first || m_last == &m_first);
1934+
DBUG_PRINT("info", ("fetched queue of %d transactions", my_atomic_load32(&m_size)));
19151935
DBUG_PRINT("return", ("result: 0x%llx", (ulonglong) result));
19161936
DBUG_ASSERT(my_atomic_load32(&m_size) >= 0);
19171937
my_atomic_store32(&m_size, 0);
@@ -8411,17 +8431,17 @@ MYSQL_BIN_LOG::process_commit_stage_queue(THD *thd, THD *first)
84118431
DBUG_PRINT("debug", ("commit_error: %d, flags.pending: %s",
84128432
head->commit_error,
84138433
YESNO(head->get_transaction()->m_flags.pending)));
8434+
}
84148435

8415-
/*
8416-
Handle the GTID of the thread.
8417-
gtid_executed table is kept updated even though transactions fail to be
8418-
logged. That's required by slave auto positioning.
8419-
*/
8420-
if (head->commit_error != THD::CE_COMMIT_ERROR)
8421-
gtid_state->update_on_commit(head);
8422-
else
8423-
gtid_state->update_on_rollback(head);
8436+
/*
8437+
Handle the GTID of the threads.
8438+
gtid_executed table is kept updated even though transactions fail to be
8439+
logged. That's required by slave auto positioning.
8440+
*/
8441+
gtid_state->update_commit_group(first);
84248442

8443+
for (THD *head= first ; head ; head = head->next_to_commit)
8444+
{
84258445
/*
84268446
Decrement the prepared XID counter after storage engine commit.
84278447
We also need decrement the prepared XID when encountering a

sql/binlog.h

+5-1
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,11 @@ class Stage_manager {
108108
return m_first == NULL;
109109
}
110110

111-
/** Append a linked list of threads to the queue */
111+
/**
112+
Append a linked list of threads to the queue.
113+
@retval true The queue was empty before this operation.
114+
@retval false The queue was non-empty before this operation.
115+
*/
112116
bool append(THD *first);
113117

114118
/**

sql/mysqld.cc

+1
Original file line numberDiff line numberDiff line change
@@ -9177,6 +9177,7 @@ static PSI_memory_info all_server_memory[]=
91779177
{ &key_memory_Gtid_set_Interval_chunk, "Gtid_set::Interval_chunk", 0},
91789178
{ &key_memory_Owned_gtids_sidno_to_hash, "Owned_gtids::sidno_to_hash", 0},
91799179
{ &key_memory_Sid_map_Node, "Sid_map::Node", 0},
9180+
{ &key_memory_Gtid_state_group_commit_sidno, "Gtid_state::group_commit_sidno_locks", 0},
91809181
{ &key_memory_Mutex_cond_array_Mutex_cond, "Mutex_cond_array::Mutex_cond", 0},
91819182
{ &key_memory_TABLE_RULE_ENT, "TABLE_RULE_ENT", 0},
91829183

0 commit comments

Comments
 (0)