Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-14310

Possible corruption by table-rebuilding or index-creating ALTER TABLE…ALGORITHM=INPLACE

Details

    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).

      Attachments

        Issue Links

          Activity

            marko Marko Mäkelä added a comment - - edited

            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.

            marko Marko Mäkelä added a comment - - edited 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.

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

            marko Marko Mäkelä added a comment - This turned out to be a merge error of MDEV-13328 . Upstream (MySQL 5.7) is not affected by this.

            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.

            marko Marko Mäkelä added a comment - 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.

            People

              marko Marko Mäkelä
              marko Marko Mäkelä
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.