Commit bd2ae787 authored by Kristian Nielsen's avatar Kristian Nielsen

MDEV-7825: Parallel replication race condition on gco->flags, possibly resulting in slave hang

The patch for optimistic parallel replication as a memory optimisation moved
the gco->installed field into a bit in gco->flags. However, that is just plain
wrong. The gco->flags field is owned by the SQL driver thread, but
gco->installed is used by the worker threads, so this will cause a race
condition.

The user-visible problem might be conflicts between transactions and/or slave
threads hanging.

So revert this part of the optimistic parallel replication patch, going back
to using a separate field gco->installed like in 10.0.
parent ec68494b
...@@ -744,7 +744,7 @@ handle_rpl_parallel_thread(void *arg) ...@@ -744,7 +744,7 @@ handle_rpl_parallel_thread(void *arg)
in parallel with. in parallel with.
*/ */
mysql_mutex_lock(&entry->LOCK_parallel_entry); mysql_mutex_lock(&entry->LOCK_parallel_entry);
if (!(gco->flags & group_commit_orderer::INSTALLED)) if (!gco->installed)
{ {
group_commit_orderer *prev_gco= gco->prev_gco; group_commit_orderer *prev_gco= gco->prev_gco;
if (prev_gco) if (prev_gco)
...@@ -752,7 +752,7 @@ handle_rpl_parallel_thread(void *arg) ...@@ -752,7 +752,7 @@ handle_rpl_parallel_thread(void *arg)
prev_gco->last_sub_id= gco->prior_sub_id; prev_gco->last_sub_id= gco->prior_sub_id;
prev_gco->next_gco= gco; prev_gco->next_gco= gco;
} }
gco->flags|= group_commit_orderer::INSTALLED; gco->installed= true;
} }
wait_count= gco->wait_count; wait_count= gco->wait_count;
if (wait_count > entry->count_committing_event_groups) if (wait_count > entry->count_committing_event_groups)
...@@ -1401,6 +1401,7 @@ rpl_parallel_thread::get_gco(uint64 wait_count, group_commit_orderer *prev, ...@@ -1401,6 +1401,7 @@ rpl_parallel_thread::get_gco(uint64 wait_count, group_commit_orderer *prev,
gco->prev_gco= prev; gco->prev_gco= prev;
gco->next_gco= NULL; gco->next_gco= NULL;
gco->prior_sub_id= prior_sub_id; gco->prior_sub_id= prior_sub_id;
gco->installed= false;
gco->flags= 0; gco->flags= 0;
return gco; return gco;
} }
......
...@@ -66,7 +66,8 @@ struct group_commit_orderer { ...@@ -66,7 +66,8 @@ struct group_commit_orderer {
This flag is set when this GCO has been installed into the next_gco pointer This flag is set when this GCO has been installed into the next_gco pointer
of the previous GCO. of the previous GCO.
*/ */
static const uint8 INSTALLED = 0x01; bool installed;
/* /*
This flag is set for a GCO in which we have event groups with multiple This flag is set for a GCO in which we have event groups with multiple
different commit_id values from the master. This happens when we different commit_id values from the master. This happens when we
...@@ -77,13 +78,13 @@ struct group_commit_orderer { ...@@ -77,13 +78,13 @@ struct group_commit_orderer {
of current commit_id, as DDL is not safe to speculatively apply in parallel of current commit_id, as DDL is not safe to speculatively apply in parallel
with prior event groups. with prior event groups.
*/ */
static const uint8 MULTI_BATCH = 0x02; static const uint8 MULTI_BATCH = 0x01;
/* /*
This flag is set for a GCO that contains DDL. If set, it forces a switch to This flag is set for a GCO that contains DDL. If set, it forces a switch to
a new GCO upon seeing a new commit_id, as DDL is not safe to speculatively a new GCO upon seeing a new commit_id, as DDL is not safe to speculatively
replicate in parallel with subsequent transactions. replicate in parallel with subsequent transactions.
*/ */
static const uint8 FORCE_SWITCH = 0x04; static const uint8 FORCE_SWITCH = 0x02;
uint8 flags; uint8 flags;
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment