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

page_zip_dir_insert() may corrupt ROW_FORMAT=COMPRESSED tables

Details

    Description

      A data corruption bug that affects ROW_FORMAT=COMPRESSED tables was found by RQG testing:

      mysqld: /data/Server/bb-10.6-markoO/storage/innobase/row/row0purge.cc:208: bool row_purge_remove_clust_if_poss_low(purge_node_t*, ulint): Assertion `rec_get_deleted_flag(rec, rec_offs_comp(offsets))' failed.
      

      The cause turned out to be corruption that had been introduced in page_zip_dir_insert() before the server had been killed and restarted. After the following fix had been applied, this assertion was not seen again:

      diff --git a/storage/innobase/page/page0zip.cc b/storage/innobase/page/page0zip.cc
      index f1da35d12ec..959256bf105 100644
      --- a/storage/innobase/page/page0zip.cc
      +++ b/storage/innobase/page/page0zip.cc
      @@ -4248,8 +4248,13 @@ page_zip_dir_insert(
       	}
       
       	/* Write the entry for the inserted record.
      -	The "owned" and "deleted" flags must be zero. */
      -	mach_write_to_2(slot_rec - PAGE_ZIP_DIR_SLOT_SIZE, page_offset(rec));
      +	The "owned" flag must be zero. */
      +	uint16_t offs = page_offset(rec);
      +	if (rec_get_deleted_flag(rec, true)) {
      +		offs |= PAGE_ZIP_DIR_SLOT_DEL;
      +	}
      +
      +	mach_write_to_2(slot_rec - PAGE_ZIP_DIR_SLOT_SIZE, offs);
       	mtr->zmemcpy(*cursor->block, slot_rec - page_zip->data
       		     - PAGE_ZIP_DIR_SLOT_SIZE, PAGE_ZIP_DIR_SLOT_SIZE);
       }
      

      It turns out that btr_cur_pessimistic_update() may invoke this function on a delete-marked record. The scenario involved ROLLBACK. I did not examine it deeper, but I suspect that a purgeable delete-marked record had been "resurrected" by an insert that was executed as delete-unmarking the record. Either the test deleted and later inserted the same record, or it updated a PRIMARY KEY value back and forth.

      We failed to create a test case for this.

      Attachments

        Issue Links

          Activity

            After some deeper analysis, I came to the conclusion that this bug should only affect 10.5 and later releases. Before MDEV-12353, modifications to ROW_FORMAT=COMPRESSED pages would be recovered based on the uncompressed page contents, and this error would have been corrected later in page_zip_write_rec(). An additional patch for 10.5 could be necessary:

            diff --git a/storage/innobase/page/page0zip.cc b/storage/innobase/page/page0zip.cc
            index 331ecbfb0c4..864c04e95d7 100644
            --- a/storage/innobase/page/page0zip.cc
            +++ b/storage/innobase/page/page0zip.cc
            @@ -3676,6 +3676,7 @@ void page_zip_write_rec(buf_block_t *block, const byte *rec,
             
             	slot = page_zip_dir_find(page_zip, page_offset(rec));
             	ut_a(slot);
            +	byte s = *slot;
             	/* Copy the delete mark. */
             	if (rec_get_deleted_flag(rec, TRUE)) {
             		/* In delete-marked records, DB_TRX_ID must
            @@ -3683,11 +3684,12 @@ void page_zip_write_rec(buf_block_t *block, const byte *rec,
             		On non-leaf pages, the delete-mark flag is garbage. */
             		ut_ad(!index->is_primary() || !page_is_leaf(page)
             		      || row_get_rec_trx_id(rec, index, offsets));
            -		*slot |= PAGE_ZIP_DIR_SLOT_DEL >> 8;
            +		s |= PAGE_ZIP_DIR_SLOT_DEL >> 8;
             	} else {
            -		*slot &= byte(~(PAGE_ZIP_DIR_SLOT_DEL >> 8));
            +		s &= byte(~(PAGE_ZIP_DIR_SLOT_DEL >> 8));
             	}
             
            +	mtr->write<1,mtr_t::MAYBE_NOP>(*block, slot, s);
             	ut_ad(rec_get_start((rec_t*) rec, offsets) >= page + PAGE_ZIP_START);
             	ut_ad(rec_get_end((rec_t*) rec, offsets) <= page + srv_page_size
             	      - PAGE_DIR - PAGE_DIR_SLOT_SIZE
            

            marko Marko Mäkelä added a comment - After some deeper analysis, I came to the conclusion that this bug should only affect 10.5 and later releases. Before MDEV-12353 , modifications to ROW_FORMAT=COMPRESSED pages would be recovered based on the uncompressed page contents, and this error would have been corrected later in page_zip_write_rec() . An additional patch for 10.5 could be necessary: diff --git a/storage/innobase/page/page0zip.cc b/storage/innobase/page/page0zip.cc index 331ecbfb0c4..864c04e95d7 100644 --- a/storage/innobase/page/page0zip.cc +++ b/storage/innobase/page/page0zip.cc @@ -3676,6 +3676,7 @@ void page_zip_write_rec(buf_block_t *block, const byte *rec, slot = page_zip_dir_find(page_zip, page_offset(rec)); ut_a(slot); + byte s = *slot; /* Copy the delete mark. */ if (rec_get_deleted_flag(rec, TRUE)) { /* In delete-marked records, DB_TRX_ID must @@ -3683,11 +3684,12 @@ void page_zip_write_rec(buf_block_t *block, const byte *rec, On non-leaf pages, the delete-mark flag is garbage. */ ut_ad(!index->is_primary() || !page_is_leaf(page) || row_get_rec_trx_id(rec, index, offsets)); - *slot |= PAGE_ZIP_DIR_SLOT_DEL >> 8; + s |= PAGE_ZIP_DIR_SLOT_DEL >> 8; } else { - *slot &= byte(~(PAGE_ZIP_DIR_SLOT_DEL >> 8)); + s &= byte(~(PAGE_ZIP_DIR_SLOT_DEL >> 8)); } + mtr->write<1,mtr_t::MAYBE_NOP>(*block, slot, s); ut_ad(rec_get_start((rec_t*) rec, offsets) >= page + PAGE_ZIP_START); ut_ad(rec_get_end((rec_t*) rec, offsets) <= page + srv_page_size - PAGE_DIR - PAGE_DIR_SLOT_SIZE

            People

              marko Marko Mäkelä
              marko Marko Mäkelä
              Votes:
              0 Vote for this issue
              Watchers:
              1 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.