Re: [Maria-developers] [Commits] d44c14e: MDEV-10812 WSREP causes responses being sent to protocol commands that must not send a response
Hi Sachin,
On Mon, Jan 30, 2017 at 12:10 AM, Sachin Setiya
wrote:
> HI Nirbhay!
>
> On Sun, Jan 29, 2017 at 11:42 PM, Nirbhay Choubey
> wrote:
> > Hi Sachin!
> >
> > On Fri, Jan 27, 2017 at 12:53 AM, SachinSetiya <
> sachin.set...@mariadb.com>
> > wrote:
> >>
> >> revision-id: d44c14e1e0dea312779ba0a8633583ee94284295
> >> (mariadb-10.1.21-2-gd44c14e)
> >> parent(s): a14638581b4c8ef175e68dccff07967d819b3b7e
> >> author: Sachin Setiya
> >> committer: Sachin Setiya
> >> timestamp: 2017-01-27 11:23:17 +0530
> >> message:
> >>
> >> MDEV-10812 WSREP causes responses being sent to protocol commands that
> >> must not send a response
> >>
> >> Problem:- When using wsrep (w/ galera) and issuing commands that can
> cause
> >> deadlocks, deadlock exception errors are sent in responses to commands
> >> such as close prepared statement and close connection which, by spec,
> must
> >> not send a
> >
> >
> > In commit message, we normally limit line width to 60 chars.
> Changed, But does this rule also apply to First line ?
>
Yes, I normally break it too.
> >
> >>
> >> response.
> >>
> >> Solution:- In dispatch_command, we will handle COM_QUIT and
> COM_STMT_CLOSE
> >> commands even in case of error.
> >
> >
> > I think you forgot to add the patch credit you had in previous commits.
> >
> >>
> >>
> >> ---
> >> mysql-test/suite/galera/r/galera_mdev_10812.result | 11 +
> >> mysql-test/suite/galera/t/galera_mdev_10812.test | 27
> >> ++
> >> sql/sql_parse.cc | 10 +++-
> >> 3 files changed, 47 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/mysql-test/suite/galera/r/galera_mdev_10812.result
> >> b/mysql-test/suite/galera/r/galera_mdev_10812.result
> >> new file mode 100644
> >> index 000..fba1000
> >> --- /dev/null
> >> +++ b/mysql-test/suite/galera/r/galera_mdev_10812.result
> >> @@ -0,0 +1,11 @@
> >> +#
> >> +# MDEV-10812: On COM_STMT_CLOSE/COM_QUIT, when wsrep_conflict_state
> >> +# is ABORTED, it causes wrong response to be sent to the client
> >> +#
> >> +SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;
> >> +CREATE TABLE t1(a INT PRIMARY KEY);
> >> +INSERT INTO t1 VALUES(1),(2),(3);
> >> +START TRANSACTION ;
> >> +UPDATE t1 SET a=a+100;
> >> +UPDATE t1 SET a=a+100;
> >> +DROP TABLE t1;
> >> diff --git a/mysql-test/suite/galera/t/galera_mdev_10812.test
> >> b/mysql-test/suite/galera/t/galera_mdev_10812.test
> >> new file mode 100644
> >> index 000..4539ab6
> >> --- /dev/null
> >> +++ b/mysql-test/suite/galera/t/galera_mdev_10812.test
> >> @@ -0,0 +1,27 @@
> >> +--source include/galera_cluster.inc
> >> +--source include/have_innodb.inc
> >> +
> >> +--echo #
> >> +--echo # MDEV-10812: On COM_STMT_CLOSE/COM_QUIT, when
> >> wsrep_conflict_state
> >> +--echo # is ABORTED, it causes wrong response to be sent to the client
> >> +--echo #
> >> +
> >> +# First create a deadlock
> >> +--connect node_1a, 127.0.0.1, root, , test, $NODE_MYPORT_1
> >> +SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;
> >> +CREATE TABLE t1(a INT PRIMARY KEY);
> >> +INSERT INTO t1 VALUES(1),(2),(3);
> >> +START TRANSACTION ;
> >> +UPDATE t1 SET a=a+100;
> >> +
> >> +--sleep 2
> >> +--connection node_2
> >> +UPDATE t1 SET a=a+100;
> >> +
> >> +--sleep 2
> >> +--connection node_1a
> >> +# here we get deadlock error
> >> +--disconnect node_1a
> >> +
> >> +--connection node_2
> >> +DROP TABLE t1;
> >> diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
> >> index 033e88a..cb0bd41 100644
> >> --- a/sql/sql_parse.cc
> >> +++ b/sql/sql_parse.cc
> >> @@ -1265,7 +1265,9 @@ bool dispatch_command(enum enum_server_command
> >> command, THD *thd,
> >> {
> >>wsrep_client_rollback(thd);
> >> }
> >> -if (thd->wsrep_conflict_state== ABORTED)
> >> +/* We let COM_QUIT and COM_STMT_CLOSE to execute even if wsrep
> >> aborted. */
> >> +if (thd->wsrep_conflict_state == ABORTED &&
> >> +command != COM_STMT_CLOSE && command != COM_QUIT)
> >> {
> &g
Re: [Maria-developers] [Commits] 5190ebd: MDEV-11817: Altering a table with more rows than ..
Hi Sachin,
On Wed, Jan 25, 2017 at 1:17 AM, Sachin Setiya
wrote:
> Hi Nirbhay!
>
> On Tue, Jan 24, 2017 at 8:02 AM, Nirbhay Choubey
> 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 #
> > +--connec
Re: [Maria-developers] [Commits] d44c14e: MDEV-10812 WSREP causes responses being sent to protocol commands that must not send a response
Hi Sachin!
On Fri, Jan 27, 2017 at 12:53 AM, SachinSetiya
wrote:
> revision-id: d44c14e1e0dea312779ba0a8633583ee94284295
> (mariadb-10.1.21-2-gd44c14e)
> parent(s): a14638581b4c8ef175e68dccff07967d819b3b7e
> author: Sachin Setiya
> committer: Sachin Setiya
> timestamp: 2017-01-27 11:23:17 +0530
> message:
>
> MDEV-10812 WSREP causes responses being sent to protocol commands that
> must not send a response
>
> Problem:- When using wsrep (w/ galera) and issuing commands that can cause
> deadlocks, deadlock exception errors are sent in responses to commands
> such as close prepared statement and close connection which, by spec, must
> not send a
>
In commit message, we normally limit line width to 60 chars.
> response.
>
> Solution:- In dispatch_command, we will handle COM_QUIT and COM_STMT_CLOSE
> commands even in case of error.
I think you forgot to add the patch credit you had in previous commits.
>
---
> mysql-test/suite/galera/r/galera_mdev_10812.result | 11 +
> mysql-test/suite/galera/t/galera_mdev_10812.test | 27
> ++
> sql/sql_parse.cc | 10 +++-
> 3 files changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/mysql-test/suite/galera/r/galera_mdev_10812.result
> b/mysql-test/suite/galera/r/galera_mdev_10812.result
> new file mode 100644
> index 000..fba1000
> --- /dev/null
> +++ b/mysql-test/suite/galera/r/galera_mdev_10812.result
> @@ -0,0 +1,11 @@
> +#
> +# MDEV-10812: On COM_STMT_CLOSE/COM_QUIT, when wsrep_conflict_state
> +# is ABORTED, it causes wrong response to be sent to the client
> +#
> +SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;
> +CREATE TABLE t1(a INT PRIMARY KEY);
> +INSERT INTO t1 VALUES(1),(2),(3);
> +START TRANSACTION ;
> +UPDATE t1 SET a=a+100;
> +UPDATE t1 SET a=a+100;
> +DROP TABLE t1;
> diff --git a/mysql-test/suite/galera/t/galera_mdev_10812.test
> b/mysql-test/suite/galera/t/galera_mdev_10812.test
> new file mode 100644
> index 000..4539ab6
> --- /dev/null
> +++ b/mysql-test/suite/galera/t/galera_mdev_10812.test
> @@ -0,0 +1,27 @@
> +--source include/galera_cluster.inc
> +--source include/have_innodb.inc
> +
> +--echo #
> +--echo # MDEV-10812: On COM_STMT_CLOSE/COM_QUIT, when wsrep_conflict_state
> +--echo # is ABORTED, it causes wrong response to be sent to the client
> +--echo #
> +
> +# First create a deadlock
> +--connect node_1a, 127.0.0.1, root, , test, $NODE_MYPORT_1
> +SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;
> +CREATE TABLE t1(a INT PRIMARY KEY);
> +INSERT INTO t1 VALUES(1),(2),(3);
> +START TRANSACTION ;
> +UPDATE t1 SET a=a+100;
> +
> +--sleep 2
> +--connection node_2
> +UPDATE t1 SET a=a+100;
> +
> +--sleep 2
> +--connection node_1a
> +# here we get deadlock error
> +--disconnect node_1a
> +
> +--connection node_2
> +DROP TABLE t1;
> diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
> index 033e88a..cb0bd41 100644
> --- a/sql/sql_parse.cc
> +++ b/sql/sql_parse.cc
> @@ -1265,7 +1265,9 @@ bool dispatch_command(enum enum_server_command
> command, THD *thd,
> {
>wsrep_client_rollback(thd);
> }
> -if (thd->wsrep_conflict_state== ABORTED)
> +/* We let COM_QUIT and COM_STMT_CLOSE to execute even if wsrep
> aborted. */
> +if (thd->wsrep_conflict_state == ABORTED &&
> +command != COM_STMT_CLOSE && command != COM_QUIT)
> {
>my_error(ER_LOCK_DEADLOCK, MYF(0), "wsrep aborted transaction");
>WSREP_DEBUG("Deadlock error for: %s", thd->query());
> @@ -1918,6 +1920,12 @@ bool dispatch_command(enum enum_server_command
> command, THD *thd,
>
>if (WSREP(thd))
>{
> +/*
> + MDEV-10812
> + In the case of COM_QUIT/COM_CLOSE thread status should be disabled.
>
s/COM_CLOSE/COM_STMT_CLOSE/
> +*/
> +DBUG_ASSERT((command != COM_QUIT && command != COM_STMT_CLOSE)
> +|| thd->get_stmt_da()->is_disabled());
>
Could you check the indentation of the above line?
Considering above changes, I have no more suggestions. Ok to push.
Best,
Nirbhay
> /* wsrep BF abort in query exec phase */
> mysql_mutex_lock(&thd->LOCK_wsrep_thd);
> do_end_of_statement= thd->wsrep_conflict_state != REPLAYING &&
> ___
> commits mailing list
> comm...@mariadb.org
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
___
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
Re: [Maria-developers] [Commits] d1124c9: MDEV-10812 WSREP causes responses being sent to protocol commands that must not send a response
Hi Sachin,
My comments below.
On Sun, Jan 22, 2017 at 7:18 AM, SachinSetiya
wrote:
> revision-id: d1124c98cbede6366e4b4d27a23dcf5b9d207cf0
> (mariadb-10.1.21-2-gd1124c9)
> parent(s): a14638581b4c8ef175e68dccff07967d819b3b7e
> author: Sachin Setiya
> committer: Sachin Setiya
> timestamp: 2017-01-22 17:47:29 +0530
> message:
>
> MDEV-10812 WSREP causes responses being sent to protocol commands that
> must not send a response
>
> Problem:- When using wsrep (w/ galera) and issuing commands that can cause
> deadlocks, deadlock exception errors are sent in responses to commands
> such as close prepared statement () which, by spec, must not send a
>
s/close prepared statement ()/close prepared statement and close connection/
response.
>
> Solution:- We will not jump to dispatch_end: when there is deadlock and
> query is either COM_QUIT or COM_STMT_CLOSE.
>
Or, I would say, in dispatch_command, we handle COM_QUIT and COM_STMT_CLOSE
commands even in case of error.
>
> Patch Credit:- Jaka Močnik
>
> ---
> .../galera/r/galera_prepared_statement.result | 18
> .../suite/galera/t/galera_prepared_statement.test | 25
> ++
> sql/sql_parse.cc | 6 +-
> 3 files changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/mysql-test/suite/galera/r/galera_prepared_statement.result
> b/mysql-test/suite/galera/r/galera_prepared_statement.result
> index de5ac9c..3425648 100644
> --- a/mysql-test/suite/galera/r/galera_prepared_statement.result
> +++ b/mysql-test/suite/galera/r/galera_prepared_statement.result
> @@ -27,7 +27,25 @@ ALTER TABLE t1 ADD COLUMN f2 INTEGER;
> ALTER TABLE t1 DROP COLUMN f1;
> EXECUTE st1;
> ERROR 22007: Incorrect integer value: 'abc' for column 'f2' at row 1
> +SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;
> +CREATE TABLE t5(a int primary key);
> +INSERT INTO t5 VALUES(1),(2),(3);
> +START TRANSACTION ;
> +PREPARE st5 FROM 'SELECT * FROM t5';
> +EXECUTE st5;
> +a
> +1
> +2
> +3
> +update t5 set a=a+100;
> +EXECUTE st5;
> +a
> +101
> +102
> +103
> +update t5 set a=a+100;
> DROP TABLE t1;
> DROP TABLE t2;
> DROP TABLE t3;
> DROP TABLE t4;
> +DROP TABLE t5;
diff --git a/mysql-test/suite/galera/t/galera_prepared_statement.test
> b/mysql-test/suite/galera/t/galera_prepared_statement.test
> index 3bee097..debc00e 100644
> --- a/mysql-test/suite/galera/t/galera_prepared_statement.test
> +++ b/mysql-test/suite/galera/t/galera_prepared_statement.test
> @@ -38,8 +38,33 @@ ALTER TABLE t1 DROP COLUMN f1;
> --error ER_TRUNCATED_WRONG_VALUE_FOR_FIELD
> EXECUTE st1;
>
> +# MDEV-10812
> +# On Closing Prepare Stamemnt, When wsrep_conflict_state is ABORTED
> +# It causes wrong response to be sent on Clients.
> +
> +# First Create a Deadlock
> --connection node_1
> +SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;
> +CREATE TABLE t5(a int primary key);
> +INSERT INTO t5 VALUES(1),(2),(3);
> +START TRANSACTION ;
> +PREPARE st5 FROM 'SELECT * FROM t5';
> +EXECUTE st5;
> +update t5 set a=a+100;
> +EXECUTE st5;
> +
> +--sleep 2
> +--connection node_2
> +update t5 set a=a+100;
> +
> +--sleep 2
> +--connection node_1
> +# here we have deadlock
> +--disconnect node_1
> +
> +--connection node_2
> DROP TABLE t1;
> DROP TABLE t2;
> DROP TABLE t3;
> DROP TABLE t4;
> +DROP TABLE t5;
>
The test always hits command != COM_QUIT assert if I revert :
+if (thd->wsrep_conflict_state == ABORTED &&
+command != COM_STMT_CLOSE && command != COM_QUIT)
So, I modified your test a bit (simplified) to not use PS.
--echo #
--echo # MDEV-10812: On closing prepare stamemnt, when wsrep_conflict_state
--echo # is ABORTED, it causes wrong response to be sent to the client
--echo #
# First create a deadlock
--connect node_1a, 127.0.0.1, root, , test, $NODE_MYPORT_1
SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;
CREATE TABLE t1(a INT PRIMARY KEY);
INSERT INTO t1 VALUES(1),(2),(3);
START TRANSACTION ;
UPDATE t1 SET a=a+100;
--sleep 2
--connection node_2
UPDATE t1 SET a=a+100;
--sleep 2
--connection node_1a
# here we get deadlock error
--disconnect node_1a
--connection node_2
DROP TABLE t1;
I think it should be ok.
> diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
> index 033e88a..d45b196 100644
> --- a/sql/sql_parse.cc
> +++ b/sql/sql_parse.cc
> @@ -1265,7 +1265,9 @@ bool dispatch_command(enum enum_server_command
> command, THD *thd,
> {
>wsrep_client_rollback(thd);
> }
> -if (thd->wsrep_conflict_state== ABORTED)
> +/* we let COM_QUIT and STMT_CLOSE execute even if wsrep aborted */
>
We let COM_QUIT and COM_STMT_CLOSE to execute even if wsrep aborted.
+if (thd->wsrep_conflict_state == ABORTED &&
> +command != COM_STMT_CLOSE && command != COM_QUIT)
> {
>my_error(ER_LOCK_DEADLOCK, MYF(0), "wsrep aborted transaction");
>WSREP_DEBUG("Deadlock error for: %s", thd->query());
> @@ -1918,6 +1920,8 @@ bool dispatch_command(enum enum_server_command
> command, THD *thd,
>
>i
Re: [Maria-developers] [Commits] 98b2a9c: MDEV-4774 Strangeness with max_binlog_stmt_cache_size Settings
Hi Sachin, On Fri, Jan 20, 2017 at 3:04 AM, Sachin Setiya wrote: > Hi Nirbhay! > > On Fri, Jan 20, 2017 at 7:09 AM, Nirbhay Choubey > wrote: > >> Hi Sachin, >> >> The overall patch looks ok. I, however, have a few minor comments inline. >> >> >> On Thu, Jan 19, 2017 at 1:21 AM, SachinSetiya >> wrote: >> >>> revision-id: 98b2a9c967a5eaa1f99bb3ef229ff2af62018ffe >>> (mariadb-10.0.28-34-g98b2a9c) >>> parent(s): 9bf92706d19761722b46d66a671734466cb6e98e >>> author: Sachin Setiya >>> committer: Sachin Setiya >>> timestamp: 2017-01-19 11:50:59 +0530 >>> message: >>> >>> MDEV-4774 Strangeness with max_binlog_stmt_cache_size Settings >>> >>> Problem:- When setting max_binlog_stmt_cache_size=18446744073709547520 >>> from either command line or .cnf file, server fails to start. >>> >>> Solution:- Added one more function eval_num_suffix_ull , which uses >>> strtoull to get unsigned ulonglong from string. And getopt_ull calles >>> this >>> >> .. cut .. > diff --git a/mysql-test/suite/binlog/t/binlog_max_binlog_stmt_cache_size.test >>> b/mysql-test/suite/binlog/t/binlog_max_binlog_stmt_cache_size.test >>> new file mode 100644 >>> index 000..bc30b48 >>> --- /dev/null >>> +++ b/mysql-test/suite/binlog/t/binlog_max_binlog_stmt_cache_size.test >>> @@ -0,0 +1,11 @@ >>> +source include/have_log_bin.inc; >>> +select @@max_binlog_stmt_cache_size; >>> + >>> +--let $cache_size=`select @@max_binlog_stmt_cache_size;` >>> + >>> +set global max_binlog_stmt_cache_size= 18446744073709547520; >>> +select @@max_binlog_stmt_cache_size; >>> + >>> +set global max_binlog_stmt_cache_size= 18446744073709547519; >>> +select @@max_binlog_stmt_cache_size; >>> >> >> I would also add tests for ULLONG_MAX and ULLONG_MAX +/- 1. >> >> Added the test for ULLONG_MAX+1, I am already testing for ULLONG_MAX and > ULLONG_MAX-1. > Not exactly. What you currently test is the maximum allowed value '18446744073709547520' for this variable (which is good). But, I would adittionally like to have it ULL_MAX (& +/1) which is 18446744073709551615. Considering this change, I have no more remarks. Best, Nirbhay ___ 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
Re: [Maria-developers] [Commits] 98b2a9c: MDEV-4774 Strangeness with max_binlog_stmt_cache_size Settings
Hi Sachin,
The overall patch looks ok. I, however, have a few minor comments inline.
On Thu, Jan 19, 2017 at 1:21 AM, SachinSetiya
wrote:
> revision-id: 98b2a9c967a5eaa1f99bb3ef229ff2af62018ffe
> (mariadb-10.0.28-34-g98b2a9c)
> parent(s): 9bf92706d19761722b46d66a671734466cb6e98e
> author: Sachin Setiya
> committer: Sachin Setiya
> timestamp: 2017-01-19 11:50:59 +0530
> message:
>
> MDEV-4774 Strangeness with max_binlog_stmt_cache_size Settings
>
> Problem:- When setting max_binlog_stmt_cache_size=18446744073709547520
> from either command line or .cnf file, server fails to start.
>
> Solution:- Added one more function eval_num_suffix_ull , which uses
> strtoull to get unsigned ulonglong from string. And getopt_ull calles this
>
Typo s/calles/calls/
> function instead of eval_num_suffix. Also changed previous eval_num_suffix
> to
> eval_num_suffix_ll to remain consistent.
>
> ---
> .../r/binlog_max_binlog_stmt_cache_size.result | 14 +
> .../binlog/t/binlog_max_binlog_stmt_cache_size.opt | 1 +
> .../t/binlog_max_binlog_stmt_cache_size.test | 11
> mysys/my_getopt.c | 66
> ++
> 4 files changed, 81 insertions(+), 11 deletions(-)
>
> diff --git
> a/mysql-test/suite/binlog/r/binlog_max_binlog_stmt_cache_size.result
> b/mysql-test/suite/binlog/r/binlog_max_binlog_stmt_cache_size.result
> new file mode 100644
> index 000..6fbec90
> --- /dev/null
> +++ b/mysql-test/suite/binlog/r/binlog_max_binlog_stmt_cache_size.result
> @@ -0,0 +1,14 @@
> +select @@max_binlog_stmt_cache_size;
> +@@max_binlog_stmt_cache_size
> +18446744073709547520
> +set global max_binlog_stmt_cache_size= 18446744073709547520;
> +select @@max_binlog_stmt_cache_size;
> +@@max_binlog_stmt_cache_size
> +18446744073709547520
> +set global max_binlog_stmt_cache_size= 18446744073709547519;
> +Warnings:
> +Warning1292Truncated incorrect max_binlog_stmt_cache_size
> value: '18446744073709547519'
> +select @@max_binlog_stmt_cache_size;
> +@@max_binlog_stmt_cache_size
> +18446744073709543424
> +set global max_binlog_stmt_cache_size= 18446744073709547520;
> diff --git a/mysql-test/suite/binlog/t/binlog_max_binlog_stmt_cache_size.opt
> b/mysql-test/suite/binlog/t/binlog_max_binlog_stmt_cache_size.opt
> new file mode 100644
> index 000..c52ef14
> --- /dev/null
> +++ b/mysql-test/suite/binlog/t/binlog_max_binlog_stmt_cache_size.opt
> @@ -0,0 +1 @@
> +--max_binlog_stmt_cache_size=18446744073709547520
> diff --git a/mysql-test/suite/binlog/t/binlog_max_binlog_stmt_cache_size.test
> b/mysql-test/suite/binlog/t/binlog_max_binlog_stmt_cache_size.test
> new file mode 100644
> index 000..bc30b48
> --- /dev/null
> +++ b/mysql-test/suite/binlog/t/binlog_max_binlog_stmt_cache_size.test
> @@ -0,0 +1,11 @@
> +source include/have_log_bin.inc;
> +select @@max_binlog_stmt_cache_size;
> +
> +--let $cache_size=`select @@max_binlog_stmt_cache_size;`
> +
> +set global max_binlog_stmt_cache_size= 18446744073709547520;
> +select @@max_binlog_stmt_cache_size;
> +
> +set global max_binlog_stmt_cache_size= 18446744073709547519;
> +select @@max_binlog_stmt_cache_size;
>
I would also add tests for ULLONG_MAX and ULLONG_MAX +/- 1.
+--eval set global max_binlog_stmt_cache_size= $cache_size
> diff --git a/mysys/my_getopt.c b/mysys/my_getopt.c
> index 2a45571..0934a50 100644
> --- a/mysys/my_getopt.c
> +++ b/mysys/my_getopt.c
> @@ -895,11 +895,32 @@ my_bool getopt_compare_strings(register const char
> *s, register const char *t,
> /*
>function: eval_num_suffix
>
> + Transforms suffix like k/m/g to there real value.
>
Typo : s/there/their/
> +*/
> +static inline long eval_num_suffix(char *suffix, int *error)
> +{
> + long num= 1;
> + if (*suffix == 'k' || *suffix == 'K')
> +num*= 1024L;
> + else if (*suffix == 'm' || *suffix == 'M')
> +num*= 1024L * 1024L;
> + else if (*suffix == 'g' || *suffix == 'G')
> +num*= 1024L * 1024L * 1024L;
> + else if (*suffix)
> + {
> +*error= 1;
> +return 0;
> + }
> + return num;
> +}
>
Please add a new line here (as a separator).
> +/*
> + function: eval_num_suffix_ll
> +
>Transforms a number with a suffix to real number. Suffix can
>be k|K for kilo, m|M for mega or g|G for giga.
> */
>
> -static longlong eval_num_suffix(char *argument, int *error, char
> *option_name)
> +static longlong eval_num_suffix_ll(char *argument, int *error, char
> *option_name)
> {
>char *endchar;
>longlong num;
>
DBUG_ENTER("eval_num_suffix");
You should also update the function name.
> @@ -916,23 +937,46 @@ static longlong eval_num_suffix(char *argument, int
> *error, char *option_name)
> *error= 1;
> DBUG_RETURN(0);
>}
> - if (*endchar == 'k' || *endchar == 'K')
> -num*= 1024L;
> - else if (*endchar == 'm' || *endchar == 'M')
> -num*= 1024L * 1024L;
> - else if (*endchar == 'g' || *endchar == 'G')
> -num*= 1024L * 1024L * 1024L;
> - else if (*endchar)
> - {
> + num*= eval_num_
Re: [Maria-developers] [JIRA] (MDEV-11685) sql_mode can't be set with non-ascii connection charset
Hi Bar,
On Tue, Jan 10, 2017 at 2:41 AM, Alexander Barkov wrote:
> Hello Nirbhay,
>
> On 01/09/2017 09:48 PM, Nirbhay Choubey wrote:
> > Hi Bar,
> >
> > On Mon, Jan 9, 2017 at 1:00 AM, Alexander Barkov > <mailto:b...@mariadb.org>> wrote:
> >
> > Hello Nirbhay,
> >
> > IIRC, you was working on this issue.
> > What is the current status?
> >
> >
> > Yes, I have just committed a patch.
> > Could you please take a look at it?
>
> The patch
> http://lists.askmonty.org/pipermail/commits/2017-January/010405.html
> looks Ok.
>
> I have a proposal for tests.
> The problem is that utf32, utf16 and ucs2 are not always compiled-in.
> So in case when some of the affected charsets is missing, sql_mode.test
> will fail.
>
>
> Can you please move the tests into ctype_utf32.test, ctype_utf16.test,
> ctype_ucs2.test ?
>
> I also suggest to add "SELECT @@sql_mode", to make sure that the
> assignment actually does what it's supposed to do.
>
>
> Remembering sql_mode is not necessary. It's OK to restore it to DEFAULT.
> Remembering character_set_connection is not necessary. It's Ok to do
> "SET NAMES utf8" at the end.
>
> Collecting all together, I'd suggest this chuck for ctype_utf32.test:
>
> --echo #
> --echo # MDEV-11685: sql_mode can't be set with non-ascii connection
> charset
> --echo #
> SET character_set_connection=utf32;
> SET sql_mode='NO_ENGINE_SUBSTITUTION';
> SELECT @@sql_mode;
> SET sql_mode=DEFAULT;
> SET NAMES utf8;
>
>
> and similar chunks for ctype_utf16.test and ctype_ucs2.test.
>
Done.
http://lists.askmonty.org/pipermail/commits/2017-January/010414.html
Thanks for the review.
Best,
Nirbhay
>
>
> Thanks!
>
> >
> > Best,
> > Nirbhay
> >
> >
> >
> > Thanks!
> >
> > On 01/03/2017 09:31 PM, Elena Stepanova (JIRA) wrote:
> > >
> > > [
> > https://jira.mariadb.org/browse/MDEV-11685?page=com.
> atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
> > <https://jira.mariadb.org/browse/MDEV-11685?page=com.
> atlassian.jira.plugin.system.issuetabpanels:all-tabpanel>
> > ]
> > >
> > > Elena Stepanova reassigned MDEV-11685:
> > > --
> > >
> > >Labels: upstream (was: )
> > > Affects Version/s: 5.5
> > >10.0
> > >10.1
> > >10.2
> > > Assignee: Alexander Barkov
> > >
> > >> sql_mode can't be set with non-ascii connection charset
> > >> ---
> > >>
> > >> Key: MDEV-11685
> > >> URL: https://jira.mariadb.org/browse/MDEV-11685
> > <https://jira.mariadb.org/browse/MDEV-11685>
> > >> Project: MariaDB Server
> > >> Issue Type: Bug
> > >> Components: Character Sets
> > >>Affects Versions: 10.2.3, 5.5, 10.0, 10.1, 10.2
> > >>Reporter: Nirbhay Choubey
> > >>Assignee: Alexander Barkov
> > >> Labels: upstream
> > >>
> > >> {code}
> > >> MariaDB [test]> set sql_mode = 'NO_ENGINE_SUBSTITUTION';
> > >> Query OK, 0 rows affected (0.00 sec)
> > >> MariaDB [test]> set character_set_connection=utf32;
> > >> Query OK, 0 rows affected (0.00 sec)
> > >> MariaDB [test]> set sql_mode = 'NO_ENGINE_SUBSTITUTION';
> > >> ERROR 1231 (42000): Variable 'sql_mode' can't be set to the value
> > of 'NO_ENGINE_SUBSTITUTION'
> > >> {code}
> > >
> > >
> > >
> > > --
> > > This message was sent by Atlassian JIRA
> > > (v7.2.1#72003)
> > >
> >
> >
>
___
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
Re: [Maria-developers] MDEV-11558 Split Item_type_holder::display_length into virtual methods in Type_handler
Hi Bar, The patch is ok to push. Perhaps a similar virtual method to disintegrate Item_type_holder::get_real_type()? Best, Nirbhay On Tue, Dec 13, 2016 at 8:03 AM, Alexander Barkov wrote: > Hello Nirbhay, > > Can you please review a patch for MDEV-11558? > > Thanks! > ___ 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
Re: [Maria-developers] [Commits] bbc52df: MDEV-11490 Galera_3nodes test suite does not suppress Warnings.
Hi Sachin,
I see that you have added a lot of warnings to the suite-level suppression
list.
Does *all* the new warnings show up for all the tests?
I'd only add ones that make sense for the entire suite, and add the
individuals
to the specific test by calling mtr.add_suppression("").
Ping me on slack if you need to discuss this further.
Best,
Nirbhay
On Tue, Dec 6, 2016 at 1:53 AM, wrote:
> revision-id: bbc52df39b7dedca50943a7cd99ee80b9e2f5b12
> (mariadb-10.1.19-24-gbbc52df)
> parent(s): 611f91605adce17df87acf96b5aede0b73d0fc12
> author: SachinSetiya
> committer: SachinSetiya
> timestamp: 2016-12-06 12:21:16 +0530
> message:
>
> MDEV-11490 Galera_3nodes test suite does not suppress Warnings.
>
> Problem:- While running individual tests of Galera_3nodes ,
> We get warnings like '[Warning] WSREP: Could not open state file
> for reading: '. And because of this individual tests fails.
>
> Solution:- We change suite.pm of Galera_3nodes to supress these warnings.
>
> ---
> mysql-test/suite/galera_3nodes/suite.pm | 34
> -
> 1 file changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/mysql-test/suite/galera_3nodes/suite.pm
> b/mysql-test/suite/galera_3nodes/suite.pm
> index 39d5acb..74f7dfa 100644
> --- a/mysql-test/suite/galera_3nodes/suite.pm
> +++ b/mysql-test/suite/galera_3nodes/suite.pm
> @@ -25,7 +25,8 @@ return "No my_print_defaults" unless $epath;
> push @::global_suppressions,
>(
> qr(WSREP: wsrep_sst_receive_address is set to '127.0.0.1),
> - qr(WSREP: Could not open saved state file for reading: ),
> + qr(WSREP: Could not open saved state file for reading: .*),
> + qr(WSREP: Could not open state file for reading: .*),
> qr(WSREP: Gap in state sequence. Need state transfer.),
> qr(WSREP: Failed to prepare for incremental state transfer:),
> qr(WSREP:.*down context.*),
> @@ -33,16 +34,39 @@ push @::global_suppressions,
> qr(WSREP: last inactive check more than .* skipping check),
> qr(WSREP: SQL statement was ineffective),
> qr(WSREP: Releasing seqno [0-9]* before [0-9]* was assigned.),
> - qr|WSREP: access file\(gvwstate.dat\) failed\(No such file or
> directory\)|,
> + qr|WSREP: access file\(.*gvwstate.dat\) failed\(No such file or
> directory\)|,
> qr(WSREP: Quorum: No node with complete state),
> qr(WSREP: Initial position was provided by configuration or SST,
> avoiding override),
> qr|WSREP: discarding established \(time wait\) .*|,
> qr(WSREP: There are no nodes in the same segment that will ever be
> able to become donors, yet there is a suitable donor outside. Will use that
> one.),
> qr(WSREP: evs::proto.*),
> - qr|WSREP: Ignoring possible split-brain (allowed by configuration)
> from view:.*|,
> + qr|WSREP: Ignoring possible split-brain \(allowed by configuration\)
> from view:.*|,
> + qr(WSREP: no nodes coming from prim view, prim not possible),
> + qr(WSREP: Member .* requested state transfer from .* but it is
> impossible to select State Transfer donor: Resource temporarily
> unavailable),
> + qr(WSREP: user message in state LEAVING),
> + qr(WSREP: .* sending install message failed: Transport endpoint is
> not connected),
> + qr(WSREP: .* sending install message failed: Resource temporarily
> unavailable),
> + qr(WSREP: Maximum writeset size exceeded by .*),
> + qr(WSREP: transaction size exceeded.*),
> + qr(WSREP: RBR event .*),
> + qr(WSREP: Ignoring error for TO isolated action: .*),
> + qr(WSREP: transaction size limit .*),
> + qr(WSREP: rbr write fail, .*),
> + qr(WSREP: .*Backend not supported: foo.*),
> + qr(WSREP: .*Failed to initialize backend using .*),
> + qr(WSREP: .*Failed to open channel 'my_wsrep_cluster' at .*),
> + qr(WSREP: gcs connect failed: Socket type not supported),
> + qr(WSREP: failed to open gcomm backend connection: 110: failed to
> reach primary view: 110 .*),
> + qr(WSREP: .*Failed to open backend connection: -110 .*),
> + qr(WSREP: .*Failed to open channel 'my_wsrep_cluster' at .*),
> + qr(WSREP: gcs connect failed: Connection timed out),
> + qr|WSREP: wsrep::connect\(.*\) failed: 7|,
> + qr(WSREP: SYNC message from member .* in non-primary configuration.
> Ignored.),
> qr(WSREP: Could not find peer:),
> - qr(WSREP: Protocol violation. JOIN message sender .*),
> - qr(WSREP: JOIN message from member [0-9]* in non-primary
> configuration. Ignored.),
> + qr(WSREP: TO isolation failed for: .*),
> + qr|WSREP: gcs_caused\(\) returned .*|,
> + qr|WSREP: Protocol violation. JOIN message sender .* is not in state
> transfer \(SYNCED\). Message ignored.|,
> + qr(WSREP: Action message in non-primary configuration from member
> [0-9]*),
> );
>
>
> ___
> commits mailing list
> comm...@mariadb.org
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
_
Re: [Maria-developers] [Commits] f79d9d4: [MDEV]-11162 Assertion failing because cleanup does not get called
Hi Varun, On Tue, Dec 6, 2016 at 9:34 AM, Varun wrote: > revision-id: f79d9d454bc3fa6fecb34fec850752a688c0ccc7 > (mariadb-10.0.28-17-gf79d9d4) > parent(s): e0c631905cd923257331ef124a2bc9276e0d > author: Varun > committer: Varun > timestamp: 2016-12-06 19:48:58 +0530 > message: > > [MDEV]-11162 Assertion failing because cleanup does not get called > Looks like you forgot to add the result file. Besides, since the test is simple, you could instead add it to an existing _debug_ select/optimizer test. Best, Nirbhay > --- > mysql-test/t/bug-11162.test | 6 ++ > sql/sql_select.cc | 1 + > 2 files changed, 7 insertions(+) > > diff --git a/mysql-test/t/bug-11162.test b/mysql-test/t/bug-11162.test > new file mode 100644 > index 000..5924851 > --- /dev/null > +++ b/mysql-test/t/bug-11162.test > @@ -0,0 +1,6 @@ > +--source include/have_innodb.inc > + > +CREATE TABLE t1 (i INT) ENGINE=InnoDB; > +SELECT ( SELECT DISTINCT GROUP_CONCAT(SLEEP(0)) FROM t1 GROUP BY i ); > +SELECT i FROM t1 order by i LIMIT 1; > +drop table t1; > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index 2db9a2b..f345d3c 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -3144,6 +3144,7 @@ JOIN::destroy() > */ > tmp_table_param.cleanup(); > tmp_join->tmp_table_param.copy_field= 0; > +cleanup(1); > DBUG_RETURN(tmp_join->destroy()); >} >cond_equal= 0; > ___ > commits mailing list > comm...@mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits ___ 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
Re: [Maria-developers] [Commits] 1c9da8d: MDEV-9312: storage engine not enforced during
Hi Sachin,
On Mon, Nov 28, 2016 at 4:49 AM, Sachin Setiya
wrote:
> Hi Nirbhay,
>
>if (IF_WSREP(thd->wsrep_applier,1))
>
> {
>
> plugin_thdvar_init(thd);
>
> }
>
> This code in commit , breaks log writing on other nodes(In galera) , It
> sets
> the thd->variables to global system variable.
> And this
>
> if (wsrep_emulate_bin_log || !(thd->variables.option_bits &
> OPTION_BIN_LOG))
> DBUG_RETURN(0);
>
> code here make write_transaction_to_binlog() function exit.
>
This was MDEV-10944. You need to pull the latest 10.1 commits and
possibly cherry-pick the fix to 10.2, in case it has not been up-merged
yet.
> I am unable to understand purpose of applying plugin_thdvar_init(thd);
> for wsrep_applier threads.
>
This is done because the startup wsrep threads (applier, replayer)
are created before some plugin-related variables are initialized.
> Because this is applied on first thread only and not applied to
> remaining threads.
>
Its only applicable to start-up wsrep threads.
Best,
Nirbhay
> --
> Regards
> Sachin Setiya
> Software Engineer at MariaDB
>
___
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
Re: [Maria-developers] MDEV-11337 Split Item::save_in_field() into virtual methods in Type_handler
Hi Bar, On Tue, Nov 22, 2016 at 12:33 PM, Alexander Barkov wrote: > Hello Nirbhay, > > Can you please review a patch for 10.3: > MDEV-11337 Split Item::save_in_field() into virtual methods in Type_handler > > > It also automatically fixed two problems: > > MDEV-11331 Wrong result for INSERT INTO t1 (datetime_field) VALUES > (hybrid_function_of_TIME_data_type) > MDEV-11333 Expect "Impossible where condition" for WHERE > timestamp_field>=DATE_ADD(TIMESTAMP'-01-01 00:00:00',INTERVAL 1000 > YEAR) > > because the new code is now symmetric for all data types. > Looks good. Ok to push. Best, Nirbhay > > Thanks! > ___ 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
Re: [Maria-developers] [Commits] a3d469b: MDEV-11035: Restore removed disallow-writes for Galera
Hi Jan!
On Wed, Oct 12, 2016 at 7:31 AM, Jan Lindström
wrote:
> revision-id: a3d469b991732dddfafe966011e996166cd98671
> (mariadb-10.2.2-39-ga3d469b)
> parent(s): 6e46de4a674c55858ec5b2528dcebb69010b34d6
> author: Jan Lindström
> committer: Jan Lindström
> timestamp: 2016-10-12 14:29:36 +0300
> message:
>
> MDEV-11035: Restore removed disallow-writes for Galera
>
> Found actually only one missing condition.
>
The patch is unfortunately incomplete. There are a bunch of
WAIT_ALLOW_WRITES'
(when compared to 10.1) that are currently missing in 10.2. Also, there is
a memory
leak as InnoDB does not free this event on shutdown (srv_free()).
You can use galera.galera_var_innodb_disallow_writes to verify the fix.
Best,
Nirbhay
> ---
> storage/innobase/handler/ha_innodb.cc | 6 --
> storage/innobase/srv/srv0srv.cc | 4 +---
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/storage/innobase/handler/ha_innodb.cc
> b/storage/innobase/handler/ha_innodb.cc
> index 1e74154..dc7e6eb 100644
> --- a/storage/innobase/handler/ha_innodb.cc
> +++ b/storage/innobase/handler/ha_innodb.cc
> @@ -23276,10 +23276,11 @@ innobase_disallow_writes_update(
> {
> *(my_bool*)var_ptr = *(my_bool*)save;
> ut_a(srv_allow_writes_event);
> - if (*(my_bool*)var_ptr)
> + if (*(my_bool*)var_ptr) {
> os_event_reset(srv_allow_writes_event);
> - else
> + } else {
> os_event_set(srv_allow_writes_event);
> + }
> }
>
> static MYSQL_SYSVAR_BOOL(disallow_writes, innobase_disallow_writes,
> @@ -23287,6 +23288,7 @@ static MYSQL_SYSVAR_BOOL(disallow_writes,
> innobase_disallow_writes,
>"Tell InnoDB to stop any writes to disk",
>NULL, innobase_disallow_writes_update, FALSE);
> #endif /* WITH_INNODB_DISALLOW_WRITES */
> +
> static MYSQL_SYSVAR_BOOL(random_read_ahead, srv_random_read_ahead,
>PLUGIN_VAR_NOCMDARG,
>"Whether to use read ahead for random access within an extent.",
> diff --git a/storage/innobase/srv/srv0srv.cc
> b/storage/innobase/srv/srv0srv.cc
> index 49de954..a46f62f 100644
> --- a/storage/innobase/srv/srv0srv.cc
> +++ b/storage/innobase/srv/srv0srv.cc
> @@ -1969,9 +1969,7 @@ DECLARE_THREAD(srv_error_monitor_thread)(
> if (sync_array_print_long_waits(&waiter, &sema)
> && sema == old_sema && os_thread_eq(waiter, old_waiter)) {
> #if defined(WITH_WSREP) && defined(WITH_INNODB_DISALLOW_WRITES)
> - if (true) {
> - // JAN: TODO: MySQL 5.7
> - //if (srv_allow_writes_event->is_set) {
> + if (os_event_is_set(srv_allow_writes_event)) {
> #endif /* WITH_WSREP */
> fatal_cnt++;
> #if defined(WITH_WSREP) && defined(WITH_INNODB_DISALLOW_WRITES)
> ___
> commits mailing list
> comm...@mariadb.org
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
___
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
Re: [Maria-developers] [Commits] 1c9da8d: MDEV-9312: storage engine not enforced during galera cluster replication
Hi Jan! On Wed, Sep 28, 2016 at 12:48 PM, Jan Lindström wrote: > Hi Nirbhay, > > This looks ok but one question (no need to change now): > > On Wed, Sep 28, 2016 at 7:36 PM, Nirbhay Choubey > wrote: > >> >> +Since some wsrep threads (THDs) are create before plugins are >> +initialized, LOCK_plugin mutex needs to be initialized here. >> + */ >> > > Is there some fundamental reason why we can't create wsrep threads after > other plugins and > related global system variables (that are really needed for wsrep) are > initialized ? > The wsrep replication thread is required during state transfer, which (rsync & xtrabackup based) must complete before plugins (InnoDB, in specific) can be initialized. Best, Nirbhay > > R: Jan > > > ___ > 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 > > ___ 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
Re: [Maria-developers] [Commits] a8162d4: MDEV-9312: storage engine not enforced during galera cluster replication
Hi Serg,
On Tue, Sep 27, 2016 at 3:49 PM, Sergei Golubchik wrote:
> Hi, Nirbhay!
>
> I don't understand, why do you need to create a dummy plugin here?
>
Since global_system_variables.table_plugin is null when the startup wsrep
threads
are created, I used dummy plugin to initialize it to avoid running into
segfaults in
intern_plugin_lock() invoked from under plugin_thdvar_init().
static plugin_ref intern_plugin_lock(LEX *lex, plugin_ref rc)
{
st_plugin_int *pi= plugin_ref_to_int(rc);
...
if (pi->state & (PLUGIN_IS_READY | PLUGIN_IS_UNINITIALIZED |
PLUGIN_IS_DELETED))
Also, with this commit plugin_thdvar_init() is free from wsrep related
changes.
Best,
Nirbhay
> On Sep 27, Nirbhay Choubey wrote:
> > revision-id: a8162d4a8737cff67889390fad0153acc175391d
> (mariadb-10.1.17-22-ga8162d4)
> > parent(s): 6a6b253a6ecbd4d3dd254044d12ec64475453275
> > author: Nirbhay Choubey
> > committer: Nirbhay Choubey
> > timestamp: 2016-09-27 09:03:26 -0400
> > message:
> >
> > MDEV-9312: storage engine not enforced during galera cluster replication
> >
> > Perform a post initialization of plugin-related variables
> > of wsrep threads after their global counterparts have been
> > initialized.
> ...
> > +#ifdef WITH_WSREP
> > +
> > +/*
> > + Placeholder for global_system_variables.table_plugin required during
> > + initialization of startup wsrep threads.
> > +*/
> > +static st_plugin_int *wsrep_dummy_plugin;
> > +
> > +/*
> > + Initialize wsrep_dummy_plugin and assign it to
> > + global_system_variables.table_plugin.
> > +*/
> > +void wsrep_plugins_pre_init()
> > +{
> > + wsrep_dummy_plugin=
> > +(st_plugin_int *) my_malloc(sizeof(st_plugin_int), MYF(0));
> > + wsrep_dummy_plugin->state= PLUGIN_IS_DISABLED;
> > + global_system_variables.table_plugin= plugin_int_to_ref(wsrep_dummy_
> plugin);
> > +}
> > +
> Regards,
> Sergei
> Chief Architect MariaDB
> and secur...@mariadb.org
>
___
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
Re: [Maria-developers] [Commits] 97d212a: MDEV-10545: Server crashed in my_copy_fix_mb on querying I_S and P_S tables
Hi Svoj, On Fri, Sep 2, 2016 at 9:10 AM, Sergey Vojtovich wrote: > Hi Nirbhay, > > On Fri, Sep 02, 2016 at 09:00:27AM -0400, Nirbhay Choubey wrote: > > > > diff --git a/sql/log_event.cc b/sql/log_event.cc > > > > index afa58af..66e7c60 100644 > > > > --- a/sql/log_event.cc > > > > +++ b/sql/log_event.cc > > > > @@ -6022,7 +6022,7 @@ int Load_log_event::do_apply_event(NET* net, > > > rpl_group_info *rgi, > > > >new_db.str= (char *) rpl_filter->get_rewrite_db(db, > &new_db.length); > > > >thd->set_db(new_db.str, new_db.length); > > > >DBUG_ASSERT(thd->query() == 0); > > > > - thd->reset_query_inner();// Should not be > needed > > > > + if (thd->query()) thd->reset_query(); // Should not be > needed > > > Why if? Original code didn't have it. > > > > > > > Now that we call reset_query(), the check has been added to avoid it and > an > > extra mutex. > > And from the above assert, I think it will be ever better have this check > > within unlikely(). > Taking into account this assert I'd say there's no point in reset_query at > all. > Ok, I will remove it. - Nirbhay > ___ > commits mailing list > comm...@mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits ___ 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
Re: [Maria-developers] [Commits] 97d212a: MDEV-10545: Server crashed in my_copy_fix_mb on querying I_S and P_S tables
Hi Svoj! Thanks for the review. On Fri, Sep 2, 2016 at 1:41 AM, Sergey Vojtovich wrote: > Hi Nirbhay, > > Ok to push, one question below. > > On Thu, Sep 01, 2016 at 12:46:58PM -0400, Nirbhay Choubey wrote: > > revision-id: 97d212a34ff4e0558126bc393bbef97036611d83 > (mariadb-10.1.17-4-g97d212a) > > parent(s): a322651b8aa702e58d473edfae26606f10a089fb > > author: Nirbhay Choubey > > committer: Nirbhay Choubey > > timestamp: 2016-09-01 12:46:56 -0400 > > message: > > > > MDEV-10545: Server crashed in my_copy_fix_mb on querying I_S and P_S > tables > > > > Once THDs have been added to the global "threads" list, > > they must modify query_string only after acquiring per- > > thread LOCK_thd_data mutex. > > > > --- > > sql/log_event.cc | 2 +- > > sql/sql_acl.cc | 8 > > sql/sql_parse.cc | 7 +++ > > 3 files changed, 8 insertions(+), 9 deletions(-) > > > > diff --git a/sql/log_event.cc b/sql/log_event.cc > > index afa58af..66e7c60 100644 > > --- a/sql/log_event.cc > > +++ b/sql/log_event.cc > > @@ -6022,7 +6022,7 @@ int Load_log_event::do_apply_event(NET* net, > rpl_group_info *rgi, > >new_db.str= (char *) rpl_filter->get_rewrite_db(db, &new_db.length); > >thd->set_db(new_db.str, new_db.length); > >DBUG_ASSERT(thd->query() == 0); > > - thd->reset_query_inner();// Should not be needed > > + if (thd->query()) thd->reset_query(); // Should not be needed > Why if? Original code didn't have it. > Now that we call reset_query(), the check has been added to avoid it and an extra mutex. And from the above assert, I think it will be ever better have this check within unlikely(). Best, Nirbhay > > Regards, > Sergey > ___ > commits mailing list > comm...@mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits ___ 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
Re: [Maria-developers] [Commits] 3bbf3d2: MDEV-10216: Assertion `strcmp(share->unique_file_name, filename) ||
Hi Monty! Can you please 2nd review this patch? http://lists.askmonty.org/pipermail/commits/2016-July/009523.html Best, Nirbhay On Tue, Jul 5, 2016 at 4:51 AM, Sergey Vojtovich wrote: > Hi Nirbhay, > > Looks good, thanks! > > On Fri, Jul 01, 2016 at 02:47:29PM -0400, Nirbhay Choubey wrote: > > revision-id: 3bbf3d2e246da1a2fd458b634f30cbe35afb5668 > (mariadb-10.2.0-142-g3bbf3d2) > > parent(s): 76f492e26d8d45c678c570be2cf4ca5d238edceb > > author: Nirbhay Choubey > > committer: Nirbhay Choubey > > timestamp: 2016-07-01 14:47:27 -0400 > > message: > > > > MDEV-10216: Assertion `strcmp(share->unique_file_name,filename) || > > > > .. share->last_version' failed in myisam/mi_open.c:67: test_if_reopen > > > > During the RENAME operation since the renamed temporary table is also > > opened and added to myisam_open_list/maria_open_list, resetting the > > last_version at the end of operation (HA_EXTRA_PREPARE_FOR_RENAME) > > will cause an assertion failure when a subsequent query tries to open > > an additional temporary table instance and thus attempts to reuse it > > from the open table list. > > > > This commit fixes the issue by skipping flush/close operations executed > > toward the end of ALTER for temporary tables. It also enables a shortcut > > for simple ALTERs (like rename, disable/enable keys) on temporary > > tables. > > > > As safety checks, added some assertions at code points that should not > > be hit for temporary tables. > ... > > Regards, > Sergey > ___ > commits mailing list > comm...@mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits ___ 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
Re: [Maria-developers] [Commits] d1de640: MDEV-10216: Assertion `strcmp(share->unique_file_name, filename) ||
Hi Svoj,
On Thu, Jun 23, 2016 at 7:35 AM, Sergey Vojtovich wrote:
> Hi Nirbhay,
>
> This solution looks better than MyISAM/Aria based one. Still a few doubts
> inline.
> I still suggest someone should do second review.
>
> On Tue, Jun 21, 2016 at 01:47:15PM -0400, Nirbhay Choubey wrote:
> > revision-id: d1de6402c8734e9a1929c2384c0cfc07c2ec48c0
> (mariadb-10.2.0-83-gd1de640)
> > parent(s): b2ae32aafdd2787ad456f38833f630182ded25e8
> > author: Nirbhay Choubey
> > committer: Nirbhay Choubey
> > timestamp: 2016-06-21 13:47:12 -0400
> > message:
> >
> > MDEV-10216: Assertion `strcmp(share->unique_file_name,filename) ||
> ...skip...
>
> > diff --git a/sql/sql_table.cc b/sql/sql_table.cc
> > index 0be329e..07d4b42 100644
> > --- a/sql/sql_table.cc
> > +++ b/sql/sql_table.cc
> > @@ -8151,12 +8151,15 @@ static bool fk_prepare_copy_alter_table(THD
> *thd, TABLE *table,
> Looks like git confused function names here and for most of other hunks of
> this
> patch. I vaguely remember this bug was affecting really old git versions
> like
> pre-2.x.
>
>
I add a new function before this stub but still no change. Pretty weird. :)
> >
> >if (keys_onoff != Alter_info::LEAVE_AS_IS)
> >{
> > -if (wait_while_table_is_used(thd, table, extra_func))
> > - DBUG_RETURN(true);
> > +if (!table->s->tmp_table)
> > +{
> > + if (wait_while_table_is_used(thd, table, extra_func))
> > +DBUG_RETURN(true);
> >
> > -// It's now safe to take the table level lock.
> > -if (lock_tables(thd, table_list, alter_ctx->tables_opened, 0))
> > - DBUG_RETURN(true);
> > + // It's now safe to take the table level lock.
> > + if (lock_tables(thd, table_list, alter_ctx->tables_opened, 0))
> > +DBUG_RETURN(true);
> > +}
> >
> > error= alter_table_manage_keys(table,
> > table->file->indexes_are_disabled(),
> Are you sure lock_tables() is not necessary for temporary tables here? It
> would
> be nice to make sure this code is covered by a test case.
>
hmm.. While I do see locks getting initialized for transactional temporary
tables (get_lock_data()),
I still do not understand why would it be necessary. I also looked into the
past related commits but
could not find one that explains the rationale.
Anyway, I have added some more tests to check the behavior and will also
ask Monty for a 2nd
review.
> > @@ -8166,43 +8169,57 @@ static bool fk_prepare_copy_alter_table(THD
> *thd, TABLE *table,
> >if (!error && alter_ctx->is_table_renamed())
> >{
> > THD_STAGE_INFO(thd, stage_rename);
> > -handlerton *old_db_type= table->s->db_type();
> > -/*
> > - Then do a 'simple' rename of the table. First we need to close all
> > - instances of 'source' table.
> > - Note that if wait_while_table_is_used() returns error here (i.e.
> if
> > - this thread was killed) then it must be that previous step of
> > - simple rename did nothing and therefore we can safely return
> > - without additional clean-up.
> > -*/
> > -if (wait_while_table_is_used(thd, table, extra_func))
> > - DBUG_RETURN(true);
> > -close_all_tables_for_name(thd, table->s,
> HA_EXTRA_PREPARE_FOR_RENAME, NULL);
> > -
> > LEX_STRING old_db_name= { alter_ctx->db, strlen(alter_ctx->db) };
> > LEX_STRING old_table_name=
> > { alter_ctx->table_name, strlen(alter_ctx->table_name) };
> > LEX_STRING new_db_name= { alter_ctx->new_db,
> strlen(alter_ctx->new_db) };
> > LEX_STRING new_table_name=
> > { alter_ctx->new_alias, strlen(alter_ctx->new_alias) };
> > -(void) rename_table_in_stat_tables(thd, &old_db_name,
> &old_table_name,
> > - &new_db_name, &new_table_name);
> >
> > -if (mysql_rename_table(old_db_type, alter_ctx->db,
> alter_ctx->table_name,
> > - alter_ctx->new_db, alter_ctx->new_alias, 0))
> > - error= -1;
> > -else if (Table_triggers_list::change_table_name(thd,
> > -alter_ctx->db,
> > -alter_ctx->alias,
> > -
> alter_ctx->table_name,
> > -alter_ctx->new_db,
> > -
> alter_ctx->new_alias))
> > -{
> > -
Re: [Maria-developers] MDEV-9423: FTWRL and Binlog checkpoint
On Wed, Jun 29, 2016 at 9:31 AM, Kristian Nielsen wrote: > Nirbhay Choubey writes: > > > Since there is no wait in reload_acl_and_cache() anymore, user's FLUSH > LOGS > > will > > create a new binary log file with binlog checkpoint event for the > > penultimate binlog and > > return, leaving it onto binlog background thread to take care of logging > > the checkpoint > > event for the current (new) binlog file. > > > > Now, if background thread kicks in _after_ the file transfer (as shown in > > #9 below), the > > same problem occurs - the joiner complains of the missing binlog file. > > Sure, I understand, what I fail to understand is how putting > wait_for_last_checkpoint_event() into the user's connection thread helps > avoid this. The user thread waits for the checkpoint event of the new > binlog > file, however the SST thread already did its wait for its own > reload_acl_and_cache(), it will not wait again... ? > Ah.. I get it now, adding the wait in reload_acl_and_cache() is futile. So, perhaps only option left is place this wait in sst_flush_tables() after reload_acl_and_cache(). - Nirbhay > > - Kristian. > ___ 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
Re: [Maria-developers] MDEV-9423: FTWRL and Binlog checkpoint
Hi Kristian, On Wed, Jun 29, 2016 at 4:31 AM, Kristian Nielsen wrote: > Nirbhay Choubey writes: > > > It wouldn't prevent the user from doing REFRESH_BINARY_LOG, but with > > wait_for_last_checkpoint_event() added to reload_acl_and_cache(), it > would > > ensure every REFRESH_BINARY_LOG (either from user or #2 above) waits > > until last checkpoint event makes into the new binary log file. > > The user's query will wait, but it's not clear how that helps your SST > (which it seems will be unaffected by the wait in the user's connection > thread). > Lets say we move the wait outside of reload_acl_and_cache() and place it after #3 and user does a REFRESH_BINARY_LOG (#4) after the wait is over but before the main thread acquires LOCK_log (#6). Since there is no wait in reload_acl_and_cache() anymore, user's FLUSH LOGS will create a new binary log file with binlog checkpoint event for the penultimate binlog and return, leaving it onto binlog background thread to take care of logging the checkpoint event for the current (new) binlog file. Now, if background thread kicks in _after_ the file transfer (as shown in #9 below), the same problem occurs - the joiner complains of the missing binlog file. 1> FTWRL 2> reload_acl_and_cache() 3> wait_for_last_checkpoint_event() 4> FLUSH LOGS (reload_acl_and_cache()) 5> SET global innodb_disallow_writes=1 6> mysql_mutex_lock(LOCK_log) 7> file transfer 8> mysql_mutex_unlock(LOCK_log) 9> MYSQL_BIN_LOG::write_binlog_checkpoint_event_already_locked() Best, Nirbhay ___ 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
Re: [Maria-developers] MDEV-9423: FTWRL and Binlog checkpoint
Hi Serg, On Tue, Jun 28, 2016 at 5:02 PM, Sergei Golubchik wrote: > Hi, Nirbhay! > > On Jun 27, Nirbhay Choubey wrote: > > > > > > That seems quite ugly, why not call it from the SST code, after it > > > has called reload_acl_and_cache()? You're basically making FLUSH > > > LOGS behave differently in Galera and non-Galera (if my > > > understanding is correct), which might lead to subtle bugs? > > > > I initially thought of adding the call after reload_acl_and_cache(), > > but there could still be a case when user performs a > > REFRESH_BINARY_LOG before LOCK_log is acquired. > > Right, but you didn't fix it. You have > > 1> FTWRL > 2> reload_acl_and_cache() > 3> wait_for_last_checkpoint_event() > 4> SET global innodb_disallow_writes=1 > 5> mysql_mutex_lock(LOCK_log) > > You've described your case correctly: "when user performs > REFRESH_BINARY_LOG before LOCK_log is acquired". That is, you care when > a user performs REFRESH_BINARY_LOG between 3 and 5. You don't care if > somebody does REFRESH_BINARY_LOG between 2 and 3. So, you can as well > move wait_for_last_checkpoint_event() out of reload_acl_and_cache(). > If wait is moved outside wait_for_last_checkpoint_event() (say 3') and user's REFRESH_BINARY_LOG kicks in right after the wait (3') but before (5), will trigger creation of another new binlog file for which the last checkpoint event (logged asynchronously by a separate thread) may not make it into time and will cause the same issue on joiner node. Another workable option was to move wait outside and after reload_acl_and_cache and not release LOCK_log until the file transfer is complete. 1> FTWRL 2> reload_acl_and_cache() 3> wait for last checkpoint event & lock(LOCK_log) 4> SET global innodb_disallow_writes=1 ... file transfer ... 5> mysql_mutex_unlock(LOCK_log) But with LOCK_log locked in #3, mysql_mutex_assert_not_owner(mysql_bin_log.get_log_lock()) will fail for #4. > With wait_for_last_checkpoint_event inside reload_acl_and_cache or > outside, you still don't have anything that would prevent user from > doing REFRESH_BINARY_LOG between 3 and 5. > It wouldn't prevent the user from doing REFRESH_BINARY_LOG, but with wait_for_last_checkpoint_event() added to reload_acl_and_cache(), it would ensure every REFRESH_BINARY_LOG (either from user or #2 above) waits until last checkpoint event makes into the new binary log file. Best, Nirbhay > Regards, > Sergei > Chief Architect MariaDB > and secur...@mariadb.org > ___ 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
Re: [Maria-developers] 0752333: MDEV-10161: wsrep_sync_wait not enabled when set to 1 in config file
Hi Serg, On Mon, Jun 27, 2016 at 7:38 AM, Sergei Golubchik wrote: > Hi, Nirbhay! > > On Jun 08, Nirbhay Choubey wrote: > > revision-id: 075233312cc5bc6c242f4629f8293d4c67937f02 > (mariadb-10.1.14-10-g0752333) > > parent(s): c9f05974e618a869563f2360ef1ca910974a4f1c > > author: Nirbhay Choubey > > committer: Nirbhay Choubey > > timestamp: 2016-06-08 16:03:01 -0400 > > message: > > > > MDEV-10161: wsrep_sync_wait not enabled when set to 1 in config file > > > > Since wsrep_sync_wait & wsrep_causal_reads variables are related, > > they are always kept in sync whenever one of them changes. > > Same is tried on server start, where wsrep_sync_wait get updated > > based on wsrep_causal_reads' value. But, since wsrep_causal_reads > > is OFF by default, wsrep_sync_wait's value gets modified and loses > > its WSREP_SYNC_WAIT_BEFORE_READ bit. > > > > Fixed by giving higher precedence to one that's explicitly set to > > a non-default value and later to wsrep_sync_wait in case both are > > set. > > > > diff --git a/sql/wsrep_mysqld.cc b/sql/wsrep_mysqld.cc > > index d0798ef..fffb0cc 100644 > > --- a/sql/wsrep_mysqld.cc > > +++ b/sql/wsrep_mysqld.cc > > @@ -567,7 +567,39 @@ int wsrep_init() > > > >wsrep_sst_auth_init(wsrep_sst_auth); > > > > - wsrep_causal_reads_update(&global_system_variables); > > + /* > > +On server start, there is no way to detect whether the default value > > +for a system variable was set explicitly by the user. > > +Now, since wsrep_causal_reads & wsrep_sync_wait are used for same > > +purpose, a rule of precedence must be defined to keep their values > > +in sync. It goes as following: > > This is wrong. First, in 10.1 there is a way to detect whether the value > was set by a user. I_S.SYSTEM_VARIABLES can show that. > sys_var::value_origin == CONFIG. I wasn't aware of that. > But even if you'd detected that, it won't help you do distinguish > between > > mysqld --wsrep-sync-wait=1 --wsrep-causal-reads=0 > mysqld --wsrep-sync-wait=0 --wsrep-causal-reads=1 > > The only correct solution is to handle this in the mysqld_get_one_option > switch, updating the dependent option when one of these two is set. > Correct. I have committed another one using this approach. http://lists.askmonty.org/pipermail/commits/2016-June/009503.html Thank you. - Nirbhay > > Regards, > Sergei > Chief Architect MariaDB > and secur...@mariadb.org > ___ 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
Re: [Maria-developers] MDEV-9423: FTWRL and Binlog checkpoint
Hi Kristian,
On Sat, Jun 25, 2016 at 3:57 AM, Kristian Nielsen
wrote:
> Nirbhay Choubey writes:
>
> >> Also, it seems reasonable that FTWRL in general could wait for
> checkpoint
> >> events so that other backup mechanisms similarly could avoid binlog
> files
>
> > That sound good to me. But, considering Percona's backup locks, it seems
> > more logical to
> > implement this in Backup locks instead, whenever they get
> > ported/implemented in MariaDB.
>
> Right. As I was thinking about the problem, it occured to me that this
> wasn't really a Galera-specific thing, my suggestion seemed a valid general
> wait-for-checkpoint mechanism.
>
> So we should put the code that waits for checkpoint in its own function
> (as you already did, MYSQL_BIN_LOG::wait_for_last_checkpoint_event()). But
> I
> agree, we can wait with actually exposing it (in FTWRL, backup locks,
> whatever) until when/if that becomes relevant/priority.
>
> I would just note that this wait does not really do anything unless there
> is
> something else (like FTWRL in your case) that prevents new commits,
> otherwise a new checkpoint could become pending at any time after
> wait_for_last_checkpoint_event() returns.
>
> > Also, in this particular case, the problem lies
> > in reload_acl_and_cache(REFRESH_BINARY_LOG),
> > (executed after FTWRL while preparing for SST) that rotates the binary
> log.
>
> Hm, I see. So you're always copying an empty binlog file? I'm wondering why
> you don't simply don't copy any binlogs and just start the new server with
> --tc-heuristic-recover=ROLLBACK ... maybe copying binlogs was just
> considered easier? Anyway, I don't have the bigger picture, so can't have
> much of an informed opinion here.
>
The joiner node also picks up the GTID state from the binary log file it
received.
> > Yes, it worked. But, to solve this issue in 10.1, I have added this wait
> to
> > REFRESH_BINARY_LOG
> > (as explained above) only when the server is acting as a Galera node.
>
> That seems quite ugly, why not call it from the SST code, after it has
> called reload_acl_and_cache()? You're basically making FLUSH LOGS behave
> differently in Galera and non-Galera (if my understanding is correct),
> which
> might lead to subtle bugs?
I initially thought of adding the call after reload_acl_and_cache(), but
there
could still be a case when user performs a REFRESH_BINARY_LOG before
LOCK_log is acquired.
> But again, I don't have the bigger picture, and
> the whole wsrep patch is garbage all over the server anyway, so I suppose
> it
> doesn't matter much to me, as long as it's #ifdef WSREP.
>
> >> and it also makes the extra lock/unlock of LOCK_log above redundant.
> >
> >
> > Not quite. The wait logic (that includes LOCK_log, as the snippet above)
> is
> > to pause
> > REFRESH_BINARY_LOG and an additional use of LOCK_log to block the RESET/
> > FLUSH commands while file transfer is in progress.
>
> Sure, it's fine to have both, probably makes the code clearer anyway.
>
Right.
>
> > --- a/sql/log.cc
> > +++ b/sql/log.cc
> > @@ -3690,7 +3690,10 @@ bool MYSQL_BIN_LOG::open(const char *log_name,
> > new_xid_list_entry->binlog_id= current_binlog_id;
> > /* Remove any initial entries with no pending XIDs. */
> > while ((b= binlog_xid_count_list.head()) && b->xid_count == 0)
> > +{
> >my_free(binlog_xid_count_list.get());
> > + mysql_cond_broadcast(&COND_xid_list);
> > +}
> > binlog_xid_count_list.push_back(new_xid_list_entry);
> > mysql_mutex_unlock(&LOCK_xid_list);
>
> There is no need to mysql_cond_broadcast() multiple times. Use just a
> single
> broadcast outside the loop (before or after, doesn't make a difference).
>
Fixed.
Best,
Nirbhay
>
> - Kristian.
>
___
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
Re: [Maria-developers] MDEV-9423: FTWRL and Binlog checkpoint
Hi Kristian,
On Thu, Jun 23, 2016 at 3:39 AM, Kristian Nielsen
wrote:
> Nirbhay Choubey writes:
>
> > While copying the last 2 binlog files would have solved this, I have
> worked
> > out
> > a solution where the donor node waits for binlog checkpoint event for
> last
> > binlog
> > file to get logged before proceeding with file transfer.
> >
> > http://lists.askmonty.org/pipermail/commits/2016-June/009483.html
>
> Urgh, please don't do this, seems there are multiple problems with this
> patch (insufficient locking, introducing a new redundant wait mechanism,
> comparing binlog file names rather than ids, ...).
>
> > By the way, I initially tried reusing
> > is_xidlist_idle_nolock()/COND_xid_list to implement the
> > waiting mechanism. But since binlog checkpoint events are written
> > asynchronously after
> > xid_count falls to 0, that did not work. So later came up with the above
>
> I think it should work if you follow the chained locking of LOCK_xid_list
> and LOCK_log. First wait under LOCK_xid_list for the binlog_xid_count_list
> to become empty. Then release LOCK_xid_list and take and immediately
> release
> LOCK_log. mark_xid_done() will hold onto LOCK_log until the checkpoint
> event
> has been written.
>
> Note that there is already a similar wait mechanism, used by RESET
> MASTER. RESET MASTER also needs to wait for checkpoint events to be
> completed before running, so we should reuse that mechanism.
>
Right.
>
> Also, it seems reasonable that FTWRL in general could wait for checkpoint
> events so that other backup mechanisms similarly could avoid binlog files
> changing during backup. So please fix this in FTWRL, in 10.2. (If you feel
> you need to fix the galera bug in 10.1, you can implement it only for
> galera
> in 10.1).
>
That sound good to me. But, considering Percona's backup locks, it seems
more logical to
implement this in Backup locks instead, whenever they get
ported/implemented in MariaDB.
Also, in this particular case, the problem lies
in reload_acl_and_cache(REFRESH_BINARY_LOG),
(executed after FTWRL while preparing for SST) that rotates the binary log.
So, FTWRL is not
directly linked to this issue. And as you rightly pointed, I will refrain
from altering FTWRL's
behavior in 10.1 at least.
> So in more detail, here is suggested way to fix:
>
> In FTWRL (somewhere near the end, after commits are blocked), wait for
> checkpoint events to be written using a similar mechanism as RESET MASTER:
>
> if (mysql_bin_log.is_open())
> {
> mysql_mutex_lock(&LOCK_xid_list);
> for (;;)
> {
> if (binlog_xid_count_list.is_last(binlog_xid_count_list.head()))
> break;
> mysql_cond_wait(&COND_xid_list, &LOCK_xid_list);
> }
> mysql_mutex_unlock(&LOCK_xid_list);
> /*
> LOCK_xid_list and LOCK_log are chained, so the LOCK_log will only be
> obtained after mark_xid_done() has written the last checkpoint event.
> */
> mysql_mutex_lock(&LOCK_log);
> mysql_mutex_unlock(&LOCK_log);
> }
>
> Now, since FTWRL is a bit different from RESET MASTER, we need a couple
> other changes:
>
> - Use mysql_cond_broadcast(&COND_xid_list) instead of mysql_cond_signal()
>in mark_xid_done() (to allow multiple waiters).
>
> - The second (but not the first mysql_cond_broadcast() in mark_xid_done()
>should be unconditional, so remove the if() here:
>
> if (unlikely(reset_master_pending))
> mysql_cond_signal(&COND_xid_list);
>
> - Also add mysql_cond_broadcast(&COND_xid_list) in two other places that
>the binlog_xid_count_list is modified. One in MYSQL_BIN_LOG::open():
>
> while ((b= binlog_xid_count_list.head()) && b->xid_count == 0)
> my_free(binlog_xid_count_list.get());
>
>And one in reset_logs():
>
> my_free(binlog_xid_count_list.get());
>
> This should make FTWRL wait for all pending binlog checkpoint events to be
> written. And with commits blocked, no new checkpoints should become
> pending.
>
> Does it seem reasonable to you? Let me know if some things are unclear or
> if
> you see any potential problems with it.
>
Yes, it worked. But, to solve this issue in 10.1, I have added this wait to
REFRESH_BINARY_LOG
(as explained above) only when the server is acting as a Galera node.
> By the way, how to you intend to handle the case where RESET MASTER is run
> during SST? I just checked, FTWRL does not seem to block RESET MASTER. Or
> do
> you have another mechanism to prevent RESET MASTER from running during SST?
> Thinking more, you should be holding LOCK_log while copying the binlog
> f
Re: [Maria-developers] MDEV-9423: FTWRL and Binlog checkpoint
Hi Kristian! On Mon, May 2, 2016 at 2:10 PM, Kristian Nielsen wrote: > Nirbhay Choubey writes: > > [Cc: maria-developers@, please always keep these discussions on the > mailing list] > > > In Galera cluster, the state transfer scripts perform FTWRL and > > copy data along with the last of all available binlog files to the > > joiner node. > > > > After MDEV-181, I understand that the binlog checkpoint can be > > in any of the binary log files (and not necessarily the last one). > > > > This seemingly has caused MDEV-9423, in which the joiner node > > complains of the missing binlog file. > > > > Now the question is : Is FTWRL not sufficient to ensure that the > > checkpoint is always the last binlog file? > > So if I understand correctly, the issue is related to having binlog files > available during XA crash recovery. When the binlog file is rotated, there > is a small window where both the latest and the previous binlog files are > needed for crash recovery. The binlog checkpoint is the earliest binlog > file > that is needed for crash recovery, and it can be seen from the binlog > checkpoint event. > > So the problem here is that a copy is made just after binlog rotation, and > Galera only copies the most recent, mostly-empty binlog file, leaving > insufficient information for XA recovery, right? > Correct. > > One option to solve this is to always copy the last two binlog files. While > it is theoretically possible to have the binlog checkpoint more than two > files back, I think it will not occur in practice. > Another option is to wait for the binlog checkpoint to reach the current > binlog file. You can see this done in the test suite: > > mysql-test/include/wait_for_binlog_checkpoint.inc > > The binlog checkpointing happens asynchroneously, I *think* it can complete > even while FTWRL is active, but I am not 100% sure though. > > The checkpoint happens after InnoDB has made its commits durable with > fsync() or similar - only after that is it safe to discard the old binlog > data and still have correct crash recovery. > While copying the last 2 binlog files would have solved this, I have worked out a solution where the donor node waits for binlog checkpoint event for last binlog file to get logged before proceeding with file transfer. http://lists.askmonty.org/pipermail/commits/2016-June/009483.html By the way, I initially tried reusing is_xidlist_idle_nolock()/COND_xid_list to implement the waiting mechanism. But since binlog checkpoint events are written asynchronously after xid_count falls to 0, that did not work. So later came up with the above patch. Best, Nirbhay > > - Kristian. > ___ 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
Re: [Maria-developers] [Commits] 511bd2c: MDEV-10216: Assertion `strcmp(share->unique_file_name, filename) ||
Hi Monty, On Tue, Jun 21, 2016 at 9:30 AM, Michael Widenius wrote: > > Hi! > > >>>>> "Nirbhay" == Nirbhay Choubey writes: > > > > Nirbhay> ALTER TABLE implementation takes a shortcut for operations not > affecting > Nirbhay> .frm files, > Nirbhay> which includes simple RENAMEs. But this is skipped for temp > tables and thus > Nirbhay> copying > Nirbhay> always takes place for temp tables. > > Nirbhay> The following commit (from 2002) added this exception for temp > tables: > Nirbhay> > https://github.com/MariaDB/server/commit/a7798dfd0a6472bf65fffc2ade543605e177ff9c > > The above old patch is not really working for the current code. > > Nirbhay> I have created a patch to also include temp tables to this > shortcut : > Nirbhay> https://gist.github.com/nirbhayc/442a0c269ce48b283543cac434aaf44e > > This solves the problem of doing a simple rename, but not for things > like: > > alter table t1 add column c int, rename to t2; > > Which will still produce crashes. > > When it comes to last_version, is still vote for that we should be using > THD::rename_temporary_table when renaming temporary tables instead of > doing any handler calls. > Right. > > Maybe we could move THD::rename_temporary_tables to ha_rename_table to > get all temporary tables handled in one place ? > handler::ha_rename_table() is currently being used only for non-temporary tables at places which do not support temporary tables like partitions, DDL logs, RENAME TABLE, etc. So, moving rename_temporary_table to ha_rename_table seem overkill and possibly inappropriate. Here's the latest version of patch : http://lists.askmonty.org/pipermail/commits/2016-June/009477.html Best, Nirbhay > Regards, > Monty > ___ 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
Re: [Maria-developers] [Commits] 511bd2c: MDEV-10216: Assertion `strcmp(share->unique_file_name, filename) ||
Hi Svoj, [Also adding Monty for his inputs.] On Thu, Jun 16, 2016 at 7:45 AM, Sergey Vojtovich wrote: > Hi Nirbhay, > > last_version of MyISAM is used for debugging only and has no functional > effect. > last_version of Aria has some functional effect. > > Effect of HA_EXTRA_PREPARE_FOR_RENAME for MyISAM is flush key blocks and > reset > last_version. > Effect of HA_EXTRA_PREPARE_FOR_RENAME for Aria is more complex. > Other engines ignore HA_EXTRA_PREPARE_FOR_RENAME. > > I don't completely understand why "ALTER TABLE ... RENAME TO ..." has to > copy > data between tables. ALTER TABLE implementation takes a shortcut for operations not affecting .frm files, which includes simple RENAMEs. But this is skipped for temp tables and thus copying always takes place for temp tables. The following commit (from 2002) added this exception for temp tables: https://github.com/MariaDB/server/commit/a7798dfd0a6472bf65fffc2ade543605e177ff9c I have created a patch to also include temp tables to this shortcut : https://gist.github.com/nirbhayc/442a0c269ce48b283543cac434aaf44e > But it is probably subject of another bug (or even task). > Right. I will open a separate MDEV for this. > copy_data_between_tables() seem to be the only function that does > HA_EXTRA_PREPARE_FOR_RENAME for temporary table (intermediate in this > case). > > I don't think the above HA_EXTRA_PREPARE_FOR_RENAME is reasonable for > MyISAM. > There's no point in resetting last_version and flushing key blocks, since > we're > going to reuse this itermediate table anyway. > > I don't think the above can be reasonable for any engine. > Makes sense. > > Said the above, I'd vote to remove this call. But please check with some > Aria > expert. > Your fix is mostly alright, but I guess we shouldn't reset last_version in > case > of HA_EXTRA_PREPARE_FOR_DROP. > Monty ^^ ? Best, Nirbhay > > Regards, > Sergey > > On Wed, Jun 15, 2016 at 08:57:06AM -0400, Nirbhay Choubey wrote: > > revision-id: 511bd2ca3269bc7bf80e30cceeab534f7f3e5666 > (mariadb-10.2.0-83-g511bd2c) > > parent(s): b2ae32aafdd2787ad456f38833f630182ded25e8 > > author: Nirbhay Choubey > > committer: Nirbhay Choubey > > timestamp: 2016-06-15 08:57:04 -0400 > > message: > > > > MDEV-10216: Assertion `strcmp(share->unique_file_name,filename) || > > > > .. share->last_version' failed in myisam/mi_open.c:67: test_if_reopen > > > > During the RENAME operation since the renamed temporary table is also > > opened and added to myisam_open_list/maria_open_list, resetting the > > last_version at the end of operation (HA_EXTRA_PREPARE_FOR_RENAME) > > will cause an assertion failure when a subsequent query tries to open > > an additional temporary table instance and thus attempts to reuse it > > from the open table list. > > > > Fixed by not resetting share->last_version for temporary tables so that > > the share gets reused when needed. > > > > --- > > mysql-test/r/reopen_temp_table.result | 18 ++ > > mysql-test/t/reopen_temp_table.test | 16 > > storage/maria/ma_extra.c | 7 ++- > > storage/myisam/mi_extra.c | 7 ++- > > 4 files changed, 46 insertions(+), 2 deletions(-) > > > > diff --git a/mysql-test/r/reopen_temp_table.result > b/mysql-test/r/reopen_temp_table.result > > index 08affaa..e63a21e 100644 > > --- a/mysql-test/r/reopen_temp_table.result > > +++ b/mysql-test/r/reopen_temp_table.result > > @@ -164,5 +164,23 @@ SELECT COUNT(*)=4 FROM t6; > > COUNT(*)=4 > > 1 > > DROP TABLE t5, t6; > > +# > > +# MDEV-10216: Assertion `strcmp(share->unique_file_name,filename) || > > +# share->last_version' failed in myisam/mi_open.c:67: test_if_reopen > > +# > > +CREATE TEMPORARY TABLE t7 (i INT) ENGINE=MYISAM; > > +INSERT INTO t7 VALUES(1); > > +ALTER TABLE t7 RENAME TO t; > > +SELECT * FROM t a, t b; > > +ii > > +11 > > +DROP TABLE t; > > +CREATE TEMPORARY TABLE t7 (i INT) ENGINE=ARIA; > > +INSERT INTO t7 VALUES(1); > > +ALTER TABLE t7 RENAME TO t; > > +SELECT * FROM t a, t b; > > +ii > > +11 > > +DROP TABLE t; > > # Cleanup > > DROP DATABASE temp_db; > > diff --git a/mysql-test/t/reopen_temp_table.test > b/mysql-test/t/reopen_temp_table.test > > index 98de983..83a5bbc 100644 > > --- a/mysql-test/t/reopen_temp_table.test > > +++ b/mysql-test/t/reopen_temp_table.test > > @@ -159,5 +159,21 @@ SELECT COUNT(*)=6 FROM t5; > > SELECT
Re: [Maria-developers] 56a2835: MDEV-5535: Cannot reopen temporary table
Hi Serg!
On Fri, Jun 3, 2016 at 4:10 PM, Sergei Golubchik wrote:
> Hi, Nirbhay!
>
> Just a couple of questions/comments, see below.
>
> And please pay attention to Kristian's email about locking and parallel
> replication. I don't think I can confidently review it in a patch, so
> you may want to spend more time testing or analyzing that code.
>
Yes, I did add some related rpl test cases.
>
> Ok to push, when you're ready! Thanks!
>
> On May 26, Nirbhay Choubey wrote:
> > revision-id: 56a2835872c4ac7296ec0ae2ff618822855b0fc0
> (mariadb-10.1.8-82-g56a2835)
> > parent(s): 28c289626f318631d707f85b057a90af99018b06
> > author: Nirbhay Choubey
> > committer: Nirbhay Choubey
> > timestamp: 2016-05-26 20:42:47 -0400
> > message:
> >
> > MDEV-5535: Cannot reopen temporary table
> >
> > mysqld maintains a list of TABLE objects for all temporary
> > tables in THD, where each table is represented by a TABLE
> > object. A query referencing a temporary table for more than
> > once, however, failed with ER_CANT_REOPEN_TABLE error.
>
> "because a TABLE_SHARE was allocate together with the TABLE, so
> temporary tables always had one TABLE per TABLE_SHARE"
>
> > This patch lift this restriction by preserving the TABLE_SHARE
> > object of each created temporary table, so that multiple instances
> > of TABLE objects can be created.
>
> better "by separating TABLE and TABLE_SHARE objects and storing
> TABLE_SHARE's in the list in a THD, and TABLE's in the list in
> the corresponding TABLE_SHARE"
>
That's better indeed. I will update that when I merge the consolidated
patches to the main branch.
>
> > diff --git a/sql/rpl_rli.cc b/sql/rpl_rli.cc
> > index 7bdd9c1..77f458b 100644
> > --- a/sql/rpl_rli.cc
> > +++ b/sql/rpl_rli.cc
> > @@ -1060,24 +1061,43 @@ void
> Relay_log_info::inc_group_relay_log_pos(ulonglong log_pos,
> >
> > void Relay_log_info::close_temporary_tables()
> > {
> > - TABLE *table,*next;
> >DBUG_ENTER("Relay_log_info::close_temporary_tables");
> >
> > - for (table=save_temporary_tables ; table ; table=next)
> > + TMP_TABLE_SHARE *share;
> > + TABLE *table;
> > +
> > + while ((share= save_temporary_tables.pop_front()))
> >{
> > -next=table->next;
> > +/*
> > + Iterate over the list of tables for this TABLE_SHARE and close
> them.
> > +*/
> > +while ((table= share->all_tmp_tables.pop_front()))
> > +{
> > + DBUG_PRINT("tmptable", ("closing table: '%s'.'%s'",
> > + table->s->db.str,
> table->s->table_name.str));
> > +
> > + /* Reset in_use as the table may have been created by another thd
> */
> > + table->in_use= 0;
> > + free_io_cache(table);
> > + /*
> > +Lets not free TABLE_SHARE here as there could be multiple
> TABLEs opened
> > +for the same table (TABLE_SHARE).
> > + */
> > + closefrm(table, false);
> > + my_free(table);
> > +}
> >
> > -/* Reset in_use as the table may have been created by another thd */
> > -table->in_use=0;
> > /*
> >Don't ask for disk deletion. For now, anyway they will be deleted
> when
> >slave restarts, but it is a better intention to not delete them.
> > */
> > -DBUG_PRINT("info", ("table: 0x%lx", (long) table));
> > -close_temporary(table, 1, 0);
> > +
> > +free_table_share(share);
> > +my_free(share);
> >}
> > - save_temporary_tables= 0;
> > - slave_open_temp_tables= 0;
> > +
> > + save_temporary_tables.empty();
>
> Is it needed?
> you've popped all shares from the list, it's empty.
> I'd rather add an assert instead.
>
Right. I realized this later. Fixed.
>
> > +
> >DBUG_VOID_RETURN;
> > }
> >
> > diff --git a/sql/sql_class.h b/sql/sql_class.h
> > index ae240ae..a70c4a9 100644
> > --- a/sql/sql_class.h
> > +++ b/sql/sql_class.h
> > @@ -1247,6 +1247,61 @@ enum enum_locked_tables_mode
> >LTM_PRELOCKED_UNDER_LOCK_TABLES
> > };
> >
> > +/**
> > + The following structure is an extension to TABLE_SHARE and is
> > + exclusively for temporary tables.
> > +
> > + @note:
> > + Although, TDC_element has data members (like next, prev &
> > + all_tables) to store the list of TABLE_SHARE & TA
Re: [Maria-developers] [Commits] c285dbe: MDEV-5535: Cannot reopen temporary table
Hi Kristian! On Fri, Jun 3, 2016 at 7:53 AM, Kristian Nielsen wrote: > Sergei Golubchik writes: > > > On May 26, Nirbhay Choubey wrote: > > >> > > + if (wait_for_prior_commit()) > > Please call this method tmptable_wait_for_prior_commit() or something like > that. To avoid confusion with THD::wait_for_prior_commit(). > I no longer have this wrapper and am calling THD::wait_for_prior_commit() directly. > >> > you're doing it in almost every method. And when one method calls > another, > >> > you're doing it twice. Or thrice. Isn't is too much? (I don't know) > >> > >> I added these additional waits to fix some failing replication tests. > But, > >> whit new TMP_TABLE_SHARE approach, this could have changed. > >> I have now reverted them to the original state and will run rpl test to > >> assure if we do not need additional waits. > > That won't mean much. Temporary table handling in statement-based > replication is very nasty code, and we surely do not have full test > coverage > of possible issues. And bugs only turn up as rare race conditions that are > hard to reproduce. > I kind of felt that. :) > > So you will have to make sure you understand in detail exactly where > wait_for_prior_commit() (and possibly other needed code) is needed to > ensure > statement-based parallel replication will work in all cases (or at least > work as well as the non-parallel case). > > The issue here is that temporary tables have no locking facilities. And the > same temporary table can be used by multiple different transactions, which > in parallel replication can be run in different threads. There is code that > keeps track of a shared list of temporary tables, and shuffles it around > between threads as needed (this is also needed for non-parallel > replication, > since temporary tables might need to be kept across SQL thread destruction > and re-creation). See Relay_log_info::save_temporary_tables (as you > probably > already saw). Maybe Monty knows something about this (ISTR that he wrote > that code originally), otherwise careful study of the code is probably the > only way. > > Since there is no locking of temporary tables (at least before your > patch?), > it would be possible for two worker threads to simultaneously try to use > the > same temptable, which fails, obviously. This is handled rather crudely in > the current code by simply serialising all transactions using temporary > tables with all prior transactions. This is what wait_for_prior_commit() > does - it waits for all transactions to commit that come before in the > replication stream (within that replication domain). > Right, and as I see it, we wait at the _entry_ point when a thread tries to access/create a temporary table. > > Hope this helps, > Thanks for sharing these details. Best, Nirbhay > > - Kristian. > ___ 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
Re: [Maria-developers] [Commits] c285dbe: MDEV-5535: Cannot reopen temporary table
Hi Serg! On Fri, Jun 3, 2016 at 7:20 AM, Sergei Golubchik wrote: > Hi, Nirbhay! > > On May 26, Nirbhay Choubey wrote: > > > > > +/* > > > > + Create a temporary table, open it and return the TABLE handle. > > > > + > > > > + @param hton [IN]Handlerton > > > > + @param frm [IN]Binary frm image > > > > + @param path [IN]File path (without extension) > > > > + @param db [IN]Schema name > > > > + @param table_name [IN] Table name > > > > + @param open_in_engine [IN] Whether open table in SE > > > > + @param created [OUT]Whether table was created? > > > > > > can it ever happen for *create==true but a return value is NULL? > > > or the other way around? how? > > > > Yes, but not the other way around. For instance, if the table was > > created successfully, but the subsequent malloc failed while an > > attempt was made to allocate TABLE object later in > > open_temporary_table(). > > But do you need to handle this practically impossible case specially? > I'd say - if that happens, just drop the table (or even don't drop it, > it'll be removed on disconnect anyway) and pretend the whole never > happened (return NULL). I mean, you don't need a separate *created > pointer. > I have update the function to remove the share from the list and free it in case it fails to open a table instance. > > > > + if (wait_for_prior_commit()) > > > > > > you're doing it in almost every method. And when one method calls > another, > > > you're doing it twice. Or thrice. Isn't is too much? (I don't know) > > > > I added these additional waits to fix some failing replication tests. > But, > > whit new TMP_TABLE_SHARE approach, this could have changed. > > I have now reverted them to the original state and will run rpl test to > > assure if we do not need additional waits. > > And? > I found some failing rpl tests. Fixed. > > > > > + free_io_cache(table); > > > > > > don't forget to remove free_io_cache() calls when rebasing your > > > work on top of the latest 10.2 > > > (they were removed from close_temporary() in 260dd476b05 commit) > > > > Okay > > Unless you've rebased already, it makes sense to do it now - as you're > apparently almost ready to push > Done. Best, Nirbhay > > Regards, > Sergei > Chief Architect MariaDB > and secur...@mariadb.org > ___ 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
Re: [Maria-developers] core review #1
Hi Shubham,
On Sun, Jun 5, 2016 at 9:25 AM, Sergei Golubchik wrote:
> Hi, Shubham!
>
> Here's a first review.
>
> Summary: looks good so far, main comments:
>
> 1. Coding style. Use the same coding style as everywhere else in the
>file you're editing. It's the same in sql/ and in myisam/ directories.
>But InnoDB uses different coding style.
>
You can refer to the link below for coding guidelines.
https://dev.mysql.com/doc/internals/en/general-development-guidelines.html
Also, if you use vim, there are some good vim suggestions to fix
indentations.
Best,
Nirbhay
>
> 2. Tests, tests, tests! Please start adding tests now, and commit them
>into your branch. They should be somewhere under mysql-test/ directory.
>The full documentation is here:
> https://mariadb.com/kb/en/mariadb/mysqltest/
>But even without it you can create your tests by starting of from
>existing test files.
>
> More comments below:
>
> > diff --git a/sql/sql_table.cc b/sql/sql_table.cc
> > index dfce503..31f282b 100644
> > --- a/sql/sql_table.cc
> > +++ b/sql/sql_table.cc
> > @@ -3876,8 +3876,9 @@ mysql_prepare_create_table(THD *thd,
> HA_CREATE_INFO *create_info,
> > column->length= MAX_LEN_GEOM_POINT_FIELD;
> > if (!column->length)
> > {
> > - my_error(ER_BLOB_KEY_WITHOUT_LENGTH, MYF(0),
> column->field_name.str);
> > - DBUG_RETURN(TRUE);
> > + key_info->algorithm=HA_KEY_ALG_HASH;
>
> There's more to it. The task title is, indeed, "unique index for
> blobs", but the actual goal is to have unique indexes for anything
> that is too long for normal btree indexes. That's what MI_UNIQUE does.
>
> so, you should support unique indexes also for blobs where column->length
> is specified, but is too large. Or unique indexes for a combination of
> ten long varchar columns, for example.
>
> So, you also need to patch the code where it issues ER_TOO_LONG_KEY.
>
> Also we'd want to use MI_UNIQUE for keys where a user has
> explicitly specified USING HASH, but let's look at it later
>
> > + // my_error(ER_BLOB_KEY_WITHOUT_LENGTH, MYF(0),
> column->field_name.str);
> > + // DBUG_RETURN(TRUE);
> > }
> > }
> > #ifdef HAVE_SPATIAL
> > diff --git a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc
> > index 72bc4c0..9aba216 100644
> > --- a/storage/myisam/ha_myisam.cc
> > +++ b/storage/myisam/ha_myisam.cc
> > @@ -216,44 +216,59 @@ static void mi_check_print_msg(HA_CHECK *param,
> const char* msg_type,
> > 0 OK
> > !0 error code
> > */
> > -
> > int table2myisam(TABLE *table_arg, MI_KEYDEF **keydef_out,
> > - MI_COLUMNDEF **recinfo_out, uint *records_out)
> > + MI_COLUMNDEF **recinfo_out,MI_UNIQUEDEF
> **uniquedef_out ,uint *records_out)
> > {
> > - uint i, j, recpos, minpos, fieldpos, temp_length, length;
> > + uint i, j, recpos, minpos, fieldpos, temp_length, length, k, l, m;
> >enum ha_base_keytype type= HA_KEYTYPE_BINARY;
> >uchar *record;
> >KEY *pos;
> >MI_KEYDEF *keydef;
> > + MI_UNIQUEDEF *uniquedef;
> >MI_COLUMNDEF *recinfo, *recinfo_pos;
> >HA_KEYSEG *keyseg;
> >TABLE_SHARE *share= table_arg->s;
> >uint options= share->db_options_in_use;
> >DBUG_ENTER("table2myisam");
> > + pos= table_arg->key_info;
> > + share->uniques=0;
> > + for (i= 0; i < share->keys; i++,pos++)
> > + {
> > +if(pos->algorithm==HA_KEY_ALG_HASH)
> > +{
> > + share->uniques++ ;
> > +}
> > + }
> >if (!(my_multi_malloc(MYF(MY_WME),
> >recinfo_out, (share->fields * 2 + 2) * sizeof(MI_COLUMNDEF),
> > - keydef_out, share->keys * sizeof(MI_KEYDEF),
> > + keydef_out, (share->keys - share->uniques) *
> sizeof(MI_KEYDEF),
> > + uniquedef_out, share->uniques * sizeof(MI_UNIQUEDEF),
> >&keyseg,
> >(share->key_parts + share->keys) * sizeof(HA_KEYSEG),
> >NullS)))
> > DBUG_RETURN(HA_ERR_OUT_OF_MEM); /* purecov: inspected */
> >keydef= *keydef_out;
> >recinfo= *recinfo_out;
> > + uniquedef= *uniquedef_out;
> > pos= table_arg->key_info;
> > + k=0;
> > + m=0;
>
> better call your indexes 'k' and 'u', for keydefs and uniques.
>
> >for (i= 0; i < share->keys; i++, pos++)
> >{
> > -keydef[i].flag= ((uint16) pos->flags & (HA_NOSAME | HA_FULLTEXT |
> HA_SPATIAL));
> > -keydef[i].key_alg= pos->algorithm == HA_KEY_ALG_UNDEF ?
> > +if(pos->algorithm!=HA_KEY_ALG_HASH)
> > +{
> > +keydef[k].flag= ((uint16) pos->flags & (HA_NOSAME | HA_FULLTEXT |
> HA_SPATIAL));
> > +keydef[k].key_alg= pos->algorithm == HA_KEY_ALG_UNDEF ?
> >(pos->flags & HA_SPATIAL ? HA_KEY_ALG_RTREE : HA_KEY_ALG_BTREE) :
> >pos->algorithm;
> > -keydef[i].block_length= pos->block_size;
> > -keydef[i].seg= keyseg;
> > -keydef[i].keysegs= pos->user_defined_key_parts;
> > +keydef[k].block_length= pos->block_size;
> > +keydef[k].
Re: [Maria-developers] [Commits] 53696a6: MDEV-10004: Galera's pc.recovery process fails in 10.1 with systemd
Hi Svoj, On Fri, May 27, 2016 at 3:27 AM, Sergey Vojtovich wrote: > Hi Nirbhay, > > Looks good to push. > > On Thu, May 26, 2016 at 11:52:04PM -0400, Nirbhay Choubey wrote: > > revision-id: 53696a63a2a517e04bf27382184162da50994ecb > (mariadb-10.1.14-4-g53696a6) > > parent(s): 9a1c4e900b98fdb9940aab57c895753f175c2bd8 > > author: Nirbhay Choubey > > committer: Nirbhay Choubey > > timestamp: 2016-05-26 23:52:04 -0400 > > message: > > > > MDEV-10004: Galera's pc.recovery process fails in 10.1 with systemd > ...skip... > > > +# Safety checks > > +if [ -n "$log_file" -a -f "$log_file" ]; then > > + [ "$euid" = "0" ] && chown $user $log_file > > + chmod 600 $log_file > > +else > > + echo "WSREP: mktemp failed" > Should this be "log" instead of "echo"? > Oh yes, indeed. Best, Nirbhay > > Regards, > Sergey > ___ > commits mailing list > comm...@mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits ___ 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
Re: [Maria-developers] [Commits] 454a9dc: MDEV-10004: Galera's pc.recovery process fails in 10.1 with systemd
Hi Svoj!
On Wed, May 25, 2016 at 3:56 AM, Sergey Vojtovich wrote:
> Hi Nirbhay,
>
> I believe this patch is acceptable. Still I didn't completely understand
> the
> need for all this complexity and your answer to Daniel's question.
>
wsrep recovery broadly involves following steps:
1.0 # Grab the last saved global transaction ID from the storage engine
(InnoDB) after performing SE recovery.
2,0 # Pass this ID to wsrep provider (galera).
3.0 # wsrep provider decides whether the node requires a state transfer.
3.1 # Perform state transfer (in case of rsync/xtrabackup SST methods,
receive files - this has
to be done before SE initialization).
3.2 # In case of rsync/xtrabackup : load the storage engine (InnoDB).
While 1.0 is done after loading InnoDB, 3.1 needs to be does before its
initialization. And since InnoDB cannot be reinitialized without a
restarting
the server, the entire recovery process requires a server restart.
1.0 : mysqld --wsrep-recover
2.0 - 3.2 : mysqld [all-other-options] --wsrep-start-position=xxx
> Some minor comments inline.
>
> On Fri, May 20, 2016 at 11:04:24PM -0400, Nirbhay Choubey wrote:
> > revision-id: 454a9dc53a5b02af316dd713c577082eb8cf79c4
> (mariadb-10.1.14-4-g454a9dc)
> > parent(s): 9a1c4e900b98fdb9940aab57c895753f175c2bd8
> > author: Nirbhay Choubey
> > committer: Nirbhay Choubey
> > timestamp: 2016-05-20 23:04:20 -0400
> > message:
> >
> > MDEV-10004: Galera's pc.recovery process fails in 10.1 with systemd
> >
> > Galera recovery process works in two phases. In the first
> > phase, mysqld is started as non-daemon with --wsrep-recover
> > to recover and fetch the last logged global transaction ID.
> > This ID is then used in second phase as the start position
> > (--wsrep-start-position=XX) to start mysqld as daemon.
> >
> > As this process was implemented in mysqld_safe script, the
> > recovery did not work when server was started using systemd.
> >
> > Fixed by introducing a shell script (wsrep_recovery.sh) that
> > mimics the first phase of the recovery process.
> >
> > ---
> > cmake/systemd.cmake | 3 +-
> > scripts/galera_recovery.sh | 94
>
> > support-files/mariadb.service.in | 11 -
> > 3 files changed, 106 insertions(+), 2 deletions(-)
> >
> > diff --git a/cmake/systemd.cmake b/cmake/systemd.cmake
> > index b0161cf..17f44f7 100644
> > --- a/cmake/systemd.cmake
> > +++ b/cmake/systemd.cmake
> > @@ -55,9 +55,10 @@ MACRO(CHECK_SYSTEMD)
> >IF(HAVE_SYSTEMD AND HAVE_SYSTEMD_SD_DAEMON_H AND
> HAVE_SYSTEMD_SD_LISTEN_FDS
> > AND HAVE_SYSTEMD_SD_NOTIFY AND HAVE_SYSTEMD_SD_NOTIFYF)
> > ADD_DEFINITIONS(-DHAVE_SYSTEMD)
> > -SET(SYSTEMD_SCRIPTS mariadb-service-convert galera_new_cluster)
> > +SET(SYSTEMD_SCRIPTS mariadb-service-convert galera_new_cluster
> galera_recovery)
> > SET(SYSTEMD_DEB_FILES "usr/bin/mariadb-service-convert
> > usr/bin/galera_new_cluster
> > + usr/bin/galera_recovery
> >
> ${INSTALL_SYSTEMD_UNITDIR}/mariadb.service
> > ${INSTALL_SYSTEMD_UNITDIR}/mariadb@
> .service
> >
> ${INSTALL_SYSTEMD_UNITDIR}/mariadb@bootstrap.service.d
> /use_galera_new_cluster.conf")
> > diff --git a/scripts/galera_recovery.sh b/scripts/galera_recovery.sh
> > new file mode 100755
> > index 000..6d4f1d5
> > --- /dev/null
> > +++ b/scripts/galera_recovery.sh
> > @@ -0,0 +1,94 @@
> > +#!/bin/sh
> > +
> > +# Copyright (c) 2016 MariaDB Corporation
> > +#
> > +# This program is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; version 2 of the License.
> > +#
> > +# This program is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program; if not, write to the Free Software
> > +# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301
> USA */
> > +
> > +
> > +# This script is intended to be executed by systemd. It starts mysqld
> with
> > +# --wsrep-recover to recover from a non-graceful shutdown, determines
> the
> > +# last stored global transaction ID
Re: [Maria-developers] 1e883e9: MDEV-5535: Cannot reopen temporary table
Hi Serg,
On Tue, May 17, 2016 at 10:26 AM, Sergei Golubchik wrote:
> Hi, Nirbhay!
>
>
> .. cut ..
>
> > > > diff --git a/mysql-test/r/temp_table.result
> > > b/mysql-test/r/temp_table.result
> > > > index ee0b3ab..94b02a6 100644
> > > > --- a/mysql-test/r/temp_table.result
> > > > +++ b/mysql-test/r/temp_table.result
> > > > @@ -308,3 +308,185 @@ show status like 'com_drop%table';
> > > > +DROP TABLE temp_t1;
> > > > +# Cleanup
> > > > +DROP DATABASE temp_db;
> > > > +# MDEV-5535: End of tests
> > >
> > > What's that? You have lots of tests for temporary tables - not that I
> mind
> > > the number, but they are mostly unrelated to MDEV-5535. Only the one
> last
> > > test is about the new feature. I'd expect it to be the other way
> around -
> > > many tests for the new functionality and few, if at all - for the
> > > unmodified
> > > behavior.
> > >
> > These generic tests were added to cover more possible use cases as the
> > patch modified a good fraction of existing code. I have now moved
> > these unrelated tests to a separate commit and added some more tests
> > specific to MDEV-5535 in reopen_temp_table.test.
>
> Good idea. I hope your separate commit with unrelated tests is *before*
> the MDEV-5535 commit, so that one could checkout before MDEV-5535, see
> unrelated tests pass, then checkout MDEV-5535 and see how unrelated
> tests still pass.
>
Yes & yes.
>
> > > > diff --git a/mysql-test/suite/handler/handler.inc
> > > b/mysql-test/suite/handler/handler.inc
> > > > index c71dc53..ca67b62 100644
> > > > --- a/mysql-test/suite/handler/handler.inc
> > > > +++ b/mysql-test/suite/handler/handler.inc
> > > > @@ -489,7 +489,7 @@ handler t1 open as a1;
> > > > handler a1 read a=(1);
> > > > handler a1 read a next;
> > > > handler a1 read a next;
> > > > ---error ER_CANT_REOPEN_TABLE
> > > > +#--error ER_CANT_REOPEN_TABLE
> > >
> > > remove it, don't comment. And, please, fix the comment before this
> test.
> >
> > Removed. What comment are you referring to?
>
> This test in the handler.inc that you've edited. It has a comment before
> the test starts:
>
> #
> # Bug#30882 Dropping a temporary table inside a stored function may
> # cause a server crash
> #
> # Test HANDLER statements in conjunction with temporary tables. While the
> temporary table
> # is open by a HANDLER, no other statement can access it.
> #
>
> You've fixed temporary tables, so this comment is no longer true.
>
I wasn't able to find it as it was already removed. :)
> > > > select a,b from t1;
> > > > handler a1 read a prev;
> > > > handler a1 read a prev;
> > > > diff --git a/sql/temporary_tables.h b/sql/temporary_tables.h
> > > > new file mode 100644
> > > > index 000..c345bea
> > > > --- /dev/null
> > > > +++ b/sql/temporary_tables.h
> ...
> > > > +class Temporary_tables {
> ...
> > > > +private:
> > > > + THD *m_thd;
> > >
> > > Hmm, is that necessary? Temporary_tables class is instantiated in only
> one
> > > place - inside the THD. So Temporary_tables always belongs to its thd,
> and
> > > storing a point to the parent THD in the Temporary_tables you
> basically add
> > > another void* to already big THD class, while thid pointer stores no
> useful
> > > information at all.
> > >
> > > A hackish way to fix is is to replace m_thd with
> > >
> > > (((char*)this) - offsetof(THD, temporary_tables))
> > >
> > > I'm not suggesting this hack, it is just to prove that m_thd is
> redundant.
> > > I hope you'll find a nicer and safer solution :)
> >
> > How about current_thd in place of m_thd?
>
> Better not. Svoj and Monty has recently done a lot of work removing
> current_thd from many places, no need to introduce new ones.
>
Ok
>
> Besides, this simply reading THD pointer from somewhere, while, in fact,
> it's unambigously defined by 'this'.
>
> Can you use TABLE::in_use pointer?
I am not sure about it. There are some methods that use THD
without directly accessing TABLE.
Or, may be, inherit THD from
> Temporary_tables, and then thd==this :)
>
Temporary_table methods also access some of the THD data members like
rgi_slave, variables, etc.
Or that pointer arithmetics, even (as above).
>
I turned that into a macro, but its gives the following warning :
warning: invalid access to non-static data member
‘Open_tables_state::temporary_tables’ of NULL object [-Winvalid-offsetof]
#define CURRENT_THD ((THD *)(((char*)this) - offsetof(THD,
temporary_tables)))
I believe offsetof is not considered safe for non-PODs?
OTOH, I can change the class methods to accept THD * as a parameter. and
that
way we can get rid of caching this this redundant pointer. A bit ugly but
works.
Best,
Nirbhay
> Regards,
> Sergei
> Chief Architect MariaDB
> and secur...@mariadb.org
>
___
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/ListH
Re: [Maria-developers] [Commits] a6fb98f: MDEV-6368: assertion xid_seqno > trx_sys_cur_xid_seqno
Hi Jan,
Thanks for the review.
On Tue, May 3, 2016 at 3:06 AM, Jan Lindström
wrote:
> Hi Nirbhay,
>
> I'm not expert on this code area
>
I am also hoping to get some remarks from Seppo/Teemu on this.
> but some questions, comments below.
>
Addressed : http://lists.askmonty.org/pipermail/commits/2016-May/009343.html
See my comments inline.
>
> On Tue, May 3, 2016 at 1:26 AM, Nirbhay Choubey
> wrote:
>
>> revision-id: a6fb98fd74ab6f14fab0c1ef4630b010ff28880f
>> (mariadb-10.1.13-6-ga6fb98f)
>> parent(s): f8cdb9e7e8fb692f8eb3b9bb4792cd60653e0eb8
>> author: Nirbhay Choubey
>> committer: Nirbhay Choubey
>> timestamp: 2016-05-02 18:26:58 -0400
>> message:
>>
>> MDEV-6368: assertion xid_seqno > trx_sys_cur_xid_seqno
>>
>> - Validate the specified wsrep_start_position value by also
>> checking the return status of wsrep->sst_received. This also
>> ensures that changes in wsrep_start_position is not allowed
>> when the node is not in JOINING state.
>> - Do not allow decrease in seqno within same UUID.
>> - The initial checkpoint in SEs should be [0...:-1].
>>
>>
>> diff --git a/sql/wsrep_sst.cc b/sql/wsrep_sst.cc
>> index 4f6cc51..44fdd03 100644
>> --- a/sql/wsrep_sst.cc
>> +++ b/sql/wsrep_sst.cc
>> @@ -264,42 +264,91 @@ void wsrep_sst_complete (const wsrep_uuid_t*
>> sst_uuid,
>>mysql_mutex_unlock (&LOCK_wsrep_sst);
>> }
>>
>> -void wsrep_sst_received (wsrep_t*const wsrep,
>> +/*
>> + If wsrep provider is loaded, inform that the new state snapshot
>> + has been received. Also update the local checkpoint.
>> +
>> + @param wsrep [IN] wsrep handle
>> + @param uuid [IN] Initial state UUID
>> + @param seqno [IN] Initial state sequence number
>> + @param state [IN] Always NULL, also ignored by wsrep
>> provider (?)
>> + @param state_len [IN] Always 0, also ignored by wsrep
>> provider (?)
>> + @param implicit [IN] Whether invoked implicitly due to
>> SST
>> + (true) or explicitly because if
>> change
>> + in wsrep_start_position by user
>> (false).
>> + @return false Success
>> + trueError
>> +
>> +*/
>> +bool wsrep_sst_received (wsrep_t*const wsrep,
>> const wsrep_uuid_t& uuid,
>> wsrep_seqno_t const seqno,
>> const void* const state,
>> - size_t const state_len)
>>
>
> Why not const size_t state_len ? Use of const here is confusing.
>
Updated.
>
>
>> + size_t const state_len,
>> + boolconst implicit)
>> {
>> -wsrep_get_SE_checkpoint(local_uuid, local_seqno);
>> + /*
>> +To keep track of whether the local uuid:seqno should be updated.
>> Also, note
>> +that local state (uuid:seqno) is updated/checkpointed only after we
>> get an
>> +OK from wsrep provider. By doing so, the values remain consistent
>> across
>> +the server & wsrep provider.
>> + */
>> + bool do_update= false;
>> +
>> + // Get the locally stored uuid:seqno.
>> + wsrep_get_SE_checkpoint(local_uuid, local_seqno);
>>
>
> What if this fails, no error handling ?
>
Done.
>
>
>>
>> -if (memcmp(&local_uuid, &uuid, sizeof(wsrep_uuid_t)) ||
>> -local_seqno < seqno || seqno < 0)
>> + if (memcmp(&local_uuid, &uuid, sizeof(wsrep_uuid_t)) ||
>> + local_seqno < seqno)
>> + {
>> +do_update= true;
>> + }
>> + else if (local_seqno > seqno)
>> + {
>> +WSREP_WARN("SST position can't be set in past. Requested: %lld,
>> Current: "
>> + " %lld.", (long long)seqno, (long long)local_seqno);
>> +/*
>> + If we are here because of SET command, simply return true (error)
>> instead of
>> + aborting.
>> +*/
>> +if (implicit)
>> {
>> -wsrep_set_SE_checkpoint(uuid, seqno);
>> -local_uuid = uuid;
>> -local_seqno = seqno;
>> + WSREP_WARN("Can't continue.");
>> + unireg_abort(1);
>> }
>> -else if (local_seqno > seqno)
Re: [Maria-developers] f9f290b: MDEV-9851: CREATE USER w/o IDENTIFIED BY clause causes crash when using cracklib plugin
Hello Serg,
On Fri, Apr 29, 2016 at 8:20 AM, Sergei Golubchik wrote:
> Hi, Nirbhay!
>
> On Mar 31, Nirbhay Choubey wrote:
> > revision-id: f9f290b6828eeb57cba611d006d2a9301dc52244
> (mariadb-10.1.13-3-gf9f290b)
> > parent(s): f4d5fe277599da4549c97c660f324c88cf9a2542
> > author: Nirbhay Choubey
> > committer: Nirbhay Choubey
> > timestamp: 2016-03-31 18:03:44 -0400
> > message:
> >
> > MDEV-9851: CREATE USER w/o IDENTIFIED BY clause causes crash when using
> cracklib plugin
> >
> > Add a check for NULL password.
> >
> > diff --git a/plugin/cracklib_password_check/cracklib_password_check.c
> b/plugin/cracklib_password_check/cracklib_password_check.c
> > index c593173..c192cdf 100644
> > --- a/plugin/cracklib_password_check/cracklib_password_check.c
> > +++ b/plugin/cracklib_password_check/cracklib_password_check.c
> > @@ -33,7 +33,8 @@ static int crackme(MYSQL_LEX_STRING *username,
> MYSQL_LEX_STRING *password)
> >if ((host= strchr(user, '@')))
> > *host++= 0;
> >
> > - if ((res= FascistCheckUser(password->str, dictionary, user, host)))
> > + if ((password->str == NULL) ||// No password
> > + (res= FascistCheckUser(password->str, dictionary, user, host)))
> >{
> > my_printf_error(ER_NOT_VALID_PASSWORD, "cracklib: %s",
> > MYF(ME_JUST_WARNING), res);
>
> You forgot to fix the simple_password_check plugin.
simple_password_check plugin was immune indirectly because of the following
check:
if (strncmp(password->str, username->str, password->length) == 0)
return 1;
And if all plugins
> need to do the same check - it's a strong indication that this should've
> been done in the server.
>
I agree.
> So, please, fix this in sql_acl.cc instead. Like this:
>
> -struct validation_data data= { &user->user, &user->pwtext };
> +struct validation_data data= { &user->user, user->pwtext.str ?
> &user->pwtext : &empy_lex_str };
>
> Ok to push with this fix and your test case.
>
Done.
Best,
Nirbhay
> Regards,
> Sergei
> Chief Architect MariaDB
> and secur...@mariadb.org
>
___
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
Re: [Maria-developers] [Commits] ca24c9d: MDEV-9383: Server fails to read master.info after upgrade 10.0 -> 10.1
Hi Kristian! Ok to push. (see a suggestion inline) Best, Nirbhay On Thu, Apr 7, 2016 at 8:55 AM, Kristian Nielsen wrote: > Hi Nirbhay, > > Do you want to review this patch for MDEV-9383? > It concerns your code added for do_domain_ids=(...). > > - Kristian. > > Kristian Nielsen writes: > > > revision-id: ca24c9d1671e90e43daa483af32fc19891d1a646 > (mariadb-10.1.13-8-gca24c9d) > > parent(s): 4b6a3518e4dc9088d1f42cd9bc487d06137d2760 > > committer: Kristian Nielsen > > timestamp: 2016-04-07 14:44:29 +0200 > > message: > > > > MDEV-9383: Server fails to read master.info after upgrade 10.0 -> 10.1 > > > > In some cases, MariaDB 10.0 could write a master.info file that was read > > incorrectly by 10.1 and could cause server to fail to start after an > upgrade. > > > > (If writing a new master.info file that is shorter than the old, extra > > junk may remain at the end of the file. This is handled properly in > > 10.1 with an END_MARKER line, but this line is not written by > > 10.0. The fix here is to make 10.1 robust at reading the master.info > > files written by 10.0). > > > > Fix several things around reading master.info and > read_mi_key_from_file(): > > > > - read_mi_key_from_file() did not distinguish between a line with and > >without an eqals '=' sign. > > > > - If a line was empty, read_mi_key_from_file() would incorrectly return > >the key from the previous call. > > > > - An extra using_gtid=X line left-over by MariaDB 10.0 might incorrectly > >be read and overwrite the correct value. > > > > - Fix incorrect usage of strncmp() which should be strcmp(). > > > > - Add test cases. > > > > --- > > mysql-test/std_data/bad2_master.info | 35 + > > mysql-test/std_data/bad3_master.info | 37 + > > mysql-test/std_data/bad4_master.info | 35 + > > mysql-test/std_data/bad5_master.info | 35 + > > mysql-test/std_data/bad6_master.info | 36 + > > mysql-test/std_data/bad_master.info| 35 + > > .../suite/rpl/r/rpl_upgrade_master_info.result | 88 +++ > > .../suite/rpl/t/rpl_upgrade_master_info.test | 163 > + > > sql/rpl_mi.cc | 81 +- > > 9 files changed, 512 insertions(+), 33 deletions(-) > > > > diff --git a/mysql-test/std_data/bad2_master.info b/mysql-test/std_data/ > bad2_master.info > > new file mode 100644 > > index 000..6172256 > > --- /dev/null > > +++ b/mysql-test/std_data/bad2_master.info > > @@ -0,0 +1,35 @@ > > +33 > > +mysql-bin.01 > > +4 > > +127.0.0.1 > > +root > > + > > +3310 > > +60 > > +0 > > + > > + > > + > > + > > + > > +0 > > +1800.000 > > + > > +0 > > + > > +0 > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > +using_gtid=1 > > +=0 > > diff --git a/mysql-test/std_data/bad3_master.info b/mysql-test/std_data/ > bad3_master.info > > new file mode 100644 > > index 000..6e632cd > > --- /dev/null > > +++ b/mysql-test/std_data/bad3_master.info > > @@ -0,0 +1,37 @@ > > +33 > > +mysql-bin.01 > > +4 > > +127.0.0.1 > > +root > > + > > +3310 > > +60 > > +0 > > + > > + > > + > > + > > + > > +0 > > +1800.000 > > + > > +0 > > + > > +0 > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > +using_gtid=1 > > + > > + > > +0 > > diff --git a/mysql-test/std_data/bad4_master.info b/mysql-test/std_data/ > bad4_master.info > > new file mode 100644 > > index 000..87572ef > > --- /dev/null > > +++ b/mysql-test/std_data/bad4_master.info > > @@ -0,0 +1,35 @@ > > +33 > > +mysql-bin.01 > > +4 > > +127.0.0.1 > > +root > > + > > +3310 > > +60 > > +0 > > + > > + > > + > > + > > + > > +0 > > +1800.000 > > + > > +0 > > + > > +0 > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > +using_gtid=1 > > +d=1 > > diff --git a/mysql-test/std_data/bad5_master.info b/mysql-test/std_data/ > bad5_master.info > > new file mode 100644 > > index 000..4ea8113 > > --- /dev/null > > +++ b/mysql-test/std_data/bad5_master.info > > @@ -0,0 +1,35 @@ > > +33 > > +mysql-bin.01 > > +4 > > +127.0.0.1 > > +root > > + > > +3310 > > +60 > > +0 > > + > > + > > + > > + > > + > > +0 > > +1800.000 > > + > > +0 > > + > > +0 > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > +using_gtid=1 > > +using_gtid > > diff --git a/mysql-test/std_data/bad6_master.info b/mysql-test/std_data/ > bad6_master.info > > new file mode 100644 > > index 000..0f48f48 > > --- /dev/null > > +++ b/mysql-test/std_data/bad6_master.info > > @@ -0,0 +1,36 @@ > > +33 > > +mysql-bin.01 > > +4 > > +127.0.0.1 > > +root > > + > > +3310 > > +60 > > +0 > > + > > + > > + > > + > > + > > +0 > > +1800.000 > > + > > +0 > > + > > +0 > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > +using_gtid=1 > > +END_MARKER > > +do_domain_ids=20 Hulubulu!!?! > > diff --git a/mysql-test/std_data/bad_master
Re: [Maria-developers] bf2eaa7: MDEV-9382: After updating mariadb server apt-configure fails
Hi Serg,
On Wed, Mar 16, 2016 at 3:02 PM, Sergei Golubchik wrote:
.. cut..
> diff --git a/debian/mariadb-server-10.1.mysql.init
> b/debian/mariadb-server-10.1.mysql.init
> > index 9e098b4..005d0e8 100644
> > --- a/debian/mariadb-server-10.1.mysql.init
> > +++ b/debian/mariadb-server-10.1.mysql.init
> > @@ -109,7 +109,7 @@ case "${1:-''}" in
> > /usr/bin/mysqld_safe "${@:2}" > /dev/null 2>&1 &
> >
> > # 6s was reported in #352070 to be too little
> > - for i in $(seq 1 "${MYSQLD_STARTUP_TIMEOUT:-30}"); do
> > + for i in $(seq 1 "${MYSQLD_STARTUP_TIMEOUT:-60}"); do
> > sleep 1
>
> This doesn't fix much. Next time somebody will need 61 second and you'll
> adjust to 120? SST might take many minutes and it's not a good idea to
> have every user wait this long.
>
> Instead you should do what MDEV-8509 says. Add a line like
>
> [ -r /etc/default/mariadb ] && . /etc/default/mariadb
>
> then one can have a very personal setting of MYSQLD_STARTUP_TIMEOUT
> in /etc/default/mariadb
>
I was also keen on changing the default 30 secs limit to some higher value
like
60 or 90 secs, so that it works for more installations by default.
I will re-commit one.
Best,
Nirbhay
>
> Regards,
> Sergei
> Chief Architect MariaDB
> and secur...@mariadb.org
>
___
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
Re: [Maria-developers] bf2eaa7: MDEV-9382: After updating mariadb server apt-configure fails
Hi Jean, On Wed, Mar 16, 2016 at 4:48 PM, Jean Weisbuch wrote: > I forgot to create the PR for upstream as i did for Otto's repo : > https://github.com/ottok/mariadb-10.0/pull/26 and > https://github.com/ottok/mariadb-10.0/pull/27 > These changes are already in Debian testing packages, should i create the > PR or you will handle it on your side? > I have just committed a patch. Best, Nirbhay > > Le 16/03/2016 20:02, Sergei Golubchik a écrit : > >> This doesn't fix much. Next time somebody will need 61 second and you'll >> adjust to 120? SST might take many minutes and it's not a good idea to >> have every user wait this long. >> >> Instead you should do what MDEV-8509 says. Add a line like >> >>[ -r /etc/default/mariadb ] && . /etc/default/mariadb >> >> then one can have a very personal setting of MYSQLD_STARTUP_TIMEOUT >> in /etc/default/mariadb >> > > ___ > 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 > ___ 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
Re: [Maria-developers] [Commits] 096fd8f: MDEV-9033 : Incorrect statements binlogged on slave with do_domain_ids=(...)
Hi Kristian, Can you please review this patch? - Nirbhay On Fri, Nov 13, 2015 at 5:23 PM, Nirbhay Choubey wrote: > revision-id: 096fd8f57664d0b3061f0e781a31bfa3cc6bf6cd > parent(s): a574407444fc3ea93ca88afa93dc18154251bf74 > committer: Nirbhay Choubey > branch nick: 10.1.b9033 > timestamp: 2015-11-13 17:23:49 -0500 > message: > > MDEV-9033 : Incorrect statements binlogged on slave with > do_domain_ids=(...) > > In domain ID based filtering, a flag is used to filter-out > the events that belong to a particular domain. This flag gets > set when IO thread receives a GTID_EVENT for the domain on > filter list and its reset at the last event in the GTID group. > The resetting, however, was wrongly done before the decision to > write/filter the event from relay log is made. As a result, the > last event in the group will always pass through the filter. > Fixed by deferring the reset logic. Also added a test case. > > --- > mysql-test/suite/rpl/r/rpl_gtid_mdev9033.result | 73 > mysql-test/suite/rpl/t/rpl_gtid_mdev9033.cnf| 20 ++ > mysql-test/suite/rpl/t/rpl_gtid_mdev9033.test | 92 > + > sql/slave.cc| 19 + > 4 files changed, 188 insertions(+), 16 deletions(-) > > diff --git a/mysql-test/suite/rpl/r/rpl_gtid_mdev9033.result > b/mysql-test/suite/rpl/r/rpl_gtid_mdev9033.result > new file mode 100644 > index 000..fdee91f > --- /dev/null > +++ b/mysql-test/suite/rpl/r/rpl_gtid_mdev9033.result > @@ -0,0 +1,73 @@ > +include/rpl_init.inc [topology=1->2->3] > +include/stop_slave.inc > +CHANGE MASTER TO > +MASTER_USE_GTID = SLAVE_POS, IGNORE_SERVER_IDS = (2,3); > +CHANGE MASTER "M_3" TO > +MASTER_HOST = "127.0.0.1", MASTER_PORT = SERVER_MYPORT_3, > +MASTER_USER = "root", MASTER_USE_GTID = SLAVE_POS, > +IGNORE_SERVER_IDS = (1,2); > +START ALL SLAVES; > +Warnings: > +Note 1937SLAVE 'M_3' started > +Note 1937SLAVE '' started > +include/stop_slave.inc > +CHANGE MASTER "M_1" TO > +MASTER_HOST = "127.0.0.1", MASTER_PORT = MASTER_MYPORT, > +MASTER_USER = "root", MASTER_USE_GTID = SLAVE_POS, > +IGNORE_SERVER_IDS = (2,3); > +CHANGE MASTER TO > +MASTER_USE_GTID = SLAVE_POS, IGNORE_SERVER_IDS = (1,3); > +START ALL SLAVES; > +Warnings: > +Note 1937SLAVE 'M_1' started > +Note 1937SLAVE '' started > +CHANGE MASTER "M_2" TO > +MASTER_HOST = "127.0.0.1", MASTER_PORT = SLAVE_MYPORT, > +MASTER_USER = "root", MASTER_USE_GTID = SLAVE_POS, > +IGNORE_SERVER_IDS = (1,3); > +CHANGE MASTER "M_3" TO > +MASTER_HOST = "127.0.0.1", MASTER_PORT = SERVER_MYPORT_3, > +MASTER_USER = "root", MASTER_USE_GTID = SLAVE_POS, > +IGNORE_SERVER_IDS = (1,2); > +START ALL SLAVES; > +Warnings: > +Note 1937SLAVE 'M_3' started > +Note 1937SLAVE 'M_2' started > +CREATE TABLE t1 (a INT PRIMARY KEY, b VARCHAR(10)) ENGINE=InnoDB; > +INSERT INTO t1 VALUES (1, "m1"); > +INSERT INTO t1 VALUES (2, "m2"), (3, "m3"), (4, "m4"); > +include/save_master_gtid.inc > +include/sync_with_master_gtid.inc > +SELECT * FROM t1 ORDER BY a; > +a b > +1 m1 > +2 m2 > +3 m3 > +4 m4 > +include/sync_with_master_gtid.inc > +SELECT * FROM t1 ORDER BY a; > +a b > +1 m1 > +2 m2 > +3 m3 > +4 m4 > +# Cleanup > +DROP TABLE t1; > +include/save_master_gtid.inc > +include/sync_with_master_gtid.inc > +include/sync_with_master_gtid.inc > +STOP SLAVE "M_3"; > +RESET SLAVE "M_3" ALL; > +STOP SLAVE "M_1"; > +RESET SLAVE "M_1" ALL; > +STOP SLAVE "M_2"; > +RESET SLAVE "M_2" ALL; > +STOP SLAVE "M_3"; > +RESET SLAVE "M_3" ALL; > +include/sync_with_master_gtid.inc > +STOP SLAVE; > +CHANGE MASTER TO MASTER_USE_GTID = NO, IGNORE_SERVER_IDS = (); > +include/sync_with_master_gtid.inc > +STOP SLAVE; > +CHANGE MASTER TO MASTER_USE_GTID = NO, IGNORE_SERVER_IDS = (); > +# End of test. > diff --git a/mysql-test/suite/rpl/t/rpl_gtid_mdev9033.cnf > b/mysql-test/suite/rpl/t/rpl_gtid_mdev9033.cnf > new file mode 100644 > index 000..80b6649 > --- /dev/null > +++ b/mysql-test/suite/rpl/t/rpl_gtid_mdev9033.cnf > @@ -0,0 +1,20 @@ > +!include ../my.cnf > + > +[mysqld.1] > +log-slave-updates > +loose-innodb > +gtid_domain_id=1 > + > +[mysqld.2] > +log-slave-updates > +loose-innodb > +gtid_domain_id=2 > + > +[mysqld.3] > +log-slave-updates > +loose-innodb >
Re: [Maria-developers] [Commits] 4685c90: mari...@.service.in should be configured for all systemd platforms.
Hi Svoj!
On Wed, Nov 4, 2015 at 1:54 AM, Sergey Vojtovich wrote:
> Hi Nirbhay,
>
> taking into account that CPackRPM won't be able to install it,
I think it does not apply to all rpm platforms, but ones with cmake
affected by
http://public.kitware.com/Bug/view.php?id=14782.
what's the purpose of having this configured for all systemd platforms?
>
It should be ok for other platforms.
Best,
Nirbhay
>
> Regards,
> Sergey
>
> On Tue, Nov 03, 2015 at 05:38:12PM -0500, Nirbhay Choubey wrote:
> > revision-id: 4685c903bc822596bcf12cbfdb5bcc4460ed0105
> > parent(s): 95289e5b665d6a3e37f938d0db978d0d0166e493
> > committer: Nirbhay Choubey
> > branch nick: 10.1
> > timestamp: 2015-11-03 17:38:11 -0500
> > message:
> >
> > mari...@.service.in should be configured for all systemd platforms.
> >
> > ---
> > support-files/CMakeLists.txt | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/support-files/CMakeLists.txt b/support-files/CMakeLists.txt
> > index 2ab4350..2929db1 100644
> > --- a/support-files/CMakeLists.txt
> > +++ b/support-files/CMakeLists.txt
> > @@ -79,9 +79,9 @@ IF(UNIX)
> >IF(HAVE_SYSTEMD)
> > CONFIGURE_FILE(mariadb.service.in
> > ${CMAKE_CURRENT_BINARY_DIR}/mariadb.service @ONLY)
> > +CONFIGURE_FILE(mari...@.service.in
> > + ${CMAKE_CURRENT_BINARY_DIR}/mariadb@.service @ONLY)
> > IF(NOT RPM)
> > - CONFIGURE_FILE(mari...@.service.in
> > - ${CMAKE_CURRENT_BINARY_DIR}/mariadb@.service
> @ONLY)
> >INSTALL(FILES use_galera_new_cluster.conf
> >${CMAKE_CURRENT_BINARY_DIR}/mariadb.service
> >${CMAKE_CURRENT_BINARY_DIR}/mariadb@.service
> > ___
> > commits mailing list
> > comm...@mariadb.org
> > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
> ___
> commits mailing list
> comm...@mariadb.org
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
___
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
Re: [Maria-developers] Problem in compiling Maria-db
Hi Vaibhav, On Fri, Oct 2, 2015 at 11:46 AM, indiavaibhav vaibhavindia < vaibhav.sanskr...@gmail.com> wrote: > Thanks Nirbhay for your guidance. > I just have one more question to ask,when I write any patch for any issue > on Jira,i would have to compile the entire source > with cmake again to test the patch right? > Right. After you are done with your changes, you have to make sure that the source builds fine. You do no have to run "cmake" every time though. cmake generates makefiles for your project, and once the makefiles are in place (after the first cmake run), all you need is to run "make".. that's it. I really appreciate your help. > You are welcome. All the best. -- Nirbhay > > Thanks, > Vaibhav Choudhary > > On Thu, Oct 1, 2015 at 10:29 PM, Nirbhay Choubey > wrote: > >> Hi! >> >> On Thu, Oct 1, 2015 at 12:46 PM, indiavaibhav vaibhavindia < >> vaibhav.sanskr...@gmail.com> wrote: >> >>> Hello everyone, >>> My name is Vaibhav.I am new to programming for open source. >>> I am facing problems in trying to compile Maria-Db.As instructed on the >>> website, I cloned the maria db source and installed all the files required >>> for compilation. >>> >>> However on using cmake,I am getting an error,screenshot of which is >>> attached is with this mail. >>> >>> >>> Can someone please help me out? >>> >> >> Looks like you are not running cmake from the correct directory? >> >> Did you follow this : >> https://mariadb.com/kb/en/mariadb/generic-build-instructions/ ? >> >> You can also build out-of-source to keep the tree clean : >> >> cd src-dir >> mkdir bld >> cd bld >> cmake .. >> >> Best, >> Nirbhay >> >> >>> >>> Thanks, >>> Vaibhav Choudhary >>> >>> >>> ___ >>> 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 >>> >>> >> > ___ 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
Re: [Maria-developers] Problem in compiling Maria-db
Hi! On Thu, Oct 1, 2015 at 12:46 PM, indiavaibhav vaibhavindia < vaibhav.sanskr...@gmail.com> wrote: > Hello everyone, > My name is Vaibhav.I am new to programming for open source. > I am facing problems in trying to compile Maria-Db.As instructed on the > website, I cloned the maria db source and installed all the files required > for compilation. > > However on using cmake,I am getting an error,screenshot of which is > attached is with this mail. > > > Can someone please help me out? > Looks like you are not running cmake from the correct directory? Did you follow this : https://mariadb.com/kb/en/mariadb/generic-build-instructions/ ? You can also build out-of-source to keep the tree clean : cd src-dir mkdir bld cd bld cmake .. Best, Nirbhay > > Thanks, > Vaibhav Choudhary > > > ___ > 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 > > ___ 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
[Maria-developers] MDEV-8831: review request
Hi Jan,
Could you please review the attached patch?
Thanks,
Nirbhay
From f9bf5f50593a181effb0cde9d18c1f0fd5c662ff Mon Sep 17 00:00:00 2001
From: Nirbhay Choubey
Date: Wed, 30 Sep 2015 21:56:50 -0400
Subject: [PATCH] MDEV-8831 : enforce_storage_engine doesn't block table
creation on other nodes
Check if the engine is supported/allowed before replicating the
statement.
---
.../suite/galera/r/enforce_storage_engine.result | 22 +++
.../suite/galera/t/enforce_storage_engine.test | 33 ++
sql/sql_parse.cc | 13 ++---
sql/sql_table.cc | 5 ++--
sql/sql_table.h| 2 ++
5 files changed, 68 insertions(+), 7 deletions(-)
create mode 100644 mysql-test/suite/galera/r/enforce_storage_engine.result
create mode 100644 mysql-test/suite/galera/t/enforce_storage_engine.test
diff --git a/mysql-test/suite/galera/r/enforce_storage_engine.result b/mysql-test/suite/galera/r/enforce_storage_engine.result
new file mode 100644
index 000..a3513fc
--- /dev/null
+++ b/mysql-test/suite/galera/r/enforce_storage_engine.result
@@ -0,0 +1,22 @@
+#
+# MDEV-8831 : enforce_storage_engine doesn't block table creation on
+# other nodes (galera cluster)
+#
+SET @@enforce_storage_engine=INNODB;
+CREATE TABLE t1(i INT) ENGINE=INNODB;
+CREATE TABLE t2(i INT) ENGINE=MYISAM;
+ERROR 42000: Unknown storage engine 'MyISAM'
+INSERT INTO t1 VALUES(1);
+SHOW TABLES;
+Tables_in_test
+t1
+SELECT COUNT(*)=1 FROM t1;
+COUNT(*)=1
+1
+CREATE TABLE t2(i INT) ENGINE=MYISAM;
+SHOW TABLES;
+Tables_in_test
+t1
+t2
+DROP TABLE t1, t2;
+# End of tests
diff --git a/mysql-test/suite/galera/t/enforce_storage_engine.test b/mysql-test/suite/galera/t/enforce_storage_engine.test
new file mode 100644
index 000..5f07dd5
--- /dev/null
+++ b/mysql-test/suite/galera/t/enforce_storage_engine.test
@@ -0,0 +1,33 @@
+--source include/galera_cluster.inc
+--source include/have_innodb.inc
+
+# enforce_storage_engine should prevent the creation of tables with
+# non-enforced storage engines on the master node and the command
+# should also not replicate to other nodes.
+
+--echo #
+--echo # MDEV-8831 : enforce_storage_engine doesn't block table creation on
+--echo # other nodes (galera cluster)
+--echo #
+
+--connection node_1
+SET @@enforce_storage_engine=INNODB;
+CREATE TABLE t1(i INT) ENGINE=INNODB;
+--error ER_UNKNOWN_STORAGE_ENGINE
+CREATE TABLE t2(i INT) ENGINE=MYISAM;
+
+INSERT INTO t1 VALUES(1);
+
+--connection node_2
+SHOW TABLES;
+SELECT COUNT(*)=1 FROM t1;
+
+CREATE TABLE t2(i INT) ENGINE=MYISAM;
+
+--connection node_1
+SHOW TABLES;
+
+# Cleanup
+DROP TABLE t1, t2;
+
+--echo # End of tests
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index 2141e68..9c6f7c0 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -3440,11 +3440,16 @@ static bool do_execute_sp(THD *thd, sp_head *sp)
}
else
{
-/* in STATEMENT format, we probably have to replicate also temporary
- tables, like mysql replication does
+/*
+ In STATEMENT format, we probably have to replicate also temporary
+ tables, like mysql replication does. Also check if the requested
+ engine is allowed/supported.
*/
-if (WSREP(thd) && (!thd->is_current_stmt_binlog_format_row() ||
-!create_info.tmp_table()))
+if (WSREP(thd) &&
+!check_engine(thd, create_table->db, create_table->table_name,
+ &create_info) &&
+(!thd->is_current_stmt_binlog_format_row() ||
+ !create_info.tmp_table()))
{
WSREP_TO_ISOLATION_BEGIN(create_table->db, create_table->table_name, NULL)
}
diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index d3a7253..b05bdc9 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -72,7 +72,6 @@ static int copy_data_between_tables(THD *thd, TABLE *from,TABLE *to,
Alter_table_ctx *alter_ctx);
static bool prepare_blob_field(THD *thd, Create_field *sql_field);
-static bool check_engine(THD *, const char *, const char *, HA_CREATE_INFO *);
static int mysql_prepare_create_table(THD *, HA_CREATE_INFO *, Alter_info *,
uint *, handler *, KEY **, uint *, int);
static uint blob_length_by_type(enum_field_types type);
@@ -9816,8 +9815,8 @@ bool mysql_checksum_table(THD *thd, TABLE_LIST *tables,
@retval true Engine not available/supported, error has been reported.
@retval false Engine available/supported.
*/
-static bool check_engine(THD *thd, const char *db_name,
- const char *table_name, HA_CREATE_INFO *create_info)
+bool check_engine(THD *thd, const char *db_name,
+ const char *table_name, HA_CREATE_INFO *create_info)
{
DBUG_ENTER("check_engine");
handlerton **
Re: [Maria-developers] [Commits] 5f461a9: MDEV-7640: CHANGE MASTER TO doesn't work with prepared statements
Hi Kristian! On Mon, Sep 21, 2015 at 4:42 AM, Kristian Nielsen wrote: > nirb...@mariadb.com (Nirbhay Choubey) writes: > > > revision-id: 5f461a94a5aa2e1a20fa559ca612d96b53201e4e > > parent(s): 29ac245dd04c5416d57a90b0ab3c4d08cbc4d723 > > committer: Nirbhay Choubey > > branch nick: 5.5 > > timestamp: 2015-09-16 18:10:06 -0400 > > message: > > > > MDEV-7640: CHANGE MASTER TO doesn't work with prepared statements > > > > When CHANGE MASTER was executed as a PS, its attributes were wrongly > > getting reset toward the end of PREPARE. As a result, the subsequent > > executions had no effect. Fixed by making sure that the CHANGE MASTER > > attributes are preserved during the lifetime of the PS. > > Hm, this MDEV seems now assigned to me, but I did not see any mail from you > about it? Did you expect a review? > That's the process I normally follow. I add a reference of the committed patch to the MDEV, use JIRA's "Request Review" and expect it to send out an email for the same. > But in any case, this code around prepare/execute is not something I am > familiar with. I seem to recall that Sanja worked on several issues related > to incorrect prepare/execute, maybe he can have some ideas or suggest a > reviewer. > Ok. @Sanja : Could you please review this bug? > Apparently this is something that used to work but started to break > sometimes in 5.5. Do you know how the problem was introduced? > It got introduced by a memory leak fix in 5.5 back in July, 2011. Best, Nirbhay > - Kristian (sadder by the hour over devs not using the mailing list). > ___ 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
Re: [Maria-developers] [Commits] 8bf23ef: MDEV-8208: Sporadic SEGFAULT on startup
Hi Serg,
Thanks for the review. My comments inline.
On Sat, Sep 12, 2015 at 7:12 AM, Sergei Golubchik wrote:
> Hi, Nirbhay!
>
> On Aug 19, Nirbhay Choubey wrote:
> > revision-id: 8bf23efc914f18c687067e9314cadf4f36a85eec
> > parent(s): 71d1f35847a575239deff856590bf6f13afd74ed
> > committer: Nirbhay Choubey
> > branch nick: 5.5-galera.b8208
> > timestamp: 2015-08-19 17:17:56 -0400
> > message:
> >
> > MDEV-8208: Sporadic SEGFAULT on startup
>
> A couple of questions:
>
> > diff --git a/sql/mysqld.cc b/sql/mysqld.cc
> > index 2008fc5..4c05999 100644
> > --- a/sql/mysqld.cc
> > +++ b/sql/mysqld.cc
> > @@ -4584,6 +4579,29 @@ static int init_server_components()
> >}
> >plugins_are_initialized= TRUE; /* Don't separate from init function
> */
> >
> > +#ifdef WITH_WSREP
> > + /* Wait for wsrep threads to get created. */
> > + if (wsrep_creating_startup_threads == 1) {
> > +mysql_mutex_lock(&LOCK_thread_count);
> > +while (wsrep_running_threads < 2)
> > +{
> > + mysql_cond_wait(&COND_thread_count, &LOCK_thread_count);
> > +}
> > +
> > +/* Now is the time to initialize threads for queries. */
> > +THD *tmp;
> > +I_List_iterator it(threads);
> > +while ((tmp= it++))
> > + if (tmp->wsrep_applier == true)
> > +tmp->init_for_queries();
>
> Do you really need to iterate over all THD's? There is only one applier
> thread, right? And it's THD is not stored anywhere, only in the global
> list?
>
2 actually : applier and replayer.
>
> > +mysql_mutex_unlock(&LOCK_thread_count);
> > + }
> > +#endif
> > +
> >have_csv= plugin_status(STRING_WITH_LEN("csv"),
> >MYSQL_STORAGE_ENGINE_PLUGIN);
> >have_ndbcluster= plugin_status(STRING_WITH_LEN("ndbcluster"),
> > @@ -4890,12 +4902,21 @@ pthread_handler_t start_wsrep_THD(void *arg)
> >//thd->version= refresh_version;
> >thd->proc_info= 0;
> >thd->command= COM_SLEEP;
> > - thd->set_time();
> > - thd->init_for_queries();
> > +
> > + if (plugins_are_initialized)
> > + {
> > +thd->init_for_queries();
> > + }
>
> What happens if plugins are not initialized yet?
>
In that case we do init_for_queries() only after plugins gets initialized.
BTW, this can only
happen for the initial 2 wsrep thds. As they are created before the
initialization of plugins.
For other wsrep threads, requested using wsrep_slave_threads, they get
created only
after plugins are initialized.
> thd->init_for_queries() will be done in that THD loop above?
> In that case why do you need to do it here?
>
For wsrep threads requested via wsrep_slave_threads.
The clarify the issue further, here is roughly what happens on the server
start :
( http://pastie.org/10362496 -> In case the below diagram gets mangled by
the mail client)
main()
|
|- wsrep_init_startup()
\ |
\ |
| |
| |- pthread_create(start_wsrep_THD()) <-- applier
| |- pthread_create(start_wsrep_THD()) <-- rollbacker
|
| <-- main thread : wait (COND_wsrep_sst)
/
/ <-- applier thread: signal(COND_wsrep_sst)
/
| (while the applier/rollbacker threads get initialized, applier
later signals COND_wsrep_sst
| post SST and main thread of execution continues. Now, the segfault
can occur when
| the rollbacker thread is still under thd->init_for_queries trying
to access maria_thon
| while its only partially initialized (e.g. maria_hton != 0 but its
other members like slot
| are not initialized) under plugin_init().
|
|- main thread: plugin_init() || rollbacker thd : THD::init_for_queries()
-> ha_maria::implicit_commit
Best,
Nirbhay
>
> Regards,
> Sergei
>
___
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
[Maria-developers] Using oqgraph/connect engines in mariadb-galera-server
Hi Serg, Svoj! I am looking into an issue where accessing oqgraph/connect engines from galera server leads to errors like "Unknown option ''" and segfaults. The issue being the differences in handlerton structure in galera and non-galera server codes. Since the connect/oqgraph packages are built from non-galera server, when they are loaded into galera server, the engine's handlerton gets improperly initialized. https://mariadb.atlassian.net/browse/MDEV-8240 I can think of the following potential solutions : a) Club connect/oqgraph and mariadb-galera-server into a single package. (this would mean adding new dependencies to mariadb-galera-server) b) New packages for connect/oqgraph out of mariadb-galera build (mariadb-connect-engine-galera-10.0, mariadb-oqgraph-engine-galera-10.0) Do you see any other option? Also, we should add some checks for plugin load to fail in first place in such scenarios. Best, Nirbhay ___ 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
Re: [Maria-developers] [Commits] aff5606: Add close-on-exec flag to open(), socket(), accept() & fopen().
Hi Serg,
On Wed, Jun 24, 2015 at 2:59 PM, Sergei Golubchik wrote:
> Hi, Nirbhay!
>
> On Jun 24, Nirbhay Choubey wrote:
> > revision-id: aff560696cac5e081d19f4460ccd9c906ddc9d84
> > parent(s): 70714d3597ec9ffa742cc9e173d4ad32f294cc51
> > committer: Nirbhay Choubey
> > branch nick: 10.0-galera.b7631
> > timestamp: 2015-06-24 11:58:11 -0400
> > message:
> >
> > Add close-on-exec flag to open(), socket(), accept() & fopen().
> >
> > @@ -1026,7 +1026,8 @@ static inline void inline_mysql_socket_register(
> >(&state, socket_listen.m_psi, PSI_SOCKET_CONNECT, (size_t)0,
> src_file, src_line);
> >
> > /* Instrumented code */
> > -socket_accept.fd= accept(socket_listen.fd, addr, &addr_length);
> > +socket_accept.fd= accept4(socket_listen.fd, addr, &addr_length,
> > + SOCK_CLOEXEC);
>
> Note that, according to the manpage, "accept4() is a nonstandard Linux
> extension".
>
> It needs a configure-time check and ifdef. Alternatively, don't use it
> at all, do fcntl(FD_CLOEXEC) instead.
>
Right. I have added a compile-time check for accept4() so that it can be
used if available,
accept()/fcntl() will be used otherwise.
Thanks!
-- Nirbhay
>
> > /* Instrumentation end */
> > if (locker != NULL)
> > @@ -1036,7 +1037,8 @@ static inline void inline_mysql_socket_register(
> > #endif
> >{
> > /* Non instrumented code */
> > -socket_accept.fd= accept(socket_listen.fd, addr, &addr_length);
> > +socket_accept.fd= accept4(socket_listen.fd, addr, &addr_length,
> > + SOCK_CLOEXEC);
> >}
>
> Regards,
> Sergei
>
___
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
Re: [Maria-developers] [Commits] 6050ab6: MDEV-6829 : SELinux/AppArmor policies for Galera server
Hi Daniel!
On Fri, Jun 19, 2015 at 2:11 AM, Daniel Black wrote:
> Nice work.
>
> https://mariadb.atlassian.net/browse/MDEV-7637 has some
> netlink_audit_socket rules that don't appear to be here.
>
No, I did not try PAM.
> Recommend contributing the selinux component to
> https://github.com/TresysTechnology/refpolicy which distros usually
> develop their policies from.
>
Sure, that's a good idea. I will wait for sometime for the policies to
stabilize and then open a pull request.
There are some version specific changes that we need to sort out. For
instance, tram_port_t (tcp/4567)
is defined in CentOS 7.0 and not in Centos 6.5. And similar stuff.
> Does this work for galera multicast? It appears to only allow tcp bind
> here.
>
No it didn't. :) I have a patch ready for this now.
>
> note for readme semanage permissive -a mysqld_t - less of a change for
> enabling just that domain to be permissive.
>
Yep, I have updated the README.
>
> Does any of
> https://mariadb.com/kb/en/mariadb/what-to-do-if-mariadb-doesnt-start/
> need changing?
>
It looks good, don't think we need to update it to reflect any change
related to this.
Thanks!
-- Nirbhay
>
>
> - On 18 Jun, 2015, at 11:59 PM, Nirbhay Choubey nirb...@mariadb.com
> wrote:
>
> > revision-id: 6050ab658696925f2a031b901eb398fff65fa92a
> > parent(s): 9eff9ed5c58e782abf383a52a7e691a55b4798a2
> > committer: Nirbhay Choubey
> > branch nick: 5.5-galera
> > timestamp: 2015-06-18 09:59:09 -0400
> > message:
> >
> > MDEV-6829 : SELinux/AppArmor policies for Galera server
> >
> > Add SELinux policy and AppArmor profile under policy/.
> >
> > ---
> > policy/apparmor/README| 5 ++
> > policy/apparmor/usr.sbin.mysqld | 150
> ++
> > policy/apparmor/usr.sbin.mysqld.local | 4 +
> > policy/selinux/README | 18
> > policy/selinux/mariadb-server.fc | 10 +++
> > policy/selinux/mariadb-server.te | 91 +
> > 6 files changed, 278 insertions(+)
> >
> > diff --git a/policy/apparmor/README b/policy/apparmor/README
> > new file mode 100644
> > index 000..271655f
> > --- /dev/null
> > +++ b/policy/apparmor/README
> > @@ -0,0 +1,5 @@
> > +Note: The included AppArmor profiles can be used for MariaDB Galera
> cluster.
> > +However, since these profiles had been tested for a limited set of
> scenarios,
> > +it is highly recommended to run them in "complain" mode and report any
> denials
> > +on mariadb.org/jira.
> > +
> > diff --git a/policy/apparmor/usr.sbin.mysqld
> b/policy/apparmor/usr.sbin.mysqld
> > new file mode 100644
> > index 000..307872c
> > --- /dev/null
> > +++ b/policy/apparmor/usr.sbin.mysqld
> > @@ -0,0 +1,150 @@
> > +# Last Modified: Fri Mar 1 18:55:47 2013
> > +# Based on usr.sbin.mysqld packaged in mysql-server in Ubuntu.
> > +# This AppArmor profile has been copied under BSD License from
> > +# Percona XtraDB Cluster, along with some additions.
> > +
> > +#include
> > +
> > +/usr/sbin/mysqld flags=(complain) {
> > + #include
> > + #include
> > + #include
> > + #include
> > + #include
> > +
> > + capability chown,
> > + capability dac_override,
> > + capability setgid,
> > + capability setuid,
> > + capability sys_rawio,
> > + capability sys_resource,
> > +
> > + network tcp,
> > +
> > + /bin/dash rcx,
> > + /dev/dm-0 r,
> > + /etc/gai.conf r,
> > + /etc/group r,
> > + /etc/hosts.allow r,
> > + /etc/hosts.deny r,
> > + /etc/ld.so.cache r,
> > + /etc/mtab r,
> > + /etc/my.cnf r,
> > + /etc/mysql/*.cnf r,
> > + /etc/mysql/*.pem r,
> > + /etc/mysql/conf.d/ r,
> > + /etc/mysql/conf.d/* r,
> > + /etc/nsswitch.conf r,
> > + /etc/passwd r,
> > + /etc/services r,
> > + /run/mysqld/mysqld.pid w,
> > + /run/mysqld/mysqld.sock w,
> > + /sys/devices/system/cpu/ r,
> > + owner /tmp/** lk,
> > + /tmp/** rw,
> > + /usr/lib/mysql/plugin/ r,
> > + /usr/lib/mysql/plugin/*.so* mr,
> > + /usr/sbin/mysqld mr,
> > + /usr/share/mysql/** r,
> > + /var/lib/mysql/ r,
> > + /var/lib/mysql/** rwk,
> > + /var/log/mysql.err rw,
> > + /var/log/mysql.log rw,
> > + /var/log/mysql/ r,
> > + /var/log/mysql/* rw,
> > + /var/run/mysqld/mysqld.pid w,
> > + /var/run/mysqld/mysqld.sock w,
> > +
> > +
> > + profile /bin/dash flags=(com
Re: [Maria-developers] Can't find messagefile '/usr/share/mysql/errmsg.sys'
Hi! On Wed, Jun 10, 2015 at 5:40 AM, Sylvain Raybaud < sylvain.rayb...@green-communications.fr> wrote: > > I built mariadb galera cluster 10.0.17 from source. Starting it with > mysql_safe fails with: Can't find messagefile > '/usr/share/mysql/errmsg.sys' > Did you try setting --basedir= ? > This file does not exist indeed. It seems to not exist at all in > mariadb-galera-10.0.17.tar.gz. Is it supposed to be installed > separately? Am I missing something? > In binary tarball, the file is placed under share/. Best, Nirbhay ___ 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
Re: [Maria-developers] [Commits] 7948cc3: MDEV-8260 : Issues related to concurrent CTAS
Hi Serg!
On Tue, Jun 9, 2015 at 1:24 AM, Sergei Golubchik wrote:
> Hi, Nirbhay!
>
> On Jun 08, Nirbhay Choubey wrote:
> > revision-id: 7948cc32b6c12c9696e8b497ae90f123cafcd234
> > parent(s): 6d5b723bdc3e04978619b9673fca266e0426916f
> > committer: Nirbhay Choubey
> > branch nick: 10.0-galera.ctas
> > timestamp: 2015-06-08 22:50:26 -0400
> > message:
> >
> > MDEV-8260 : Issues related to concurrent CTAS
> >
> > * Wait for aborted thd (victim) to release MDL locks
> > * Skip aborting an already aborted thd
> > * Defer setting OK status in case of CTAS
> > * Minor cosmetic changes
> > * Added a test case
> ...
> > +bool select_insert::send_eof()
> > +{
> > + DBUG_ENTER("select_insert::send_eof");
> > + DBUG_RETURN(prepare_eof() || send_ok_packet());
> > }
>
> Please, don't do that, don't put function calls in DBUG_RETURN.
> This will result in dbug traces like
>
>> select_insert::send_eof
>< select_insert::send_eof
>> select_insert::prepare_eof
>< select_insert::prepare_eof
>> select_insert::send_ok_packet
>< select_insert::send_ok_packet
>
> while the correct trace should've be
>
>> select_insert::send_eof
>| > select_insert::prepare_eof
>| < select_insert::prepare_eof
>| > select_insert::send_ok_packet
>| < select_insert::send_ok_packet
>< select_insert::send_eof
>
Wow.. that's interesting.
>
> So, either write like
>
> bool res= prepare_eof() || send_ok_packet();
> DBUG_RETURN(res);
>
> or fix DBUG_RETURN() macro. Whatever you prefer :)
>
I would prefer not to offset DBUG macros at least for now. :)
> Otherwise the patch is ok.
>
http://lists.askmonty.org/pipermail/commits/2015-June/008029.html
Thanks!
-- Nirbhay
>
> Regards,
> Sergei
>
___
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
Re: [Maria-developers] 1ed4683: MDEV-8260 : Issues related to concurrent CTAS
Hi Serg,
On Mon, Jun 8, 2015 at 11:07 AM, Sergei Golubchik wrote:
> Hi, Nirbhay!
>
> On Jun 02, Nirbhay Choubey wrote:
> > revision-id: 1ed46831ed3a1c842b45f7819edd48a233080ce4
> > parent(s): 6d5b723bdc3e04978619b9673fca266e0426916f
> > committer: Nirbhay Choubey
> > branch nick: 10.0-galera.ctas
> > timestamp: 2015-06-02 17:59:32 -0400
> > message:
> >
> > MDEV-8260 : Issues related to concurrent CTAS
> >
> > * Wait for aborted thd (victim) to release MDL locks
> > * Skip aborting an already aborted thd
> > * Defer setting OK status in case of CTAS
> > * Minor cosmetic changes
> > * Added a test case
>
> So, I'm only reviewing non-wsrep part, ok?
>
Ok. :)
..skip..
> I think we've discussed a slightly different fix,where the parent class
> (select_insert) wouldn't need to check thd->lex->sql_command. Like,
> create two methods
>
> select_insert::prepare_eof
> select_insert::send_ok_packet
>
> the first method will have everything of the current
> select_insert::send_eof but without ::my_ok part.
> Then select_insert::send_eof() will be just
>
> bool select_insert::send_eof()
> {
> return prepare_eof() || send_ok_packet();
> }
>
> and select_create::send_eof() will do prepare_eof(), commit and other
> stuff, send_ok_packet().
>
> In this case you won't need to test for thd->lex->sql_command in the
> parent class. And you won't need prepare_ok_info(), it'll be inside
> send_ok_packet().
>
I have redone the patch with your suggestions.
http://lists.askmonty.org/pipermail/commits/2015-June/008023.html
Please have a look.
Thanks!
-- Nirbhay
>
> Regards,
> Sergei
>
___
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
Re: [Maria-developers] 8928497: MDEV-7067: Server outputs Galera (WSREP) information, even if Galera is disabled
Hi Serg,
On Thu, May 28, 2015 at 5:56 PM, Sergei Golubchik wrote:
> Hi, Nirbhay!
>
> On May 09, Nirbhay Choubey wrote:
> > revision-id: 8928497acbe3e17a105897f405d5e5138d57a249
> > parent(s): 3674c363a7e77e534c3e0d28659dc614c53fbcbc
> > committer: Nirbhay Choubey
> > branch nick: 10.1-wsrep
> > timestamp: 2015-05-09 16:11:59 -0400
> > message:
> >
> > MDEV-7067: Server outputs Galera (WSREP) information, even if Galera is
> disabled
> >
> > * mysqld_safe: Since wsrep_on variable is mandatory in 10.1, skip wsrep
> > position recovery if its OFF.
> > * mysqld: Remove "-wsrep" from server version
> > * mysqld: Remove wsrep patch version from @@version_comment
> > * mysqld: Introduce @@wsrep_patch_version
>
> Looks ok, I just didn't like the wsrep.cmake change. See below.
>
> > diff --git a/cmake/wsrep.cmake b/cmake/wsrep.cmake
> > index c37cf74..305ab27 100644
> > --- a/cmake/wsrep.cmake
> > +++ b/cmake/wsrep.cmake
> > @@ -28,29 +28,22 @@ OPTION(WITH_WSREP "WSREP replication API (to use,
> e.g. Galera Replication librar
> > # Set the patch version
> > SET(WSREP_PATCH_VERSION "10")
> >
> > -# MariaDB addition: Revision number of the last revision merged from
> > -# codership branch visible in @@version_comment.
> > -# Branch : codership-mysql/5.6
> > -SET(WSREP_PATCH_REVNO "4144") # Should be updated on every merge.
> > -
> > -# MariaDB addition: Revision number of the last revision merged from
> > -# Branch : lp:maria/maria-10.0-galera
> > -SET(WSREP_PATCH_REVNO2 "3919") # Should be updated on every merge.
> > -
> > -# MariaDB: Obtain patch revision number:
> > -# Update WSREP_PATCH_REVNO if WSREP_REV environment variable is set.
> > -IF (DEFINED ENV{WSREP_REV})
> > - SET(WSREP_PATCH_REVNO $ENV{WSREP_REV})
> > -ENDIF()
> >
> > -SET(WSREP_INTERFACE_VERSION 25)
> > +# Obtain wsrep API version
> > +EXECUTE_PROCESS(
> > + COMMAND sh -c "grep WSREP_INTERFACE_VERSION
> ${MySQL_SOURCE_DIR}/wsrep/wsrep_api.h | cut -d '\"' -f 2"
> > + OUTPUT_VARIABLE WSREP_API_VERSION
> > + RESULT_VARIABLE RESULT
> > +)
>
> I don't like calling grep|cut from cmake files. If you want to extract
> the version from wsrep_api.h, use cmake commands, e.g.
>
> FILE(STRINGS "${MySQL_SOURCE_DIR}/wsrep/wsrep_api.h" WSREP_API_H
>LIMIT_COUNT 1 REGEX "WSREP_INTERFACE_VERSION")
> STRING(REGEX MATCH
>
Done.
>
> > +STRING(REGEX REPLACE "(\r?\n)+$" "" WSREP_API_VERSION
> "${WSREP_API_VERSION}")
> >
> > -SET(WSREP_VERSION
> > -
> "${WSREP_INTERFACE_VERSION}.${WSREP_PATCH_VERSION}.r${WSREP_PATCH_REVNO}")
> > +SET(WSREP_VERSION "${WSREP_API_VERSION}.${WSREP_PATCH_VERSION}"
> > + CACHE STRING "WSREP version")
>
> make it INTERNAL, not STRING
>
Done.
>
> > SET(WSREP_PROC_INFO ${WITH_WSREP})
> >
> > IF(WITH_WSREP)
> > - SET(COMPILATION_COMMENT "${COMPILATION_COMMENT},
> wsrep_${WSREP_VERSION}")
> > + SET(WSREP_PATCH_VERSION "wsrep_${WSREP_VERSION}")
> > ENDIF()
> >
> > diff --git a/mysql-test/suite/sys_vars/r/sysvars_wsrep.result
> b/mysql-test/suite/sys_vars/r/sysvars_wsrep.result
> > index 9c392b1..1763078 100644
> > --- a/mysql-test/suite/sys_vars/r/sysvars_wsrep.result
> > +++ b/mysql-test/suite/sys_vars/r/sysvars_wsrep.result
> > @@ -365,6 +365,20 @@ NUMERIC_BLOCK_SIZE NULL
> > ENUM_VALUE_LIST TOI,RSU
> > READ_ONLYNO
> > COMMAND_LINE_ARGUMENTOPTIONAL
> > +VARIABLE_NAMEWSREP_PATCH_VERSION
> > +SESSION_VALUENULL
> > +GLOBAL_VALUE wsrep_25.10
> > +GLOBAL_VALUE_ORIGIN COMPILE-TIME
> > +DEFAULT_VALUENULL
> > +VARIABLE_SCOPE GLOBAL
> > +VARIABLE_TYPEVARCHAR
> > +VARIABLE_COMMENT wsrep patch version
> > +NUMERIC_MIN_VALUENULL
> > +NUMERIC_MAX_VALUENULL
> > +NUMERIC_BLOCK_SIZE NULL
> > +ENUM_VALUE_LIST NULL
> > +READ_ONLYYES
> > +COMMAND_LINE_ARGUMENTNULL
> > VARIABLE_NAMEWSREP_PROVIDER
> > SESSION_VALUENULL
> > GLOBAL_VALUE none
> > diff --git
> a/mysql-test/suite/sys_vars/r/wsrep_patch_version_basic.result
> b/mysql-test/suite/sys_vars/r/wsrep_patch_version_basic.result
> > new file mode 100644
> > index 000..80f29e6
> > --- /dev/null
> > +++ b/mysql-test/suite/sys_vars/r/wsrep_patch_version_basic.result
>
> I've a
Re: [Maria-developers] [Commits] 845edb6: MDEV-7067: Server outputs Galera (WSREP) information, even if Galera is disabled
Hi Serg,
On Thu, May 7, 2015 at 4:27 AM, Sergei Golubchik wrote:
> Hi, Nirbhay!
>
> On Apr 08, Nirbhay Choubey wrote:
> > revision-id: 845edb6259559bed39eb7365860aa5d588990775
> > parent(s): 3674c363a7e77e534c3e0d28659dc614c53fbcbc
> > committer: Nirbhay Choubey
> > branch nick: 10.1-wsrep
> > timestamp: 2015-04-08 16:49:57 -0400
> > message:
> >
> > MDEV-7067: Server outputs Galera (WSREP) information, even if Galera is
> disabled
> >
> > - mysqld_safe: Skip wsrep position recovery if wsrep_on variable
> > (mandatory since 10.1) is not specified
>
> ok
>
> > - Do not load "dummy" wsrep provider if wsrep_on is not ON
>
> see below
>
> > - Remove "-wsrep" from server version
>
> ok
>
> I'd also move "wsrep_25.10.r4144" from version_comment to a separate
> variable, like, wsrep_version.
>
Good idea, introduced @@wsrep_patch_version.
> > diff --git a/sql/wsrep_var.cc b/sql/wsrep_var.cc
> > index ca921b7..1cd0a54 100644
> > --- a/sql/wsrep_var.cc
> > +++ b/sql/wsrep_var.cc
> > @@ -295,6 +295,8 @@ bool wsrep_provider_options_check(sys_var *self,
> THD* thd, set_var* var)
> >
> > bool wsrep_provider_options_update(sys_var *self, THD* thd,
> enum_var_type type)
> > {
> > + if (!wsrep) return false;
> > +
>
> What about that comment by Alexey Yurchenko?
> The one where he says that checking for wsrep being not NULL inside
> wsrep functions is never needed, because wsrep functions should be never
> even called if wsrep is not enabled.
>
Since wsrep_on and wsrep_provider variables are dynamic, we need to have
the dummy provider
loaded in case galera in not available.
Here is the patch :
http://lists.askmonty.org/pipermail/commits/2015-May/007831.html
Best,
Nirbhay
> Regards,
> Sergei
>
___
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
Re: [Maria-developers] MariaDB / Galera BUG
Hi Andy! On Fri, Apr 24, 2015 at 3:27 PM, Andrew W Elble wrote: > > Nirbhay, > > I'm going to have a number of patches/suggestions from chasing > this. Hopefully I'll have them in a consumable fashion soon. > Is it preferred to send them all to the list? > Great! Will it be possible for you to create a pull request? > > This is a rough summary of what we've found so far: > > 1.) MDEV-6924: either: >fix because CTAS uses THD::STMT_QUERY_TYPE >alternatively: Query_log_event::Query_log_event() >flips the setting of "direct" when binlog is not row/ >picks inapproriate setting of use_cache > I have pushed a related fix recently in 5.5-galera (to be upmerged to higher versions). https://github.com/MariaDB/server/commit/581b49dd3d3e2e253812bb24fa881148675320b4 Perhaps, you can take a look to see if it does not conflict with your additions. > 1a.) Revert patch for MDEV-7673, as it apparently can cause a crash > with WSREP: FSM: no such a transition REPLICATING -> REPLICATING > > 2.) select_insert::send_eof() will call my_ok() when called from > select_create::send_eof() even if abort_result_set() is going to > be called. Rectify for CTAS case. > > 3.) wsrep_applier thread tends to spin and try to apply the same > transaction multiple times to cluster failure even though the > selected victim thread is slowly trying to abort. > a.) increase timeout if a victim has been selected > b.) don't downcall from wsrep_abort_thd if victim is already > aborting > > 4.) select_create::send_eof() sets exit_done before seeing if galera > is going to call abort_result_set(), which can lead to unexpected > tables being present + cluster failure as result. > > 5.) handle_select() resets thd->killed() even when thread was a victim > thread, causing crash. > > 6.) cherry-picking upstream commit cc3d09bc8d5a78abc064d289045b20363aab9d28 > (I believe you're already aware of this one seeing as how your > name is on it) > Is this correct? Which repo/branch are you referring to? $ git branch --contains cc3d09bc8d5a78abc064d289045b20363aab9d28 error: no such commit cc3d09bc8d5a78abc064d289045b20363aab9d28 Best. -- Nirbhay > Thanks, > > Andy > > -- > Andrew W. Elble > awe...@discipline.rit.edu > Infrastructure Engineer, Communications Technical Lead > Rochester Institute of Technology > PGP: BFAD 8461 4CCF DC95 DA2C B0EB 965B 082E 863E C912 > ___ 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
Re: [Maria-developers] MariaDB / Galera BUG
Hi Andrew! On Wed, Apr 15, 2015 at 11:45 AM, Andrew W Elble wrote: > > Nirbhay, > >We've been poking at this some more. Seems like we're retracing some >steps as documented here: > >https://bugs.launchpad.net/codership-mysql/+bug/1052002 > >My patch will need to be reverted and replaced with something else. > Yes, after fixing MDEV-7995, I realized that your patch can be reverted. >MDEV-7673 is a regression caused by a portion of the fix for MDEV-6924. > >Specifically this: > > in sql/sql_class.cc -> THD::binlog_query() > #ifdef WITH_WSREP > /* > Even though wsrep only supports ROW binary log format, a user can set > binlog format to STATEMENT (wsrep_forced_binlog_format). In which > case > the control might reach here even when binary logging (--log-bin) is > not enabled. This is possible because wsrep patch partially enables > binary logging by setting wsrep_emulate_binlog. > */ > if (mysql_bin_log.is_open()) > #endif /* WITH_WSREP */ > That's right, removed as part of fix for MDEV-7995. > see sql/sql_insert.cc -> select_create::binlog_show_create_table() > >There are other issues here which we're still trying to track down >(simultaneously running 'CREATE TABLE t1 AS SELECT SLEEP(30)' on >multiple nodes is bad, for instance). I wanted to at least start the > Thanks! -- Nirbhay ___ 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
Re: [Maria-developers] 1cda762: MDEV-6069: Remove old logic for 3.23-to-higher upgrades from upgrade SQL scripts
On Thu, Mar 19, 2015 at 8:49 AM, Peter Laursen wrote: > Are you referring > > "When upgrading to MySQL 5.1.18 or later from a previous MySQL version > and scheduled events have been used, the upgrade utilities do not > accommodate changes in event-related system tables. As a workaround, you > can dump events before the upgrade, then restore them from the dump > afterward. This issue was fixed in MySQL 5.1.20." (from > http://dev.mysql.com/doc/relnotes/mysql/5.1/en/news-5-1-18.html > > ?? > Yes. That pretty much explains the main.system_mysql_db_fix50117 test. Best, Nirbhay > > > -- Peter > > On Thu, Mar 19, 2015 at 1:35 PM, Nirbhay Choubey > wrote: > >> Hi Serg, >> >> On Thu, Mar 19, 2015 at 8:02 AM, Sergei Golubchik >> wrote: >> >>> Hi, Nirbhay! >>> >>> On Mar 17, Nirbhay Choubey wrote: >>> > revision-id: 1cda762d3f1132e6696e1c451435746db0a1420a >>> > parent(s): b0542b78c866fb32383f759914b10f060d0e14cd >>> > committer: Nirbhay Choubey >>> > branch nick: server >>> > timestamp: 2015-03-17 11:20:49 -0400 >>> > message: >>> > >>> > MDEV-6069: Remove old logic for 3.23-to-higher upgrades from upgrade >>> SQL scripts >>> > >>> > Fix for failing tests. >>> > >>> > * Update mysql_system_tables_fix.sql to makeup the differences in >>> system >>> > tables in 5.1.17 (main.system_mysql_db_fix50117) >>> >>> I don't quite understand that. >>> Why these ALTER TABLEs were not needed before? >>> >> >> The system table structures in 5.1.17 were slightly different from the >> later 5.1 versions. >> While the original/main patch that I prepared was based on a later >> mysql/maria-5.1. >> These extra ALTERs would fix the differences. >> >> Best, >> Nirbhay >> >> >>> > * Removed system_mysql_db tests for versions 5.0.30 & 4.1.23. >>> >>> ok >>> >>> Regards, >>> Sergei >>> >> >> >> ___ >> 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 >> >> > ___ 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
Re: [Maria-developers] 1cda762: MDEV-6069: Remove old logic for 3.23-to-higher upgrades from upgrade SQL scripts
Hi Serg, On Thu, Mar 19, 2015 at 8:02 AM, Sergei Golubchik wrote: > Hi, Nirbhay! > > On Mar 17, Nirbhay Choubey wrote: > > revision-id: 1cda762d3f1132e6696e1c451435746db0a1420a > > parent(s): b0542b78c866fb32383f759914b10f060d0e14cd > > committer: Nirbhay Choubey > > branch nick: server > > timestamp: 2015-03-17 11:20:49 -0400 > > message: > > > > MDEV-6069: Remove old logic for 3.23-to-higher upgrades from upgrade SQL > scripts > > > > Fix for failing tests. > > > > * Update mysql_system_tables_fix.sql to makeup the differences in system > > tables in 5.1.17 (main.system_mysql_db_fix50117) > > I don't quite understand that. > Why these ALTER TABLEs were not needed before? > The system table structures in 5.1.17 were slightly different from the later 5.1 versions. While the original/main patch that I prepared was based on a later mysql/maria-5.1. These extra ALTERs would fix the differences. Best, Nirbhay > > * Removed system_mysql_db tests for versions 5.0.30 & 4.1.23. > > ok > > Regards, > Sergei > ___ 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
Re: [Maria-developers] GSoC 2015: Indexes on Virtual Columns and Array Based UDFs
On Sun, Mar 15, 2015 at 7:48 AM, Richa Sehgal wrote: > Dear Sergei and Maria community, > > Finally I have settled on one project: Indexing over virtual columns. > > I spent the entire day today trying to compile MariaDB in Ubuntu and > reading the code. On reading code side, I spent some time in the > /storage/myiasm/ : read fiels like mi_key.cc, mi_create.cc, ha_myisam and > so on. Took a brief overview arounf _mi_make_key function. I also looked > around update_virtual_fields() function. > > The code path is still not very clear to me - especially the transition > point between sql folders to /storage folders. I chatted on #maria IRC and > they gave an idea to run Maria in gdb mode. I tried compiling the code it > was almost successful! At the very final step, I got the following error: > > [ 98%] Built target thr_lock > Linking CXX shared library libmysqlclient.so > /usr/bin/ld: error: > /home/shikhar/mariadb/maria/trunk/libmysql/libmysql_versions.ld:155:9: > invalid use of VERSION in input file > collect2: ld returned 1 exit status > make[2]: *** [libmysql/libmysqlclient.so.18.0.0] Error 1 > make[1]: *** [libmysql/CMakeFiles/libmysql.dir/all] Error 2 > make: *** [all] Error 2 > > I am not able to proceed now. I tred googling around, but could not find > the fix. Can you please guide me in my next steps? > I think you hit the ld/gold issue. https://mariadb.atlassian.net/browse/MDEV-5982 Best, Nirbhay > Thanks > Richa > > On Sat, Mar 14, 2015 at 10:46 AM, Sergei Golubchik > wrote: > >> Hi, Richa! >> >> On Mar 14, Richa Sehgal wrote: >> > Thanks Sergei for the explanation. The modified project for MDEV-5199 >> > (Table Functions) now makes sense - as in the proposals mentioned in the >> > ticket as possible solutions now looks appropriate :) >> > >> > I am looking into the index of virtual columns. One question: can we >> submit >> > proposals for 2 projects at MariaDB? >> >> We don't mind, but rules are set by Google, if they allow it - you can, >> otherwise you cannot. >> >> > Now as for Table Functions: >> > >> > As mentioned there are two ways: >> > 1. Materializing the table - this has a disadvantage for extra space. >> > 2. Treating the UDF result as an "abstract table". For eg, the code >> might >> > have a base class called Table. Then we can create a derived class >> called >> > UDFTable and pass this instance in the regular query processing. This >> class >> > would implement all functionalities like Init(), Next(), Read(int >> > col_index), Seek(), etc >> > Consider: >> > Select A where A.x > 10 FROM UDF_TABLE >> > >> > In this case, we will first construct the table UDF_TABLE through >> regular >> > query - evaluation of the UDF. Then we can create an instance of >> UDFTable >> > from the table and run normal query processing on this. >> > >> > Is this the right approach? Can you send me the next steps? >> >> the "abstract table" that can do Next, Read, Seek, etc in MariaDB is >> represented by the "handler" class. All real storage engines inherit >> from the handler class. >> >> See the SEQUENCE engine, it's in the storage/sequence/sequence.cc >> This is, exactly, the engine that generates data on the fly, so is a >> table function, in a sense. But it's a hard-coded function and the way >> of passing arguments is not very nice. Still, it'll give you an idea >> what a table function handler might look like. >> >> If you want an even simpler storage engines - there are some in our >> book, "MySQL 5.1 Plugin Development". >> >> Regards, >> Sergei >> >> > > ___ > 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 > > ___ 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
Re: [Maria-developers] MariaDB / Galera BUG
Hi Andy!
On Thu, Mar 5, 2015 at 2:27 PM, Andrew W Elble wrote:
>
> Environment:
> MariaDB-10.0.16
> Galera-3-25.3.9
>
> This statement does nasty things to a MariaDB+Galera cluster:
>
> CREATE TABLE u91rw_zoo_version (`version` varchar(255) NOT NULL)
>ENGINE=innoDB DEFAULT CHARSET=utf8
>SELECT '3.3.3' as version;
>
> Statement fails to replicate to the other nodes because the target
> table doesn't exist. Cluster then disassembles itself.
>
Right.
>
> This patch makes the problem disappear. Please evaluate?
> (note: patch based against github.com/MariaDB/server.git)
>
> =
>
> diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
> index be57be9b223a..d0521ed896b8 100644
> --- a/sql/sql_parse.cc
> +++ b/sql/sql_parse.cc
> @@ -3325,6 +3325,11 @@ mysql_execute_command(THD *thd)
> /* Store reference to table in case of LOCK TABLES */
> create_info.table= create_table->table;
>
> +if (WSREP(thd) && (!thd->is_current_stmt_binlog_format_row() ||
> +!create_info.tmp_table()))
> +{
> + WSREP_TO_ISOLATION_BEGIN(create_table->db,
> create_table->table_name, NULL)
> +}
> /*
>select_create is currently not re-execution friendly and
>needs to be created for every execution of a PS/SP.
>
> =
>
The patch looks good to me.
>
>
> We have a full environment setup in which to test. debug/trace available
> on request. Will open bug if required for your workflow.
>
I have filed a bug reporting this issue :
https://mariadb.atlassian.net/browse/MDEV-7673
Thanks for the patch.
Cheers!
-- Nirbhay
> Thanks,
>
> Andy
>
>
> --
> Andrew W. Elble
> awe...@discipline.rit.edu
> Infrastructure Engineer, Communications Technical Lead
> Rochester Institute of Technology
> PGP: BFAD 8461 4CCF DC95 DA2C B0EB 965B 082E 863E C912
>
> ___
> 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
>
___
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
Re: [Maria-developers] [Commits] 101d7eb: MDEV-4987: Sort by domain_id when list of GTIDs are output
Hi Kristian,
On Mon, Feb 2, 2015 at 5:06 AM, Kristian Nielsen
wrote:
> nirb...@mariadb.com writes:
>
> > revision-id: 101d7eb963816514362da8a98fc7db7135181910
> > committer: Nirbhay Choubey
> > timestamp: 2015-01-31 21:48:14 -0500
> > message:
> >
> > MDEV-4987: Sort by domain_id when list of GTIDs are output
> >
> > Added logic to sort gtid list based on domain_id before
> > populating them in string. Added a test case.
>
> Thanks!
> The patch looks ok to push after fixing / considering below in-line
> comments:
>
> > diff --git a/mysql-test/suite/rpl/t/rpl_gtid_sort.test
> b/mysql-test/suite/rpl/t/rpl_gtid_sort.test
> > new file mode 100644
> > index 000..d3b8c6d
> > --- /dev/null
> > +++ b/mysql-test/suite/rpl/t/rpl_gtid_sort.test
>
> > +--connection server_2
> > +SHOW VARIABLES LIKE '%GTID%';
> > +#SET GLOBAL gtid_slave_pos="";
>
> This commented-out SET looks like some left-over, probably you meant to
> remove
> it?
>
Yes, it was a leftover, removed.
>
> > diff --git a/sql/rpl_gtid.cc b/sql/rpl_gtid.cc
> > index e5620ec..a0471f3 100644
> > --- a/sql/rpl_gtid.cc
> > +++ b/sql/rpl_gtid.cc
> > @@ -247,11 +247,13 @@
> > {
> >my_hash_init(&hash, &my_charset_bin, 32, offsetof(element, domain_id),
> > sizeof(uint32), NULL, rpl_slave_state_free_element,
> HASH_UNIQUE);
> > + my_init_dynamic_array(>id_cache, sizeof(rpl_gtid), 8, 8, MYF(0));
>
> The name 'cache' confused me slightly, as I think of a cache as something
> that
> stores some state between invocations. But it is actually just a temporary
> buffer used for sorting.
>
> A suggestion for a better name could be 'gtid_sort_array'.
>
Renamed.
>
>
> > rpl_slave_state::~rpl_slave_state()
> > {
> > + delete_dynamic(>id_cache);
> > }
>
> The other parts of rpl_slave_state are freed in rpl_slave_state::deinit().
> I
> think this is because there is a global variable of this class, and
> destructors in global variables can cause various issues.
>
> So better put this delete_dynamic() into deinit() as well (for
> consistency, if
> nothing else).
>
I agree, done.
>
>
> > @@ -705,7 +707,13 @@ class Gtid_db_intact : public Table_check_intact
> >return sub_id;
> > }
> >
> > +/* A callback used in sorting of gtid list based on domain_id. */
> > +static int rpl_gtid_cmp_cb(const void *id1, const void *id2)
> > +{
> > + return (((rpl_gtid *)id1)->domain_id - ((rpl_gtid *)id2)->domain_id);
> > +}
>
> I think this is wrong, you can get signed/unsigned integer overflow (eg. it
> will consider 0x as being less than 0x).
>
> We might be able to do the subtraction in longlong with a cast (though that
> would still be susceptible to overflow when converting to the int return
> value, right?), but it's probably clearer to just do:
>
> uint32 d1= ((rpl_gtid *)id1)->domain_id;
> uint32 d2= ((rpl_gtid *)id2)->domain_id;
> if (d1 < d2)
> return -1;
> else if (d1 > f2)
> return 1;
> else
> return 0;
>
> You should probably also modify the test case to have a domain id >
> 0x8000, just to check.
>
You are right. It's been modified now. I have also added some related tests
to check this.
I originally thought of pushing it to 10.1 (10.1.4). Do you want me to push
it to 10.0 as well?
Best,
Nirbhay
>
> Otherwise looks good, thanks!
>
> - Kristian.
>
___
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
Re: [Maria-developers] MDEV-4412 : SLOW QUERY LOG - add affected rows (UPDATE / DELETE) in slow query log
Hi Serg, On Sat, Nov 29, 2014 at 1:54 PM, Sergei Golubchik wrote: > Hi, Nirbhay! > > On Nov 28, nirb...@mariadb.com wrote: > > revision-id: 7a5559421f4f3dda44d04bf946e79661d09b4dc2 > > parent(s): 7b55b67de5fdfe91283357fe6c7ccc3f9e355925 > > committer: Nirbhay Choubey > > branch nick: b4412 > > timestamp: 2014-11-28 20:53:02 -0500 > > message: > > > > MDEV-4412 : SLOW QUERY LOG - add affected rows (UPDATE / DELETE) in slow > query log > > > > Added Rows_affected to slow query log & mysql.slow_log table. > > Just a couple of comments: > > > diff --git a/scripts/mysql_system_tables_fix.sql > b/scripts/mysql_system_tables_fix.sql > > index b9bb59c..4973dc9 100644 > > --- a/scripts/mysql_system_tables_fix.sql > > +++ b/scripts/mysql_system_tables_fix.sql > > @@ -262,7 +262,8 @@ ALTER TABLE slow_log > >MODIFY insert_id INTEGER NOT NULL, > >MODIFY server_id INTEGER UNSIGNED NOT NULL, > >MODIFY sql_text MEDIUMTEXT NOT NULL, > > - MODIFY thread_id BIGINT(21) UNSIGNED NOT NULL; > > + MODIFY thread_id BIGINT(21) UNSIGNED NOT NULL, > > + MODIFY rows_affected INTEGER NOT NULL; > > Eh? That makes no sense. This file is for mysql_upgrade. > Your ALTER TABLE would help if there was some MariaDB version > where rows_affected column would've been nullable and you > wanted to change it to NOT NULL. But there was no such MariaDB version. > There was MariaDB version without rows_affected column at all. > So, instead of this MODIFY rows_affected, you need to put > another ALTER TABLE statement that adds rows_affected INTEGER NOT NULL > column. > Right, I have fixed this. > > > SET GLOBAL slow_query_log = @old_log_state; > > > > ALTER TABLE plugin > > diff --git a/sql/log.cc b/sql/log.cc > > index ff04105..24e5283 100644 > > --- a/sql/log.cc > > +++ b/sql/log.cc > > @@ -2918,12 +2924,16 @@ bool MYSQL_QUERY_LOG::write(THD *thd, time_t > current_time, > > sprintf(lock_time_buff, "%.6f", > ulonglong2double(lock_utime)/100.0); > > if (my_b_printf(&log_file, > > "# Thread_id: %lu Schema: %s QC_hit: %s\n" \ > > -"# Query_time: %s Lock_time: %s Rows_sent: %lu > Rows_examined: %lu\n", > > +"# Query_time: %s Lock_time: %s Rows_sent: %lu > Rows_examined: %lu\n" \ > > +"# Rows_affected: %lu\n", > > I'd rather have Rows_affected on the same line with Rows_sent and > Rows_examined > Based on how mysqldumpslow parses the slow log file, IMO adding a new attribute to the same line may render the existing line non-parsable to older versions. I also had to make a tiny modification to the script for it to consider "QC_hit:". Best, Nirbhay > > (ulong) thd->thread_id, (thd->db ? thd->db : ""), > > ((thd->query_plan_flags & QPLAN_QC) ? "Yes" : "No"), > > query_time_buff, lock_time_buff, > > (ulong) thd->get_sent_row_count(), > > -(ulong) thd->get_examined_row_count()) == (size_t) > -1) > > +(ulong) thd->get_examined_row_count(), > > +thd->get_stmt_da()->is_ok() ? > > +(ulong) thd->get_stmt_da()->affected_rows() : > > +0) == (size_t) -1) > >tmp_errno= errno; > > if ((thd->variables.log_slow_verbosity & > LOG_SLOW_VERBOSITY_QUERY_PLAN) && > > (thd->query_plan_flags & > > Regards, > Sergei > ___ 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
Re: [Maria-developers] [Commits] Rev 3879: MDEV-6593 : domain_id based replication filters in lp:~maria-captains/maria/maria-10.0-galera
Hi!
On Fri, Nov 14, 2014 at 3:58 AM, Kristian Nielsen
wrote:
> Nirbhay Choubey writes:
>
> >> > # Case 7: Stop slave into the middle of a transaction being
> filtered
> >> and
> >> > # start it back with filtering disabled.
> >> >
> >> > --echo # On master
> >> > connection master;
> >> > SET @@session.gtid_domain_id=1;
> >> > BEGIN;
> >> > INSERT INTO t2 VALUES(3);
> >> > INSERT INTO t3 VALUES(3);
> >> > sync_slave_with_master;
> >>
> >> No, this does not work. Transactions are always binlogged as a whole on
> the
> >> master, during COMMIT.
> >>
> >
> > You are right. My original intent was to test a transaction which
> modifies
> > both MyISAM and
> > InnoDB tables, where first modification is done in MyISAM table. In which
> > case the changes
> > to MyISAM is sent to the slave right away, while rest of trx is sent on
> > commit. I have modified
> > the test accordingly.
>
> I'm still not sure you understand the scenario I had in mind. It's not
> about
> what happens on the master during the transaction. It is about what
> happens in
> case the slave disconnects in the middle of receiving an event
> group/transaction.
>
You are perhaps looking at an older version of the test. The latest says :
# Case 7: Stop slave before a transaction (involving MyISAM and InnoDB
# table) being filtered commits and start it back with filtering
# disabled.
...
> In general in replication, the major part of the work is not implementing
> the
> functionality for the normal case - that is usually relatively easy. The
> major
> part is handling and testing all the special cases that can occur in
> special
> scenarios, especially various error cases. The replication code is really
> complex in this respect, and the fact that things by their nature happen in
> parallel between different threads and different servers make things even
> more
> complex.
>
> What I wanted you to think about here is what happens if the slave is
> disconnected from the master after having received the first half of an
> event
> group. For example due to network error. This will not happen normally in a
> mysql-test-case run, and if it happens in a production site for a user, it
> will be extremely hard to track down.
>
> In this case, the second half of the event group could be received much
> later
> than the first half. The IO thread could have been stopped (or even the
> whole
> mysqld server could have been stopped) in-between, and the replication
> could
> have been re-configured with CHANGE MASTER. Since the IO thread is doing
> the
> filtering, it seems very important to consider what will happen if eg.
> filters
> are enabled while receiving the first half of the transaction, but disabled
> while receiving the second half:
> Suppose we have this transaction:
>
> BEGIN GTID 2-1-100
> INSERT INTO t1 VALUES (1);
> INSERT INTO t1 VALUES (2);
> COMMIT;
>
> What happens in the following scenario?
>
> CHANGE MASTER TO master_use_gtid=current_pos, ignore_domain_ids=(2);
> START SLAVE;
> # slave IO thread connects to master;
> # slave receives: BEGIN GTID 2-1-100; INSERT INTO t1 VALUES (1);
> # slave IO thread is disconnected from master
> STOP SLAVE;
> # slave mysqld process is stopped and restarted.
> CHANGE MASTER TO master_use_gtid=no, ignore_domain_ids=();
> START SLAVE;
> # slave IO thread connects to master;
> # slave IO thread receives: INSERT INTO t1 VALUES (2); COMMIT;
>
> Are you sure that this will work correctly? And what does "work correctly"
> mean in this case? Will the transaction be completely ignored? Or will it
> be
> completely replicated on the slave? The bug would be if the first half
> would
> be ignored, but the second half still written into the relay log.
>
> To test this, you would need to use DBUG error insertion. There are already
> some tests that do this. They use for example
>
> SET GLOBAL debug_dbug="+d,binlog_force_reconnect_after_22_events";
>
> The code will then (in debug builds) simulate a disconnect at some
> particular
> point in the replication stream, allowing this rare but important case to
> be
> tested. This is done using DBUG_EXECUTE_IF() in the code.
>
I had already added multiple cases under rpl_domain_id_filter_io_crash.test
using
DBUG_EXECUTE_IF("+d,"kill_io_slave_before_commit") in the previous commit.
Even though, it is not exactly similar to what you suggest, it does,
however,try to
kill I/O thread whe
Re: [Maria-developers] [Commits] Rev 3879: MDEV-6593 : domain_id based replication filters in lp:~maria-captains/maria/maria-10.0-galera
Hi!
On Mon, Oct 27, 2014 at 9:12 AM, Kristian Nielsen
wrote:
> Nirbhay Choubey writes:
>
> > [NC] You are right, setting both DO_ and IGNORE_ at the same time would
> not
> > make any sense.
> > But I actually followed the pattern of the existing replication, where
> both
> > _do_ and _ignore_
> > filters are allowed to hold values at the same time and _do_ is preferred
> > in case both are set.
>
> But if it does not make any sense, why allow it? Not giving an error will
> just
> risk confuse users, there seems no benefit.
>
Right.
>
> So I still think we need an error.
>
Ok. CHANGE MASTER would now fail when both DO_ and INGORE_ are non-empty
at the same time or when MASTER_USE_GTID=no with non-empty DO_ or IGNORE_.
> >> - Add a test for the following scenario when using GTID mode: Some
> domain
> >> id
> >>is ignored, some events are received from the master and ignored.
> Then
> >> the
> >>slave is stopped, and the ignore rule is removed. What happens when
> the
> >>slave is restarted and reconnects? I think the previously ignored
> >> domain id
> >>events will now be re-fetched and replicated. I guess that's ok, but
> it
> >>should be documented. Also test what happens when binlog files on the
> >>master have been rotated and purged, so that the events are no longer
> >>available on the master (an error should occur).
> >>
> >
> > [NC] As the filtering takes place in the IO thread. I have added some
> test
> > cases covering
> > various scenarios including ones where IO thread crashes while reading
> > COMMIT/XID event.
> > In this scenario if the filtering is ON the GTID is added to the ignored
> > list (ign_gtids) on GTID
> > event, so when the crash happens before COMMIT/XID event, that particular
> > GTID gets ignored
> > after the slave reconnects.
>
> Ok, I'd forgotten about the ign_gtids.
>
> So if I understand correctly, this means that when we ignore a transaction
> due
> to filtered domain, the IO thread logs a GTID_LIST event to the relay log,
> and
> the SQL thread then updates the slave position to after the ignored events.
>
> So this means that an ignored event stays ignored, even if the domain id
> filter is later removed, that is good.
>
Yes.
>
> Crash tests are always good and needed. However, the test I was asking for
> is
> what happens when the configuration is changed:
>
> CHANGE MASTER TO ignore_domain_ids=(2), master_use_gtid=current_pos;
> START SLAVE;
> # Replicate a number of events in domain 2.
> STOP SLAVE;
> CHANGE MASTER TO ignore_domain_ids=();
> START SLAVE;
> # Replicate some more events in domain 2 from master.
> # Test that the events received before the filter was removed are still
> # ignored, while those received after were applied.
>
I have added a test covering this scenario.
>
> >> > @@ -5601,6 +5610,10 @@
> >> > mi->last_queued_gtid= event_gtid;
> >> > mi->last_queued_gtid_standalone=
> >> >(gtid_flag & Gtid_log_event::FL_STANDALONE) != 0;
> >> > +
> >> > +/* Should filter all the subsequent events in the current GTID
> >> group? */
> >> > +mi->domain_id_filter.set_filter(event_gtid.domain_id);
> >>
> >> So, here we will set m_filter to true, if we want to skip the following
> >> group.
> >>
> >> But I don't see any code that will set it to false at the end of the
> >> group?
> >
> >
> > [NC] With the previous logic m_filter was set/reset only at GTID event. I
> > have now added the logic
> > to reset this flag on COMMIT/XID events too.
>
> I guess you mean this code?
>
> if ((mi->using_gtid != Master_info::USE_GTID_NO) &&
> mi->domain_id_filter.is_group_filtered() &&
> is_commit_rollback)
> {
> mi->domain_id_filter.reset_filter();
> }
>
> But that is not sufficient. This is only valid for GTIDs that have the
> FL_STANDALONE flag clear. For GTIDs with FL_STANDALONE set, the event
> groups
> ends after then first event for which Log_event::is_part_of_group() returns
> false.
>
> (There are two kinds of event groups. Some use BEGIN ... COMMIT or BEGIN
> ... XID; Others have just a single statement, possibly with INTVAR or so
> events before, like CREATE TABLE ...).
>
I see. Fixed.
>
> >
> >
> >> That seems a problem, as we could then end up wrongly skipping all
> kinds of
> >> other events, such as Gti
Re: [Maria-developers] [Commits] Rev 3900: Fix for test failures on 64-bit platform. in lp:~maria-captains/maria/10.0-galera-6593
Hi Kristian,
On Fri, Oct 17, 2014 at 5:53 AM, Kristian Nielsen
wrote:
> Nirbhay Choubey writes:
>
> > message:
> > Fix for test failures on 64-bit platform.
> > === modified file 'sql/rpl_mi.cc'
> > --- a/sql/rpl_mi.cc 2014-10-10 23:06:40 +
> > +++ b/sql/rpl_mi.cc 2014-10-17 02:08:55 +
> > @@ -1466,7 +1466,7 @@
> > {
> >for (int i= DO_DOMAIN_IDS; i <= IGNORE_DOMAIN_IDS; i ++)
> >{
> > -my_init_dynamic_array(&m_domain_ids[i], sizeof(uint32), 16, 16,
> MYF(0));
> > +my_init_dynamic_array(&m_domain_ids[i], sizeof(ulong), 16, 16,
> MYF(0));
> >}
> > }
> >
>
> Why do you need to use ulong? Domain ids should always be uint32, also on
> 64-bit.
> What is the real problem? Eg. why is there a test failure?
It was due to the different sizes for ulong on different platforms. While
trying to reuse
the (modified) auxiliary functions for IGNORE_SERVER_IDS where ulong is
used for
ids (server_id), uint32 store for domain_id did not work on 64-bit.
To solve this, templates did not work well, so was left with either
changing the
global server_id to uint32 (which seem right to me as the valid range for
@@server_id
is (0, UINT_MAX32)) or using ulong in domain id filtering implementation,
which would
work for both 32/64 archs. I chose later to not offset the existing code.
I'd expect that the
> right fix was to convert to uint32 in the places where a domain_id might
> come
> into the code as ulong...
>
Sorry, I did not follow that.
> > @@ -1483,7 +1483,7 @@
> >domain ids list. DO_DOMAIN_IDS list is only looked-up is both (do &
> ignore)
> >list are non-empty.
> > */
> > -void Domain_id_filter::do_filter(uint32 domain_id)
> > +void Domain_id_filter::do_filter(ulong domain_id)
> > {
> >DYNAMIC_ARRAY *do_domain_ids= &m_domain_ids[DO_DOMAIN_IDS];
> >DYNAMIC_ARRAY *ignore_domain_ids= &m_domain_ids[IGNORE_DOMAIN_IDS];
> > @@ -1491,21 +1491,21 @@
> >if (do_domain_ids->elements > 0)
> >{
> > if (likely(do_domain_ids->elements == 1))
> > - m_filter= ((* (uint32*) dynamic_array_ptr(do_domain_ids, 0))
> > + m_filter= ((* (ulong *) dynamic_array_ptr(do_domain_ids, 0))
> > != domain_id);
> > else
> > - m_filter= (bsearch((const uint32 *) &domain_id,
> do_domain_ids->buffer,
> > - do_domain_ids->elements, sizeof(uint32),
> > + m_filter= (bsearch((const ulong *) &domain_id,
> do_domain_ids->buffer,
> > + do_domain_ids->elements, sizeof(ulong),
>
> Why is a cast needed here? If it's because of const, could you avoid the
> cast
> just by declaring domain_id const in the argument of the function?
>
Right cast isn't really needed here, the param should be const instead.
will update.
Thanks,
Nirbhay
___
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
Re: [Maria-developers] [Commits] Rev 3879: MDEV-6593 : domain_id based replication filters in lp:~maria-captains/maria/maria-10.0-galera
Hi Kristian! Thank you for the review remarks. I have committed a patch addressing your comments. http://lists.askmonty.org/pipermail/commits/2014-October/006775.html See inline for my response on specific comments. On Mon, Sep 15, 2014 at 9:25 AM, Kristian Nielsen wrote: > Nirbhay Choubey writes: > > > > > revno: 3879 > > revision-id: nirb...@skysql.com-20140912181622-s33e7r0vtgpju95q > > parent: nirb...@skysql.com-20140814224304-rtkeal7p9lk06li1 > > committer: Nirbhay Choubey > > branch nick: maria-10.0-galera_6593 > > timestamp: Fri 2014-09-12 14:16:22 -0400 > > message: > > MDEV-6593 : domain_id based replication filters > > > > Added support for IGNORE_DOMAIN_IDS and DO_DOMAIN_IDS. > > Here is my review. > > Some general comments, followed by some detailed comments in-line in the > patch: > > 1. Did you prepare any documentation? You could either add it to the > knowledgebase directly, or write up a text file and ask Daniel to add it in > relevant places. > [NC] I haven't but I will prepare it and pass it down to Daniel. > > 2. Did you run the full test suite? I would have expected at least some > tests > in the multi_source suite to need modification. The chance touches enough > things that you should test it in a feature tree in Buildbot to ensure > that it > is green before pushing it to a main tree. > [NC] You are right. I did that now and updated the result files. > 3. I think some extra test cases are needed: > > - You should test (and document) what happens when the same domain_id is > in >both the DO and the IGNORE list. Is it actually ever sensible to have > both >DO_DOMAIN_IDS and IGNORE_DOMAIN_IDS ? If not, I think it should be an > error >to try. > [NC] You are right, setting both DO_ and IGNORE_ at the same time would not make any sense. But I actually followed the pattern of the existing replication, where both _do_ and _ignore_ filters are allowed to hold values at the same time and _do_ is preferred in case both are set. > - You should also add a test using non-GTID mode. > [NC] Done. > > - You should also add a test where parallel replication is used >(--slave-parallel-threads > 0). > [NC] Done. > > - Add a test for the following scenario when using GTID mode: Some domain > id >is ignored, some events are received from the master and ignored. Then > the >slave is stopped, and the ignore rule is removed. What happens when the >slave is restarted and reconnects? I think the previously ignored > domain id >events will now be re-fetched and replicated. I guess that's ok, but it >should be documented. Also test what happens when binlog files on the >master have been rotated and purged, so that the events are no longer >available on the master (an error should occur). > [NC] As the filtering takes place in the IO thread. I have added some test cases covering various scenarios including ones where IO thread crashes while reading COMMIT/XID event. In this scenario if the filtering is ON the GTID is added to the ignored list (ign_gtids) on GTID event, so when the crash happens before COMMIT/XID event, that particular GTID gets ignored after the slave reconnects. > - Add a similar test for when not using GTID mode, the last event > replicated >by the slave is ignored, the slave is stopped, the ignore rule removed > and >the slave restarted; will the last event now be re-fetched or not (I > think >not). > [NC] Done. > > - Add a test that checks that seconds_behind_master is properly updated to >zero even when the last event is ignored due to >DO_DOMAIN_IDS/IGNORE_DOMAIN_IDS. It can perhaps be a bit tricky to test, >but I think you can use this existing test as a basis: >mysql-test/suite/rpl/t/rpl_parallel2.test > [NC] Done. > > - It would also be good to check that the code works when used on an old >master (which does not send GTID events). You can do this by creating a >binlog file with a 5.5 server and storing it in mysql-test/std_data/, > and >then installing it in a master; there should be several other test cases >that already do this. > [NC] Done. > > 4. Also see detailed comments for some possible problems with the > implementation. The most serious is probably to ensure that events are not > skipped after the end of the group, we need a couple of tests for this, see > below. Also, there is a small memory leak, see below. > > > +DO_DOMAIN_IDS (before) : 1 > > +IGNORE_DOMAIN_IDS (before) : 2 > > +CHANGE MASTER TO DO_DOMAIN_IDS=(4,4,5,1,7,7,7,1,1,2,6,8,1,4,5,5,
Re: [Maria-developers] [Commits] Rev 3879: MDEV-6593 : domain_id based replication filters in lp:~maria-captains/maria/maria-10.0-galera
Hi! On Mon, Sep 15, 2014 at 10:09 AM, Kristian Nielsen wrote: > Kristian Nielsen writes: > > > 4. Also see detailed comments for some possible problems with the > > implementation. The most serious is probably to ensure that events are > not > > skipped after the end of the group, we need a couple of tests for this, > see > > Hm, actually I thought of another potential problem. > > What happens if the slave disconnects from the master in the middle of > receiving an event group? There are several tricky issues around this part > of > replication, we definitely need some test cases for this as well. > > There are different cases, for example whether using GTID or non-GTID mode, > whether the slave just reconnects, the I/O and/or SQL threads are > restarted, > or the entire server restarts. And if the filters are reconfigured before > reconnecting. > I have tried to add multiple test scenarios to cover these aspects. > For example, it seems to me that in non-GTID mode, if we restart the > server in > the middle of receiving an event group, we can easily end up with ignoring > one > half of the group and not the other, which is very bad? > The filtering would only work when slave is configured with GTID. > > And in GTID mode, the reconnect issue is quite tricky also, we need a test > case to check that everything is ok. Though in this case we always > reconnect > at the start of an event group, so maybe things are easier to handle. > > For non-GTID mode, maybe we need to handle the ignoring in the SQL thread > instead? Or alternatively, we can make ignoring events based on domain_id > only > legal in GTID mode, and give an error in non-GTID? > I think the patch follows the 2nd approach minus the error. Could you please elaborate on the error? Best. - Nirbhay ___ 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
