Hi Nirbhay!

On Tue, Jan 24, 2017 at 8:02 AM, Nirbhay Choubey <nirbhay@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.

>
> ---
>  .../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.