innodb_scrub_background done. Not yet sure what is wrong with two others.

I do not know, sometimes test passes for 5 times and then fails. Sometimes less.

encryption.innodb_scrub fails to do a slow shutdown, and it unnecessarily does multiple shutdowns while it could easily test all combinations of row_format and operations with a single restart.
Once I fixed that, I get a test failure once in 10 to 20 runs. I believe that that is due to the shutdown bug MDEV-11638.

While cleaning up encryption.innodb_scrub_background I noticed some problems with the background scrubbing itself:

1. Undo log pages fail to get scrubbed. The data for INDEX(b) is clearly visible. This is repeatable in encryption.innodb_scrub by adding INDEX(b) to each table.
2. Sometimes, background scrubbing fails also for tables.

bb-10.1-mdev-8139

ok to push

The test encryption.innodb_scrub is now enabled in MariaDB 10.1.21. It is still disabled in MariaDB 10.2.4, because the end of page 2 (inode list page) of delete_3.ibd randomly gets corrupted with the string "repairmanun". That string should only have been written to B-tree pages, not to an allocation metadata page. This looks like a sign of a serious corruption bug in MariaDB 10.2.

The test encryption.innodb_scrub_background demonstrates that more work is needed in the scrubbing. Clearly, undo pages are not being scrubbed as they should, and background scrubbing of data files does not work reliably.

Even though I pushed some commits related to the tests, this issue must remain open until all unresolved issues have been addressed.

The test below (reduced from encryption.innodb_scrub) demonstrates 2 problems in 10.2.4:

1. despite ROW_FORMAT=COMPRESSED, delete_3.ibd contains the strings "infimum" and "supremum" which should not be present in that format (see page_zip_compress())
2. At byte offset 0x9ff5 to 0x9fff (when using innodb_page_size=16k) we sometimes get the string "repairmanun". This seems to be at the very end of a page where we should have some page trailer (at the minimum, 4 bytes for the LSN). Index pages would additionally have a directory of records at the end of a page. It is as if some bytes were written to the wrong page.

-- source include/have_innodb.inc

-- source include/not_embedded.inc

-- source include/have_example_key_management_plugin.inc

let $MYSQLD_DATADIR=`select @@datadir`;

let $t= delete_3;

eval create table $t (

  a int auto_increment primary key,

  b varchar(256),

  c text) engine = innodb row_format=compressed;

let $numinserts = 500;

--disable_query_log

begin;

while ($numinserts)

{

  dec $numinserts;

  eval insert into $t(b,c) values ('repairman', repeat('unicycle', 1000));

}

commit;

--enable_query_log

eval delete from $t;

SET GLOBAL innodb_fast_shutdown=0;

-- source include/shutdown_mysqld.inc

let SEARCH_ABORT= FOUND;

let SEARCH_PATTERN= (un|b|tr)icycle|(repair|breakhu|wonderwo)man;

let SEARCH_RANGE= 12582912;

let SEARCH_FILE= $MYSQLD_DATADIR/test/$t.ibd;

-- source include/search_pattern_in_file.inc

-- source include/start_mysqld.inc

eval drop table $t;

Note: Remember to copy encryption/t/innodb_scrub.opt if you write the above test into a new file.

I am able to repeat the "repairmanun" problem on MariaDB 10.1.21. I can also see "infimum" and "supremum" strings in the file. FSP_SPACE_FLAGS contains 0x29 both in 10.1 and 10.2.

I think that the scrubbing feature should be an integral part of the purge subsystem. Alas, as noted in MDEV-11802, purge does not always run when there is work to do.

We should redesign this in a later version (10.3 or 10.4) where MDEV-12288 and MDEV-13536 are available.

I believe that the scrubbing as it is implemented in MariaDB 10.1 to 10.3 is incomplete and cannot be fixed easily.

I think that the correct way to implement scrubbing would be to introduce a new redo log record MLOG_PAGE_FREE for marking a page freed. It would be written in the same mini-transaction that marks the page freed in the allocation bitmap.

