Correctly updating the before image inconsistency counter

Summary: Each worker was updating it's own counter instead of incrementing the one in global rli. Reviewed By: anirbanr-fb Differential Revision: D18049251 fbshipit-source-id: bfaca17
facebook · Oct 23, 2019 · 90f0fbf · 90f0fbf
1 parent 182d59c
commit 90f0fbf
Show file tree

Hide file tree

Showing 7 changed files with 78 additions and 4 deletions.
diff --git a/mysql-test/extra/rpl_tests/rpl_slave_check_before_image_consistency.inc b/mysql-test/extra/rpl_tests/rpl_slave_check_before_image_consistency.inc
@@ -27,6 +27,7 @@ update t1 set b = 2;
 connection slave;
 let $slave_sql_errno= 1032; # HA_ERR_END_OF_FILE
 source include/wait_for_slave_sql_error.inc;
+show status like 'Slave_before_image_inconsistencies';
 set @@sql_log_bin = 0;
 update t1 set c = 1;
 set @@sql_log_bin = 1;
@@ -40,6 +41,7 @@ update t2 set b = 2;
 connection slave;
 let $slave_sql_errno= 1032; # HA_ERR_END_OF_FILE
 source include/wait_for_slave_sql_error.inc;
+show status like 'Slave_before_image_inconsistencies';
 set @@sql_log_bin = 0;
 update t2 set c = 1;
 set @@sql_log_bin = 1;
@@ -53,6 +55,7 @@ update t3 set b = 2;
 connection slave;
 let $slave_sql_errno= 1032; # HA_ERR_END_OF_FILE
 source include/wait_for_slave_sql_error.inc;
+show status like 'Slave_before_image_inconsistencies';
 set @@sql_log_bin = 0;
 update t3 set c = 1;
 set @@sql_log_bin = 1;
@@ -65,6 +68,7 @@ connection master;
 update t4 set b = 2;
 connection slave;
 let $slave_sql_errno= 1032; # HA_ERR_END_OF_FILE
+show status like 'Slave_before_image_inconsistencies';
 source include/wait_for_slave_sql_error.inc;
 set @@sql_log_bin = 0;
 update t4 set c = 1;

diff --git a/mysql-test/suite/rocksdb_rpl/r/rocksdb_slave_check_before_image_consistency.result b/mysql-test/suite/rocksdb_rpl/r/rocksdb_slave_check_before_image_consistency.result
@@ -21,6 +21,9 @@ update t4 set c = 2;
 set @@sql_log_bin = 1;
 update t1 set b = 2;
 include/wait_for_slave_sql_error.inc [errno=1032]
+show status like 'Slave_before_image_inconsistencies';
+Variable_name	Value
+Slave_before_image_inconsistencies	1
 set @@sql_log_bin = 0;
 update t1 set c = 1;
 set @@sql_log_bin = 1;
@@ -29,6 +32,9 @@ include/start_slave.inc
 include/sync_slave_sql_with_master.inc
 update t2 set b = 2;
 include/wait_for_slave_sql_error.inc [errno=1032]
+show status like 'Slave_before_image_inconsistencies';
+Variable_name	Value
+Slave_before_image_inconsistencies	2
 set @@sql_log_bin = 0;
 update t2 set c = 1;
 set @@sql_log_bin = 1;
@@ -37,13 +43,19 @@ include/start_slave.inc
 include/sync_slave_sql_with_master.inc
 update t3 set b = 2;
 include/wait_for_slave_sql_error.inc [errno=1032]
+show status like 'Slave_before_image_inconsistencies';
+Variable_name	Value
+Slave_before_image_inconsistencies	2
 set @@sql_log_bin = 0;
 update t3 set c = 1;
 set @@sql_log_bin = 1;
 include/stop_slave.inc
 include/start_slave.inc
 include/sync_slave_sql_with_master.inc
 update t4 set b = 2;
+show status like 'Slave_before_image_inconsistencies';
+Variable_name	Value
+Slave_before_image_inconsistencies	2
 include/wait_for_slave_sql_error.inc [errno=1032]
 set @@sql_log_bin = 0;
 update t4 set c = 1;
@@ -100,6 +112,9 @@ update t4 set c = 2;
 set @@sql_log_bin = 1;
 update t1 set b = 2;
 include/wait_for_slave_sql_error.inc [errno=1032]
+show status like 'Slave_before_image_inconsistencies';
+Variable_name	Value
+Slave_before_image_inconsistencies	3
 set @@sql_log_bin = 0;
 update t1 set c = 1;
 set @@sql_log_bin = 1;
@@ -108,6 +123,9 @@ include/start_slave.inc
 include/sync_slave_sql_with_master.inc
 update t2 set b = 2;
 include/wait_for_slave_sql_error.inc [errno=1032]
+show status like 'Slave_before_image_inconsistencies';
+Variable_name	Value
+Slave_before_image_inconsistencies	3
 set @@sql_log_bin = 0;
 update t2 set c = 1;
 set @@sql_log_bin = 1;
@@ -116,13 +134,19 @@ include/start_slave.inc
 include/sync_slave_sql_with_master.inc
 update t3 set b = 2;
 include/wait_for_slave_sql_error.inc [errno=1032]
+show status like 'Slave_before_image_inconsistencies';
+Variable_name	Value
+Slave_before_image_inconsistencies	3
 set @@sql_log_bin = 0;
 update t3 set c = 1;
 set @@sql_log_bin = 1;
 include/stop_slave.inc
 include/start_slave.inc
 include/sync_slave_sql_with_master.inc
 update t4 set b = 2;
+show status like 'Slave_before_image_inconsistencies';
+Variable_name	Value
+Slave_before_image_inconsistencies	3
 include/wait_for_slave_sql_error.inc [errno=1032]
 set @@sql_log_bin = 0;
 update t4 set c = 1;

diff --git a/mysql-test/suite/rpl/r/rpl_slave_check_before_image_consistency.result b/mysql-test/suite/rpl/r/rpl_slave_check_before_image_consistency.result
@@ -23,6 +23,9 @@ update t4 set c = 2;
 set @@sql_log_bin = 1;
 update t1 set b = 2;
 include/wait_for_slave_sql_error.inc [errno=1032]
+show status like 'Slave_before_image_inconsistencies';
+Variable_name	Value
+Slave_before_image_inconsistencies	1
 set @@sql_log_bin = 0;
 update t1 set c = 1;
 set @@sql_log_bin = 1;
@@ -31,6 +34,9 @@ include/start_slave.inc
 include/sync_slave_sql_with_master.inc
 update t2 set b = 2;
 include/wait_for_slave_sql_error.inc [errno=1032]
+show status like 'Slave_before_image_inconsistencies';
+Variable_name	Value
+Slave_before_image_inconsistencies	2
 set @@sql_log_bin = 0;
 update t2 set c = 1;
 set @@sql_log_bin = 1;
@@ -39,13 +45,19 @@ include/start_slave.inc
 include/sync_slave_sql_with_master.inc
 update t3 set b = 2;
 include/wait_for_slave_sql_error.inc [errno=1032]
+show status like 'Slave_before_image_inconsistencies';
+Variable_name	Value
+Slave_before_image_inconsistencies	2
 set @@sql_log_bin = 0;
 update t3 set c = 1;
 set @@sql_log_bin = 1;
 include/stop_slave.inc
 include/start_slave.inc
 include/sync_slave_sql_with_master.inc
 update t4 set b = 2;
+show status like 'Slave_before_image_inconsistencies';
+Variable_name	Value
+Slave_before_image_inconsistencies	2
 include/wait_for_slave_sql_error.inc [errno=1032]
 set @@sql_log_bin = 0;
 update t4 set c = 1;
@@ -102,6 +114,9 @@ update t4 set c = 2;
 set @@sql_log_bin = 1;
 update t1 set b = 2;
 include/wait_for_slave_sql_error.inc [errno=1032]
+show status like 'Slave_before_image_inconsistencies';
+Variable_name	Value
+Slave_before_image_inconsistencies	3
 set @@sql_log_bin = 0;
 update t1 set c = 1;
 set @@sql_log_bin = 1;
@@ -110,6 +125,9 @@ include/start_slave.inc
 include/sync_slave_sql_with_master.inc
 update t2 set b = 2;
 include/wait_for_slave_sql_error.inc [errno=1032]
+show status like 'Slave_before_image_inconsistencies';
+Variable_name	Value
+Slave_before_image_inconsistencies	3
 set @@sql_log_bin = 0;
 update t2 set c = 1;
 set @@sql_log_bin = 1;
@@ -118,13 +136,19 @@ include/start_slave.inc
 include/sync_slave_sql_with_master.inc
 update t3 set b = 2;
 include/wait_for_slave_sql_error.inc [errno=1032]
+show status like 'Slave_before_image_inconsistencies';
+Variable_name	Value
+Slave_before_image_inconsistencies	3
 set @@sql_log_bin = 0;
 update t3 set c = 1;
 set @@sql_log_bin = 1;
 include/stop_slave.inc
 include/start_slave.inc
 include/sync_slave_sql_with_master.inc
 update t4 set b = 2;
+show status like 'Slave_before_image_inconsistencies';
+Variable_name	Value
+Slave_before_image_inconsistencies	3
 include/wait_for_slave_sql_error.inc [errno=1032]
 set @@sql_log_bin = 0;
 update t4 set c = 1;

diff --git a/sql/log_event.cc b/sql/log_event.cc
@@ -11643,7 +11643,7 @@ int Rows_log_event::do_index_scan_and_update(Relay_log_info const *rli,
       !(m_table->file->ha_table_flags() & HA_READ_BEFORE_WRITE_REMOVAL) &&
       record_compare(m_table, tabledef, &m_cols))
   {
-    ++const_cast<Relay_log_info*>(rli)->before_image_inconsistencies;
+    const_cast<Relay_log_info*>(rli)->inc_before_image_inconsistencies();
     if (log_warnings > 1)
     {
       sql_print_warning("Slave before-image consistency check failed at "

diff --git a/sql/mysqld.cc b/sql/mysqld.cc
@@ -9962,7 +9962,7 @@ static int show_slave_before_image_inconsistencies(THD *thd, SHOW_VAR *var,
     var->type= SHOW_LONGLONG;
     var->value= buff;
     *((ulonglong *)buff)=
-      (ulonglong) active_mi->rli->before_image_inconsistencies.load();
+      (ulonglong) active_mi->rli->get_before_image_inconsistencies();
   }
   else
     var->type= SHOW_UNDEF;

diff --git a/sql/rpl_rli.h b/sql/rpl_rli.h
@@ -1097,6 +1097,10 @@ class Relay_log_info : public Rpl_info
 
   std::unordered_set<std::string> rbr_column_type_mismatch_whitelist;
 
+  /* counter for the number of BI inconsistencies found */
+  /* @see opt_slave_check_before_image_consistency */
+  std::atomic<ulong> before_image_inconsistencies{0};
+
 public:
   // store value to propagate to handler in open_tables
   bool skip_unique_check;
@@ -1114,8 +1118,16 @@ class Relay_log_info : public Rpl_info
 
   /* @see opt_slave_check_before_image_consistency */
   ulong check_before_image_consistency= 0;
-  /* counter for the number of inconsistencies found */
-  std::atomic<ulong> before_image_inconsistencies{0};
+
+  virtual void inc_before_image_inconsistencies()
+  {
+    ++before_image_inconsistencies;
+  }
+
+  virtual ulong get_before_image_inconsistencies() const
+  {
+    return before_image_inconsistencies.load();
+  }
 
   virtual const std::unordered_set<std::string>*
         get_rbr_column_type_mismatch_whitelist() const

diff --git a/sql/rpl_rli_pdb.h b/sql/rpl_rli_pdb.h
@@ -513,6 +513,16 @@ class Slave_worker : public Relay_log_info
   {
     return current_db;
   }
+  virtual void inc_before_image_inconsistencies() override
+  {
+    DBUG_ASSERT(c_rli);
+    if (likely(c_rli)) c_rli->inc_before_image_inconsistencies();
+  }
+  virtual ulong get_before_image_inconsistencies() const override
+  {
+    DBUG_ASSERT(c_rli);
+    return c_rli ? c_rli->get_before_image_inconsistencies() : 0;
+  }
   virtual const std::unordered_set<std::string>*
         get_rbr_column_type_mismatch_whitelist() const override
   {