WL#7083 step 6.3. GTID-violations: Introduce global counters of GTID-violating transactions.

Sven Sandberg · Sven Sandberg · commit 02112f56ed3d · 2015-02-05T20:59:45.000+01:00
@rpl_gtid.h
- Introduce global counters, and wrappers to set and get them.
- Update comment to account changes in rpl_gtid_state.cc

@sql_class.h
- Introduce per-thread flag to keep track of whether the current
transaction violates GTID consistency.

@sql_class.cc
- Initialize per-thread flag in THD constructor.

@binlog.cc
- When starting to execute a GTID-violating transaction, increase global
counter and set per-thread flag.

@rpl_gtid_state.cc
- update_gtids_impl: When ending a GTID-violating transaction, decrease global counter and clear per-thread flag.
- update_on_rollback: Ensure counters are decreased on rollback even if nothing is owned.
diff --git a/mysql-test/suite/binlog/r/binlog_enforce_gtid_consistency_multiple_warnings.result b/mysql-test/suite/binlog/r/binlog_enforce_gtid_consistency_multiple_warnings.result
@@ -0,0 +1,35 @@
+call mtr.add_suppression("Statement violates GTID consistency:");
+call mtr.add_suppression("Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT");
+CREATE TABLE t1 (c1 INT) Engine=InnoDB;
+CREATE TABLE t2 (c1 INT) Engine=MyISAM;
+SET GLOBAL enforce_gtid_consistency=WARN;
+BEGIN;
+CREATE TEMPORARY TABLE temp1 (a INT);
+Warnings:
+Warning	1787	Statement violates GTID consistency: CREATE TEMPORARY TABLE and DROP TEMPORARY TABLE can only be executed outside transactional context.
+DROP TEMPORARY TABLE temp1;
+Warnings:
+Warning	1787	Statement violates GTID consistency: CREATE TEMPORARY TABLE and DROP TEMPORARY TABLE can only be executed outside transactional context.
+INSERT INTO t1 VALUES (1);
+INSERT INTO t2 VALUES (1);
+Warnings:
+Warning	1785	Statement violates GTID consistency: Updates to non-transactional tables can only be done in either autocommitted statements or single-statement transactions, and never in the same statement as updates to transactional tables.
+COMMIT;
+BEGIN;
+INSERT INTO t1 VALUES (1);
+INSERT INTO t2 VALUES (1);
+Warnings:
+Warning	1785	Statement violates GTID consistency: Updates to non-transactional tables can only be done in either autocommitted statements or single-statement transactions, and never in the same statement as updates to transactional tables.
+INSERT INTO t1 VALUES (1);
+COMMIT;
+BEGIN;
+UPDATE t1, t2 SET t1.c1=2, t2.c1=1000;
+Warnings:
+Warning	1785	Statement violates GTID consistency: Updates to non-transactional tables can only be done in either autocommitted statements or single-statement transactions, and never in the same statement as updates to transactional tables.
+Note	1592	Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. Statement accesses nontransactional table as well as transactional or temporary table, and writes to any of them.
+INSERT INTO t1 VALUES (1);
+COMMIT;
+DROP TABLE t1;
+DROP TABLE t2;
+SET GLOBAL enforce_gtid_consistency=SAVED_LEVEL;
+RESET MASTER;
diff --git a/mysql-test/suite/binlog/t/binlog_enforce_gtid_consistency_multiple_warnings.test b/mysql-test/suite/binlog/t/binlog_enforce_gtid_consistency_multiple_warnings.test
@@ -0,0 +1,54 @@
+#
+# BUG#20414559: WL7083: ER1787, ER1785 NOT GENERATED ALWAYS WHEN ENFORCE_GTID_CONSISTENCY=WARN
+# WL#7083: GTIDs: set gtid_mode=ON online
+#
+# Verify that when enforce_gtid_consistency=WARN, and multiple
+# statements in the same transaction violate GTID-consistency, there
+# is one warning for each such statement.
+#
+--source include/have_binlog_format_statement.inc
+--source include/not_gtid_enabled.inc
+--source include/have_myisam.inc
+--source include/have_innodb.inc
+
+#
+# BUG#20414559
+#
+
+call mtr.add_suppression("Statement violates GTID consistency:");
+call mtr.add_suppression("Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT");
+
+--let $saved_enforce_gtid_consistency=`SELECT @@global.enforce_gtid_consistency`
+
+CREATE TABLE t1 (c1 INT) Engine=InnoDB;
+CREATE TABLE t2 (c1 INT) Engine=MyISAM;
+
+SET GLOBAL enforce_gtid_consistency=WARN;
+BEGIN;
+# warning is emitted
+CREATE TEMPORARY TABLE temp1 (a INT);
+# warning is emitted
+DROP TEMPORARY TABLE temp1;
+INSERT INTO t1 VALUES (1);
+# warning is emitted
+INSERT INTO t2 VALUES (1);
+COMMIT;
+
+BEGIN;
+INSERT INTO t1 VALUES (1);
+# warning is emitted
+INSERT INTO t2 VALUES (1);
+INSERT INTO t1 VALUES (1);
+COMMIT;
+
+BEGIN;
+# warning is emitted
+UPDATE t1, t2 SET t1.c1=2, t2.c1=1000;
+INSERT INTO t1 VALUES (1);
+COMMIT;
+
+DROP TABLE t1;
+DROP TABLE t2;
+--replace_result $saved_enforce_gtid_consistency SAVED_LEVEL
+--eval SET GLOBAL enforce_gtid_consistency=$saved_enforce_gtid_consistency
+RESET MASTER;
diff --git a/sql/binlog.cc b/sql/binlog.cc
@@ -9491,7 +9491,6 @@ static bool handle_gtid_consistency_violation(THD *thd, int error_code)
   enum_gtid_consistency_mode gtid_consistency_mode=
     get_gtid_consistency_mode();
   enum_gtid_mode gtid_mode= get_gtid_mode(GTID_MODE_LOCK_SID);
