Merge lp:~yan.zhang/codership-mysql/5.5 into lp:codership-mysql/5.5

[Yan Zhang]

add wsrep_causal_mask to control optional causal check to auto_commit UPDATE/DELETE.

variable name is not finalized yet, 'wsrep_causal_level' or 'wsrep_synchronous_level' would be fine too. I need your opinion.

[Alex Yurchenko (ayurchen)]

General comment: as we discussed in chat, 'causal' is the bad word here, we definitely want to get rid of it (except for 'wsrep_causal_reads' variable that stays for backward compatibility). We are still undecided whether it should be 'wsrep_synchronous_reads' or 'wsrep_syncronous_wait', but not 'causal' for sure, so it should be gone, both from the option name and from the code.
Not sure that 'mask' should be present in the name as well, it is a type of a variable, not its purpose. You don't say 'wsrep_node_address_string', you just say 'wsrep_node_address'.

review: Needs Fixing

[Yan Zhang (yan.zhang)]

comment replied. and about new variable name, how about:
1. wsrep_sync_reads (new name for wsrep_causal_reads)
2. wsrep_sync_wait (bitmask).

[lp:~yan.zhang/codership-mysql/5.5]

4008. By Yan Zhang on 2014-07-17

References lp:1277053 renaming wsrep_casual_mask to wsrep_sync_wait and some related enums

4009. By Yan Zhang on 2014-07-17

Merge trunk

[Alex Yurchenko (ayurchen)]

I think you would agree that it is still not ready

review: Needs Fixing

[lp:~yan.zhang/codership-mysql/5.5]

4010. By Yan Zhang on 2014-07-24

References lp:1277053 do some refactor according to review.
1. remove duplicated checks.
2. add optional causal check to autocommit INSERT and REPLACE stmt.
3. rename some related functions.

4011. By Yan Zhang on 2014-07-28

merge trunk

[Alex Yurchenko (ayurchen)]

Some minor fixes are still needed.

review: Needs Fixing

[lp:~yan.zhang/codership-mysql/5.5]

4012. By Yan Zhang on 2014-07-31

References lp:1277053 remove 'wsrep_sync_wait_by_mask', use 'wsrep_sync_wait' with default parameter value.
change description of 'wsrep_causal_reads'.

[Yan Zhang (yan.zhang)]

fixed in http://bazaar.launchpad.net/~yan.zhang/codership-mysql/5.5/revision/4012

[Alex Yurchenko (ayurchen)]

some cleanups still needed

[Alex Yurchenko (ayurchen)]

and I would leave it to compiler to decide about inlining - just give it a chance... WSREP_DEBUG of course is a mean macro, but other than that - this function is going to be called for every request.

[Yan Zhang (yan.zhang)]

inline this function is little bit tricky.

Because in wsrep_mysqld.h 'struct THD' is forward declared which means we can not access its members. We can put '#include <sql_class.h>' which contains definition of 'struct THD' into wsrep_mysqld.h, but unfortunately there is '#include <wsrep_mysqld.h>' in sql_class.h too which causes cross reference of header files.

so what I plan is to put all inline functions into a separate header file 'wsrep_mysqld_inl.h'. Any compilation unit who needs any inline function in this file could include this header file.

[lp:~yan.zhang/codership-mysql/5.5]

4013. By Yan Zhang on 2014-08-01

References lp:1277053 some cleanups. move 'wsrep_sync_wait' to wsrep_mysqld_inl.h for inline

[Alex Yurchenko (ayurchen)]

1. I'll be damned if that compiles ;) wsrep_mysqld.cc should fail compilation.
2. wsrep_sync_wait is a part of the same interface provided by wsrep integration layer as, e.g. enum_wsrep_sync_wait which is in wsrep_mysqld.h. Why did you decide to put it in a separate header? It would be logical to keep them together. If it is to reduce compilation overhead, then probably it should be named more descriptively.

[Yan Zhang (yan.zhang)]

1. there are several recent patches(4011,4012,4013) in trunk cause conflict with my branch. My branch just contains the code in trunk up to 4010. Maybe that's why compilation fails. I'll handle that conflict.

2. The mainly reason is header file cross reference. I have to put wsrep_sync_wait into header file(wsrep_mysqld.h) for inline. Since I use some specific fields in 'THD' structure(which is defined sql_class.h) in wsrep_sync_wait, I have to include sql_class.h in wsrep_mysql.h too. But unfortunately sql_class.h already includes wsrep_mysql.h. So wsrep_mysql.h will include sql_class.h, and sql_class.h will include wsrep_mysql.h, that's header file cross reference situation which will make compilation fail.

Although actually we can make it compiles by moving '#include <wsrep_mysqld.h>' in sql_class.h to the position after definition of 'THD' structure, it's just much more ugly.

[Alex Yurchenko (ayurchen)]

1. Just look at the patch. There are merge artifacts.
2. You're right, this is quite ugly. Sorry for pushing you into that. I think this moving of wsrep_sync_wait() definition to header file was not a good idea after all.

I'll leave it for you to decide what to do about it and merge to trunk. All the principal issues have been resolved IMO.

[lp:~yan.zhang/codership-mysql/5.5]

4014. By Yan Zhang on 2014-08-12

References lp:1277053 cancel inlining function 'wsrep_sync_wait'

[Yan Zhang (yan.zhang)]

OK. I'd like not to inline 'wsrep_sync_wait' function atm, and put definition of it back to 'wsrep_mysqld.cc'

[lp:~yan.zhang/codership-mysql/5.5]

4015. By Yan Zhang on 2014-08-12

merge trunk to r4013

=== modified file 'sql/sql_class.h'
--- sql/sql_class.h	2014-04-28 16:57:30 +0000
+++ sql/sql_class.h	2014-08-12 03:33:55 +0000
@@ -520,6 +520,7 @@
 #ifdef WITH_WSREP
   my_bool wsrep_on;
   my_bool wsrep_causal_reads;
+  uint wsrep_sync_wait;
   ulong wsrep_retry_autocommit;
 #endif
   double long_query_time_double;

=== modified file 'sql/sql_parse.cc'
--- sql/sql_parse.cc	2014-07-30 21:09:47 +0000
+++ sql/sql_parse.cc	2014-08-12 03:33:55 +0000
@@ -2393,7 +2393,7 @@
   case SQLCOM_SHOW_STATUS_PROC:
   case SQLCOM_SHOW_STATUS_FUNC:
 #ifdef WITH_WSREP
-    if (WSREP_CLIENT(thd) && wsrep_causal_wait(thd)) goto error;
+    if (WSREP_CLIENT(thd) && wsrep_sync_wait(thd)) goto error;
 #endif /* WITH_WSREP */
     if ((res= check_table_access(thd, SELECT_ACL, all_tables, FALSE,
                                   UINT_MAX, FALSE)))
@@ -2405,7 +2405,7 @@
     system_status_var old_status_var= thd->status_var;
     thd->initial_status_var= &old_status_var;
 #ifdef WITH_WSREP
-    if (WSREP_CLIENT(thd) && wsrep_causal_wait(thd)) goto error;
+    if (WSREP_CLIENT(thd) && wsrep_sync_wait(thd)) goto error;
 #endif /* WITH_WSREP */
     if (!(res= check_table_access(thd, SELECT_ACL, all_tables, FALSE,
                                   UINT_MAX, FALSE)))
@@ -2444,7 +2444,7 @@
 #endif /* WITH_WSREP */
   case SQLCOM_SELECT:
 #ifdef WITH_WSREP
-    if (WSREP_CLIENT(thd) && wsrep_causal_wait(thd)) goto error;
+    if (WSREP_CLIENT(thd) && wsrep_sync_wait(thd)) goto error;
   case SQLCOM_SHOW_VARIABLES:
   case SQLCOM_SHOW_CHARSETS:
   case SQLCOM_SHOW_COLLATIONS:
