Move XID_STATE::rm_error to XID_cache_element

XID_STATE::rm_error is never used by internal 2PC, it is intended to be
used by explicit XA only.

Also removed redundant xid reset from THD::init_for_queries(). Must've
been done already either by THD::transaction constructor or by
THD::cleanup().

Part of MDEV-7974 - backport fix for mysql bug#12161 (XA and binlog)

sql/handler.cc

@@ -1867,14 +1867,6 @@ int ha_rollback_trans(THD *thd, bool all)
     trans->no_2pc=0;
   }
 
-  /*
-    Thanks to possibility of MDL deadlock rollback request can come even if
-    transaction hasn't been started in any transactional storage engine.
-  */
-  if (is_real_trans && thd->transaction_rollback_request &&
-      thd->transaction.xid_state.xa_state != XA_NOTR)
-    thd->transaction.xid_state.rm_error= thd->get_stmt_da()->sql_errno();
-
 #ifdef WITH_WSREP
   if (thd->is_error())
   {
@@ -1887,6 +1879,13 @@ int ha_rollback_trans(THD *thd, bool all)
   /* Always cleanup. Even if nht==0. There may be savepoints. */
   if (is_real_trans)
   {
+    /*
+      Thanks to possibility of MDL deadlock rollback request can come even if
+      transaction hasn't been started in any transactional storage engine.
+    */
+    if (thd->transaction_rollback_request)
+      thd->transaction.xid_state.set_error(thd->get_stmt_da()->sql_errno());
+
     thd->has_waiter= false;
     thd->transaction.cleanup();
   }

sql/sql_class.cc

@@ -1385,7 +1385,8 @@ void THD::init_for_queries()
   reset_root_defaults(&transaction.mem_root,
                       variables.trans_alloc_block_size,
                       variables.trans_prealloc_size);
-  transaction.xid_state.xid.null();
+  DBUG_ASSERT(!transaction.xid_state.is_explicit_XA());
+  DBUG_ASSERT(transaction.xid_state.xid.is_null());
 }
 
 
@@ -1529,7 +1530,6 @@ void THD::cleanup(void)
   close_temporary_tables();
 
   transaction.xid_state.xa_state= XA_NOTR;
-  transaction.xid_state.rm_error= 0;
   trans_rollback(this);
   xid_cache_delete(this, &transaction.xid_state);
 

sql/sql_class.h

@@ -2603,16 +2603,11 @@ class THD: public THD_count, /* this must be first */
     MEM_ROOT mem_root; // Transaction-life memory allocation pool
     void cleanup()
     {
-      DBUG_ENTER("thd::cleanup");
+      DBUG_ENTER("THD::st_transactions::cleanup");
       changed_tables= 0;
       savepoints= 0;
-      /*
-        If rm_error is raised, it means that this piece of a distributed
-        transaction has failed and must be rolled back. But the user must
-        rollback it explicitly, so don't start a new distributed XA until
-        then.
-      */
-      if (!xid_state.rm_error)
+      /* xid_cache_delete() resets xid of explicitly started XA transaction */
+      if (!xid_state.is_explicit_XA())
         xid_state.xid.null();
       free_root(&mem_root,MYF(MY_KEEP_PREALLOC));
       DBUG_VOID_RETURN;

sql/xa.cc

@@ -61,6 +61,8 @@ class XID_cache_element
   static const int32 ACQUIRED= 1 << 30;
   static const int32 RECOVERED= 1 << 29;
   XID_STATE *m_xid_state;
+  /* Error reported by the Resource Manager (RM) to the Transaction Manager. */
+  uint rm_error;
   bool is_set(int32_t flag)
   { return m_state.load(std::memory_order_relaxed) & flag; }
   void set(int32_t flag)
@@ -108,6 +110,7 @@ class XID_cache_element
                                   XID_STATE *xid_state)
   {
     DBUG_ASSERT(!element->is_set(ACQUIRED | RECOVERED));
+    element->rm_error= 0;
     element->m_xid_state= xid_state;
     xid_state->xid_cache_element= element;
   }
