MDEV-14092 NEXTVAL fails on slave

The problem was that the code in replication didn't distinguish between a setval() failing because the stored sequence number was bigger than the current (should have been ignored) and a failure from the storage engine.
MariaDB · Oct 19, 2017 · 211f9ee · 211f9ee
1 parent e156db8
commit 211f9ee
Show file tree

Hide file tree

Showing 5 changed files with 260 additions and 12 deletions.
diff --git a/mysql-test/suite/sql_sequence/slave_nextval.result b/mysql-test/suite/sql_sequence/slave_nextval.result
@@ -0,0 +1,108 @@
+include/master-slave.inc
+[connection master]
+CREATE SEQUENCE s;
+INSERT INTO s VALUES (1,1,4,1,1,1,0,0);
+show create sequence s;
+Table	Create Table
+s	CREATE SEQUENCE `s` start with 1 minvalue 1 maxvalue 4 increment by 1 cache 1 nocycle ENGINE=MyISAM
+SELECT NEXTVAL(s);
+NEXTVAL(s)
+1
+connection slave;
+SELECT NEXTVAL(s);
+NEXTVAL(s)
+2
+SELECT NEXTVAL(s);
+NEXTVAL(s)
+3
+connection master;
+SELECT NEXTVAL(s);
+NEXTVAL(s)
+2
+SELECT NEXTVAL(s);
+NEXTVAL(s)
+3
+SELECT NEXTVAL(s);
+NEXTVAL(s)
+4
+select * from s;
+next_not_cached_value	minimum_value	maximum_value	start_value	increment	cache_size	cycle_option	cycle_count
+5	1	4	1	1	1	0	0
+connection slave;
+select * from s;
+next_not_cached_value	minimum_value	maximum_value	start_value	increment	cache_size	cycle_option	cycle_count
+5	1	4	1	1	1	0	0
+connection master;
+DROP SEQUENCE s;
+CREATE SEQUENCE s;
+INSERT INTO s VALUES (1,1,3,1,1,1,1,0);
+show create sequence s;
+Table	Create Table
+s	CREATE SEQUENCE `s` start with 1 minvalue 1 maxvalue 3 increment by 1 cache 1 cycle ENGINE=MyISAM
+SELECT NEXTVAL(s);
+NEXTVAL(s)
+1
+connection slave;
+SELECT NEXTVAL(s);
+NEXTVAL(s)
+2
+SELECT NEXTVAL(s);
+NEXTVAL(s)
+3
+connection master;
+SELECT NEXTVAL(s);
+NEXTVAL(s)
+2
+SELECT NEXTVAL(s);
+NEXTVAL(s)
+3
+SELECT NEXTVAL(s);
+NEXTVAL(s)
+1
+select * from s;
+next_not_cached_value	minimum_value	maximum_value	start_value	increment	cache_size	cycle_option	cycle_count
+2	1	3	1	1	1	1	1
+connection slave;
+select * from s;
+next_not_cached_value	minimum_value	maximum_value	start_value	increment	cache_size	cycle_option	cycle_count
+2	1	3	1	1	1	1	1
+connection master;
+DROP SEQUENCE s;
+CREATE SEQUENCE s;
+INSERT INTO s VALUES (1,1,3,1,1,1,1,0);
+SELECT NEXTVAL(s);
+NEXTVAL(s)
+1
+CREATE PROCEDURE pr(n INT)
+BEGIN
+DECLARE i INT DEFAULT 0;
+WHILE i < n
+DO
+SELECT NEXTVAL(s);
+SELECT NEXTVAL(s);
+SELECT NEXTVAL(s);
+SET i= i+1;
+END WHILE;
+END $
+connect  con1,localhost,root,,;
+CALL pr(100);
+connect  con2,localhost,root,,;
+CALL pr(100);
+connect  con3,localhost,root,,;
+CALL pr(100);
+connect  con4,localhost,root,,;
+CALL pr(100);
+connect  con5,localhost,root,,;
+CALL pr(100);
+connect  con6,localhost,root,,;
+CALL pr(100);
+connect  con7,localhost,root,,;
+CALL pr(100);
+connect  con8,localhost,root,,;
+CALL pr(100);
+connection master;
+connection slave;
+connection master;
+DROP SEQUENCE s;
+DROP PROCEDURE pr;
+include/rpl_end.inc
diff --git a/mysql-test/suite/sql_sequence/slave_nextval.test b/mysql-test/suite/sql_sequence/slave_nextval.test
@@ -0,0 +1,132 @@
+--source include/master-slave.inc
+--source include/have_binlog_format_row.inc
+
+#
+# MDEV-14092  NEXTVAL() fails on slave
+#
+
+CREATE SEQUENCE s;
+INSERT INTO s VALUES (1,1,4,1,1,1,0,0);
+show create sequence s;
+SELECT NEXTVAL(s);
+
+--sync_slave_with_master
+SELECT NEXTVAL(s);
+SELECT NEXTVAL(s);
+
+--connection master
+SELECT NEXTVAL(s);
+SELECT NEXTVAL(s);
+SELECT NEXTVAL(s);
+
+select * from s;
+
+--sync_slave_with_master
+
+select * from s;
+--connection master
+DROP SEQUENCE s;
+
+#
+# Same as above, but with cycles
+#
+
+CREATE SEQUENCE s;
+INSERT INTO s VALUES (1,1,3,1,1,1,1,0);
+show create sequence s;
+SELECT NEXTVAL(s);
+
+--sync_slave_with_master
+SELECT NEXTVAL(s);
+SELECT NEXTVAL(s);
+
+--connection master
+SELECT NEXTVAL(s);
+SELECT NEXTVAL(s);
+SELECT NEXTVAL(s);
+
+select * from s;
+
+--sync_slave_with_master
+
+select * from s;
+
+--connection master
+DROP SEQUENCE s;
+
+# Here is a bit more complicated concurrent scenario that
+# causes the same effect without any updates on the slave. You might
+# need to replace 100 with a bigger value if it doesn't happen on your
+# machine right away.
+
+CREATE SEQUENCE s;
+INSERT INTO s VALUES (1,1,3,1,1,1,1,0);
+SELECT NEXTVAL(s);
+
+--delimiter $
+CREATE PROCEDURE pr(n INT)
+BEGIN
+DECLARE i INT DEFAULT 0;
+WHILE i < n
+DO
+SELECT NEXTVAL(s);
+SELECT NEXTVAL(s);
+SELECT NEXTVAL(s);
+SET i= i+1;
+END WHILE;
+END $
+--delimiter ;
+
+--connect (con1,localhost,root,,)
+--send CALL pr(100)
+--connect (con2,localhost,root,,)
+--send CALL pr(100)
+--connect (con3,localhost,root,,)
+--send CALL pr(100)
+--connect (con4,localhost,root,,)
+--send CALL pr(100)
+--connect (con5,localhost,root,,)
+--send CALL pr(100)
+--connect (con6,localhost,root,,)
+--send CALL pr(100)
+--connect (con7,localhost,root,,)
+--send CALL pr(100)
+--connect (con8,localhost,root,,)
+--send CALL pr(100)
+
+
+--disable_query_log
+--disable_result_log
+
+--connection con1
+--reap
+--connection con2
+--reap
+--connection con3
+--reap
+--connection con4
+--reap
+--connection con5
+--reap
+--connection con6
+--reap
+--connection con7
+--reap
+--connection con8
+--reap
+
+--enable_query_log
+--enable_result_log
+
+--connection master
+
+--sync_slave_with_master
+
+--connection master
+DROP SEQUENCE s;
+DROP PROCEDURE pr;
+
+#
+# Cleanup
+#
+--source include/rpl_end.inc
diff --git a/sql/log_event.cc b/sql/log_event.cc
@@ -12707,7 +12707,7 @@ int Rows_log_event::update_sequence()
     longlong round= table->field[ROUND_FIELD_NO]->val_int();
     dbug_tmp_restore_column_map(table->read_set, old_map);
 
-    return table->s->sequence->set_value(table, nextval, round, 0);
+    return table->s->sequence->set_value(table, nextval, round, 0) > 0;
   }
 
   /*
@@ -12726,6 +12726,7 @@ Write_rows_log_event::do_exec_row(rpl_group_info *rgi)
   DBUG_ASSERT(m_table != NULL);
   const char *tmp= thd->get_proc_info();
   const char *message= "Write_rows_log_event::write_row()";
+  int error;
 
 #ifdef WSREP_PROC_INFO
   my_snprintf(thd->wsrep_info, sizeof(thd->wsrep_info) - 1,
@@ -12735,7 +12736,7 @@ Write_rows_log_event::do_exec_row(rpl_group_info *rgi)
 #endif /* WSREP_PROC_INFO */
 
   thd_proc_info(thd, message);
-  int error= write_row(rgi, slave_exec_mode == SLAVE_EXEC_MODE_IDEMPOTENT);
+  error= write_row(rgi, slave_exec_mode == SLAVE_EXEC_MODE_IDEMPOTENT);
   thd_proc_info(thd, tmp);
 
   if (error && !thd->is_error())

diff --git a/sql/sql_sequence.cc b/sql/sql_sequence.cc
@@ -758,12 +758,12 @@ void SEQUENCE_LAST_VALUE::set_version(TABLE *table)
 
    @param in   table       Sequence table
    @param in   next_val    Next free value
-   @param in   next_round  Round for 'next_value' (in cace of cycles)
+   @param in   next_round  Round for 'next_value' (in case of cycles)
    @param in   is_used     1 if next_val is already used
 
    @retval     0      ok, value adjusted
-               1      value was less than current value or
-                      error when storing value
+               -1     value was less than current value
+               1      error when storing value
 
    @comment
    A new value is set only if "nextval,next_round" is less than
@@ -773,10 +773,10 @@ void SEQUENCE_LAST_VALUE::set_version(TABLE *table)
    contain the highest used value when the slave is promoted to a master.
 */
 
-bool SEQUENCE::set_value(TABLE *table, longlong next_val, ulonglong next_round,
+int SEQUENCE::set_value(TABLE *table, longlong next_val, ulonglong next_round,
                          bool is_used)
 {
-  bool error= 1;
+  int error= -1;
   bool needs_to_be_stored= 0;
   longlong org_reserved_until=  reserved_until;
   longlong org_next_free_value= next_free_value;
@@ -788,21 +788,27 @@ bool SEQUENCE::set_value(TABLE *table, longlong next_val, ulonglong next_round,
     next_val= increment_value(next_val);
 
   if (round > next_round)
-    goto end;
+    goto end;                                   // error = -1
   if (round == next_round)
   {
     if (real_increment > 0  ?
         next_val < next_free_value :
         next_val > next_free_value)
-      goto end;
+      goto end;                                 // error = -1
     if (next_val == next_free_value)
     {
       error= 0;
       goto end;
     }
   }
   else if (cycle == 0)
-    goto end;                       // round < next_round && no cycles
+  {
+    // round < next_round && no cycles, which is impossible
+    my_error(ER_SEQUENCE_RUN_OUT, MYF(0), table->s->db.str,
+             table->s->table_name.str);
+    error= 1;
+    goto end;
+  }
   else
     needs_to_be_stored= 1;
 
@@ -819,6 +825,7 @@ bool SEQUENCE::set_value(TABLE *table, longlong next_val, ulonglong next_round,
       reserved_until=  org_reserved_until;
       next_free_value= org_next_free_value;
       round= org_round;
+      error= 1;
       goto end;
     }
   }

diff --git a/sql/sql_sequence.h b/sql/sql_sequence.h
@@ -105,8 +105,8 @@ class SEQUENCE :public sequence_definition
     all_values_used= 0;
   }
   longlong next_value(TABLE *table, bool second_round, int *error);
-  bool set_value(TABLE *table, longlong next_value, ulonglong round_arg,
-                 bool is_used);
+  int set_value(TABLE *table, longlong next_value, ulonglong round_arg,
+                bool is_used);
   longlong increment_value(longlong value)
   {
     if (real_increment > 0)