WL#3072 Maria Recovery

All statements doing an implicit commit now also do one in Maria.
This is useful because LOCK TABLES; REPAIR; crash; is not rollback-able,
the implicit commit of REPAIR avoid that Recovery tries to rollback
and fails.
Fix for BUG#33827 "COMMIT AND CHAIN causes serious Valgrind error"
(maybe not the definite one, depends on the assigned dev).


mysql-test/t/maria-recovery.test:
  test of REPAIR's implicit commit. I cannot commit the result file
  because maria-recovery fails in vanilla tree (seen in pushbuild) but
  its new section looks like:
  repair table t1;
  Table   Op      Msg_type        Msg_text
  mysqltest.t1    repair  status  OK
  insert into t1 values(2);
  select * from t1;
  a
  1
  2
  3
  SET SESSION debug="+d,maria_flush_whole_log,maria_flush_whole_page_cache,maria_crash";
  * crashing mysqld intentionally
  set global maria_checkpoint_interval=1;
  ERROR HY000: Lost connection to MySQL server during query
  * recovery happens
  check table t1 extended;
  Table   Op      Msg_type        Msg_text
  mysqltest.t1    check   status  OK
  * testing that checksum after recovery is as expected
  Checksum-check
  failure
  use mysqltest;
  select * from t1;
  a
  1
  3
  Which is as it should be.
sql/rpl_injector.cc:
  fix for BUG#33827
sql/sql_parse.cc:
  - All DDLs and mysql_admin_table() (REPAIR etc) use end_actrive_trans()
  to do an implicit commit so we add there an implicit commit of the
  Maria transaction.
  - Fix for BUG#33827
storage/maria/ha_maria.cc:
  - A method to do implicit commit in Maria
  - After an implicit commit, if it was under LOCK TABLES, the locked
  tables have a stale file->trn: update it.
storage/maria/ha_maria.h:
  new static method
storage/maria/ma_check.c:
  bugfix: this disabling of transactionality had the effect that if
  LOCK TABLES; REPAIR; INSERT then the INSERT ran non-transactional
  (so couldn't be undone in case of crash, if, by bad chance, its
  effect on pages went to disk).
storage/maria/ma_checkpoint.c:
  indentation
storage/maria/ma_recovery.c:
  dbug statements
storage/maria/trnman.c:
  When doing an implicit commit we need to know the number of locked
  tables of the committed transaction and copy it to the new transaction
storage/maria/trnman_public.h:
  prototype change

mysql-test/r/bdb_notembedded.result

@@ -0,0 +1,35 @@
+set autocommit=1;
+reset master;
+create table bug16206 (a int);
+insert into bug16206 values(1);
+start transaction;
+insert into bug16206 values(2);
+commit;
+show binlog events;
+Log_name	Pos	Event_type	Server_id	End_log_pos	Info
+f	n	Format_desc	1	n	Server ver: VERSION, Binlog ver: 4
+f	n	Query	1	n	use `test`; create table bug16206 (a int)
+f	n	Query	1	n	use `test`; insert into bug16206 values(1)
+f	n	Query	1	n	use `test`; insert into bug16206 values(2)
+drop table bug16206;
+reset master;
+create table bug16206 (a int) engine=         bdb;
+insert into bug16206 values(0);
+insert into bug16206 values(1);
+start transaction;
+insert into bug16206 values(2);
+commit;
+insert into bug16206 values(3);
+show binlog events;
+Log_name	Pos	Event_type	Server_id	End_log_pos	Info
+f	n	Format_desc	1	n	Server ver: VERSION, Binlog ver: 4
+f	n	Query	1	n	use `test`; create table bug16206 (a int) engine=         bdb
+f	n	Query	1	n	use `test`; insert into bug16206 values(0)
+f	n	Query	1	n	use `test`; insert into bug16206 values(1)
+f	n	Query	1	n	use `test`; BEGIN
+f	n	Query	1	n	use `test`; insert into bug16206 values(2)
+f	n	Query	1	n	use `test`; COMMIT
+f	n	Query	1	n	use `test`; insert into bug16206 values(3)
+drop table bug16206;
+set autocommit=0;
+End of 5.0 tests

