Hi Nirbhay! On Sun, Jan 29, 2017 at 11:57 PM, Nirbhay Choubey <nirb...@mariadb.com> wrote: > Hi Sachin, > > On Wed, Jan 25, 2017 at 1:17 AM, Sachin Setiya <sachin.set...@mariadb.com> > wrote: >> >> Hi Nirbhay! >> >> On Tue, Jan 24, 2017 at 8:02 AM, Nirbhay Choubey <nirb...@mariadb.com> >> wrote: >> > >> > revision-id: 5190ebde3aa142219e1c703df6ea1a78f171182c >> > (mariadb-10.1.21-7-g5190ebd) >> > parent(s): 15f46d517435f3570e2c788349637a06d818a619 >> > author: Nirbhay Choubey >> > committer: Nirbhay Choubey >> > timestamp: 2017-01-23 21:32:50 -0500 >> > message: >> > >> > MDEV-11817: Altering a table with more rows than .. >> > >> > .. wsrep_max_ws_rows causes cluster to break when running >> > Galera cluster in TOI mode >> > >> > During ALTER TABLE, while copying records to temporary >> > table, since the records are not placed into the binary >> > log, wsrep_affected_rows must also not be incremented. >> > >> I think, you should write a more detailed commit message, So that >> user/developer >> can link this commit to jira task body. >> May be add something like this >> Problem:- >> 1. When doing altering table in galera cluster if table has more >> records than wsrep_max_ws_rows >> it gives error. >> 2. Doing this also breaks replication. >> >> Solution:- >> 1. Your commit message. >> 2. Replication is broke because setting up wsrep_max_ws_rows is not >> transferred to another cluster. >> And as ALTER command is passed in statement format and it is passed >> much earlier. >> here in Sql_cmd_alter_table::execute(THD *thd) function. >> >> #ifdef WITH_WSREP >> TABLE *find_temporary_table(THD *thd, const TABLE_LIST *tl); >> >> if ((!thd->is_current_stmt_binlog_format_row() || >> !find_temporary_table(thd, first_table))) >> { >> WSREP_TO_ISOLATION_BEGIN(((lex->name.str) ? select_lex->db >> : NULL), >> ((lex->name.str) ? lex->name.str >> : NULL), >> first_table); >> } >> this send alter command to another node , before it is executed on node. >> Galera assumes that if alter command is failed on one node it should >> also be failing on other nodes. >> >> Or may be this can be added in comment. > > > IMHO, the above explanation (though correct) seem too verbose for the change > that > the patch introduces. :) I have redone the commit message. > >> >> > >> > --- >> > .../suite/galera/r/galera_var_max_ws_rows.result | 23 ++++++++ >> > .../suite/galera/t/galera_var_max_ws_rows.test | 28 +++++++++- >> > sql/handler.cc | 61 >> > ++++++++++++---------- >> > 3 files changed, 83 insertions(+), 29 deletions(-) >> > >> > diff --git a/mysql-test/suite/galera/r/galera_var_max_ws_rows.result >> > b/mysql-test/suite/galera/r/galera_var_max_ws_rows.result >> > index 6e239c7..555bd1a 100644 >> > --- a/mysql-test/suite/galera/r/galera_var_max_ws_rows.result >> > +++ b/mysql-test/suite/galera/r/galera_var_max_ws_rows.result >> > @@ -113,3 +113,26 @@ INSERT INTO t1 (f2) VALUES (2); >> > ERROR HY000: wsrep_max_ws_rows exceeded >> > DROP TABLE t1; >> > DROP TABLE ten; >> > +# >> > +# MDEV-11817: Altering a table with more rows than >> > +# wsrep_max_ws_rows causes cluster to break when running >> > +# Galera cluster in TOI mode >> > +# >> > +CREATE TABLE t1(c1 INT)ENGINE = INNODB; >> > +SET GLOBAL wsrep_max_ws_rows= DEFAULT; >> > +INSERT INTO t1 VALUES(1); >> > +INSERT INTO t1 SELECT * FROM t1; >> > +INSERT INTO t1 SELECT * FROM t1; >> > +INSERT INTO t1 SELECT * FROM t1; >> > +INSERT INTO t1 SELECT * FROM t1; >> > +SET GLOBAL wsrep_max_ws_rows= 10; >> > +ALTER TABLE t1 CHANGE COLUMN c1 c1 BIGINT; >> > +SHOW CREATE TABLE t1; >> > +Table Create Table >> > +t1 CREATE TABLE `t1` ( >> > + `c1` bigint(20) DEFAULT NULL >> > +) ENGINE=InnoDB DEFAULT CHARSET=latin1 >> > +SELECT COUNT(*) FROM t1; >> > +COUNT(*) >> > +16 >> > +DROP TABLE t1; >> > diff --git a/mysql-test/suite/galera/t/galera_var_max_ws_rows.test >> > b/mysql-test/suite/galera/t/galera_var_max_ws_rows.test >> > index 944238b..4b2ba60 100644 >> > --- a/mysql-test/suite/galera/t/galera_var_max_ws_rows.test >> > +++ b/mysql-test/suite/galera/t/galera_var_max_ws_rows.test >> > @@ -146,10 +146,34 @@ INSERT INTO t1 (f2) VALUES (1); >> > --error ER_ERROR_DURING_COMMIT >> > INSERT INTO t1 (f2) VALUES (2); >> > >> > +DROP TABLE t1; >> > +DROP TABLE ten; >> > + >> > +--echo # >> > +--echo # MDEV-11817: Altering a table with more rows than >> > +--echo # wsrep_max_ws_rows causes cluster to break when running >> > +--echo # Galera cluster in TOI mode >> > +--echo # >> > +--connection node_1 >> > +CREATE TABLE t1(c1 INT)ENGINE = INNODB; >> > +SET GLOBAL wsrep_max_ws_rows= DEFAULT; >> > +INSERT INTO t1 VALUES(1); >> > +INSERT INTO t1 SELECT * FROM t1; >> > +INSERT INTO t1 SELECT * FROM t1; >> > +INSERT INTO t1 SELECT * FROM t1; >> > +INSERT INTO t1 SELECT * FROM t1; >> > +SET GLOBAL wsrep_max_ws_rows= 10; >> > +ALTER TABLE t1 CHANGE COLUMN c1 c1 BIGINT; >> > + >> > +--connection node_2 >> > +SHOW CREATE TABLE t1; >> > +SELECT COUNT(*) FROM t1; >> > +DROP TABLE t1; >> > + >> > +--connection node_1 >> > >> > --disable_query_log >> > --eval SET GLOBAL wsrep_max_ws_rows = $wsrep_max_ws_rows_orig >> > --enable_query_log >> > >> > -DROP TABLE t1; >> > -DROP TABLE ten; >> > +--source include/galera_end.inc >> > diff --git a/sql/handler.cc b/sql/handler.cc >> > index af06427..48946de 100644 >> > --- a/sql/handler.cc >> > +++ b/sql/handler.cc >> > @@ -5724,6 +5724,30 @@ static int write_locked_table_maps(THD *thd) >> > } >> > >> > >> > +static int check_wsrep_max_ws_rows() >> > +{ >> > +#ifdef WITH_WSREP >> > + if (wsrep_max_ws_rows) >> > + { >> > + THD *thd= current_thd; >> > + >> > + if (!WSREP(thd)) >> > + return 0; >> > + >> > + thd->wsrep_affected_rows++; >> > + if (thd->wsrep_exec_mode != REPL_RECV && >> > + thd->wsrep_affected_rows > wsrep_max_ws_rows) >> > + { >> > + trans_rollback_stmt(thd) || trans_rollback(thd); >> > + my_message(ER_ERROR_DURING_COMMIT, "wsrep_max_ws_rows exceeded", >> > MYF(0)); >> > + return ER_ERROR_DURING_COMMIT; >> > + } >> > + } >> > +#endif /* WITH_WSREP */ >> > + return 0; >> > +} >> > + >> > + >> I think declaring check_wsrep_max_ws_rows() on top would be a much >> better option. > > > Done. > > http://lists.askmonty.org/pipermail/commits/2017-January/010555.html > > Best, > Nirbhay > This , lookes okay to me.
Regards sachin _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp