Skip to content

MDEV-40332 InnoDB: Defragmentation of BASE_IDX in SYS_VIRTUAL failed: Data structure corruption#5385

Open
Thirunarayanan wants to merge 1 commit into
11.4from
MDEV-40332
Open

MDEV-40332 InnoDB: Defragmentation of BASE_IDX in SYS_VIRTUAL failed: Data structure corruption#5385
Thirunarayanan wants to merge 1 commit into
11.4from
MDEV-40332

Conversation

@Thirunarayanan

Copy link
Copy Markdown
Member

Problem:

  • While shrinking the system tablespace, InnoDB defragments the system tables to move used pages out of the extents near the end of the file so the tail can be truncated. defragment_level() relocates every page of a to-be-moved extent and, to do so, rewrites the node pointer in the page's parent.

A B-tree root page has no parent node pointer, so it is never recorded in m_parent_pages.
When the root of a system-table index happens to reside in an extent that was selected for relocation, the parent lookup fails and defragmentation is aborted with DB_CORRUPTION even though nothing is corrupt.

Solution:

Because a root page cannot move, the system tablespace cannot
shrink below the highest root page, and relocating any page at or below it cannot reduce the file size.
Exclude that region from relocation up front.

SpaceDefragmenter::max_root_extent(): New function returning the highest extent that holds a root page of a system-table index

SpaceDefragmenter::find_new_extents(): Use max_root_extent() together with the minimum tablespace size as the lower bound (floor) of the relocation scan, so no extent at or below the highest root is ever added to the relocation map.
This avoids the false corruption and also avoids reserving a destination extent for a move that would never happen. The free tail above the highest root is still reclaimed by truncation.

… Data structure corruption

Problem:
========
 - While shrinking the system tablespace, InnoDB defragments the
system tables to move used pages out of the extents
near the end of the file so the tail can be truncated.
defragment_level() relocates every page of a to-be-moved extent
and, to do so, rewrites the node pointer in the page's parent.

A B-tree root page has no parent node pointer, so it is
never recorded in m_parent_pages.
When the root of a system-table index happens to reside in an extent
that was selected for relocation, the parent lookup fails and
defragmentation is aborted with DB_CORRUPTION even though nothing is
corrupt.

Solution:
=========
  Because a root page cannot move, the system tablespace cannot
shrink below the highest root page, and relocating any
page at or below it cannot reduce the file size.
Exclude that region from relocation up front.

SpaceDefragmenter::max_root_extent(): New function returning the
highest extent that holds a root page of a system-table index

SpaceDefragmenter::find_new_extents(): Use max_root_extent()
together with the minimum tablespace size as the lower bound
(floor) of the relocation scan, so no extent at or below the
highest root is ever added to the relocation map.
This avoids the false corruption and also avoids reserving a
destination extent for a move that would never happen.
The free tail above the highest root is still reclaimed by truncation.
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses MDEV-40332 by preventing the InnoDB system tablespace from shrinking below the highest B-tree root page of system tables, as root pages cannot be relocated. It introduces a max_root_extent() helper to calculate this boundary and includes a test case to verify the fix. However, a critical issue was identified in max_root_extent() where system tables like dict_sys.sys_virtual can be nullptr, potentially leading to a null pointer dereference and server crash. A null check should be added for each table before retrieving its indexes.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +5330 to +5339
for (dict_table_t *table :
{dict_sys.sys_tables, dict_sys.sys_columns, dict_sys.sys_indexes,
dict_sys.sys_fields, dict_sys.sys_foreign,
dict_sys.sys_foreign_cols, dict_sys.sys_virtual})
for (dict_index_t *index= dict_table_get_first_index(table);
index; index= dict_table_get_next_index(index))
if (index->page != FIL_NULL)
max_extent= std::max(max_extent,
(index->page / m_extent_size) * m_extent_size);
return max_extent;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The system tables in the initializer list, particularly dict_sys.sys_virtual (and potentially others like dict_sys.sys_foreign or dict_sys.sys_foreign_cols depending on the server configuration or initialization state), can be nullptr. Passing a nullptr to dict_table_get_first_index will result in a null pointer dereference and crash the server. A null check should be added for each table before retrieving its indexes.

    for (dict_table_t *table :
         {dict_sys.sys_tables, dict_sys.sys_columns, dict_sys.sys_indexes,
          dict_sys.sys_fields, dict_sys.sys_foreign,
          dict_sys.sys_foreign_cols, dict_sys.sys_virtual})
    {
      if (!table)
      {
        continue;
      }
      for (dict_index_t *index= dict_table_get_first_index(table);
           index; index= dict_table_get_next_index(index))
        if (index->page != FIL_NULL)
          max_extent= std::max(max_extent,
                               (index->page / m_extent_size) * m_extent_size);
    }
    return max_extent;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants