Skip to content

Commit

Permalink
MDEV-25236 Online log apply fails for ROW_FORMAT=REDUNDANT tables
Browse files Browse the repository at this point in the history
In other ROW_FORMAT than REDUNDANT, the InnoDB record header
size calculation depends on dict_index_t::n_core_null_bytes.

In ROW_FORMAT=REDUNDANT, the record header always is 6 bytes
plus n_fields or 2*n_fields bytes, depending on the maximum
record size. But, during online ALTER TABLE, the log records
in the temporary file always use a format similar to
ROW_FORMAT=DYNAMIC, even omitting the 5-byte fixed-length part
of the header.

While creating a temporary file record for a ROW_FORMAT=REDUNDANT
table, InnoDB must refer to dict_index_t::n_nullable.
The field dict_index_t::n_core_null_bytes is only valid for
other than ROW_FORMAT=REDUNDANT tables.

The bug does not affect MariaDB 10.3, because only
commit 7a27db7 (MDEV-15563)
allowed an ALGORITHM=INSTANT change of a NOT NULL column to
NULL in a ROW_FORMAT=REDUNDANT table.

The fix was developed by Thirunarayanan Balathandayuthapani
and tested by Matthias Leich. The test case was simplified by me.
  • Loading branch information
dr-m committed Jul 2, 2021
1 parent 372ea88 commit a635588
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 13 deletions.
23 changes: 22 additions & 1 deletion mysql-test/suite/innodb/r/instant_alter_debug.result
Original file line number Diff line number Diff line change
Expand Up @@ -443,10 +443,31 @@ SET GLOBAL innodb_limit_optimistic_insert_debug=@save_limit;
SELECT * FROM t1;
c2 c
DROP TABLE t1;
#
# MDEV-25236 Online log apply fails for ROW_FORMAT=REDUNDANT tables
#
CREATE TABLE t1
(a INT NOT NULL, b INT, c INT, d INT, e INT, f INT, g INT, h INT, i TEXT)
ENGINE=InnoDB;
ALTER TABLE t1 MODIFY a INT NULL;
SET DEBUG_SYNC='innodb_inplace_alter_table_enter SIGNAL alter WAIT_FOR go';
ALTER TABLE t1 ADD PRIMARY KEY (a);
connect con1,localhost,root,,;
set DEBUG_SYNC='now WAIT_FOR alter';
BEGIN;
INSERT INTO t1 SET a=0, i=REPEAT('1', 10000);
ROLLBACK;
set DEBUG_SYNC='now SIGNAL go';
connection default;
disconnect con1;
SELECT * FROM t1;
a b c d e f g h i
DROP TABLE t1;
SET DEBUG_SYNC=RESET;
# End of 10.4 tests
SET GLOBAL innodb_purge_rseg_truncate_frequency = @save_frequency;
SELECT variable_value-@old_instant instants
FROM information_schema.global_status
WHERE variable_name = 'innodb_instant_alter_column';
instants
32
33
26 changes: 26 additions & 0 deletions mysql-test/suite/innodb/t/instant_alter_debug.test
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,32 @@ SET GLOBAL innodb_limit_optimistic_insert_debug=@save_limit;
SELECT * FROM t1;
DROP TABLE t1;

--echo #
--echo # MDEV-25236 Online log apply fails for ROW_FORMAT=REDUNDANT tables
--echo #

CREATE TABLE t1
(a INT NOT NULL, b INT, c INT, d INT, e INT, f INT, g INT, h INT, i TEXT)
ENGINE=InnoDB;

ALTER TABLE t1 MODIFY a INT NULL;

SET DEBUG_SYNC='innodb_inplace_alter_table_enter SIGNAL alter WAIT_FOR go';
send ALTER TABLE t1 ADD PRIMARY KEY (a);
connect(con1,localhost,root,,);
set DEBUG_SYNC='now WAIT_FOR alter';
BEGIN;
INSERT INTO t1 SET a=0, i=REPEAT('1', 10000);
ROLLBACK;
set DEBUG_SYNC='now SIGNAL go';
connection default;
reap;

disconnect con1;
SELECT * FROM t1;
DROP TABLE t1;
SET DEBUG_SYNC=RESET;

--echo # End of 10.4 tests

SET GLOBAL innodb_purge_rseg_truncate_frequency = @save_frequency;
Expand Down
45 changes: 33 additions & 12 deletions storage/innobase/rem/rem0rec.cc
Original file line number Diff line number Diff line change
Expand Up @@ -248,14 +248,16 @@ enum rec_leaf_format {
in ROW_FORMAT=COMPACT,DYNAMIC,COMPRESSED.
This is a special case of rec_init_offsets() and rec_get_offsets_func().
@tparam mblob whether the record includes a metadata BLOB
@tparam redundant_temp whether the record belongs to a temporary file
of a ROW_FORMAT=REDUNDANT table
@param[in] rec leaf-page record
@param[in] index the index that the record belongs in
@param[in] n_core number of core fields (index->n_core_fields)
@param[in] def_val default values for non-core fields, or
NULL to refer to index->fields[].col->def_val
@param[in,out] offsets offsets, with valid rec_offs_n_fields(offsets)
@param[in] format record format */
template<bool mblob = false>
template<bool mblob = false, bool redundant_temp = false>
static inline
void
rec_init_offsets_comp_ordinary(
Expand Down Expand Up @@ -286,7 +288,9 @@ rec_init_offsets_comp_ordinary(
const unsigned n_core_null_bytes = UNIV_UNLIKELY(index->n_core_fields
!= n_core)
? UT_BITS_IN_BYTES(unsigned(index->get_n_nullable(n_core)))
: index->n_core_null_bytes;
: (redundant_temp
? UT_BITS_IN_BYTES(index->n_nullable)
: index->n_core_null_bytes);

if (mblob) {
ut_ad(index->is_dummy || index->table->instant);
Expand Down Expand Up @@ -1109,8 +1113,8 @@ rec_get_nth_field_offs_old(
}

/** Determine the size of a data tuple prefix in ROW_FORMAT=COMPACT.
@tparam mblob whether the record includes a metadata BLOB
@tparam redundant_temp whether to use the ROW_FORMAT=REDUNDANT format
@tparam mblob whether the record includes a metadata BLOB
@tparam redundant_temp whether to use the ROW_FORMAT=REDUNDANT format
@param[in] index record descriptor; dict_table_is_comp()
is assumed to hold, even if it doesn't
@param[in] dfield array of data fields
Expand Down Expand Up @@ -1157,7 +1161,9 @@ rec_get_converted_size_comp_prefix_low(
- n_core_fields);
} else {
ut_ad(n_fields <= n_core_fields);
extra_size += index->n_core_null_bytes;
extra_size += redundant_temp
? UT_BITS_IN_BYTES(index->n_nullable)
: index->n_core_null_bytes;
}

ulint data_size = 0;
Expand Down Expand Up @@ -1837,10 +1843,19 @@ rec_init_offsets_temp(
if it was emptied during an ALTER TABLE operation. */
ut_ad(index->n_core_fields == n_core || !index->is_instant());
ut_ad(index->n_core_fields >= n_core);
rec_init_offsets_comp_ordinary(rec, index, offsets, n_core, def_val,
status == REC_STATUS_INSTANT
? REC_LEAF_TEMP_INSTANT
: REC_LEAF_TEMP);
if (index->table->not_redundant()) {
rec_init_offsets_comp_ordinary(
rec, index, offsets, n_core, def_val,
status == REC_STATUS_INSTANT
? REC_LEAF_TEMP_INSTANT
: REC_LEAF_TEMP);
} else {
rec_init_offsets_comp_ordinary<false, true>(
rec, index, offsets, n_core, def_val,
status == REC_STATUS_INSTANT
? REC_LEAF_TEMP_INSTANT
: REC_LEAF_TEMP);
}
}

/** Determine the offset to each field in temporary file.
Expand All @@ -1855,9 +1870,15 @@ rec_init_offsets_temp(
rec_offs* offsets)
{
ut_ad(!index->is_instant());
rec_init_offsets_comp_ordinary(rec, index, offsets,
index->n_core_fields, NULL,
REC_LEAF_TEMP);
if (index->table->not_redundant()) {
rec_init_offsets_comp_ordinary(
rec, index, offsets,
index->n_core_fields, NULL, REC_LEAF_TEMP);
} else {
rec_init_offsets_comp_ordinary<false, true>(
rec, index, offsets,
index->n_core_fields, NULL, REC_LEAF_TEMP);
}
}

/** Convert a data tuple prefix to the temporary file format.
Expand Down

0 comments on commit a635588

Please sign in to comment.