mysql-test/t/bdb_notembedded.test

@@ -0,0 +1,38 @@
+-- source include/not_embedded.inc
+-- source include/have_bdb.inc
+
+#
+# Bug #16206: Superfluous COMMIT event in binlog when updating BDB in autocommit mode
+#
+set autocommit=1;
+
+let $VERSION=`select version()`;
+
+reset master;
+create table bug16206 (a int);
+insert into bug16206 values(1);
+start transaction;
+insert into bug16206 values(2);
+commit;
+--replace_result $VERSION VERSION
+--replace_column 1 f 2 n 5 n
+show binlog events;
+drop table bug16206;
+
+reset master;
+create table bug16206 (a int) engine=         bdb;
+insert into bug16206 values(0);
+insert into bug16206 values(1);
+start transaction;
+insert into bug16206 values(2);
+commit;
+insert into bug16206 values(3);
+--replace_result $VERSION VERSION
+--replace_column 1 f 2 n 5 n
+show binlog events;
+drop table bug16206;
+
+set autocommit=0;
+
+
+--echo End of 5.0 tests

mysql-test/t/maria-recovery.test

@@ -234,6 +234,29 @@ let $mvr_crash_statement= set global maria_checkpoint_interval=1;
 -- source include/maria_verify_recovery.inc
 drop table t1;
 
+--echo Test of REPAIR's implicit commit
+
+let $mms_tables=1;
+create table t1 (a varchar(100), key(a)) engine=maria;
+let $mvr_restore_old_snapshot=0;
+let $mms_compare_physically=0;
+let $mvr_crash_statement= set global maria_checkpoint_interval=1;
+
+let $mvr_debug_option="+d,maria_flush_whole_log,maria_flush_whole_page_cache,maria_crash";
+insert into t1 values(3);
+-- source include/maria_make_snapshot_for_comparison.inc
+lock tables t1 write;
+insert into t1 values (1);
+repair table t1;
+insert into t1 values(2);
+select * from t1;
+
+# checksum comparison failure is expected, SELECT output matters
+-- source include/maria_verify_recovery.inc
+# 2 should be missing (rolled back) but 1 should be committed
+select * from t1;
+drop table t1;
+
 #
 # Test with big blobs
 #

sql/rpl_injector.cc

@@ -35,6 +35,7 @@ injector::transaction::transaction(MYSQL_BIN_LOG *log, THD *thd)
   m_start_pos.m_file_name= my_strdup(log_info.log_file_name, MYF(0));
   m_start_pos.m_file_pos= log_info.pos;
 
+  m_thd->lex->start_transaction_opt= 0; /* for begin_trans() */
   begin_trans(m_thd);
 
   thd->set_current_stmt_binlog_row_based();

sql/sql_parse.cc

@@ -28,6 +28,10 @@
 #include "events.h"
 #include "sql_trigger.h"
 
+#ifdef WITH_MARIA_STORAGE_ENGINE
+#include "../storage/maria/ha_maria.h"
+#endif
+
 /**
   @defgroup Runtime_Environment Runtime Environment
   @{
@@ -122,6 +126,9 @@ bool end_active_trans(THD *thd)
     thd->server_status&= ~SERVER_STATUS_IN_TRANS;
     if (ha_commit(thd))
       error=1;
+#ifdef WITH_MARIA_STORAGE_ENGINE
+    ha_maria::implicit_commit(thd);
+#endif
   }
   thd->options&= ~(OPTION_BEGIN | OPTION_KEEP_LOG);
   thd->transaction.all.modified_non_trans_table= FALSE;
@@ -627,6 +634,7 @@ int end_trans(THD *thd, enum enum_mysql_completiontype completion)
              xa_state_names[thd->transaction.xid_state.xa_state]);
     DBUG_RETURN(1);
   }
+  thd->lex->start_transaction_opt= 0; /* for begin_trans() */
   switch (completion) {
   case COMMIT:
     /*

storage/maria/ha_maria.cc

@@ -2184,12 +2184,20 @@ skip_transaction:
 
 int ha_maria::start_stmt(THD *thd, thr_lock_type lock_type)
 {
-  TRN *trn= THD_TRN;
+  TRN *trn;
   if (file->s->base.born_transactional)
   {
+    trn= THD_TRN;
     DBUG_ASSERT(trn); // this may be called only after external_lock()
     DBUG_ASSERT(trnman_has_locked_tables(trn));
     DBUG_ASSERT(lock_type != F_UNLCK);
+    /*
+      If there was an implicit commit under this LOCK TABLES by a previous
+      statement (like a DDL), at least if that previous statement was about a
+      different ha_maria than 'this' then this->file->trn is a stale
+      pointer. We fix it:
+    */
+    file->trn= trn;
     /*
       As external_lock() was already called, don't increment locked_tables.
       Note that we call the function below possibly several times when
@@ -2202,6 +2210,49 @@ int ha_maria::start_stmt(THD *thd, thr_lock_type lock_type)
   return 0;
 }
 
+
+/**
+  Performs an implicit commit of the Maria transaction and creates a new
+  one.
+
+  This can be considered a hack. When Maria loses HA_NO_TRANSACTIONS it will
+  be participant in the connection's transaction and so the implicit commits
+  (ha_commit()) (like in end_active_trans()) will do the implicit commit
+  without need to call this function which can then be removed.
+*/
+
+int ha_maria::implicit_commit(THD *thd)
+{
+#ifndef MARIA_CANNOT_ROLLBACK
+#error this method should be removed
+#endif
+  TRN *trn;
+  int error= 0;
+  DBUG_ENTER("ha_maria::implicit_commit");
+  if ((trn= THD_TRN) != NULL)
+  {
+    uint locked_tables= trnman_has_locked_tables(trn);
+    if (unlikely(ma_commit(trn)))
+      error= 1;
+    /*
+      We need to create a new transaction and put it in THD_TRN. Indeed,
+      tables may be under LOCK TABLES, and so they will start the next
+      statement assuming they have a trn (see ha_maria::start_stmt()).
+    */
+    trn= trnman_new_trn(& thd->mysys_var->mutex,
+                        & thd->mysys_var->suspend,
+                        thd->thread_stack + STACK_DIRECTION *
+                        (my_thread_stack_size - STACK_MIN_SIZE));
+    /* This is just a commit, tables stay locked if they were: */
+    trnman_reset_locked_tables(trn, locked_tables);
+    THD_TRN= trn;
+    if (unlikely(trn == NULL))
+      error= HA_ERR_OUT_OF_MEM;
+  }
+  DBUG_RETURN(error);
+}
+
+
 THR_LOCK_DATA **ha_maria::store_lock(THD *thd,
                                      THR_LOCK_DATA **to,
                                      enum thr_lock_type lock_type)
@@ -2494,7 +2545,7 @@ static int maria_commit(handlerton *hton __attribute__ ((unused)),
 {
   TRN *trn= THD_TRN;
   DBUG_ENTER("maria_commit");
-  trnman_reset_locked_tables(trn);
+  trnman_reset_locked_tables(trn, 0);
   /* statement or transaction ? */
   if ((thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)) && !all)
     DBUG_RETURN(0); // end of statement
@@ -2509,7 +2560,7 @@ static int maria_rollback(handlerton *hton __attribute__ ((unused)),
 {
   TRN *trn= THD_TRN;
   DBUG_ENTER("maria_rollback");
-  trnman_reset_locked_tables(trn);
+  trnman_reset_locked_tables(trn, 0);
   /* statement or transaction ? */
   if ((thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)) && !all)
   {

storage/maria/ha_maria.h

@@ -152,4 +152,6 @@ public:
                                      *engine_callback,
                                      ulonglong *engine_data);
 #endif
+
+  static int implicit_commit(THD *thd);
 };

storage/maria/ma_check.c

@@ -5907,7 +5907,6 @@ static my_bool create_new_data_handle(MARIA_SORT_PARAM *param, File new_file)
                                         HA_OPEN_COPY | HA_OPEN_FOR_REPAIR)))
     DBUG_RETURN(1);
 
