maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #10185
Re: [Commits] 27af988: MDEV-11479 Improved wsrep_dirty_reads
Hi Vicentiu!
Please review the updated patch.
On Thu, Dec 8, 2016 at 4:42 PM, Vicențiu Ciorbaru <vicentiu@xxxxxxxxxxx> wrote:
> Hi Sachin!
>
> Comments inline:
>
> On Sun, 4 Dec 2016 at 08:36 <sachin.setiya@xxxxxxxxxxx> wrote:
>>
>> revision-id: 27af98805c27c8e0c41dc8c19bf80aefff4c6d3d
>> (mariadb-galera-5.5.53-3-g27af988)
>> parent(s): 72fd15f7c31aa3e3705ae1b005a3247a985c5bb3
>> author: SachinSetiya
>> committer: SachinSetiya
>
>
> NIT: To get these to display correctly you should have a space between
> Sachin and Setiya.
>
Done :)
>>
>> timestamp: 2016-12-04 12:05:05 +0530
>> message:
>>
>> MDEV-11479 Improved wsrep_dirty_reads
>>
>> Tasks:-
>> Changes in wsrep_dirty_reads variable
>> 1.) Global + Session scope (Current: session-only)
>> 2.) Allow all commands that do not change data (besides SELECT)
>> 3.) Allow prepared Statements that do not change data
>> 4.) Works with wsrep_sync_wait enabled
>
>
> You also seemed to have made the variable available as a command line
> option. Please specify that as well.
>
Done.
>>
>> ---
>> .../suite/galera/r/galera_var_dirty_reads.result | 29
>> +++++++++++++++++++---
>> .../suite/galera/t/galera_var_dirty_reads.test | 25
>> ++++++++++++++++++-
>> .../sys_vars/r/wsrep_dirty_reads_basic.result | 19 ++++++++++++--
>> .../suite/sys_vars/t/wsrep_dirty_reads_basic.test | 13 ++++++++--
>> sql/sql_parse.cc | 18 ++++++++++----
>> sql/sys_vars.cc | 4 +--
>> sql/wsrep_mysqld.cc | 3 ++-
>> sql/wsrep_mysqld.h | 1 +
>> 8 files changed, 96 insertions(+), 16 deletions(-)
>>
>> diff --git a/mysql-test/suite/galera/r/galera_var_dirty_reads.result
>> b/mysql-test/suite/galera/r/galera_var_dirty_reads.result
>> index 6a2aa1e..d009c1b 100644
>> --- a/mysql-test/suite/galera/r/galera_var_dirty_reads.result
>> +++ b/mysql-test/suite/galera/r/galera_var_dirty_reads.result
>> @@ -28,9 +28,32 @@ SELECT @@max_allowed_packet;
>> SELECT 2+2 from DUAL;
>> 2+2
>> 4
>> -SELECT sysdate() from DUAL;
>> -sysdate()
>> -2016-10-28 23:13:06
>> +SET @@session.wsrep_dirty_reads=ON;
>> +SELECT * FROM t1;
>> +i
>> +1
>> +prepare stmt_show from 'select 1';
>> +prepare stmt_select from 'select * from t1';
>> +prepare stmt_insert from 'insert into t1 values(1)';
>> +execute stmt_show;
>> +1
>> +1
>> +execute stmt_select;
>> +i
>> +1
>> +execute stmt_insert;
>> +ERROR 08S01: WSREP has not yet prepared node for application use
>> +select * from t1;
>> +i
>> +1
>> +execute stmt_show;
>> +1
>> +1
>> +execute stmt_select;
>> +i
>> +1
>> +execute stmt_insert;
>> +ERROR 08S01: WSREP has not yet prepared node for application use
>> SELECT * FROM t1;
>> i
>> 1
>
>
> You're not really testing the global aspect of the variable here. You're
> only testing the sesssion behaviour of it. Also, test with it both ON and
> OFF (when setting it globally). Here's a sample that tests both session and
> global behaviour:
>
> --enable_connect_log
>
> SELECT @@expensive_subquery_limit;
> SET @@session.expensive_subquery_limit=300;
> SELECT @@expensive_subquery_limit;
>
> create user user1;
> create user user2;
> --connect (con1, localhost, user1,,)
> SELECT @@expensive_subquery_limit;
> SET @@session.expensive_subquery_limit=300;
> SELECT @@expensive_subquery_limit;
>
> connection default;
>
> SELECT @@expensive_subquery_limit;
> SET @@global.expensive_subquery_limit=400;
> SELECT @@expensive_subquery_limit;
>
> --connect (con2, localhost, user1,,)
> SELECT @@expensive_subquery_limit;
>
>
> connection default;
> drop user user1;
> drop user user2;
>
> ************************************
>
> SELECT @@expensive_subquery_limit;
> @@expensive_subquery_limit
> 100
> SET @@session.expensive_subquery_limit=300;
> SELECT @@expensive_subquery_limit;
> @@expensive_subquery_limit
> 300
> create user user1;
> create user user2;
> connect con1, localhost, user1,,;
> SELECT @@expensive_subquery_limit;
> @@expensive_subquery_limit
> 100
> SET @@session.expensive_subquery_limit=300;
> SELECT @@expensive_subquery_limit;
> @@expensive_subquery_limit
> 300
> connection default;
> SELECT @@expensive_subquery_limit;
> @@expensive_subquery_limit
> 300
> SET @@global.expensive_subquery_limit=400;
> SELECT @@expensive_subquery_limit;
> @@expensive_subquery_limit
> 300
> connect con2, localhost, user1,,;
> SELECT @@expensive_subquery_limit;
> @@expensive_subquery_limit
> 400
> connection default;
> drop user user1;
> drop user user2;
>
> Notice how the last connection has the default value 400 as that's the
> global value. For your use case, you'd also have to execute the prepared
> statements to see if it works (execute select_stmt).
>
Done.
>
>>
>> diff --git a/mysql-test/suite/galera/t/galera_var_dirty_reads.test
>> b/mysql-test/suite/galera/t/galera_var_dirty_reads.test
>> index dd7bc88..44931aa 100644
>> --- a/mysql-test/suite/galera/t/galera_var_dirty_reads.test
>> +++ b/mysql-test/suite/galera/t/galera_var_dirty_reads.test
>> @@ -44,7 +44,30 @@ SET @@session.wsrep_dirty_reads=OFF;
>> SELECT 2;
>> SELECT @@max_allowed_packet;
>> SELECT 2+2 from DUAL;
>> -SELECT sysdate() from DUAL;
>> +
>> +#Query which does not change data should be allowed MDEV-11479
>> +SET @@session.wsrep_dirty_reads=ON;
>> +SELECT * FROM t1;
>> +
>> +#Prepared statement creation should be allowed MDEV-11479
>> +prepare stmt_show from 'select 1';
>> +prepare stmt_select from 'select * from t1';
>> +prepare stmt_insert from 'insert into t1 values(1)';
>> +
>> +#Only prepare statement which does not change data should be allowed
>> +execute stmt_show;
>> +execute stmt_select;
>> +--error ER_UNKNOWN_COM_ERROR
>> +execute stmt_insert;
>>
>> +
>> +#wsrep_dirty_read should work when wsrep_sync_wait is 1 or non zero
>> +#because we already are disconnected , So It does not make any sense
>> +#to wait for other nodes
>
>
> I don't think you're testing this correctly. See my comment on the if
> statement which is supposed to govern this.
>
>>
>> +select * from t1;
>> +execute stmt_show;
>> +execute stmt_select;
>> +--error ER_UNKNOWN_COM_ERROR
>> +execute stmt_insert;
>>
>> --disable_query_log
>> --eval SET @@global.wsrep_cluster_address =
>> '$wsrep_cluster_address_saved'
>> diff --git a/mysql-test/suite/sys_vars/r/wsrep_dirty_reads_basic.result
>> b/mysql-test/suite/sys_vars/r/wsrep_dirty_reads_basic.result
>> index d2a62d6..649ac83 100644
>> --- a/mysql-test/suite/sys_vars/r/wsrep_dirty_reads_basic.result
>> +++ b/mysql-test/suite/sys_vars/r/wsrep_dirty_reads_basic.result
>> @@ -5,12 +5,13 @@
>> SET @wsrep_dirty_reads_session_saved = @@session.wsrep_dirty_reads;
>> # default
>> SELECT @@global.wsrep_dirty_reads;
>> -ERROR HY000: Variable 'wsrep_dirty_reads' is a SESSION variable
>> +@@global.wsrep_dirty_reads
>> +0
>> SELECT @@session.wsrep_dirty_reads;
>> @@session.wsrep_dirty_reads
>> 0
>>
>> -# scope and valid values
>> +# valid values for session
>> SET @@session.wsrep_dirty_reads=OFF;
>> SELECT @@session.wsrep_dirty_reads;
>> @@session.wsrep_dirty_reads
>> @@ -24,6 +25,20 @@ SELECT @@session.wsrep_dirty_reads;
>> @@session.wsrep_dirty_reads
>> 0
>>
>> +# valid values for global
>> +SET @@global.wsrep_dirty_reads=OFF;
>> +SELECT @@global.wsrep_dirty_reads;
>> +@@global.wsrep_dirty_reads
>> +0
>> +SET @@global.wsrep_dirty_reads=ON;
>> +SELECT @@global.wsrep_dirty_reads;
>> +@@global.wsrep_dirty_reads
>> +1
>> +SET @@global.wsrep_dirty_reads=default;
>> +SELECT @@global.wsrep_dirty_reads;
>> +@@global.wsrep_dirty_reads
>> +0
>> +
>> # invalid values
>> SET @@session.wsrep_dirty_reads=NULL;
>> ERROR 42000: Variable 'wsrep_dirty_reads' can't be set to the value of
>> 'NULL'
>
>
> Test invalid values for global scope as well. It should not change obviously
> but since we're writing tests, might as well do that too.
>
Done.
>>
>> diff --git a/mysql-test/suite/sys_vars/t/wsrep_dirty_reads_basic.test
>> b/mysql-test/suite/sys_vars/t/wsrep_dirty_reads_basic.test
>> index a47524f..000c4c4 100644
>> --- a/mysql-test/suite/sys_vars/t/wsrep_dirty_reads_basic.test
>> +++ b/mysql-test/suite/sys_vars/t/wsrep_dirty_reads_basic.test
>> @@ -8,12 +8,12 @@
>> SET @wsrep_dirty_reads_session_saved = @@session.wsrep_dirty_reads;
>>
>> --echo # default
>> ---error ER_INCORRECT_GLOBAL_LOCAL_VAR
>> +
>> SELECT @@global.wsrep_dirty_reads;
>> SELECT @@session.wsrep_dirty_reads;
>>
>> --echo
>> ---echo # scope and valid values
>> +--echo # valid values for session
>> SET @@session.wsrep_dirty_reads=OFF;
>> SELECT @@session.wsrep_dirty_reads;
>> SET @@session.wsrep_dirty_reads=ON;
>> @@ -22,6 +22,15 @@ SET @@session.wsrep_dirty_reads=default;
>> SELECT @@session.wsrep_dirty_reads;
>>
>> --echo
>> +--echo # valid values for global
>> +SET @@global.wsrep_dirty_reads=OFF;
>> +SELECT @@global.wsrep_dirty_reads;
>> +SET @@global.wsrep_dirty_reads=ON;
>> +SELECT @@global.wsrep_dirty_reads;
>> +SET @@global.wsrep_dirty_reads=default;
>> +SELECT @@global.wsrep_dirty_reads;
>> +
>> +--echo
>> --echo # invalid values
>> --error ER_WRONG_VALUE_FOR_VAR
>> SET @@session.wsrep_dirty_reads=NULL;
>> diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
>> index eb4f714..a56d694 100644
>> --- a/sql/sql_parse.cc
>> +++ b/sql/sql_parse.cc
>> @@ -2393,10 +2393,11 @@ mysql_execute_command(THD *thd)
>> */
>> if (thd->variables.wsrep_on && !thd->wsrep_applier && !wsrep_ready &&
>> lex->sql_command != SQLCOM_SET_OPTION &&
>> - !(thd->variables.wsrep_dirty_reads &&
>> - lex->sql_command == SQLCOM_SELECT) &&
>> - !(lex->sql_command == SQLCOM_SELECT &&
>> - !all_tables) &&
>> + !(thd->variables.wsrep_dirty_reads &&
>> + (sql_command_flags[lex->sql_command] &
>> + CF_CHANGES_DATA) == 0) &&
>
>
> Put this line on one line only, the single & is confusing, or better, turn
> it into a small inline function, such as command_changes_data()
Okay.
>
>>
>> + !(lex->sql_command == SQLCOM_SELECT &&
>> + !all_tables) &&
>
> Can't tell for certain but this condition might not be necessary if you have
> the previous one.
This is required by https://jira.mariadb.org/browse/MDEV-11016
previous condition is only applied when wsrep_dirty reads is true,
this is applied independent of wsrep_dirty_reads
What happens when you have a stored procedure that inserts
> into a table? Does the CF_CHANGES_DATA flag filter out the query or not?
> ex:
> select function_with_side_efect(1);
> This seems like a good test case that I'm not sure is tested for this
> variable anyway. Please try it.
This wont work, because CREATE PROCEDURE , does changes the data.
sql_command_flags[SQLCOM_CREATE_PROCEDURE]= CF_CHANGES_DATA |
CF_AUTO_COMMIT_TRANS;
>
>
>>
>> !wsrep_is_show_query(lex->sql_command))
>> {
>> #if DIRTY_HACK
>> @@ -2512,7 +2513,14 @@ mysql_execute_command(THD *thd)
>> case SQLCOM_SHOW_INDEX_STATS:
>> case SQLCOM_SELECT:
>> #ifdef WITH_WSREP
>> - if (WSREP_CLIENT(thd) && wsrep_sync_wait(thd)) goto error;
>> + /*
>> + wsrep_dirty_read should work when wsrep_sync_wait is 1 or non-zero
>> + because we already are disconnected. So, it does not make any sense
>> + to wait for other nodes
>> + */
>>
> This here is not correct. This will not execute the select if
> wsrep_dirty_reads is true and wsrep_ready is true and wsrep_sync_wait(thd)
> is non-zero.
This is expected. If wsrep_ready is true, that means cluster is online and if
wsrep_sync_wait returns some non zero value then we have to goto error.
> if (WSREP_CLIENT(thd) && thd->variables.wsrep_dirty_reads(wsrep_ready
>>
>> + if (WSREP_CLIENT(thd) && (wsrep_ready ||
>> !thd->variables.wsrep_dirty_reads)
>> + && wsrep_sync_wait(thd))
>> + goto error
>>
Actually original condition was
if (WSREP_CLIENT(thd))
{
if (!wsrep_ready && thd->variables.wsrep_dirty_reads)
;
else if(wsrep_sync_wait(thd))
goto error;
}
which I (with the help of wlad ) reduced it to above IF.
>> case SQLCOM_SHOW_VARIABLES:
>> case SQLCOM_SHOW_CHARSETS:
>> case SQLCOM_SHOW_COLLATIONS:
>> diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc
>> index bdcedff..0d864f4 100644
>> --- a/sql/sys_vars.cc
>> +++ b/sql/sys_vars.cc
>> @@ -3967,8 +3967,8 @@ static Sys_var_mybool Sys_wsrep_restart_slave(
>>
>> static Sys_var_mybool Sys_wsrep_dirty_reads(
>> "wsrep_dirty_reads", "Do not reject SELECT queries even when the
>> node "
>> - "is not ready.", SESSION_ONLY(wsrep_dirty_reads), NO_CMD_LINE,
>> - DEFAULT(FALSE), NO_MUTEX_GUARD, NOT_IN_BINLOG);
>> + "is not ready.", SESSION_VAR(wsrep_dirty_reads),
>>
>> + CMD_LINE(OPT_ARG), DEFAULT(FALSE), NO_MUTEX_GUARD, NOT_IN_BINLOG);
>
>
> We might need a guard here since global variables can be changed from
> multiple places. Not 100% sure about this but that's my intuition. Need to
> look this up.
I am not sure if I get this because global variables are guarded by
LOCK_global_system_variables mutex.
/*
the define below means that there's no *second* mutex guard,
LOCK_global_system_variables always guards all system variables
*/
#define NO_MUTEX_GUARD ((PolyLock*)0)
And auto increment uses the same approach.
static Sys_var_ulong Sys_auto_increment_increment(
"auto_increment_increment",
"Auto-increment columns are incremented by this",
SESSION_VAR(auto_increment_increment),
CMD_LINE(OPT_ARG),
VALID_RANGE(1, 65535), DEFAULT(1), BLOCK_SIZE(1),
NO_MUTEX_GUARD, IN_BINLOG)
>
>>
>> #endif /* WITH_WSREP */
>>
>> diff --git a/sql/wsrep_mysqld.cc b/sql/wsrep_mysqld.cc
>> index 48114fc..f21ff92 100644
>> --- a/sql/wsrep_mysqld.cc
>> +++ b/sql/wsrep_mysqld.cc
>> @@ -65,7 +65,8 @@ my_bool wsrep_restart_slave_activated = 0; // node has
>> dropped, and slave
>> // restart will be needed
>> my_bool wsrep_slave_UK_checks = 0; // slave thread does UK
>> checks
>> my_bool wsrep_slave_FK_checks = 0; // slave thread does FK
>> checks
>> -
>> +bool wsrep_dirty_reads = 0; // is dirty reads allowed for
>> + // disconnectd/non major node
>
>
> Typo: disconnectd -> disconnected.
> I really hate how the whole file is set up with TRUE/FALSE/0 for either bool
> or my_bool. It's so inconsistent. I'd prefer 'true' here to complicate stuff
> further. :) To be "consistent" I'd make it a my_bool, but we should clean up
> all of this mess at one point.
>
Done, I value changed to false.
>>
>> /*
>> Set during the creation of first wsrep applier and rollback threads.
>> Since these threads are critical, abort if the thread creation fails.
>> diff --git a/sql/wsrep_mysqld.h b/sql/wsrep_mysqld.h
>> index 3fb5189..9b9179a 100644
>> --- a/sql/wsrep_mysqld.h
>> +++ b/sql/wsrep_mysqld.h
>> @@ -85,6 +85,7 @@ extern ulong wsrep_max_ws_size;
>> extern ulong wsrep_max_ws_rows;
>> extern const char* wsrep_notify_cmd;
>> extern my_bool wsrep_certify_nonPK;
>> +extern bool wsrep_dirty_reads;
>
>
> Remember to update the type here too.
Done.
>
>>
>> extern long wsrep_max_protocol_version;
>> extern long wsrep_protocol_version;
>> extern ulong wsrep_forced_binlog_format;
>
>
--
Regards
Sachin Setiya
Software Engineer at MariaDB
commit 7e1ce93bf5887accf2d8ff7dc1daa5310cd68238
Author: SachinSetiya <sachinsetia1001@xxxxxxxxx>
Date: Sat Dec 3 17:28:12 2016 +0530
MDEV-11479 Improved wsrep_dirty_reads
Tasks:-
Changes in wsrep_dirty_reads variable
1.) Global + Session scope (Current: session-only)
2.) Can be Set using command line.
3.) Allow all commands that do not change data (besides SELECT)
4.) Allow prepared Statements that do not change data
5.) Works with wsrep_sync_wait enabled
diff --git a/mysql-test/suite/galera/r/galera_var_dirty_reads.result b/mysql-test/suite/galera/r/galera_var_dirty_reads.result
index 6a2aa1e..cd132a0 100644
--- a/mysql-test/suite/galera/r/galera_var_dirty_reads.result
+++ b/mysql-test/suite/galera/r/galera_var_dirty_reads.result
@@ -3,6 +3,10 @@ INSERT INTO t1 VALUES(1);
SELECT * FROM t1;
i
1
+create user user1;
+grant all privileges on *.* to user1;
+create user user2;
+grant all privileges on *.* to user2;
SET @@global.wsrep_cluster_address = '';
SET @@session.wsrep_dirty_reads=OFF;
SET SESSION wsrep_sync_wait=0;
@@ -28,11 +32,81 @@ SELECT @@max_allowed_packet;
SELECT 2+2 from DUAL;
2+2
4
-SELECT sysdate() from DUAL;
-sysdate()
-2016-10-28 23:13:06
+SET @@session.wsrep_dirty_reads=ON;
+SELECT * FROM t1;
+i
+1
+connect con1, localhost, user1,,test,$NODE_MYPORT_2,$NODE_MYSOCK_2;
+SET SESSION wsrep_sync_wait=0;
+select @@session.wsrep_dirty_reads;
+@@session.wsrep_dirty_reads
+0
+set session wsrep_dirty_reads=1;
+prepare stmt_show from 'select 1';
+prepare stmt_select from 'select * from t1';
+prepare stmt_insert from 'insert into t1 values(1)';
+set session wsrep_dirty_reads=0;
+execute stmt_show;
+ERROR 08S01: WSREP has not yet prepared node for application use
+execute stmt_select;
+ERROR 08S01: WSREP has not yet prepared node for application use
+execute stmt_insert;
+ERROR 08S01: WSREP has not yet prepared node for application use
+SET wsrep_dirty_reads=ON;
+select @@session.wsrep_dirty_reads;
+@@session.wsrep_dirty_reads
+1
+execute stmt_show;
+1
+1
+execute stmt_select;
+i
+1
+execute stmt_insert;
+ERROR 08S01: WSREP has not yet prepared node for application use
+SET @@global.wsrep_dirty_reads=ON;
+connect con2, localhost, user2,,test,$NODE_MYPORT_2,$NODE_MYSOCK_2;
+select @@session.wsrep_dirty_reads;
+@@session.wsrep_dirty_reads
+1
+prepare stmt_show from 'select 1';
+prepare stmt_select from 'select * from t1';
+prepare stmt_insert from 'insert into t1 values(1)';
+execute stmt_show;
+1
+1
+execute stmt_select;
+i
+1
+execute stmt_insert;
+ERROR 08S01: WSREP has not yet prepared node for application use
+SET SESSION wsrep_sync_wait=1;
+execute stmt_show;
+1
+1
+execute stmt_select;
+i
+1
+execute stmt_insert;
+ERROR 08S01: WSREP has not yet prepared node for application use
+SET SESSION wsrep_sync_wait=7;
+execute stmt_show;
+1
+1
+execute stmt_select;
+i
+1
+execute stmt_insert;
+ERROR 08S01: WSREP has not yet prepared node for application use
+connection node_2;
+SET @@global.wsrep_dirty_reads=OFF;
+connection node_1;
SELECT * FROM t1;
i
1
DROP TABLE t1;
+drop user user1;
+drop user user2;
+disconnect node_2;
+disconnect node_1;
# End of test
diff --git a/mysql-test/suite/galera/t/galera_var_dirty_reads.test b/mysql-test/suite/galera/t/galera_var_dirty_reads.test
index 0c81779..9891849 100644
--- a/mysql-test/suite/galera/t/galera_var_dirty_reads.test
+++ b/mysql-test/suite/galera/t/galera_var_dirty_reads.test
@@ -17,6 +17,11 @@ CREATE TABLE t1(i INT) ENGINE=INNODB;
INSERT INTO t1 VALUES(1);
SELECT * FROM t1;
+create user user1;
+grant all privileges on *.* to user1;
+create user user2;
+grant all privileges on *.* to user2;
+
SET @@global.wsrep_cluster_address = '';
SET @@session.wsrep_dirty_reads=OFF;
@@ -41,7 +46,72 @@ SET @@session.wsrep_dirty_reads=OFF;
SELECT 2;
SELECT @@max_allowed_packet;
SELECT 2+2 from DUAL;
-SELECT sysdate() from DUAL;
+
+#Query which does not change data should be allowed MDEV-11479
+SET @@session.wsrep_dirty_reads=ON;
+SELECT * FROM t1;
+
+--enable_connect_log
+--connect (con1, localhost, user1,,test,$NODE_MYPORT_2,$NODE_MYSOCK_2)
+#Just test the session behavior
+SET SESSION wsrep_sync_wait=0;
+select @@session.wsrep_dirty_reads;
+
+set session wsrep_dirty_reads=1;
+#Prepared statement creation should be allowed MDEV-11479
+prepare stmt_show from 'select 1';
+prepare stmt_select from 'select * from t1';
+prepare stmt_insert from 'insert into t1 values(1)';
+set session wsrep_dirty_reads=0;
+
+#No Preapare stmt/proceure will be allowed
+--error ER_UNKNOWN_COM_ERROR
+execute stmt_show;
+--error ER_UNKNOWN_COM_ERROR
+execute stmt_select;
+--error ER_UNKNOWN_COM_ERROR
+execute stmt_insert;
+
+SET wsrep_dirty_reads=ON;
+select @@session.wsrep_dirty_reads;
+#Only prepare statement which does not change data should be allowed
+execute stmt_show;
+execute stmt_select;
+--error ER_UNKNOWN_COM_ERROR
+execute stmt_insert;
+SET @@global.wsrep_dirty_reads=ON;
+
+--connect (con2, localhost, user2,,test,$NODE_MYPORT_2,$NODE_MYSOCK_2)
+#Just test the session behavior
+select @@session.wsrep_dirty_reads;
+
+prepare stmt_show from 'select 1';
+prepare stmt_select from 'select * from t1';
+prepare stmt_insert from 'insert into t1 values(1)';
+
+#Only prepare statement which does not change data should be allowed
+execute stmt_show;
+execute stmt_select;
+--error ER_UNKNOWN_COM_ERROR
+execute stmt_insert;
+
+#wsrep_dirty_read should work when wsrep_sync_wait is 1 or non zero
+#because we already are disconnected , So It does not make any sense
+#to wait for other nodes
+SET SESSION wsrep_sync_wait=1;
+execute stmt_show;
+execute stmt_select;
+--error ER_UNKNOWN_COM_ERROR
+execute stmt_insert;
+
+SET SESSION wsrep_sync_wait=7;
+execute stmt_show;
+execute stmt_select;
+--error ER_UNKNOWN_COM_ERROR
+execute stmt_insert;
+
+--connection node_2
+SET @@global.wsrep_dirty_reads=OFF;
--disable_query_log
--eval SET @@global.wsrep_cluster_address = '$wsrep_cluster_address_saved'
@@ -52,6 +122,8 @@ SELECT sysdate() from DUAL;
SELECT * FROM t1;
# Cleanup
DROP TABLE t1;
+drop user user1;
+drop user user2;
# Restore original auto_increment_offset values.
--source include/auto_increment_offset_restore.inc
diff --git a/mysql-test/suite/sys_vars/r/wsrep_dirty_reads_basic.result b/mysql-test/suite/sys_vars/r/wsrep_dirty_reads_basic.result
index d2a62d6..1968103 100644
--- a/mysql-test/suite/sys_vars/r/wsrep_dirty_reads_basic.result
+++ b/mysql-test/suite/sys_vars/r/wsrep_dirty_reads_basic.result
@@ -5,12 +5,13 @@
SET @wsrep_dirty_reads_session_saved = @@session.wsrep_dirty_reads;
# default
SELECT @@global.wsrep_dirty_reads;
-ERROR HY000: Variable 'wsrep_dirty_reads' is a SESSION variable
+@@global.wsrep_dirty_reads
+0
SELECT @@session.wsrep_dirty_reads;
@@session.wsrep_dirty_reads
0
-# scope and valid values
+# valid values for session
SET @@session.wsrep_dirty_reads=OFF;
SELECT @@session.wsrep_dirty_reads;
@@session.wsrep_dirty_reads
@@ -24,11 +25,29 @@ SELECT @@session.wsrep_dirty_reads;
@@session.wsrep_dirty_reads
0
+# valid values for global
+SET @@global.wsrep_dirty_reads=OFF;
+SELECT @@global.wsrep_dirty_reads;
+@@global.wsrep_dirty_reads
+0
+SET @@global.wsrep_dirty_reads=ON;
+SELECT @@global.wsrep_dirty_reads;
+@@global.wsrep_dirty_reads
+1
+SET @@global.wsrep_dirty_reads=default;
+SELECT @@global.wsrep_dirty_reads;
+@@global.wsrep_dirty_reads
+0
+
# invalid values
SET @@session.wsrep_dirty_reads=NULL;
ERROR 42000: Variable 'wsrep_dirty_reads' can't be set to the value of 'NULL'
SET @@session.wsrep_dirty_reads='junk';
ERROR 42000: Variable 'wsrep_dirty_reads' can't be set to the value of 'junk'
+SET @@global.wsrep_dirty_reads=NULL;
+ERROR 42000: Variable 'wsrep_dirty_reads' can't be set to the value of 'NULL'
+SET @@global.wsrep_dirty_reads='junk';
+ERROR 42000: Variable 'wsrep_dirty_reads' can't be set to the value of 'junk'
# restore the initial values
SET @@session.wsrep_dirty_reads = @wsrep_dirty_reads_session_saved;
diff --git a/mysql-test/suite/sys_vars/t/wsrep_dirty_reads_basic.test b/mysql-test/suite/sys_vars/t/wsrep_dirty_reads_basic.test
index a47524f..ffe767a 100644
--- a/mysql-test/suite/sys_vars/t/wsrep_dirty_reads_basic.test
+++ b/mysql-test/suite/sys_vars/t/wsrep_dirty_reads_basic.test
@@ -8,12 +8,12 @@
SET @wsrep_dirty_reads_session_saved = @@session.wsrep_dirty_reads;
--echo # default
---error ER_INCORRECT_GLOBAL_LOCAL_VAR
+
SELECT @@global.wsrep_dirty_reads;
SELECT @@session.wsrep_dirty_reads;
--echo
---echo # scope and valid values
+--echo # valid values for session
SET @@session.wsrep_dirty_reads=OFF;
SELECT @@session.wsrep_dirty_reads;
SET @@session.wsrep_dirty_reads=ON;
@@ -22,11 +22,24 @@ SET @@session.wsrep_dirty_reads=default;
SELECT @@session.wsrep_dirty_reads;
--echo
+--echo # valid values for global
+SET @@global.wsrep_dirty_reads=OFF;
+SELECT @@global.wsrep_dirty_reads;
+SET @@global.wsrep_dirty_reads=ON;
+SELECT @@global.wsrep_dirty_reads;
+SET @@global.wsrep_dirty_reads=default;
+SELECT @@global.wsrep_dirty_reads;
+
+--echo
--echo # invalid values
--error ER_WRONG_VALUE_FOR_VAR
SET @@session.wsrep_dirty_reads=NULL;
--error ER_WRONG_VALUE_FOR_VAR
SET @@session.wsrep_dirty_reads='junk';
+--error ER_WRONG_VALUE_FOR_VAR
+SET @@global.wsrep_dirty_reads=NULL;
+--error ER_WRONG_VALUE_FOR_VAR
+SET @@global.wsrep_dirty_reads='junk';
--echo
--echo # restore the initial values
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index 7d5c59f..008393f 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -2155,6 +2155,15 @@ int prepare_schema_table(THD *thd, LEX *lex, Table_ident *table_ident,
}
+/*
+ See whether the command changes the data or not.
+ */
+static int inline
+command_changes_data(uint sql_command)
+{
+ return (sql_command_flags[sql_command] &
+ CF_CHANGES_DATA) != 0;
+}
/**
Read query from packet and store in thd->query.
Used in COM_QUERY and COM_STMT_PREPARE.
@@ -2656,7 +2665,7 @@ mysql_execute_command(THD *thd)
if (lex->sql_command != SQLCOM_SET_OPTION &&
!wsrep_is_show_query(lex->sql_command) &&
!(thd->variables.wsrep_dirty_reads &&
- lex->sql_command == SQLCOM_SELECT) &&
+ !command_changes_data(lex->sql_command))&&
!(lex->sql_command == SQLCOM_SELECT &&
!all_tables) &&
!wsrep_node_is_ready(thd))
@@ -2934,8 +2943,14 @@ mysql_execute_command(THD *thd)
case SQLCOM_SHOW_FIELDS:
case SQLCOM_SHOW_KEYS:
case SQLCOM_SELECT:
- if (WSREP_CLIENT(thd) && wsrep_sync_wait(thd))
- goto error;
+ /*
+ wsrep_dirty_read should work when wsrep_sync_wait is 1 or non-zero
+ because we already are disconnected. So, it does not make any sense
+ to wait for other nodes
+ */
+ if (WSREP_CLIENT(thd) && (wsrep_ready || !thd->variables.wsrep_dirty_reads)
+ && wsrep_sync_wait(thd))
+ goto error;
case SQLCOM_SHOW_PLUGINS:
case SQLCOM_SHOW_VARIABLES:
diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc
index 0518e43..42fd46c 100644
--- a/sql/sys_vars.cc
+++ b/sql/sys_vars.cc
@@ -4977,8 +4977,8 @@ static Sys_var_mybool Sys_wsrep_restart_slave(
static Sys_var_mybool Sys_wsrep_dirty_reads(
"wsrep_dirty_reads", "Do not reject SELECT queries even when the node "
- "is not ready.", SESSION_ONLY(wsrep_dirty_reads), NO_CMD_LINE,
- DEFAULT(FALSE), NO_MUTEX_GUARD, NOT_IN_BINLOG);
+ "is not ready.", SESSION_VAR(wsrep_dirty_reads),
+ CMD_LINE(OPT_ARG), DEFAULT(FALSE), NO_MUTEX_GUARD, NOT_IN_BINLOG);
static Sys_var_uint Sys_wsrep_gtid_domain_id(
"wsrep_gtid_domain_id", "When wsrep_gtid_mode is set, this value is "
diff --git a/sql/wsrep_mysqld.cc b/sql/wsrep_mysqld.cc
index 0deb19d..a6f55b4 100644
--- a/sql/wsrep_mysqld.cc
+++ b/sql/wsrep_mysqld.cc
@@ -94,6 +94,8 @@ bool wsrep_new_cluster = false; // Bootstrap the cluster ?
// Use wsrep_gtid_domain_id for galera transactions?
bool wsrep_gtid_mode = 0;
+my_bool wsrep_dirty_reads = false; // is dirty reads allowed for
+ // disconnected/non major node
// gtid_domain_id for galera transactions.
uint32 wsrep_gtid_domain_id = 0;
diff --git a/sql/wsrep_mysqld.h b/sql/wsrep_mysqld.h
index 04ccc1a..8bdfdd5 100644
--- a/sql/wsrep_mysqld.h
+++ b/sql/wsrep_mysqld.h
@@ -88,6 +88,7 @@ extern my_bool wsrep_slave_UK_checks;
extern ulong wsrep_running_threads;
extern bool wsrep_new_cluster;
extern bool wsrep_gtid_mode;
+extern my_bool wsrep_dirty_reads;
extern uint32 wsrep_gtid_domain_id;
enum enum_wsrep_OSU_method {
References