-  global_sid_lock->unlock();
 
   DBUG_PRINT("info", ("gtid_next.type=%d gtid_mode=%s "
                       "gtid_consistency_mode=%d error=%d query=%s",
@@ -9514,18 +9513,59 @@ static bool handle_gtid_consistency_violation(THD *thd, int error_code)
       gtid_next_type == GTID_GROUP ||
       gtid_consistency_mode == GTID_CONSISTENCY_MODE_ON)
   {
+    global_sid_lock->unlock();
     my_error(error_code, MYF(0));
     DBUG_RETURN(false);
   }
-  else if (gtid_consistency_mode == GTID_CONSISTENCY_MODE_WARN)
+  else
   {
-    // Need to print to log so that replication admin knows when users
-    // have adjusted their workloads.
-    sql_print_warning("%s", ER(error_code));
-    // Need to print to client so that users can adjust their workload.
-    push_warning(thd, Sql_condition::SL_WARNING, error_code, ER(error_code));
+    /*
+      If we are not generating an error, we must increase the counter
+      of GTID-violating transactions.  This will prevent a concurrent
+      client from executing a SET GTID_MODE or SET
+      ENFORCE_GTID_CONSISTENCY statement that would be incompatible
+      with this transaction.
+
+      If the transaction had already been accounted as a gtid violating
+      transaction, then don't increment the counters, just issue the
+      warning below. This prevents calling
+      begin_automatic_gtid_violating_transaction or
+      begin_anonymous_gtid_violating_transaction multiple times for the
+      same transaction, which would make the counter go out of sync.
+    */
+    if (!thd->has_gtid_consistency_violation)
+    {
+      if (gtid_next_type == AUTOMATIC_GROUP)
+        gtid_state->begin_automatic_gtid_violating_transaction();
+      else
+      {
+        DBUG_ASSERT(gtid_next_type == ANONYMOUS_GROUP);
+        gtid_state->begin_anonymous_gtid_violating_transaction();
+      }
+
+      /*
+        If a transaction generates multiple GTID violation conditions,
+        it must still only update the counters once.  Hence we use
+        this per-thread flag to keep track of whether the thread has a
+        consistency or not.  This function must only be called if the
+        transaction does not already have a GTID violation.
+      */
+      thd->has_gtid_consistency_violation= true;
+    }
+
+    global_sid_lock->unlock();
+
+    // Generate warning if ENFORCE_GTID_CONSISTENCY = WARN.
+    if (gtid_consistency_mode == GTID_CONSISTENCY_MODE_WARN)
+    {
+      // Need to print to log so that replication admin knows when users
+      // have adjusted their workloads.
+      sql_print_warning("%s", ER(error_code));
+      // Need to print to client so that users can adjust their workload.
+      push_warning(thd, Sql_condition::SL_WARNING, error_code, ER(error_code));
+    }
+    DBUG_RETURN(true);
   }
-  DBUG_RETURN(true);
 }
 
 
diff --git a/sql/rpl_gtid.h b/sql/rpl_gtid.h
@@ -2460,6 +2460,96 @@ class Gtid_state
     return anonymous_gtid_count.atomic_get();
   }
 
