maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #10337
Re: [Commits] 5190ebd: MDEV-11817: Altering a table with more rows than ..
Hi Sachin,
On Wed, Jan 25, 2017 at 1:17 AM, Sachin Setiya <sachin.setiya@xxxxxxxxxxx>
wrote:
> Hi Nirbhay!
>
> On Tue, Jan 24, 2017 at 8:02 AM, Nirbhay Choubey <nirbhay@xxxxxxxxxxx>
> 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
Follow ups
References