| Bug #99021 | BUF_REMOVE_ALL_NO_WRITE is not needed for undo tablespace | ||
|---|---|---|---|
| Submitted: | 22 Mar 2020 9:22 | Modified: | 23 Mar 2020 14:27 |
| Reporter: | Fungo Wang (OCA) | Email Updates: | |
| Status: | Closed | Impact on me: | |
| Category: | MySQL Server: InnoDB storage engine | Severity: | S3 (Non-critical) |
| Version: | 8.0.19 | OS: | Any |
| Assigned to: | CPU Architecture: | Any | |
| Tags: | BUF_REMOVE_ALL_NO_WRITE, LRU, undo | ||
[23 Mar 2020 4:47]
Sunny Bains
The undo truncate has been solved in a more elegant manner. In the solution that will be available in the next release there is no need to scan any list.
[23 Mar 2020 5:30]
Fungo Wang
That's cool if no buffer pool list will be scanned at undo truncation , can not wait for 8.0.20 :) Thanks!
[23 Mar 2020 14:08]
Satya Bodapati
its good to know that the LRU list iteration is avoided in next release! Looked further, Coming to this bug, it is not completely true that BUF_REMOVE_ALL_NO_WRITE is wrong. We need to remove from LRU list as well but without the buf_LRU_drop_page_hash_for_tablespace(). So calling buf_LRU_remove_pages() directly with BUF_REMOVE_ALL_NO_WRITE is correct.
[23 Mar 2020 14:14]
MySQL Verification Team
Thank you all. Closed since this optimisation has been already pushed to the next patch-fix-release.
[23 Mar 2020 14:27]
Fungo Wang
Hi Satya, I guess it's safe to keep old clean buf page in the LRU list, even if undo space_id could be reused. Say if an undo page (4294967160, 5) is clean when truncate undo space 4294967160, and this buf page is left in the LRU list. The next round when a new undo space reuse the space id 4294967160, and the first time to access buf page (4294967160, 5) is through fsp_page_create() -> buf_page_create() -> fsp_init_file_page(block, init_mtr); fsp_init_file_page will reset the page frame. Or I maybe miss something that needs scanning the LRU list? Thanks.
[24 Mar 2020 3:19]
Satya Bodapati
It is necessary only if "space_id" is reused. Otherwise, the pages in LRU will "age" out. Incase of undo, space_id is reused (after 127 truncates?) So this needs LRU eviction. So if there is no space_id confilct in buf pool and no AHI, there is no need to scan any list in buf pool. May be this is what Sunny upcoming solution will rely on.
[6 Jul 2020 13:11]
Satya Bodapati
I see that 8.0.20 code has the same issue (uses BUF_REMOVE_ALL_NO_WRITE). Can the MySQL team reopen this? If not, please set to the correct version that has the fix?
[6 Jul 2020 15:13]
MySQL Verification Team
Satya, I think that it is in 8.0.21. When it is out, can you, please, let me know ......
[7 Jul 2020 5:13]
Satya Bodapati
ok, I can do that. Strange that you are asking me to verify. Wouldn't you know from the 8.0.21 release notes? or from devs?
[7 Jul 2020 12:25]
MySQL Verification Team
Actually, no, since a fix is not based on the bug, but on the WorkLog entry, whose number we truly do not know.
Description: As AHI is not used for undo tablespace, so there is no need to use BUF_REMOVE_ALL_NO_WRITE (which will scan LUR list) when delete/truncate undo tablespace, BUF_REMOVE_FLUSH_NO_WRITE (which will scan FLUSH list, and which is normally much shorter than LRU, also scan with mutex yield) is enough for such undo operation, and much more efficient/elegant. How to repeat: Check the related code logic: 1. undo tablespace truncate by purge trx_undo_truncate_tablespace() -> fil_replace_tablespace() -> fil_delete_tablespace(old_space_id, BUF_REMOVE_ALL_NO_WRITE) 2. undo tablespace upgrade srv_undo_tablespaces_upgrade() -> fil_delete_tablespace(undo_space.id(), BUF_REMOVE_ALL_NO_WRITE) 3. undo tablespace drop innodb_drop_undo_tablespace() -> buf_LRU_flush_or_remove_pages(space_id, BUF_REMOVE_ALL_NO_WRITE, NULL); Suggested fix: diff --git a/storage/innobase/fil/fil0fil.cc b/storage/innobase/fil/fil0fil.cc index 030ee54..63b5a85 100644 --- a/storage/innobase/fil/fil0fil.cc +++ b/storage/innobase/fil/fil0fil.cc @@ -4347,7 +4347,7 @@ bool fil_replace_tablespace(space_id_t old_space_id, space_id_t new_space_id, std::string file_name(space->files.front().name); /* Delete the old file and space object. */ - dberr_t err = fil_delete_tablespace(old_space_id, BUF_REMOVE_ALL_NO_WRITE); + dberr_t err = fil_delete_tablespace(old_space_id, BUF_REMOVE_FLUSH_NO_WRITE); if (err != DB_SUCCESS) { return (false); } diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 75c2b84..01e4977 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -15373,7 +15373,7 @@ static int innodb_drop_undo_tablespace(handlerton *hton, THD *thd, /* Invalidate buffer pool pages belonging to this undo tablespace before dropping it. */ - buf_LRU_flush_or_remove_pages(space_id, BUF_REMOVE_ALL_NO_WRITE, NULL); + buf_LRU_flush_or_remove_pages(space_id, BUF_REMOVE_FLUSH_NO_WRITE, NULL); /* Empty and inactive, take it out of view. */ std::string file_name{undo_space->file_name()}; diff --git a/storage/innobase/srv/srv0start.cc b/storage/innobase/srv/srv0start.cc index 9ed698d..0382b5b 100644 --- a/storage/innobase/srv/srv0start.cc +++ b/storage/innobase/srv/srv0start.cc @@ -1361,7 +1361,7 @@ dberr_t srv_undo_tablespaces_upgrade() { fil_space_close(undo_space.id()); - auto err = fil_delete_tablespace(undo_space.id(), BUF_REMOVE_ALL_NO_WRITE); + auto err = fil_delete_tablespace(undo_space.id(), BUF_REMOVE_FLUSH_NO_WRITE); if (err != DB_SUCCESS) { ib::warn(ER_IB_MSG_57_UNDO_SPACE_DELETE_FAIL, undo_space.space_name());