+  /**
+    Increase the global counter when starting a GTID-violating
+    transaction having GTID_NEXT=AUTOMATIC.
+  */
+  void begin_automatic_gtid_violating_transaction()
+  {
+    DBUG_ENTER("Gtid_state::begin_automatic_gtid_violating_transaction");
+    DBUG_ASSERT(get_gtid_mode(GTID_MODE_LOCK_SID) <= GTID_MODE_OFF_PERMISSIVE);
+    DBUG_ASSERT(get_gtid_consistency_mode() != GTID_CONSISTENCY_MODE_ON);
+#ifndef DBUG_OFF
+    int32 old_value=
+#endif
+      automatic_gtid_violation_count.atomic_add(1);
+    DBUG_PRINT("info", ("ongoing_automatic_gtid_violating_transaction_count increased to %d", old_value + 1));
+    DBUG_ASSERT(old_value >= 0);
+    DBUG_VOID_RETURN;
+  }
+
+  /**
+    Decrease the global counter when ending a GTID-violating
+    transaction having GTID_NEXT=AUTOMATIC.
+  */
+  void end_automatic_gtid_violating_transaction()
+  {
+    DBUG_ENTER("Gtid_state::end_automatic_gtid_violating_transaction");
+    DBUG_ASSERT(get_gtid_mode(GTID_MODE_LOCK_SID) <= GTID_MODE_OFF_PERMISSIVE);
+    DBUG_ASSERT(get_gtid_consistency_mode() != GTID_CONSISTENCY_MODE_ON);
+#ifndef DBUG_OFF
+    int32 old_value=
+#endif
+      automatic_gtid_violation_count.atomic_add(-1);
+    DBUG_PRINT("info", ("ongoing_automatic_gtid_violating_transaction_count decreased to %d", old_value - 1));
+    DBUG_ASSERT(old_value >= 1);
+    DBUG_VOID_RETURN;
+  }
+
+  /**
+    Return the number of ongoing GTID-violating transactions having
+    GTID_NEXT=AUTOMATIC.
+  */
+  int32 get_automatic_gtid_violating_transaction_count()
+  {
+    return automatic_gtid_violation_count.atomic_get();
+  }
+
+  /**
+    Increase the global counter when starting a GTID-violating
+    transaction having GTID_NEXT=ANONYMOUS.
+  */
+  void begin_anonymous_gtid_violating_transaction()
+  {
+    DBUG_ENTER("Gtid_state::begin_anonymous_gtid_violating_transaction");
+    DBUG_ASSERT(get_gtid_mode(GTID_MODE_LOCK_SID) != GTID_MODE_ON);
+    DBUG_ASSERT(get_gtid_consistency_mode() != GTID_CONSISTENCY_MODE_ON);
+#ifndef DBUG_OFF
+    int32 old_value=
+#endif
+      anonymous_gtid_violation_count.atomic_add(1);
+    DBUG_PRINT("info", ("ongoing_anonymous_gtid_violating_transaction_count increased to %d", old_value + 1));
+    DBUG_ASSERT(old_value >= 0);
+    DBUG_VOID_RETURN;
+  }
+
+  /**
+    Decrease the global counter when ending a GTID-violating
+    transaction having GTID_NEXT=ANONYMOUS.
+  */
+  void end_anonymous_gtid_violating_transaction()
+  {
+    DBUG_ENTER("Gtid_state::end_anonymous_gtid_violating_transaction");
+    DBUG_ASSERT(get_gtid_mode(GTID_MODE_LOCK_SID) != GTID_MODE_ON);
+    DBUG_ASSERT(get_gtid_consistency_mode() != GTID_CONSISTENCY_MODE_ON);
+#ifndef DBUG_OFF
+    int32 old_value=
+#endif
+      anonymous_gtid_violation_count.atomic_add(-1);
+    DBUG_PRINT("info", ("ongoing_anonymous_gtid_violating_transaction_count decreased to %d", old_value - 1));
+    DBUG_ASSERT(old_value >= 1);
+    DBUG_VOID_RETURN;
+  }
+
+  /**
+    Return the number of ongoing GTID-violating transactions having
+    GTID_NEXT=AUTOMATIC.
+  */
+  int32 get_anonymous_gtid_violating_transaction_count()
+  {
+    return anonymous_gtid_violation_count.atomic_get();
+  }
+
 #endif // ifndef MYSQL_CLIENT
 private:
   /**
@@ -2754,10 +2844,13 @@ class Gtid_state
 
     This will:
 
-    - release ownership of all GTIDs owned by the THD;
-    - add all GTIDs in the Group_cache to executed_gtids is only done if the
-      is_commit flag is set.
-    - send a broadcast on the condition variable for every sidno for
+    - Release ownership of all GTIDs owned by the THD. This removes
+      the GTID from Owned_gtids and clears the ownership status in the
+      THD object.
+    - Add the owned GTID to executed_gtids if the is_commit flag is
+      set.
+    - Decrease counters of GTID-violating transactions.
+    - Send a broadcast on the condition variable for every sidno for
       which we released ownership.
 
     @param[in] thd - Thread for which owned groups are updated.
@@ -2795,6 +2888,10 @@ class Gtid_state
 
   /// The number of anonymous transactions owned by any client.
   Atomic_int32 anonymous_gtid_count;
+  /// The number of GTID-violating transactions that use GTID_NEXT=AUTOMATIC.
+  Atomic_int32 automatic_gtid_violation_count;
+  /// The number of GTID-violating transactions that use GTID_NEXT=AUTOMATIC.
+  Atomic_int32 anonymous_gtid_violation_count;
 
   /// Used by unit tests that need to access private members.
 #ifdef FRIEND_OF_GTID_STATE
diff --git a/sql/rpl_gtid_state.cc b/sql/rpl_gtid_state.cc
@@ -158,19 +158,42 @@ void Gtid_state::update_on_rollback(THD *thd)
 {
   DBUG_ENTER("Gtid_state::update_on_rollback");
 
-  if (!thd->owned_gtid.is_empty())
+  /*
+    If we don't own anything, there is nothing to do, so we do an
+    early return.  Except if there is a GTID consistency violation;
+    then we need to decrement the counter, so then we go ahead and
+    call update_gtids_impl.
+  */
+  if (thd->owned_gtid.is_empty() && !thd->has_gtid_consistency_violation)
   {
-    if (thd->skip_gtid_rollback)
-    {
-      DBUG_PRINT("info",("skipping the gtid_rollback"));
-      DBUG_VOID_RETURN;
-    }
+    DBUG_PRINT("info", ("skipping gtid rollback because "
+                        "thread does not own anything and does not violate "
+                        "gtid consistency"));
+    DBUG_VOID_RETURN;
+  }
 
