MDEV-34150 Assertion failure in Diagnostics_area::set_error_status upon binary...

MDEV-34150 Assertion failure in Diagnostics_area::set_error_status upon binary logging hitting tmp space limit

- Moved writing to binlog_cache from close_thread_tables() to
  binlog_commit().
- In select_create() delete cached row events instead of flushing them
  to disk. This was done to avoid possible disk write error in this code.

mysql-test/include/show_binlog_events.inc

@@ -20,7 +20,7 @@
 #
 #   $binlog_start
 #     Position for the 'FROM' clause of SHOW BINLOG EVENTS. If none
-#     given, starts right after the  Binlog_checkpoint_log_even.
+#     given, starts right after the  Binlog_checkpoint_log_event.
 #
 #   $binlog_limit
 #     Limit for the 'LIMIT' clause of SHOW BINLOG EVENTS, i.e.:

mysql-test/suite/binlog/r/binlog_unsafe.result

@@ -2542,11 +2542,7 @@ master-bin.000001	#	Annotate_rows	#	#	insert into t2(a) values(new.a)
 master-bin.000001	#	Table_map	#	#	table_id: # (test.t1)
 master-bin.000001	#	Table_map	#	#	table_id: # (test.t3)
 master-bin.000001	#	Table_map	#	#	table_id: # (test.t2)
-master-bin.000001	#	Write_rows_v1	#	#	table_id: # flags: STMT_END_F
-master-bin.000001	#	Annotate_rows	#	#	insert into t3(a) values(new.a)
-master-bin.000001	#	Table_map	#	#	table_id: # (test.t1)
-master-bin.000001	#	Table_map	#	#	table_id: # (test.t3)
-master-bin.000001	#	Table_map	#	#	table_id: # (test.t2)
+master-bin.000001	#	Write_rows_v1	#	#	table_id: #
 master-bin.000001	#	Write_rows_v1	#	#	table_id: #
 master-bin.000001	#	Write_rows_v1	#	#	table_id: # flags: STMT_END_F
 master-bin.000001	#	Query	#	#	COMMIT

sql/log.cc

@@ -437,6 +437,28 @@ class binlog_cache_mngr {
   binlog_cache_mngr(const binlog_cache_mngr& info);
 };
 
+
+/*
+  Remove all row event from all binlog caches and clear all caches.
+  This is only called from CREATE .. SELECT, in which case it safe to delete
+  also events from the statement cache.
+*/
+
+void THD::binlog_remove_rows_events()
+{
+  binlog_cache_mngr *cache_mngr= binlog_get_cache_mngr();
+  DBUG_ENTER("THD::binlog_remove_rows_events");
+
+  if (!cache_mngr ||
+      (!WSREP_EMULATE_BINLOG_NNULL(this) && !mysql_bin_log.is_open()))
+    DBUG_VOID_RETURN;
+
+  MYSQL_BIN_LOG::remove_pending_rows_event(this, &cache_mngr->stmt_cache);
+  MYSQL_BIN_LOG::remove_pending_rows_event(this, &cache_mngr->trx_cache);
+  cache_mngr->reset(1,1);
+  DBUG_VOID_RETURN;
+}
+
 /**
   The function handles the first phase of two-phase binlogged ALTER.
   On master binlogs START ALTER when that is configured to do so.
@@ -1897,20 +1919,19 @@ binlog_commit_flush_xid_caches(THD *thd, binlog_cache_mngr *cache_mngr,
 static int
 binlog_truncate_trx_cache(THD *thd, binlog_cache_mngr *cache_mngr, bool all)
 {
+  int error=0;
   DBUG_ENTER("binlog_truncate_trx_cache");
 
   if(!WSREP_EMULATE_BINLOG_NNULL(thd) && !mysql_bin_log.is_open())
     DBUG_RETURN(0);
 
-  int error=0;
-
   DBUG_PRINT("info", ("thd->options={ %s %s}, transaction: %s",
                       FLAGSTR(thd->variables.option_bits, OPTION_NOT_AUTOCOMMIT),
                       FLAGSTR(thd->variables.option_bits, OPTION_BEGIN),
                       all ? "all" : "stmt"));
 
-  auto &trx_cache= cache_mngr->trx_cache;
-  MYSQL_BIN_LOG::remove_pending_rows_event(thd, &trx_cache);
+  binlog_cache_data *trx_cache= &cache_mngr->trx_cache;
+  MYSQL_BIN_LOG::remove_pending_rows_event(thd, trx_cache);
   thd->reset_binlog_for_next_statement();
 
   /*
@@ -1919,7 +1940,7 @@ binlog_truncate_trx_cache(THD *thd, binlog_cache_mngr *cache_mngr, bool all)
   */
   if (ending_trans(thd, all))
   {
-    if (trx_cache.has_incident())
+    if (trx_cache->has_incident())
       error= mysql_bin_log.write_incident(thd);
 
     DBUG_ASSERT(thd->binlog_table_maps == 0);
@@ -1931,9 +1952,9 @@ binlog_truncate_trx_cache(THD *thd, binlog_cache_mngr *cache_mngr, bool all)
     transaction cache to remove the statement.
   */
   else
-    trx_cache.restore_prev_position();
+    trx_cache->restore_prev_position();
 
-  DBUG_ASSERT(trx_cache.pending() == NULL);
+  DBUG_ASSERT(trx_cache->pending() == NULL);
   DBUG_RETURN(error);
 }
 