@@ -3035,7 +3035,7 @@
 #else
     {
 #ifdef WITH_WSREP
-      if (WSREP_CLIENT(thd) && wsrep_causal_wait(thd)) goto error;
+      if (WSREP_CLIENT(thd) && wsrep_sync_wait(thd)) goto error;
 #endif /* WITH_WSREP */
 
      /*
@@ -3094,7 +3094,7 @@
   {
     DBUG_ASSERT(first_table == all_tables && first_table != 0);
 #ifdef WITH_WSREP
-    if (WSREP_CLIENT(thd) && wsrep_causal_wait(thd)) goto error;
+    if (WSREP_CLIENT(thd) && wsrep_sync_wait(thd)) goto error;
 #endif /* WITH_WSREP */
 
     if (check_table_access(thd, SELECT_ACL, all_tables,
@@ -3105,6 +3105,10 @@
     break;
   }
   case SQLCOM_UPDATE:
+#ifdef WITH_WSREP
+      if (WSREP_CLIENT(thd) &&
+          wsrep_sync_wait(thd, WSREP_SYNC_WAIT_BEFORE_UPDATE_DELETE)) goto error;
+#endif /* WITH_WSREP */      
   {
     ha_rows found= 0, updated= 0;
     DBUG_ASSERT(first_table == all_tables && first_table != 0);
@@ -3144,6 +3148,10 @@
     /* if we switched from normal update, rights are checked */
     if (up_result != 2)
     {
+#ifdef WITH_WSREP
+      if (WSREP_CLIENT(thd) &&
+          wsrep_sync_wait(thd, WSREP_SYNC_WAIT_BEFORE_UPDATE_DELETE)) goto error;
+#endif /* WITH_WSREP */
       if ((res= multi_update_precheck(thd, all_tables)))
         break;
     }
@@ -3213,6 +3221,10 @@
     break;
   }
   case SQLCOM_REPLACE:
+#ifdef WITH_WSREP
+      if (WSREP_CLIENT(thd) &&
+          wsrep_sync_wait(thd, WSREP_SYNC_WAIT_BEFORE_INSERT_REPLACE)) goto error;
+#endif /* WITH_WSREP */
 #ifndef DBUG_OFF
     if (mysql_bin_log.is_open())
     {
@@ -3248,6 +3260,10 @@
     }
 #endif
   case SQLCOM_INSERT:
+#ifdef WITH_WSREP
+      if (WSREP_CLIENT(thd) &&
+          wsrep_sync_wait(thd, WSREP_SYNC_WAIT_BEFORE_INSERT_REPLACE)) goto error;
+#endif /* WITH_WSREP */      
   {
     DBUG_ASSERT(first_table == all_tables && first_table != 0);
     if ((res= insert_precheck(thd, all_tables)))
@@ -3281,6 +3297,10 @@
   }
   case SQLCOM_REPLACE_SELECT:
   case SQLCOM_INSERT_SELECT:
+#ifdef WITH_WSREP
+      if (WSREP_CLIENT(thd) &&
+          wsrep_sync_wait(thd, WSREP_SYNC_WAIT_BEFORE_INSERT_REPLACE)) goto error;
+#endif /* WITH_WSREP */      
   {
     select_result *sel_result;
     DBUG_ASSERT(first_table == all_tables && first_table != 0);
@@ -3373,6 +3393,10 @@
     break;
   }
   case SQLCOM_DELETE:
+#ifdef WITH_WSREP
+    if (WSREP_CLIENT(thd) && 
+        wsrep_sync_wait(thd, WSREP_SYNC_WAIT_BEFORE_UPDATE_DELETE)) goto error;
+#endif /* WITH_WSREP */
   {
     DBUG_ASSERT(first_table == all_tables && first_table != 0);
     if ((res= delete_precheck(thd, all_tables)))
@@ -3388,6 +3412,10 @@
     break;
   }
   case SQLCOM_DELETE_MULTI:
+#ifdef WITH_WSREP
+    if (WSREP_CLIENT(thd) && 
+        wsrep_sync_wait(thd, WSREP_SYNC_WAIT_BEFORE_UPDATE_DELETE)) goto error;
+#endif /* WITH_WSREP */
   {
     DBUG_ASSERT(first_table == all_tables && first_table != 0);
     TABLE_LIST *aux_tables= thd->lex->auxiliary_table_list.first;

=== modified file 'sql/sys_vars.cc'
--- sql/sys_vars.cc	2014-06-19 09:19:53 +0000
+++ sql/sys_vars.cc	2014-08-12 03:33:55 +0000
@@ -3465,9 +3465,21 @@
        CMD_LINE(OPT_ARG), DEFAULT(TRUE));
 
 static Sys_var_mybool Sys_wsrep_causal_reads(
-       "wsrep_causal_reads", "Enable \"strictly synchronous\" semantics for read operations",
+       "wsrep_causal_reads", "(DEPRECATED) setting this variable is equivalent to setting wsrep_sync_wait READ flag",
        SESSION_VAR(wsrep_causal_reads), 
-       CMD_LINE(OPT_ARG), DEFAULT(FALSE));
+       CMD_LINE(OPT_ARG), DEFAULT(FALSE),
+       NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(0),
+       ON_UPDATE(wsrep_causal_reads_update));
+
+static Sys_var_uint Sys_wsrep_sync_wait(
+       "wsrep_sync_wait", "Ensure \"synchronous\" read view before executing an operation of the type specified by bitmask: 1 - READ(includes SELECT, SHOW and BEGIN/START TRANSACTION); 2 - UPDATE and DELETE; 4 - INSERT and REPLACE",
+       SESSION_VAR(wsrep_sync_wait),
+       CMD_LINE(OPT_ARG),
+       VALID_RANGE(WSREP_SYNC_WAIT_NONE, WSREP_SYNC_WAIT_MAX),
+       DEFAULT(WSREP_SYNC_WAIT_NONE),
+       BLOCK_SIZE(1),
+       NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(0),
+       ON_UPDATE(wsrep_sync_wait_update));
 
 static const char *wsrep_OSU_method_names[]= { "TOI", "RSU", NullS };
 static Sys_var_enum Sys_wsrep_OSU_method(

=== modified file 'sql/transaction.cc'
--- sql/transaction.cc	2013-10-27 11:22:28 +0000
+++ sql/transaction.cc	2014-08-12 03:33:55 +0000
@@ -161,7 +161,7 @@
 
 #ifdef WITH_WSREP
   thd->wsrep_PA_safe= true;
-  if (WSREP_CLIENT(thd) && wsrep_causal_wait(thd))
+  if (WSREP_CLIENT(thd) && wsrep_sync_wait(thd))
     DBUG_RETURN(TRUE);
 #endif /* WITH_WSREP */
 

=== modified file 'sql/wsrep_mysqld.cc'
--- sql/wsrep_mysqld.cc	2014-07-31 12:59:23 +0000
+++ sql/wsrep_mysqld.cc	2014-08-12 03:33:55 +0000
@@ -847,13 +847,15 @@
   return true;
 }
 
-bool
-wsrep_causal_wait (THD* thd)
+bool wsrep_sync_wait (THD* thd, uint mask)
 {
-  if (thd->variables.wsrep_causal_reads && thd->variables.wsrep_on &&
+  if ((thd->variables.wsrep_sync_wait & mask) &&
+      thd->variables.wsrep_on &&
       !thd->in_active_multi_stmt_transaction() &&
       thd->wsrep_conflict_state != REPLAYING)
   {
+    WSREP_DEBUG("wsrep_sync_wait: thd->variables.wsrep_sync_wait = %u, mask = %u",
+                thd->variables.wsrep_sync_wait, mask);
     // This allows autocommit SELECTs and a first SELECT after SET AUTOCOMMIT=0
     // TODO: modify to check if thd has locked any rows.
     wsrep_gtid_t  gtid;
@@ -877,7 +879,7 @@
         err= ER_NOT_SUPPORTED_YET;
         break;
       default:
-        msg= "Causal wait failed.";
+        msg= "Synchronous wait failed.";
         err= ER_LOCK_WAIT_TIMEOUT; // NOTE: the above msg won't be displayed
                                    //       with ER_LOCK_WAIT_TIMEOUT
       }

=== modified file 'sql/wsrep_mysqld.h'
--- sql/wsrep_mysqld.h	2014-06-19 09:19:53 +0000
+++ sql/wsrep_mysqld.h	2014-08-12 03:33:55 +0000
@@ -101,6 +101,14 @@
 extern my_bool     wsrep_slave_UK_checks;
 
 enum enum_wsrep_OSU_method { WSREP_OSU_TOI, WSREP_OSU_RSU };
+enum enum_wsrep_sync_wait {
+    WSREP_SYNC_WAIT_NONE = 0x0,
+    // show, select, begin
+    WSREP_SYNC_WAIT_BEFORE_READ = 0x1,
+    WSREP_SYNC_WAIT_BEFORE_UPDATE_DELETE = 0x2,
+    WSREP_SYNC_WAIT_BEFORE_INSERT_REPLACE = 0x4,
+    WSREP_SYNC_WAIT_MAX = 0x7
+};
 
 // MySQL status variables
 extern my_bool     wsrep_connected;
@@ -171,9 +179,10 @@
 /* new defines */
 extern void wsrep_stop_replication(THD *thd);
 extern bool wsrep_start_replication();
-extern bool wsrep_causal_wait(THD* thd);
+extern bool wsrep_sync_wait (THD* thd, uint mask = WSREP_SYNC_WAIT_BEFORE_READ);
 extern int  wsrep_check_opts (int argc, char* const* argv);
 extern void wsrep_prepend_PATH (const char* path);
+/* some inline functions are defined in wsrep_mysqld_inl.h */
 
 /* Other global variables */
 extern wsrep_seqno_t wsrep_locked_seqno;

=== modified file 'sql/wsrep_var.cc'
--- sql/wsrep_var.cc	2014-01-07 21:49:58 +0000
+++ sql/wsrep_var.cc	2014-08-12 03:33:55 +0000
@@ -63,11 +63,29 @@
   return false;
 }
 
-void wsrep_causal_reads_update (sys_var *self, THD* thd, enum_var_type var_type)
+bool wsrep_causal_reads_update (sys_var *self, THD* thd, enum_var_type var_type)
 {
-  if (var_type == OPT_GLOBAL) {
-    thd->variables.wsrep_causal_reads = global_system_variables.wsrep_causal_reads;
+  // global setting should not affect session setting.
+  // if (var_type == OPT_GLOBAL) {
+  //   thd->variables.wsrep_causal_reads = global_system_variables.wsrep_causal_reads;
+  // }
+  if (thd->variables.wsrep_causal_reads) {
+    thd->variables.wsrep_sync_wait |= WSREP_SYNC_WAIT_BEFORE_READ;
+  } else {
+    thd->variables.wsrep_sync_wait &= ~WSREP_SYNC_WAIT_BEFORE_READ;
   }
+  return false;
+}
+
+bool wsrep_sync_wait_update (sys_var* self, THD* thd, enum_var_type var_type)
+{
+  // global setting should not affect session setting.
+  // if (var_type == OPT_GLOBAL) {
+  //   thd->variables.wsrep_sync_wait = global_system_variables.wsrep_sync_wait;
+  // }
+  thd->variables.wsrep_causal_reads = thd->variables.wsrep_sync_wait &
+          WSREP_SYNC_WAIT_BEFORE_READ;
+  return false;
 }
 
 static int wsrep_start_position_verify (const char* start_str)

=== modified file 'sql/wsrep_var.h'
--- sql/wsrep_var.h	2013-10-11 16:03:06 +0000
+++ sql/wsrep_var.h	2014-08-12 03:33:55 +0000
@@ -33,7 +33,8 @@
 #define INIT_ARGS    (const char* opt)
 
 extern bool wsrep_on_update                  UPDATE_ARGS;
-extern void wsrep_causal_reads_update        UPDATE_ARGS;
+extern bool wsrep_causal_reads_update        UPDATE_ARGS;
+extern bool wsrep_sync_wait_update           UPDATE_ARGS;
 extern bool wsrep_start_position_check       CHECK_ARGS;
 extern bool wsrep_start_position_update      UPDATE_ARGS;
 extern void wsrep_start_position_init        INIT_ARGS;