@@ -147,6 +150,13 @@ bool THD::fix_xid_hash_pins()
 }
 
 
+void XID_STATE::set_error(uint error)
+{
+  if (is_explicit_XA())
+    xid_cache_element->rm_error= error;
+}
+
+
 void xid_cache_init()
 {
   xid_cache_inited= true;
@@ -204,7 +214,6 @@ bool xid_cache_insert(XID *xid, enum xa_states xa_state)
   {
     xs->xa_state=xa_state;
     xs->xid.set(xid);
-    xs->rm_error=0;
 
     if ((res= lf_hash_insert(&xid_cache, pins, xs)))
       my_free(xs);
@@ -248,9 +257,13 @@ void xid_cache_delete(THD *thd, XID_STATE *xid_state)
     xid_state->xid_cache_element->mark_uninitialized();
     lf_hash_delete(&xid_cache, thd->xid_hash_pins,
                    xid_state->xid.key(), xid_state->xid.key_length());
-    xid_state->xid_cache_element= 0;
     if (recovered)
       my_free(xid_state);
+    else
+    {
+      xid_state->xid_cache_element= 0;
+      xid_state->xid.null();
+    }
   }
 }
 
@@ -296,9 +309,10 @@ static int xid_cache_iterate(THD *thd, my_hash_walk_action action, void *arg)
 */
 static bool xa_trans_rolled_back(XID_STATE *xid_state)
 {
-  if (xid_state->rm_error)
+  DBUG_ASSERT(xid_state->is_explicit_XA());
+  if (xid_state->xid_cache_element->rm_error)
   {
-    switch (xid_state->rm_error) {
+    switch (xid_state->xid_cache_element->rm_error) {
     case ER_LOCK_WAIT_TIMEOUT:
       my_error(ER_XA_RBTIMEOUT, MYF(0));
       break;
@@ -318,21 +332,11 @@ static bool xa_trans_rolled_back(XID_STATE *xid_state)
 /**
   Rollback the active XA transaction.
 
-  @note Resets rm_error before calling ha_rollback(), so
-        the thd->transaction.xid structure gets reset
-        by ha_rollback() / THD::transaction::cleanup().
-
   @return TRUE if the rollback failed, FALSE otherwise.
 */
 
 static bool xa_trans_force_rollback(THD *thd)
 {
-  /*
-    We must reset rm_error before calling ha_rollback(),
-    so thd->transaction.xid structure gets reset
-    by ha_rollback()/THD::transaction::cleanup().
-  */
-  thd->transaction.xid_state.rm_error= 0;
   if (ha_rollback_trans(thd, true))
   {
     my_error(ER_XAER_RMERR, MYF(0));
@@ -377,7 +381,6 @@ bool trans_xa_start(THD *thd)
   {
     DBUG_ASSERT(thd->transaction.xid_state.xid.is_null());
     thd->transaction.xid_state.xa_state= XA_ACTIVE;
-    thd->transaction.xid_state.rm_error= 0;
     thd->transaction.xid_state.xid.set(thd->lex->xid);
     if (xid_cache_insert(thd, &thd->transaction.xid_state))
     {

sql/xa.h

@@ -25,8 +25,6 @@ struct XID_STATE {
   /* For now, this is only used to catch duplicated external xids */
   XID  xid;                           // transaction identifier
   enum xa_states xa_state;            // used by external XA only
-  /* Error reported by the Resource Manager (RM) to the Transaction Manager. */
-  uint rm_error;
   XID_cache_element *xid_cache_element;
 
   /**
@@ -51,6 +49,9 @@ struct XID_STATE {
     }
     return false;
   }
+
+  bool is_explicit_XA() const { return xid_cache_element != 0; }
+  void set_error(uint error);
 };
 
 void xid_cache_init(void);