MySQL Bugs: #68588: remove most calls to os_thread

Bug #68588	remove most calls to os_thread_sleep from innodb
Submitted:	6 Mar 2013 22:00	Modified:	22 Apr 2013 1:19
Reporter:	Mark Callaghan	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: InnoDB storage engine	Severity:	S5 (Performance)
Version:	5.6.10	OS:	Any
Assigned to:	Inaam Rana	CPU Architecture:	Any

Description:
Surely I jest but most of these assume 1990's disk performance. Most were also added as a shortcut/stopgap/temp solution. This task is open for the sleep in fil_flush, but I wish that most were removed. Condition variables are usually better.

I noticed this because of http://bugs.mysql.com/bug.php?id=68555

How to repeat:
Find this in fil_flush

retry:
                        if (node->n_pending_flushes > 0) {
                                /* We want to avoid calling os_file_flush() on
                                the file twice at the same time, because we do
                                not know what bugs OS's may contain in file
                                i/o; sleep for a while */

                                mutex_exit(&fil_system->mutex);

                                os_thread_sleep(20000);

Suggested fix:
replace sleep with a condvar

Getting rid of sleeps inside InnoDB is on the agenda.

Perf results after replacing the call to sleep with an event wait:
http://mysqlha.blogspot.com/2013/03/mysql-56-odirect-filflush-and-sleep.html

The diff I used to replace sleep with an event wait

--- orig5610/storage/innobase/fil/fil0fil.cc	2013-01-22 08:54:49.000000000 -0800
+++ orig5610flushcond/storage/innobase/fil/fil0fil.cc	2013-03-12 11:11:45.000000000 -0700
@@ -307,6 +307,7 @@
 					/* !< TRUE if fil_space_create()
 					has issued a warning about
 					potential space_id reuse */
+	os_event_t	flush_event;
 };
 
 /** The tablespace memory cache. This variable is NULL before the module is
@@ -1661,6 +1662,8 @@
 	mutex_create(fil_system_mutex_key,
 		     &fil_system->mutex, SYNC_ANY_LATCH);
 
+	fil_system->flush_event = os_event_create();
+
 	fil_system->spaces = hash_create(hash_size);
 	fil_system->name_hash = hash_create(hash_size);
 
@@ -5513,15 +5516,17 @@
 #endif /* __WIN__ */
 retry:
 			if (node->n_pending_flushes > 0) {
+				ib_int64_t signal_count;
+
 				/* We want to avoid calling os_file_flush() on
 				the file twice at the same time, because we do
 				not know what bugs OS's may contain in file
 				i/o; sleep for a while */
 
+				signal_count = os_event_reset(fil_system->flush_event);
 				mutex_exit(&fil_system->mutex);
 
-				os_thread_sleep(20000);
-
+				os_event_wait_low(fil_system->flush_event, signal_count);
 				mutex_enter(&fil_system->mutex);
 
 				if (node->flush_counter >= old_mod_counter) {
@@ -5546,6 +5551,8 @@
 			mutex_enter(&fil_system->mutex);
 
 			node->n_pending_flushes--;
+			os_event_set(fil_system->flush_event);
+
 skip_flush:
 			if (node->flush_counter < old_mod_counter) {
 				node->flush_counter = old_mod_counter;
@@ -5786,6 +5793,7 @@
 	ut_a(UT_LIST_GET_LEN(fil_system->unflushed_spaces) == 0);
 	ut_a(UT_LIST_GET_LEN(fil_system->space_list) == 0);
 
+	os_event_free(fil_system->flush_event);
 	mem_free(fil_system);
 
 	fil_system = NULL;

Added changelog entry for 5.6.12, 5.7.2:

"This fix removes most calls to "OS_THREAD_SLEEP" from InnoDB.

I wasn't joking when I asked that you stop adding calls to os_thread_sleep.

C symbol: TRX_DOUBLEWRITE_BATCH_POLL_DELAY

  File         Function                        Line
0 buf0dblwr.cc <global>                          44 #define TRX_DOUBLEWRITE_BATCH_POLL_DELAY 10000
1 buf0dblwr.cc buf_dblwr_flush_buffered_writes  780 os_thread_sleep(TRX_DOUBLEWRITE_BATCH_POLL_DELAY);
2 buf0dblwr.cc buf_dblwr_add_to_batch           900 os_thread_sleep(TRX_DOUBLEWRITE_BATCH_POLL_DELAY);
3 buf0dblwr.cc buf_dblwr_write_single_page     1004 os_thread_sleep(TRX_DOUBLEWRITE_BATCH_POLL_DELAY);

Mark,

I have already removed it. It will be gone in the next MR of 5.6.