BUG#25232042: ASSERT "!CONTAINS_GTID(GTID)" RPL_GTID_OWNED.CC:94 OWNED_GTIDS::ADD_GTID_OWNER

When a existing GTID_NEXT transaction gets a conflicting GTID generated by server, the Group Replication plugin will generate a assert due having two transactions with same gtid. On Group Replication the gtid is generated after conflict detection, later than normal replication. Some conditions were relaxed to allow only be called when commit is done and add message to alert when gtid is already used. The check for macro HAVE_REPLICATION was removed due to being now correctly defined on cmake.

Author: Anibal Pinto

rapid/plugin/group_replication/src/certifier.cc

@@ -776,6 +776,27 @@ rpl_gno Certifier::certify(Gtid_set *snapshot_version,
  }
  else
  {
+ /*
+ Check if it is an already used GTID
+ */
+ rpl_sidno sidno_for_group_gtid_sid_map= gle->get_sidno(group_gtid_sid_map);
+ if (sidno_for_group_gtid_sid_map < 1)
+ {
+ log_message(MY_ERROR_LEVEL,
+ "Error fetching transaction sidno after transaction"
+ " being positively certified"); /* purecov: inspected */
+ goto end; /* purecov: inspected */
+ }
+ if (group_gtid_executed->contains_gtid(sidno_for_group_gtid_sid_map, gle->get_gno()))
+ {
+ char buf[rpl_sid::TEXT_LENGTH + 1];
+ gle->get_sid()->to_string(buf);
+
+ log_message(MY_ERROR_LEVEL,
+ "The requested GTID '%s:%lld' was already used, the transaction will rollback"
+ , buf, gle->get_gno());
+ goto end;
+ }
  /*
  Add received transaction GTID to transaction snapshot version.
  */
@@ -787,6 +808,7 @@ rpl_gno Certifier::certify(Gtid_set *snapshot_version,
  " being positively certified"); /* purecov: inspected */
  goto end; /* purecov: inspected */
  }
+
  if (snapshot_version->ensure_sidno(sidno) != RETURN_STATUS_OK)
  {
  log_message(MY_ERROR_LEVEL,

rapid/plugin/group_replication/tests/mtr/r/gr_gtid_next_conflict.result

@@ -0,0 +1,46 @@
+include/group_replication.inc
+Warnings:
+Note	####	Sending passwords in plain text without SSL/TLS is extremely insecure.
+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.
+[connection server1]
+[connection server2]
+set sql_log_bin=0;
+call mtr.add_suppression("The requested GTID .* was already used, the transaction will rollback");
+set sql_log_bin=1;
+
+#############################################################
+# 1. Establish another connection to server, to execute
+# transactions in parallel.
+[connection server1]
+set sql_log_bin=0;
+call mtr.add_suppression("The requested GTID .* was already used, the transaction will rollback");
+set sql_log_bin=1;
+CREATE TABLE t1(a INT PRIMARY KEY);
+include/rpl_sync.inc
+
+#############################################################
+# 2. Create a table and set a manual gtid_next equal to next
+# automatic gtid. Start a transaction.
+include/assert.inc [GTID_EXECUTED should contain three GTIDs.]
+set GTID_NEXT='c3b5927f-2a51-11e7-94b3-0010e0734796:4';
+BEGIN;
+INSERT INTO t1 VALUES (1);
+
+#############################################################
+# 3. Using the other connection execute a transaction.
+[connection server_1]
+INSERT INTO t1 VALUES (2);
+
+#############################################################
+# 4. Connect to first session and commit pending transaction.
+# Server will rollback it due to already used gtid.
+[connection server1]
+COMMIT;
+ERROR HY000: Plugin instructed the server to rollback the current transaction.
+set GTID_NEXT='automatic';
+include/assert_grep.inc [The GR plug-in reported an error stating that a transaction will rollback]
+
+#############################################################
+# 5. Clean up.
+DROP TABLE t1;
+include/group_replication_end.inc

rapid/plugin/group_replication/tests/mtr/t/gr_gtid_next_conflict-master.opt

@@ -0,0 +1 @@
+--log_error=$MYSQLTEST_VARDIR/tmp/gr_gtid_next_conflict.1.err

rapid/plugin/group_replication/tests/mtr/t/gr_gtid_next_conflict.test

@@ -0,0 +1,91 @@
+###############################################################################
+#
+# This test validate that if we have two sessions and one set gtid_next to the
+# next automatic gtid, if both session execute a transaction in parallel one
+# of the transactions will rollback.
+#
+# Test:
+# 0. Test requires two servers.
+# 1. Establish another connection to server, to execute transactions in
+# parallel.
+# 2. Create a table and set a manual gtid_next equal to next automatic gtid.
+# Start a transaction.
+# 3. Using the other connection execute a transaction.
+# 4. Connect to first session and commit pending transaction. Server will
+# rollback it due to already used gtid.
+# 5. Clean up.
+#
+###############################################################################
+
+--let $group_replication_group_name= c3b5927f-2a51-11e7-94b3-0010e0734796
+--source ../inc/have_group_replication_plugin.inc
+--source ../inc/group_replication.inc
+
+--let $rpl_connection_name= server2
+--source include/rpl_connection.inc
+set sql_log_bin=0;
+call mtr.add_suppression("The requested GTID .* was already used, the transaction will rollback");
+set sql_log_bin=1;
+
+--echo
+--echo #############################################################
+--echo # 1. Establish another connection to server, to execute
+--echo # transactions in parallel.
+
+--let $rpl_connection_name= server1
+--source include/rpl_connection.inc
+
+set sql_log_bin=0;
+call mtr.add_suppression("The requested GTID .* was already used, the transaction will rollback");
+set sql_log_bin=1;
+
+CREATE TABLE t1(a INT PRIMARY KEY);
+--source include/rpl_sync.inc
+
+--echo
+--echo #############################################################
+--echo # 2. Create a table and set a manual gtid_next equal to next
+--echo # automatic gtid. Start a transaction.
+
+--let $assert_cond= "[SELECT @@GLOBAL.GTID_EXECUTED]" = "c3b5927f-2a51-11e7-94b3-0010e0734796:1-3"
+--let $assert_text= GTID_EXECUTED should contain three GTIDs.
+--source include/assert.inc
+
+set GTID_NEXT='c3b5927f-2a51-11e7-94b3-0010e0734796:4';
+BEGIN;
+INSERT INTO t1 VALUES (1);
+
+--echo
+--echo #############################################################
+--echo # 3. Using the other connection execute a transaction.
+
+--let $rpl_connection_name= server_1
+--source include/rpl_connection.inc
+INSERT INTO t1 VALUES (2);
+
+--echo
+--echo #############################################################
+--echo # 4. Connect to first session and commit pending transaction.
+--echo # Server will rollback it due to already used gtid.
+
+--let $rpl_connection_name= server1
+--source include/rpl_connection.inc
+
+--let $wait_condition= SELECT @@GLOBAL.GTID_EXECUTED = 'c3b5927f-2a51-11e7-94b3-0010e0734796:1-4'
+--source include/wait_condition.inc
+
+--error ER_TRANSACTION_ROLLBACK_DURING_COMMIT
+COMMIT;
+set GTID_NEXT='automatic';
+
+--let $assert_file= $MYSQLTEST_VARDIR/tmp/gr_gtid_next_conflict.1.err
+--let $assert_count= 1
+--let $assert_select= .* was already used, the transaction will rollback
+--let $assert_text= The GR plug-in reported an error stating that a transaction will rollback
+--source include/assert_grep.inc
+
+--echo
+--echo #############################################################
+--echo # 5. Clean up.
+DROP TABLE t1;
+--source ../inc/group_replication_end.inc

sql/rpl_group_replication.cc

@@ -13,10 +13,6 @@
  along with this program; if not, write to the Free Software Foundation,
  51 Franklin Street, Suite 500, Boston, MA 02110-1335 USA */
 
-#ifndef HAVE_REPLICATION
-#define HAVE_REPLICATION
-#endif
-
 #include "rpl_group_replication.h"
 #include "rpl_channel_service_interface.h"
 #include "rpl_info_factory.h"
@@ -142,6 +138,7 @@ Group_replication_handler* group_replication_handler= NULL;
 /*
  Group Replication plugin handler function accessors.
 */
+#ifdef HAVE_REPLICATION
 int group_replication_init(const char* plugin_name)
 {
  if (initialize_channel_service_interface())
@@ -165,6 +162,7 @@ int group_replication_init(const char* plugin_name)
 
  return 1;
 }
+#endif
 
 int group_replication_cleanup()
 {
@@ -323,6 +321,7 @@ unsigned int get_group_replication_members_number_info()
  Server methods exported to plugin through
  include/mysql/group_replication_priv.h
 */
+#ifdef HAVE_REPLICATION
 void get_server_parameters(char **hostname, uint *port, char** uuid,
  unsigned int *out_server_version,
  st_server_ssl_variables* server_ssl_variables)
@@ -380,6 +379,7 @@ void get_server_parameters(char **hostname, uint *port, char** uuid,
 
  return;
 }
+#endif
 
 ulong get_server_id()
 {

sql/rpl_gtid.h

@@ -2026,8 +2026,11 @@ struct Gtid_set_or_null
  lock and then degrades it to a read lock again; there will be a
  short period when the lock is not held at all.
 
- The internal representation is a dynamic array that maps SIDNO to
- HASH, where each HASH maps GNO to my_thread_id.
+ The internal representation is a multi-valued map from GTIDs to
+ threads, mapping GTIDs to one or more threads that owns it.
+
+ In Group Replication multiple threads can own a GTID whereas if GR
+ is disabeld there is at most one owner per GTID.
 */
 class Owned_gtids
 {
@@ -2049,14 +2052,6 @@ class Owned_gtids
  @return RETURN_STATUS_OK or RETURN_STATUS_REPORTED_ERROR.
  */
  enum_return_status add_gtid_owner(const Gtid &gtid, my_thread_id owner);
- /**
- Returns the owner of the given GTID, or 0 if the GTID is not owned.
-
- @param Gtid The Gtid to query.
- @return my_thread_id of the thread that owns the group, or
- 0 if the group is not owned.
- */
- my_thread_id get_owner(const Gtid &gtid) const;
 
  /*
  Fill all gtids into the given Gtid_set object. It doesn't clear the given
@@ -2070,8 +2065,9 @@ class Owned_gtids
  nothing.
 
  @param gtid The Gtid.
+ @param owner thread_id of the owner thread
  */
- void remove_gtid(const Gtid &gtid);
+ void remove_gtid(const Gtid &gtid, const my_thread_id owner);
  /**
  Ensures that this Owned_gtids object can accomodate SIDNOs up to
  the given SIDNO.
@@ -2205,6 +2201,13 @@ class Owned_gtids
  my_free(str);
 #endif
  }
+
+ /**
+ If thd_id==0, returns true when gtid is not owned by any thread.
+ If thd_id!=0, returns true when gtid is owned by that thread.
+ */
+ bool is_owned_by(const Gtid &gtid, const my_thread_id thd_id) const;
+
 private:
  /// Represents one owned group.
  struct Node
@@ -2223,23 +2226,9 @@ class Owned_gtids
  sid_lock->assert_some_lock();
  return sidno_to_hash[sidno - 1];
  }
- /**
- Returns the Node for the given HASH and GNO, or NULL if the GNO
- does not exist in the HASH.
- */
- Node *get_node(const HASH *hash, rpl_gno gno) const
- {
- sid_lock->assert_some_lock();
- return (Node *)my_hash_search(hash, (const uchar *)&gno, sizeof(rpl_gno));
- }
- /**
- Returns the Node for the given group, or NULL if the group does
- not exist in this Owned_gtids object.
- */
- Node *get_node(const Gtid &gtid) const
- { return get_node(get_hash(gtid.sidno), gtid.gno); };
  /// Return true iff this Owned_gtids object contains the given group.
- bool contains_gtid(const Gtid &gtid) const { return get_node(gtid) != NULL; }
+ bool contains_gtid(const Gtid &gtid) const;
+
  /// Growable array of hashes.
  Prealloced_array<HASH*, 8, true> sidno_to_hash;
 
@@ -2412,14 +2401,14 @@ class Gtid_state
  DBUG_RETURN(ret);
  }
  /**
- Returns the owner of the given GTID, or 0 if the group is not owned.
+ Returns true if GTID is owned, otherwise returns 0.
 
  @param gtid The Gtid to check.
- @return my_thread_id of the thread that owns the group, or
- 0 if the group is not owned.
+ @return true if some thread owns the group, false if the group is
+ not owned
  */
- my_thread_id get_owner(const Gtid &gtid) const
- { return owned_gtids.get_owner(gtid); }
+ bool is_owned(const Gtid &gtid) const
+ { return !owned_gtids.is_owned_by(gtid, 0); }
 #ifndef MYSQL_CLIENT
  /**
  Acquires ownership of the given GTID, on behalf of the given thread.

sql/rpl_gtid_execution.cc

@@ -113,9 +113,9 @@ bool set_gtid_next(THD *thd, const Gtid_specification &spec)
  */
  break;
  }
- my_thread_id owner= gtid_state->get_owner(spec.gtid);
+
  // GTID not owned by anyone: acquire ownership
- if (owner == 0)
+ if (!gtid_state->is_owned(spec.gtid))
  {
  // acquire_ownership can't fail
  gtid_state->acquire_ownership(thd, spec.gtid);