@@ -2190,6 +2211,11 @@ int binlog_commit(THD *thd, bool all, bool ro_1pc)
         cache_mngr->need_unlog= false;
       }
   }
+  else if (thd->lock)
+  {
+    /* If not in LOCK TABLES, flush the transaction log */
+    error= thd->binlog_flush_pending_rows_event(TRUE, TRUE);
+  }
   /*
     This is part of the stmt rollback.
   */
@@ -6583,6 +6609,26 @@ int binlog_flush_pending_rows_event(THD *thd, bool stmt_end,
 }
 
 
+/*
+  Check if there are pending row events in the binlog cache
+
+  @return 0  no
+  @return 1  rows in stmt_cache
+  @return 2  rows in trx_cache
+  @return 3  rows in both
+*/
+
+uint THD::has_pending_row_events()
+{
+  binlog_cache_mngr *cache_mngr;
+  if (!mysql_bin_log.is_open() ||
+      !(cache_mngr= binlog_get_cache_mngr()))
+    return 0;
+  return ((cache_mngr->stmt_cache.pending() ? 1 : 0) |
+          (cache_mngr->trx_cache.pending() ? 2 : 0));
+}
+
+
 /**
   This function removes the pending rows event, discarding any outstanding
   rows. If there is no pending rows event available, this is effectively a

sql/online_alter.cc

@@ -222,6 +222,8 @@ int online_alter_end_trans(Online_alter_cache_list &cache_list, THD *thd,
     {
       // Do not set STMT_END for last event to leave table open in altering thd
       error= binlog_flush_pending_rows_event(thd, false, true, binlog, &cache);
+      if (error)
+        do_commit= commit= 0;
     }
 
     if (do_commit)

sql/sql_base.cc

@@ -933,7 +933,7 @@ int close_thread_tables(THD *thd)
 
       Note that even if we are in LTM_LOCK_TABLES mode and statement
       requires prelocking (e.g. when we are closing tables after
-      failing ot "open" all tables required for statement execution)
+      failing at "open" all tables required for statement execution)
       we will exit this function a few lines below.
     */
     if (! thd->lex->requires_prelocking())
@@ -964,16 +964,13 @@ int close_thread_tables(THD *thd)
   if (thd->lock)
   {
     /*
-      For RBR we flush the pending event just before we unlock all the
-      tables.  This means that we are at the end of a topmost
-      statement, so we ensure that the STMT_END_F flag is set on the
-      pending event.  For statements that are *inside* stored
-      functions, the pending event will not be flushed: that will be
-      handled either before writing a query log event (inside
-      binlog_query()) or when preparing a pending event.
+      Ensure binlog caches has been flushed in binlog_query() or
+      binlog_commit().
     */
-    (void)thd->binlog_flush_pending_rows_event(TRUE);
-    error= mysql_unlock_tables(thd, thd->lock);
+    DBUG_ASSERT((thd->state_flags & Open_tables_state::BACKUPS_AVAIL) ||
+                !thd->has_pending_row_events());
+    if (mysql_unlock_tables(thd, thd->lock))
+      error= 1;
     thd->lock=0;
   }
   /*

sql/sql_class.cc

@@ -7553,10 +7553,11 @@ int THD::binlog_flush_pending_rows_event(bool stmt_end, bool is_transactional)
   if (variables.option_bits & OPTION_GTID_BEGIN)
     is_transactional= 1;
 
-  auto *cache_mngr= binlog_get_cache_mngr();
+  binlog_cache_mngr *cache_mngr= binlog_get_cache_mngr();
   if (!cache_mngr)
     DBUG_RETURN(0);
-  auto *cache= binlog_get_cache_data(cache_mngr,
+  binlog_cache_data *cache=
+    binlog_get_cache_data(cache_mngr,
                           use_trans_cache(this, is_transactional));
 
   int error=
@@ -7852,22 +7853,14 @@ int THD::binlog_query(THD::enum_binlog_query_type qtype, char const *query_arg,
   }
 
   /*
-    If we are not in prelocked mode, mysql_unlock_tables() will be
-    called after this binlog_query(), so we have to flush the pending
-    rows event with the STMT_END_F set to unlock all tables at the
-    slave side as well.
-
-    If we are in prelocked mode, the flushing will be done inside the
-    top-most close_thread_tables().
+    We should not flush row events for sub-statements, like a trigger or
+    function. The main statement will take care of the flushing when
+    calling binlog_query().
   */
-  if (this->locked_tables_mode <= LTM_LOCK_TABLES)
-  {
-    int error;
-    if (unlikely(error= binlog_flush_pending_rows_event(TRUE, is_trans)))
+  if (!in_sub_stmt)
   {
-      DBUG_ASSERT(error > 0);
-      DBUG_RETURN(error);
-    }
+    if (unlikely(binlog_flush_pending_rows_event(TRUE, is_trans)))
+      DBUG_RETURN(my_errno);          // Return error code as required
   }
 
   /*

sql/sql_class.h

@@ -3279,7 +3279,8 @@ class THD: public THD_count, /* this must be first */
             binlog_flush_pending_rows_event(stmt_end, TRUE));
   }
   int binlog_flush_pending_rows_event(bool stmt_end, bool is_transactional);
-
+  void binlog_remove_rows_events();
+  uint has_pending_row_events();
   bool binlog_need_stmt_format(bool is_transactional) const
   {
     if (!log_current_statement())

sql/sql_insert.cc

@@ -5342,14 +5342,15 @@ void select_create::abort_result_set()
   thd->transaction->stmt.modified_non_trans_table= FALSE;
   thd->variables.option_bits= save_option_bits;
 
-  /* possible error of writing binary log is ignored deliberately */
-  (void) thd->binlog_flush_pending_rows_event(TRUE, TRUE);
-
   if (table)
   {
     bool tmp_table= table->s->tmp_table;
     bool table_creation_was_logged= (!tmp_table ||
                                      table->s->table_creation_was_logged);
+
+    /* CREATE SELECT failed. Remove all row events and clear caches */
+    thd->binlog_remove_rows_events();
+
     if (tmp_table)
     {
       DBUG_ASSERT(saved_tmp_table_share);