-  info->s->now_transactional= 0;
   new_info= sort_info->new_info;
   _ma_bitmap_set_pagecache_callbacks(&new_info->s->bitmap.file,
                                      new_info->s);

storage/maria/ma_checkpoint.c

@@ -363,7 +363,7 @@ static void flush_all_tables(int what_to_flush)
       {
       case 0:
         res= _ma_flush_table_files(info, MARIA_FLUSH_DATA | MARIA_FLUSH_INDEX,
-                              FLUSH_KEEP, FLUSH_KEEP);
+                                   FLUSH_KEEP, FLUSH_KEEP);
         break;
       case 1:
         res= _ma_state_info_write(info->s, 1|4);

storage/maria/ma_recovery.c

@@ -3023,6 +3023,7 @@ void _ma_tmp_disable_logging_for_table(MARIA_HA *info,
                                        my_bool log_incomplete)
 {
   MARIA_SHARE *share= info->s;
+  DBUG_ENTER("_ma_tmp_disable_logging_for_table");
   if (log_incomplete)
   {
     uchar log_data[FILEID_STORE_SIZE];
@@ -3051,6 +3052,7 @@ void _ma_tmp_disable_logging_for_table(MARIA_HA *info,
   _ma_set_data_pagecache_callbacks(&info->dfile, share);
   _ma_set_index_pagecache_callbacks(&share->kfile, share);
   _ma_bitmap_set_pagecache_callbacks(&share->bitmap.file, share);
+  DBUG_VOID_RETURN;
 }
 
 
@@ -3063,9 +3065,10 @@ void _ma_tmp_disable_logging_for_table(MARIA_HA *info,
 void _ma_reenable_logging_for_table(MARIA_HA *info)
 {
   MARIA_SHARE *share= info->s;
+  DBUG_ENTER("_ma_reenable_logging_for_table");
 
   if (share->now_transactional == share->base.born_transactional)
-    return;
+    DBUG_VOID_RETURN;
 
   if ((share->now_transactional= share->base.born_transactional))
   {
@@ -3080,6 +3083,7 @@ void _ma_reenable_logging_for_table(MARIA_HA *info)
   _ma_set_data_pagecache_callbacks(&info->dfile, share);
   _ma_set_index_pagecache_callbacks(&share->kfile, share);
   _ma_bitmap_set_pagecache_callbacks(&share->bitmap.file, share);
+  DBUG_VOID_RETURN;
 }
 
 

storage/maria/trnman.c

@@ -62,9 +62,9 @@ uint trnman_increment_locked_tables(TRN *trn)
   return trn->locked_tables++;
 }
 
-my_bool trnman_has_locked_tables(TRN *trn)
+uint trnman_has_locked_tables(TRN *trn)
 {
-  return trn->locked_tables != 0;
+  return trn->locked_tables;
 }
 
 uint trnman_decrement_locked_tables(TRN *trn)
@@ -72,9 +72,9 @@ uint trnman_decrement_locked_tables(TRN *trn)
   return --trn->locked_tables;
 }
 
-void trnman_reset_locked_tables(TRN *trn)
+void trnman_reset_locked_tables(TRN *trn, uint locked_tables)
 {
-  trn->locked_tables= 0;
+  trn->locked_tables= locked_tables;
 }
 
 

storage/maria/trnman_public.h

@@ -51,8 +51,8 @@ my_bool trnman_collect_transactions(LEX_STRING *str_act, LEX_STRING *str_com,
 
 uint trnman_increment_locked_tables(TRN *trn);
 uint trnman_decrement_locked_tables(TRN *trn);
-my_bool trnman_has_locked_tables(TRN *trn);
-void trnman_reset_locked_tables(TRN *trn);
+uint trnman_has_locked_tables(TRN *trn);
+void trnman_reset_locked_tables(TRN *trn, uint locked_tables);
 TRN *trnman_recreate_trn_from_recovery(uint16 shortid, TrID longid);
 TRN *trnman_get_any_trn();
 #define TRANSID_SIZE		6