Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions mysql-test/suite/galera/r/MDEV-37990.result
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
connection node_2;
connection node_1;
USE test;
SET SESSION autocommit=0;
SHOW VARIABLES LIKE 'binlog_format';
Variable_name Value
binlog_format ROW
CREATE TABLE master (
id INT PRIMARY KEY,
name VARCHAR(50) NOT NULL,
description VARCHAR(100)
) ENGINE=InnoDB;
CREATE TABLE dependent_no_cascade (
id INT PRIMARY KEY AUTO_INCREMENT,
master_id INT NOT NULL,
data VARCHAR(50),
FOREIGN KEY (master_id) REFERENCES master(id)
) ENGINE=InnoDB;
CREATE TABLE dependent_with_cascade (
id INT PRIMARY KEY AUTO_INCREMENT,
master_id INT NOT NULL,
data VARCHAR(50),
FOREIGN KEY (master_id) REFERENCES master(id) ON DELETE CASCADE
) ENGINE=InnoDB;
INSERT INTO master (id, name, description) VALUES
(1, 'Master 1', 'First master record'),
(2, 'Master 2', 'Second master record'),
(3, 'Master 3', 'Third master record'),
(4, 'Master 4', 'Fourth master record'),
(5, 'Master 5', 'Fifth master record');
COMMIT;
INSERT INTO dependent_no_cascade (master_id, data) VALUES
(1, 'Dependent no cascade 1'),
(2, 'Dependent no cascade 2'),
(3, 'Dependent no cascade 3'),
(4, 'Dependent no cascade 4'),
(5, 'Dependent no cascade 5');
COMMIT;
INSERT INTO dependent_with_cascade (master_id, data) VALUES
(1, 'Dependent with cascade 1'),
(2, 'Dependent with cascade 2'),
(3, 'Dependent with cascade 3'),
(4, 'Dependent with cascade 4'),
(5, 'Dependent with cascade 5');
COMMIT;
INSERT INTO master (id, name, description) VALUES (6, 'Master 6', 'Sixth master record - newly inserted');
COMMIT;
ALTER TABLE dependent_with_cascade
DROP FOREIGN KEY dependent_with_cascade_ibfk_1,
ADD FOREIGN KEY (master_id) REFERENCES master(id) ON DELETE CASCADE ON UPDATE CASCADE;
UPDATE master SET name = 'Master 6 UPDATE 1' WHERE id = 6;
COMMIT;
# Capture master's binlog
# Capture slave's binlog
connection node_2;
# Compare per-(transaction, table) Table_map counts
no duplicate Table_map events on slave
connection node_1;
DROP TABLE dependent_with_cascade;
DROP TABLE dependent_no_cascade;
DROP TABLE master;
disconnect node_2;
disconnect node_1;
# End of test
178 changes: 178 additions & 0 deletions mysql-test/suite/galera/t/MDEV-37990.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
#
# MDEV-37990 - redundant Table map events in binlog
#
# Background:
# On an InnoDB foreign-key cascade, the master performs the cascade
# entirely inside the storage engine and does not take SQL-layer
# MYSQL_LOCKs on the child tables. The Galera writeset still carries
# row events for both the parent and every cascaded child table.
#
# On the secondary node, the applier replays the writeset event-by-
# event. THD::binlog_write_table_maps() is invoked per replayed row
# event, and the per-statement THD::binlog_table_maps flag does not
# span multiple row events within a single Galera transaction.
# Consequently, the applier could emit the same Table_map_log_event
# for the same table_map_id more than once into its own binary log,
# even though one Table_map per table per transaction is sufficient
# for downstream consumers.
#
# Scenario:
# 1. Parent table `master` and two child tables, one with
# ON DELETE CASCADE and one with ON DELETE + ON UPDATE CASCADE.
# 2. Populate the tables, then run a statement that exercises the
# cascade path on the parent. Each statement is committed in its
# own Galera transaction.
#
# Assertion:
# After the workload, both nodes' binary logs are dumped with
# mysqlbinlog and parsed in an embedded --perl block. For every
# (transaction, table) pair the number of Table_map events is
# counted. The test passes when, on the slave (node_2), no pair
# has a count greater than 1. A regression prints the offending
# transaction and table name and fails the recorded .result.
#
# Notes:
# - autocommit is disabled and explicit COMMITs delimit
# transactions, so each cascading statement becomes a single
# Galera writeset - the shape that triggers the original bug.
# - FLUSH BINARY LOGS is issued before dumping to make sure the
# active binlog file is closed and fully visible to mysqlbinlog.
#

--source include/galera_cluster.inc
--source include/have_innodb.inc
--source include/have_log_bin.inc

# set database
USE test;

# disable auto commit
SET SESSION autocommit=0;

SHOW VARIABLES LIKE 'binlog_format';

# Create master table
CREATE TABLE master (
id INT PRIMARY KEY,
name VARCHAR(50) NOT NULL,
description VARCHAR(100)
) ENGINE=InnoDB;

# Create dependent table WITHOUT ON DELETE CASCADE
CREATE TABLE dependent_no_cascade (
id INT PRIMARY KEY AUTO_INCREMENT,
master_id INT NOT NULL,
data VARCHAR(50),
FOREIGN KEY (master_id) REFERENCES master(id)
) ENGINE=InnoDB;

# Create dependent table WITH ON DELETE CASCADE
CREATE TABLE dependent_with_cascade (
id INT PRIMARY KEY AUTO_INCREMENT,
master_id INT NOT NULL,
data VARCHAR(50),
FOREIGN KEY (master_id) REFERENCES master(id) ON DELETE CASCADE
) ENGINE=InnoDB;

# Insert 5 records into master table
INSERT INTO master (id, name, description) VALUES
(1, 'Master 1', 'First master record'),
(2, 'Master 2', 'Second master record'),
(3, 'Master 3', 'Third master record'),
(4, 'Master 4', 'Fourth master record'),
(5, 'Master 5', 'Fifth master record');
COMMIT;

#SELECT * FROM master;

# Insert 5 records into dependent_no_cascade table
INSERT INTO dependent_no_cascade (master_id, data) VALUES
(1, 'Dependent no cascade 1'),
(2, 'Dependent no cascade 2'),
(3, 'Dependent no cascade 3'),
(4, 'Dependent no cascade 4'),
(5, 'Dependent no cascade 5');
COMMIT;

# Insert 5 records into dependent_with_cascade table
INSERT INTO dependent_with_cascade (master_id, data) VALUES
(1, 'Dependent with cascade 1'),
(2, 'Dependent with cascade 2'),
(3, 'Dependent with cascade 3'),
(4, 'Dependent with cascade 4'),
(5, 'Dependent with cascade 5');
COMMIT;

# Perform INSERT operation on master table
INSERT INTO master (id, name, description) VALUES (6, 'Master 6', 'Sixth master record - newly inserted');
COMMIT;

ALTER TABLE dependent_with_cascade
DROP FOREIGN KEY dependent_with_cascade_ibfk_1,
ADD FOREIGN KEY (master_id) REFERENCES master(id) ON DELETE CASCADE ON UPDATE CASCADE;

UPDATE master SET name = 'Master 6 UPDATE 1' WHERE id = 6;
COMMIT;

# Check that binlog on the slave contains one Table_map event per
# table for each Galera transaction

--echo # Capture master's binlog
let $MYSQLD_DATADIR= `SELECT @@datadir`;
--let $master_binlog= query_get_value(SHOW MASTER STATUS, File, 1)
--let $master_path= $MYSQLTEST_VARDIR/tmp/master_binlog.txt
--exec $MYSQL_BINLOG $MYSQLD_DATADIR/$master_binlog > $master_path

--echo # Capture slave's binlog
--connection node_2
let $MYSQLD_DATADIR= `SELECT @@datadir`;
--let $slave_binlog= query_get_value(SHOW MASTER STATUS, File, 1)
--let $slave_path= $MYSQLTEST_VARDIR/tmp/slave_binlog.txt
--exec $MYSQL_BINLOG $MYSQLD_DATADIR/$slave_binlog > $slave_path

--echo # Compare per-(transaction, table) Table_map counts
--perl
use strict;
sub summarize {
my ($file) = @_;
open(my $fh, '<', $file) or die "open $file: $!";
my (%per_txn, $xid, %out);
$xid = 0;
while (my $line = <$fh>) {
$xid++ if $line =~ /^(?:COMMIT|Xid =)/;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The regex /^(?:COMMIT|Xid =)/ is anchored to the start of the line, but Xid = in mysqlbinlog output is typically preceded by metadata and a tab. This will prevent the regex from correctly identifying transaction boundaries for XID events. Removing the ^ anchor will allow it to match correctly.

      $xid++ if $line =~ /COMMIT|Xid =/;

next unless $line =~ /Table_map:\s*`[^`]+`\.`([^`]+)`/;
$per_txn{$xid}{$1}++;
}
for my $t (sort { $a <=> $b } keys %per_txn) {
for my $tab (sort keys %{$per_txn{$t}}) {
$out{"$t/$tab"} = $per_txn{$t}{$tab};
}
}
return \%out;
}
my $m = summarize("$ENV{MYSQLTEST_VARDIR}/tmp/master_binlog.txt");
my $s = summarize("$ENV{MYSQLTEST_VARDIR}/tmp/slave_binlog.txt");
my $dups = 0;
for my $k (sort keys %$s) {
if (($s->{$k} // 0) > 1) {
print "SLAVE DUPLICATE: $k = $s->{$k}\n";
$dups++;
}
}
if (!$dups) { print "no duplicate Table_map events on slave\n"; }
# Optional: also assert master == slave shape
for my $k (sort keys %$m) {
if (($m->{$k} // 0) != ($s->{$k} // 0)) {
print "MISMATCH $k: master=$m->{$k} slave=" . ($s->{$k}//"0") . "\n";
}
}
EOF

# Cleanup
--connection node_1
DROP TABLE dependent_with_cascade;
DROP TABLE dependent_no_cascade;
DROP TABLE master;

--source include/galera_end.inc
--echo # End of test
8 changes: 8 additions & 0 deletions sql/log.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6488,6 +6488,14 @@ bool THD::binlog_write_table_map(TABLE *table)
DBUG_PRINT("enter", ("table: %p (%s: #%llu)",
table, table->s->table_name.str,
table->s->table_map_id));
#ifdef WITH_WSREP
if (wsrep_thd_is_applying(this)
&& !wsrep_mark_table_mapped(table->s->table_map_id)) {
/* A table map event for this table is already recorded for this
transaction, skip writing the table map event. */
DBUG_RETURN(0);
}
#endif
Comment on lines +6491 to +6498
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The indentation and brace placement in this block do not follow the MariaDB coding style. The block inside the if statement should be indented by 2 spaces relative to the if, and the opening brace { is typically placed on a new line. Additionally, for a function returning bool, false is more idiomatic than 0 to indicate success.

#ifdef WITH_WSREP
  if (wsrep_thd_is_applying(this) &&
      !wsrep_mark_table_mapped(table->s->table_map_id))
  {
    /* A table map event for this table is already recorded for this
       transaction, skip writing the table map event. */
    DBUG_RETURN(false);
  }
#endif
References
  1. MariaDB coding style: 2-space indentation, opening braces on a new line for blocks, and use 'false' for boolean success in functions returning bool. (link)


/* Pre-conditions */
DBUG_ASSERT((table->s->table_map_id & MAX_TABLE_MAP_ID) != UINT32_MAX &&
Expand Down
11 changes: 11 additions & 0 deletions sql/sql_class.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8741,3 +8741,14 @@ bool THD::is_cursor_execution() const
{
return dynamic_cast<Select_materialize*>(this->lex->result);
}

#ifdef WITH_WSREP
void THD::wsrep_clear_table_maps() {
m_wsrep_table_maps.clear();
}

bool THD::wsrep_mark_table_mapped(ulonglong table_id) {
auto result = m_wsrep_table_maps.insert(table_id);
return result.second;
}
#endif /* WITH_WSREP */
9 changes: 9 additions & 0 deletions sql/sql_class.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ void set_thd_stage_info(void *thd,
#include "wsrep_client_state.h"
#include "wsrep_mutex.h"
#include "wsrep_condition_variable.h"
#include <unordered_set>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Including <unordered_set> in a core header like sql_class.h can increase compilation times. Additionally, std::unordered_set is relatively heavy for tracking a small number of tables. Consider using a more lightweight container like Prealloced_array or std::vector with linear search, which would be more efficient for the typical case and avoid unnecessary heap allocations.


class Wsrep_applier_service;
enum wsrep_consistency_check_mode {
Expand Down Expand Up @@ -5609,6 +5610,14 @@ class THD: public THD_count, /* this must be first */
}

#ifdef WITH_WSREP
private:
/* Track table maps for current Galera transaction */
std::unordered_set<ulonglong> m_wsrep_table_maps;

public:
void wsrep_clear_table_maps();
bool wsrep_mark_table_mapped(ulonglong table_id);

bool wsrep_applier; /* dedicated slave applier thread */
bool wsrep_applier_closing; /* applier marked to close */
bool wsrep_client_thread; /* to identify client threads*/
Expand Down
1 change: 1 addition & 0 deletions sql/wsrep_high_priority_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,7 @@ void Wsrep_applier_service::after_apply()
{
DBUG_ENTER("Wsrep_applier_service::after_apply");
wsrep_after_apply(m_thd);
m_thd->wsrep_clear_table_maps();
DBUG_VOID_RETURN;
}

Expand Down