We’d then modify the page flushing logic so that it would (when scrubbing is enabled) write zeroes or (for page_compressed=1 files) punch holes for the freed pages.

Additionally, we should consider getting rid of the mechanism that delays the deallocation and allocation of undo pages. Currently, there effectively is a separate page allocator for undo log pages, which does not zero out any preceding garbage on the pages when they are reused.

I suspect that there could also be a race condition between log scrubbing and writing. Specifically, I do not see where log_scrub_thread() would ensure that it is only overwriting log until the latest checkpoint. The log scrubbing could be implemented better after MDEV-14425, perhaps simply by punching a hole on the no-longer-needed portion of the write-ahead-log.

In MariaDB Server 10.5.2, MDEV-21870 deprecated and ignored the parameter innodb_scrub_log.

The function btr_page_free_low() was scrubbing pages in an inefficent and backup-unsafe way. Starting with MariaDB Server 10.2.24 and 10.3.15, it will write an appropriate redo log record for initializing the page.

MDEV-15528 in MariaDB 10.4 or later could make scrubbing mostly obsolete.

The scope of MDEV-15528 was reduced in order to get the necessary redo log format changes tested before the MariaDB Server 10.5 feature freeze. The rest of it will have to be done as part of this ticket (see the updated Description), to fix the scrubbing feature.

The shrinking of a file by ftruncate() (when all last pages of the file are freed) might block all concurrent access to the file. I think that we must do the ftruncate() when scrubbing is enabled. When scrubbing is disabled, we may choose to not shrink files, depending on what benchmarks indicate.

A quick review:

• mtr.set_named_space(rseg->space); is not needed, because the file names of undo tablespaces are hard-coded.
• clear_freed_ranges() should be merged to release_resources(), and we should also clear m_freed_in_system_tablespace.
• mtr_t::commit_files() must assert ut_ad(m_freed_ranges.empty()); ut_ad(!m_freed_in_system_tablespace);
• Do we really need the btr_invalidate_index_id() function? I do not think it is needed in btr_create() at all. In btr_free_if_exists(), we must skip that call for page_compressed tables (because hole-punching will be done) or if we know that scrubbing will be enabled when the page is flushed.

The interesting part of the patch is the call buf_flush_freed_pages(space, buf_flush_get_freed_pages(space)), which I would suggest to write as a single function. The preceding comment confused me, because it mentioned neighbor flushing, which was disabled on SSD by MDEV-17380, and I would expect HDD to represent a minority of the users soon. I think that we should perform that call irrespective of flush_type, no matter if we traverse buf_pool.LRU or buf_pool.flush_list. (Single-page flushing is an exception, because it should always be followed by page eviction.)

Also, we should invoke ftruncate() (after writing a TRIM_PAGES log record) when the last pages of a tablespace are freed. The recovery of TRIM_PAGES must be extended to cover all tablespaces, not only undo tablespaces. Related to this, a TODO comment about PUNCH_HOLE must be removed from trx_purge_truncate_history().

origin/bb-10.5-MDEV-8139 fd2c45631a65d3d0803a2445b965d51ecc147a34 2020-06-10T19:16:25+05:30
behaved very well when the RQG test battery for broad range functional coverage was executed.
A quite small fraction of tests failed because of already problems which are known
and affect actual 10.5 too.

Pushed the patch in

commit c92f7e287fc0e21dc1b181284b1f8e2139d1c331 (HEAD -> 10.5, origin/10.5, bb-10.5-MDEV-8139)

Author: Thirunarayanan Balathandayuthapani <thiru@mariadb.com>

Date:   Thu Jun 11 22:52:47 2020 +0530

    MDEV-8139 Fix Scrubbing

The feature was disabled in the MariaDB 10.5.4 release due to concerns over data corruption (MDEV-22970).

Bad effect 3. from above replays on actual 10.5 too.

Patch is in bb-10.5-thiru. Please take a look

I posted some review comments. It looks OK, except that crash recovery fails to scrub freed pages in page_compressed tables when innodb_immediate_scrub_data_uncompressed=OFF (which is the default setting).

OK to push after addressing my comments.