MDEV-38079: Crash Recovery Fails After ALTER TABLE…PAGE_COMPRESSED=1 #4586

mariadb-TafzeelShams · 2026-01-23T09:07:39Z

The Jira issue number for this PR is: MDEV-38079

Description

Issue:
Recovery fails because the expected space ID does not match the space
ID stored in the page.

Root Cause:

Before the crash, the nth page (n != 0) gets flushed to disk as a
compressed page.
Page 0 remains unflushed, and the compressed flag for the space is
made durable only in the redo logs.
During recovery, the compressed flag is first set to indicate a
compressed space.
Later, while applying redo logs, an earlier LSN may reset it to
non-compressed and then back to compressed.
If the nth page is read during this intermediate state, a compressed
page may be read as non-compressed, causing a space ID mismatch.

Fix:

recv_sys_t::space_flags_lsn : Added a map to track the last applied
LSN for each space and avoid stale updates from earlier LSNs.
recv_sys_t::update_space_flags() : Updates space->flags during
recovery only if the update comes from the latest LSN.

How can this PR be tested?

Tested equivalent 10.11 patch with the data directory mentioned in the Jira ticket.

Basing the PR against the correct MariaDB version

This is a new feature and the PR is based against the latest MariaDB development branch
This is a bug fix and the PR is based against the earliest branch in which the bug can be reproduced

Thirunarayanan · 2026-01-23T13:22:53Z

storage/innobase/log/log0recv.cc

-			case log_phys_t::APPLIED_TO_FSP_HEADER:
-				s->flags = mach_read_from_4(
+			case log_phys_t::APPLIED_TO_FSP_HEADER: {
+				uint32_t flags = mach_read_from_4(


we don't need this temporary variable "flags".

Thirunarayanan · 2026-01-23T13:27:31Z

storage/innobase/log/log0recv.cc

+  auto it = space_flags_lsn.find(space->id);
+  if (it == space_flags_lsn.end() || lsn >= it->second)
+  {
+    space->flags = flags;


There shouldn't be space before "="

Thirunarayanan · 2026-01-23T13:30:18Z

storage/innobase/log/log0recv.cc

 					FSP_HEADER_OFFSET + FSP_FREE
 					+ FLST_LEN + frame);
 				break;
+                        }


why do we need this curly brace?

This brace introduce a local scope, so flags is kept limited to the APPLIED_TO_FSP_HEADER case.

Thirunarayanan

Make sure to address the review comments.

Thirunarayanan · 2026-01-25T16:48:45Z

storage/innobase/log/log0recv.cc

-					FSP_HEADER_OFFSET
-					+ FSP_SPACE_FLAGS + frame);
+				recv_sys.update_space_flags(s,
+					mach_read_from_4(FSP_HEADER_OFFSET


recv_sys.update_space_flags( s, mach_read_from_4(FSP_HEADER_OFFSET + FSP_SPACE_FLAGS + frame), l->start_lsn);

Thirunarayanan

Approved with the code format changes

Issue: Recovery fails because the expected space ID does not match the space ID stored in the page. Root Cause: - Before the crash, the nth page (n != 0) gets flushed to disk as a compressed page. - Page 0 remains unflushed, and the compressed flag for the space is made durable only in the redo logs. - During recovery, the compressed flag is first set to indicate a compressed space. - Later, while applying redo logs, an earlier LSN may reset it to non-compressed and then back to compressed. - If the nth page is read during this intermediate state, a compressed page may be read as non-compressed, causing a space ID mismatch. Fix: - recv_sys_t::space_flags_lsn : Added a map to track the last applied LSN for each space and avoid stale updates from earlier LSNs. - recv_sys_t::update_space_flags() : Updates space->flags during recovery only if the update comes from the latest LSN.

dr-m

This is a step to the right direction, but I think that it needs a little more work. When it comes to changing FSP_SIZE, we can exploit the fact that the size never shrinks, except in undo tablespace truncation, whose recovery is handled separately. I believe that the LSN tracking must be kept strictly related to changes of FSP_SPACE_FLAGS.

dr-m · 2026-01-28T07:20:00Z

storage/innobase/include/fil0fil.h

 /** Set the recovered size of a tablespace in pages.
 @param	id	tablespace ID
 @param	size	recovered size in pages
-@param	flags	tablespace flags */
+@param	flags	tablespace flags
+@param	lsn	lsn of the redo log */
 void fil_space_set_recv_size_and_flags(ulint id, uint32_t size,
-                                       uint32_t flags) noexcept;
+                                       uint32_t flags, lsn_t lsn) noexcept;


The @param lsn comment is not adding any value. Which specific LSN does it refer to? Also the size and flags could be described more accurately. Let us use spaces instead of TAB also in the comment:

/** Set the recovered size and flags of a tablespace. @param id tablespace ID @param size recovered FSP_SIZE, or a less-than-equal value if no change @param flags recovered FSP_SPACE_FLAGS if applicable @param flags_lsn start LSN of modifying flags, or 0 */

This comment would also answer the question what would happen or how we would identify the situation when a mini-transaction is modifying only one of FSP_SIZE or FSP_SPACE_FLAGS.

dr-m · 2026-01-28T07:22:21Z

storage/innobase/include/log0recv.h

+  /** Last applied LSN for space->flags updates (used to prevent stale updates) */
+  std::map<uint32_t, lsn_t> space_flags_lsn;


This could significantly increase the memory footprint. Do we really need an ordered map? We already store similar information in file_name_t linked from recv_spaces.

Could we slightly repurpose fil_space_t::create_lsn for storing this information? I think that the name fil_space_t::format_lsn would also cover the meaning of create_lsn, since the format is usually determined at creation.

If the information is being read before a fil_space_t object is available, then it would seem possible to add file_name_t::format_lsn for storing this.

dr-m · 2026-01-28T07:27:43Z

storage/innobase/include/log0recv.h

+  /** Update space->flags only if lsn >= last recorded space-flags LSN */
+  void update_space_flags(fil_space_t *space, uint32_t flags, lsn_t lsn);


Which LSN? This comment should also somehow refer to fil_space_set_recv_size_and_flags().

dr-m · 2026-01-28T07:29:41Z

storage/innobase/log/log0recv.cc

+  auto it= space_flags_lsn.find(space->id);
+  if (it == space_flags_lsn.end() || lsn >= it->second)
+  {
+    space->flags= flags;
+    space_flags_lsn[space->id]= lsn;
+  }


Why not it->lsn= lsn;?

We are missing mysql_mutex_assert_owner(&recv_sys.mutex) here. The access to a shared data structure needs to be protected somehow.

dr-m · 2026-01-28T07:50:51Z

storage/innobase/log/log0recv.cc

 				if (f.size
 				    || f.flags != f.initial_flags) {
 					fil_space_set_recv_size_and_flags(
-						space->id, f.size, f.flags);
+						space->id, f.size, f.flags, lsn);
 				}


This is passing the start LSN of the mini-transaction of a FILE_ record, instead of the start LSN of the mini-transaction that actually modified FSP_SPACE_FLAGS.

dr-m · 2026-01-28T07:54:02Z

storage/innobase/log/log0recv.cc

              else if (!it->second.space)
              {
                if (has_size)
                  it->second.size= size;
                if (has_flags)
                  it->second.flags= flags;
              }
-              fil_space_set_recv_size_and_flags(space_id, size, flags);
+              fil_space_set_recv_size_and_flags(space_id, size, flags, start_lsn);


We may have to pass has_flags ? start_lsn : 0 to distinguish the case where the flags were not changed. Maybe the file_name_t::initial_flags could be removed altogether, now that we would keep track of the start_lsn of the mini-transaction that modifed FSP_SPACE_FLAGS, in a new field it->second.flags_lsn or it->second.format_lsn.

dr-m · 2026-01-28T07:57:48Z

storage/innobase/log/log0recv.cc

 			case log_phys_t::APPLIED_TO_FSP_HEADER:
-				s->flags = mach_read_from_4(
-					FSP_HEADER_OFFSET
-					+ FSP_SPACE_FLAGS + frame);
+				recv_sys.update_space_flags(
+				  s, mach_read_from_4(FSP_HEADER_OFFSET +
+						      FSP_SPACE_FLAGS + frame),
+				  l->start_lsn);


Could this code be moved to log_phys_t::apply() and the special return value APPLIED_TO_FSP_HEADER removed? That return value does not currently distinguish changes to FSP_SPACE_FLAGS and FSP_SIZE. That is, if a mini-transaction only modifies FSP_SIZE but not FSP_SPACE_FLAGS, we would execute some unnecessary code here.

mariadb-TafzeelShams added the MariaDB Corporation label Jan 23, 2026

mariadb-TafzeelShams requested review from Thirunarayanan and dr-m January 23, 2026 09:08

Thirunarayanan reviewed Jan 23, 2026

View reviewed changes

Thirunarayanan requested changes Jan 23, 2026

View reviewed changes

mariadb-TafzeelShams force-pushed the 10.6-MDEV-38079 branch from 5768c02 to 2393f5c Compare January 23, 2026 13:55

mariadb-TafzeelShams requested a review from Thirunarayanan January 23, 2026 13:58

Thirunarayanan reviewed Jan 25, 2026

View reviewed changes

Thirunarayanan approved these changes Jan 25, 2026

View reviewed changes

mariadb-TafzeelShams force-pushed the 10.6-MDEV-38079 branch 2 times, most recently from c0df4c5 to df9d316 Compare January 27, 2026 05:12

mariadb-TafzeelShams mentioned this pull request Jan 27, 2026

MDEV-38079: Crash Recovery Fails After ALTER TABLE…PAGE_COMPRESSED=1 #4565

Closed

3 tasks

mariadb-TafzeelShams force-pushed the 10.6-MDEV-38079 branch from df9d316 to b53d241 Compare January 27, 2026 08:43

dr-m requested changes Jan 28, 2026

View reviewed changes

		/** Last applied LSN for space->flags updates (used to prevent stale updates) */
		std::map<uint32_t, lsn_t> space_flags_lsn;

		/** Update space->flags only if lsn >= last recorded space-flags LSN */
		void update_space_flags(fil_space_t *space, uint32_t flags, lsn_t lsn);

Uh oh!

MDEV-38079: Crash Recovery Fails After ALTER TABLE…PAGE_COMPRESSED=1 #4586

Are you sure you want to change the base?

MDEV-38079: Crash Recovery Fails After ALTER TABLE…PAGE_COMPRESSED=1 #4586

Conversation

mariadb-TafzeelShams commented Jan 23, 2026

Description

How can this PR be tested?

Basing the PR against the correct MariaDB version

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mariadb-TafzeelShams Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Thirunarayanan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Thirunarayanan left a comment

Choose a reason for hiding this comment

Uh oh!

dr-m left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

mariadb-TafzeelShams Jan 23, 2026 •

edited

Loading