-    global_sid_lock->rdlock();
-    update_gtids_impl(thd, false);
-    global_sid_lock->unlock();
+  /*
+    The administrative commands [CHECK|REPAIR|OPTIMIZE|ANALYZE] TABLE
+    are written to the binary log even when they fail.  When the
+    commands fail, they will call update_on_rollback; later they will
+    write the binary log.  But we must not do any of the things in
+    update_gtids_impl if we are going to write the binary log.  So
+    these statements set the skip_gtid_rollback flag, which tells
+    update_on_rollback to return early.  When the statements are
+    written to the binary log they will call update_on_commit as
+    usual.
+  */
+  if (thd->skip_gtid_rollback)
+  {
+    DBUG_PRINT("info", ("skipping gtid rollback because "
+                        "thd->skip_gtid_rollback is set"));
+    DBUG_VOID_RETURN;
   }
 
+  global_sid_lock->rdlock();
+  update_gtids_impl(thd, false);
+  global_sid_lock->unlock();
+
   DBUG_VOID_RETURN;
 }
 
@@ -346,6 +369,19 @@ void Gtid_state::update_gtids_impl(THD *thd, bool is_commit)
     DBUG_ASSERT(thd->variables.gtid_next.type == AUTOMATIC_GROUP);
   }
 
+  if (thd->has_gtid_consistency_violation &&
+      !more_transactions_with_same_gtid_next)
+  {
+    if (thd->variables.gtid_next.type == AUTOMATIC_GROUP)
+      gtid_state->end_automatic_gtid_violating_transaction();
+    else
+    {
+      DBUG_ASSERT(thd->variables.gtid_next.type == ANONYMOUS_GROUP);
+      gtid_state->end_anonymous_gtid_violating_transaction();
+    }
+    thd->has_gtid_consistency_violation= false;
+  }
+
   thd->owned_gtid.dbug_print(NULL,
                              "set owned_gtid (clear) in update_gtids_impl");
 
diff --git a/sql/sql_class.cc b/sql/sql_class.cc
@@ -1197,6 +1197,7 @@ THD::THD(bool enable_plugins)
 #endif
    skip_gtid_rollback(false),
    is_commit_in_middle_of_statement(false),
+   has_gtid_consistency_violation(false),
    main_da(false),
    m_parser_da(false),
    m_query_rewrite_plugin_da(false),
diff --git a/sql/sql_class.h b/sql/sql_class.h
@@ -3617,6 +3617,11 @@ class THD :public MDL_context_owner,
     flag.
   */
   bool is_commit_in_middle_of_statement;
+  /*
+    True while the transaction is executing, if one of
+    is_ddl_gtid_consistent or is_dml_gtid_consistent returned false.
+  */
+  bool has_gtid_consistency_violation;
 
   const LEX_CSTRING &db() const
   { return m_db; }
