[MDEV-14310] Possible corruption by table-rebuilding or index-creating ALTER TABLE…ALGORITHM=INPLACE Created: 2017-11-07  Updated: 2017-11-20  Resolved: 2017-11-20

Status: Closed
Project: MariaDB Server
Component/s: Storage Engine - InnoDB
Affects Version/s: 10.2.11, 10.3.3
Fix Version/s: 10.2.11, 10.3.3

Type: Bug Priority: Blocker
Reporter: Marko Mäkelä Assignee: Marko Mäkelä
Resolution: Fixed Votes: 0
Labels: corruption, ddl, performance

Attachments: File 0001-5.7-Follow-up-fix-to-MDEV-13328.patch     File 0001-Remove-redundant-function-parameters.patch     File 0002-MDEV-13328-ALTER-TABLE-DISCARD-TABLESPACE-takes-a-lo.patch     File 5.7-MDEV-13328.patch    
Issue Links:
Relates
relates to MDEV-13328 ALTER TABLE ... DISCARD TABLESPACE ta... Closed
relates to MDEV-14317 When ALTER TABLE is aborted, do not w... Closed

 Description   

When I merged MDEV-13328 from 10.1 to 10.2, the innodb.innodb test suddenly started to fail.

The culprit seemed to be a correct-looking conflict resolution in FlushObserver::flush(). As part of the test, innodb.innodb is executing the following:

let $default=`select @@storage_engine`;
set storage_engine=INNODB;
source include/varchar.inc;

This includes the following:

--error ER_DUP_ENTRY
alter table t1 add unique(v);

The duplicate key error triggers the following code at the end of row_merge_build_indexes():

		if (error != DB_SUCCESS) {
			flush_observer->interrupted();
		}
 
		flush_observer->flush();

This in turn causes a choice of a dangerous parameter:

/** Flush dirty pages and wait. */
void
FlushObserver::flush()
{
/** Flush dirty pages and wait. */
void
FlushObserver::flush()
{
	buf_remove_t	buf_remove;
 
	if (m_interrupted) {
		buf_remove = BUF_REMOVE_FLUSH_NO_WRITE;
	} else {
		buf_remove = BUF_REMOVE_FLUSH_WRITE;
	}
 
	/* Flush or remove dirty pages. */
	buf_LRU_flush_or_remove_pages(m_space_id, buf_remove, m_trx);

The danger here is that m_space_id=0, the system tablespace. This is potentially discarding other writes to the InnoDB system tablespace, potentially corrupting the whole instance.

We must use the equivalent of BUF_REMOVE_FLUSH_WRITE for the system tablespace (and in MySQL 5.7, for any table that resides in a persistent shared tablespace). Failure to do so caused all sorts of trouble when running innodb.innodb after the merge, especially when using --innodb-buffer-pool-size=5m (the minimum).
Apparently the corruption was caused by the following sequence of events:

  1. Some pages in the system tablespace are modified.
  2. The ADD UNIQUE INDEX operation runs and fails, and finally removes system tablespace pages from the buf_pool->flush_list without writing to the file (see above).
  3. Some affected not-written-back pages are evicted from the buffer pool.
  4. A page is eventually needed and read back to the buffer pool, with too old contents, or with all-zero contents.

Various assertions failed due to a supposedly-initialized page being all-zero.

Note: For a failed ALTER TABLE in a tablename.ibd file, it is perfectly OK to discard the entries from the flush_list, and to subsequently mark the pages as freed, or to delete the file (if it was a table-rebuilding ALTER).



 Comments   
Comment by Marko Mäkelä [ 2017-11-07 ]

Actually, the impact of this bug is more widespread than I initially thought.
During online table-rebuilding ALTER (say, DROP COLUMN, or ADD COLUMN before the instant variant MDEV-11369 in MariaDB 10.3.2), the code could discard changes from INSERT, UPDATE and other statements that were run concurrently with the failed or killed ALTER TABLE statement.

Also locking ALGORITHM=INPLACE operations with innodb_file_per_table=1 may be affected, because LOCK=SHARED or LOCK=EXCLUSIVE does not prevent purge or change buffer merge from running.

Any ALGORITHM=INPLACE operation that involves ADD UNIQUE INDEX or ADD INDEX or rebuilding the table is potentially affected.

Comment by Marko Mäkelä [ 2017-11-20 ]

This turned out to be a merge error of MDEV-13328.
Upstream (MySQL 5.7) is not affected by this.

Comment by Marko Mäkelä [ 2017-11-20 ]

I tested upstream (MySQL 5.7) by porting two commits from MariaDB 10.1 to MySQL 5.6 (0001-Remove-redundant-function-parameters.patch, 0002-MDEV-13328-ALTER-TABLE-DISCARD-TABLESPACE-takes-a-lo.patch) and then merging to 5.7 as 5.7-MDEV-13328.patch. The problem is fixed by 0001-5.7-Follow-up-fix-to-MDEV-13328.patch which refactors FlushObserver and some related interfaces.

Generated at Thu Feb 08 08:12:33 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.