Commit 421973ec authored by Guilhem Bichot's avatar Guilhem Bichot

Fix for BUG#39710 "Maria assertion in maria_disable_non_unique_index".

No testcase, this requires concurrency and is automatically tested by
maria_bulk_insert.yy in pushbuild2.

storage/maria/ha_maria.cc:
  The case of BUG#39710 is:
  two threads want to INSERT SELECT into the same table.
  Thread1 (T1) starts, does external_lock, thr_lock (store_lock sees 0 records so
  upgrades to TL_WRITE), goes into bulk insert, starts writes
  T2 starts, external_lock, thr_lock (store_lock sees 0 records so
  upgrades to TL_WRITE), blocks on existing thr_lock of T1.
  T1 ends writes, ends bulk insert, commits (ha_maria::implicit_commit()
  at end of dispatch_command()), external_lock and thr_unlock
  (close_thread_tables() at end of dispatch_command()).
  T2 wakes up, gets thr_lock, goes into start_bulk_insert() where
  file->state is out-of-date and still says that file->state->records==0,
  so maria_disable_non_unique_index() is called, which asserts because
  the actual number of records (share->state.state.records) is >0.
  The solution, maybe temporary, is to also check share->state.state.records==0
  when deciding to do bulk insert, with the idea that such operation cannot
  rely on the view of the start of the transaction, as it uses repair,
  and can safely read share->state as it has acquired the exclusive
  TL_WRITE.
  Question for reviewer: if we enter the if() branch, do we also need to do:
  *(file->state)= share->state.state;
  or even call some existing function which does that?
parent d44e00e7
......@@ -1795,6 +1795,7 @@ void ha_maria::start_bulk_insert(ha_rows rows)
THD *thd= current_thd;
ulong size= min(thd->variables.read_buff_size,
(ulong) (table->s->avg_row_length * rows));
MARIA_SHARE *share= file->s;
DBUG_PRINT("info", ("start_bulk_insert: rows %lu size %lu",
(ulong) rows, size));
......@@ -1802,8 +1803,8 @@ void ha_maria::start_bulk_insert(ha_rows rows)
if (!rows || (rows > MARIA_MIN_ROWS_TO_USE_WRITE_CACHE))
maria_extra(file, HA_EXTRA_WRITE_CACHE, (void*) &size);
can_enable_indexes= (maria_is_all_keys_active(file->s->state.key_map,
file->s->base.keys));
can_enable_indexes= (maria_is_all_keys_active(share->state.key_map,
share->base.keys));
bulk_insert_single_undo= BULK_INSERT_NONE;
if (!(specialflag & SPECIAL_SAFE_MODE))
......@@ -1815,8 +1816,17 @@ void ha_maria::start_bulk_insert(ha_rows rows)
we don't want to update the key statistics based of only a few rows.
Index file rebuild requires an exclusive lock, so if versioning is on
don't do it (see how ha_maria::store_lock() tries to predict repair).
We can repair index only if we have an exclusive (TL_WRITE) lock. To
see if table is empty, we shouldn't rely on the old records' count from
our transaction's start (if that old count is 0 but now there are
records in the table, we would wrongly destroy them).
So we need to look at share->state.state.records.
As a safety net for now, we don't remove the test of
file->state->records, because there is uncertainty on what will happen
during repair if the two states disagree.
*/
if (file->state->records == 0 && can_enable_indexes &&
if ((file->state->records == 0) &&
(share->state.state.records == 0) && can_enable_indexes &&
(!rows || rows >= MARIA_MIN_ROWS_TO_DISABLE_INDEXES) &&
(file->lock.type == TL_WRITE))
{
......@@ -1825,7 +1835,7 @@ void ha_maria::start_bulk_insert(ha_rows rows)
is more costly (flushes, syncs) than a row write.
*/
maria_disable_non_unique_index(file, rows);
if (file->s->now_transactional)
if (share->now_transactional)
{
bulk_insert_single_undo= BULK_INSERT_SINGLE_UNDO_AND_NO_REPAIR;
write_log_record_for_bulk_insert(file);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment