Bug#21420180 BINLOG_GROUP_COMMIT_SYNC_DELAY SHOULD BE CONSIDERED

ONLY WHEN SYNCING BINLOG

Problem: binlog_group_commit_sync_delay is getting applied
to every binary log commit group.

Analysis: binlog_group_commit_sync_delay variable is introduced to
control how many microseconds the binary log commit waits before
synchronizing the binary log file to disk. sync_binlog variable
is introduced to control the number of binary log commit groups
to collect before synchronizing the binary log to disk

In the current code, server is waiting for binlog_group_commit_sync_delay
microseconds after entering into the sync stage of every group
i.e., every leader of BGC group is waiting the specified time. In case
of sync_binlog > 1 case or sync_binlog=0 case that every leader will not
be doing 'sync' and users expectation will be that those groups that are
doing sync will *only* hit the waiting period.

Fix: wait_count_or_timeout logic should be protected in such a way that
it will enter only if the sync stage is going to do the sync i.e.,
check if sync_counter reaches sync_binlog value, only then call the
wait_count_or_timeout function.

Author: Venkatesh Duggirala

mysql-test/suite/binlog/r/binlog_group_commit_sync_delay.result

@@ -1,7 +1,13 @@
 SET GLOBAL binlog_group_commit_sync_delay=1000000;
+SET GLOBAL sync_binlog = 1;
 SET @clock_in = SYSDATE();
 CREATE TABLE t1 (c1 INT) Engine=InnoDB;
 SET @elapsed = TIMESTAMPDIFF(MICROSECOND, @clock_in, SYSDATE());
 include/assert.inc ["Assert that the statement never takes less than the delay set."]
+SET GLOBAL sync_binlog=1000;
+SET @clock_in = SYSDATE();
+SET @elapsed = TIMESTAMPDIFF(MICROSECOND, @clock_in, SYSDATE());
+include/assert.inc ["Assert that the above statements should not take more than 1000 seconds"]
 SET GLOBAL binlog_group_commit_sync_delay=0;
+SET GLOBAL sync_binlog=1;
 DROP TABLE t1;

mysql-test/suite/binlog/t/binlog_group_commit_sync_delay.test

@@ -4,11 +4,12 @@
 
 --source include/have_binlog_format_mixed.inc
 
---let $delay=1000000
---let $delay_as_secs= `SELECT $delay / 1000000.0`
-
 --let $bgcd_saved=`SELECT @@GLOBAL.binlog_group_commit_sync_delay`
+--let $sync_binlog_saved=`SELECT @@GLOBAL.sync_binlog`
+
+--let $delay=1000000
 --eval SET GLOBAL binlog_group_commit_sync_delay=$delay
+SET GLOBAL sync_binlog = 1;
 
 SET @clock_in = SYSDATE();
 CREATE TABLE t1 (c1 INT) Engine=InnoDB;
@@ -18,6 +19,37 @@ SET @elapsed = TIMESTAMPDIFF(MICROSECOND, @clock_in, SYSDATE());
 --let $assert_cond= [SELECT @elapsed >= @@GLOBAL.binlog_group_commit_sync_delay]
 --source include/assert.inc
 
+#
+# Bug#21420180 BINLOG_GROUP_COMMIT_SYNC_DELAY SHOULD BE CONSIDERED ONLY WHEN
+#              SYNCING BINLOG
+#
+# Execute 1000 insertions (i.e., 1000 groups , one transaction per group).
+# Without the fix, it will take 1000 * binlog_group_commit_sync_delay which
+# is 1000 seconds (> 15 mints). Hence test script will be timed out.
+# But with fix, it should not take more than few seconds (that depends
+# on Pb2 machine speed and also depends on various build options too.
+#
+--let $group_count=1000
+--eval SET GLOBAL sync_binlog=$group_count
+--let $i=0
+
+SET @clock_in = SYSDATE();
+--disable_query_log
+while ($i <= $group_count)
+{
+  --eval INSERT INTO t1 VALUES ($i)
+  --inc $i
+}
+--enable_query_log
+SET @elapsed = TIMESTAMPDIFF(MICROSECOND, @clock_in, SYSDATE());
+--let $assert_text="Assert that the above statements should not take more than 1000 seconds"
+--let $assert_cond= [SELECT @elapsed < 1000 * @@GLOBAL.binlog_group_commit_sync_delay]
+--source include/assert.inc
+
+# End of Bug#21420180 test
+
+# Cleanup
 --eval SET GLOBAL binlog_group_commit_sync_delay=$bgcd_saved
+--eval SET GLOBAL sync_binlog=$sync_binlog_saved
 
 DROP TABLE t1;

sql/binlog.cc

@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2016, Oracle and/or its affiliates. All rights reserved.
+/* Copyright (c) 2009, 2017, Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -9047,10 +9047,15 @@ int MYSQL_BIN_LOG::ordered_commit(THD *thd, bool all, bool skip_commit)
     DBUG_RETURN(finish_commit(thd));
   }
 
-  /* Shall introduce a delay. */
-  stage_manager.wait_count_or_timeout(opt_binlog_group_commit_sync_no_delay_count,
-                                      opt_binlog_group_commit_sync_delay,
-                                      Stage_manager::SYNC_STAGE);
+  /*
+    Shall introduce a delay only if it is going to do sync
+    in this ongoing SYNC stage. The "+1" used below in the
+    if condition is to count the ongoing sync stage.
+  */
+  if (!flush_error && (sync_counter + 1 >= get_sync_period()))
+    stage_manager.wait_count_or_timeout(opt_binlog_group_commit_sync_no_delay_count,
+                                        opt_binlog_group_commit_sync_delay,
+                                        Stage_manager::SYNC_STAGE);
 
   final_queue= stage_manager.fetch_queue_for(Stage_manager::SYNC_STAGE);