- developers - lists.mariadb.org

[Maria-developers] optimizer_switch default settings change for 5.3.2: status?
by Sergey Petrunya 09 Oct '11

09 Oct '11

Hi Sanja, I was wondering if you have any progress with optimizer_switch default settings change that we decided to do for the 5.3.2 release? Any ETA on when that will be pushed? BR Sergey -- Sergey Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] 5.5 merge status
by Kristian Nielsen 07 Oct '11

07 Oct '11

Hi Serg, Here is the status of my efforts to merge the rpl,binlog, and innodb suites. These are the tests that still fail after my last pust to 5.5-serg: binlog.binlog_innodb_row This fails because of a problem in my merge of group commit, I will fix. binlog.binlog_killed I do not know yet, I can take a look next week. binlog.binlog_index This fails because of missing --test-expect-abort option which you said you would re-add back. rpl.rpl_create_if_not_exists Something with CREATE IF NOT EXISTS, I believe you said you would look at this (not related to replication). rpl.rpl_variables_stm This looks like microsecond related, maybe you can check. innodb.innodb innodb.innodb_no_mrricp innodb.innodb_mysql I think we should ask someone from optimiser to take a look at these. - Kristian.

3 3

[Maria-developers] Out of semi-join bugs, time for another testing?
by Sergey Petrunya 06 Oct '11

06 Oct '11

Hi Philip, I've fixed/pushed all semi-join bugs. If I recall correctly, you intended to do more semi-join testing, now everything is ready for it. BR Sergey -- Sergey Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog

2 1

Re: [Maria-developers] MySQL Bug 59449 - Accepted by MariaDB?
by Jeremy Thomerson 05 Oct '11

05 Oct '11

Cool, thank you! As I'm not very familiar with the working process between MariaDB and MySQL upstream, do you think this patch will go back upstream? My intention in my original work on it was to get a fix into all distros that track MySQL. Thanks again! Jeremy Thomerson On Wed, Oct 5, 2011 at 8:56 AM, Timour Katchaounov <timour(a)montyprogram.com>wrote: > Hello Jeremy, > > This is to inform you that I started working on > MySQL Bug 59449. In order to track the bug progress > I created a bug in Launchpad: > https://bugs.launchpad.net/**maria/+bug/861763<https://bugs.launchpad.net/maria/+bug/861763> > > If you are interested in following the progress of > this work, please subscribe to the Launchpad bug above. > > I will work on implementing the generic solution > suggested by Monty. > > Timour > > > >> Hi! >> >> "Jeremy" == Jeremy Thomerson<jeremy(a)**thomersonfamily.com<jeremy(a)thomersonfamily.com>> >>>>>>> writes: >>>>>>> >>>>>> >> Jeremy> Monty, et al... >> Jeremy> On Mon, Jun 13, 2011 at 9:36 AM, Michael Widenius< >> monty(a)askmonty.org>**wrote: >> >> >>>> Hi! >>>> >>>> "Sergei" == Sergei Golubchik<serg(a)askmonty.org> writes: >>>>>>>>> >>>>>>>> >>>> Sergei> Hi, Jeremy! >> Sergei> On Jun 10, Jeremy Thomerson wrote: >> >>> I just submitted a patch to a MySQL bug that I am also experiencing. I >>>>>> would like to see it accepted into the wider community (MySQL, >>>>>> MariaDB, >>>>>> Percona, etc). Please let me know what I need to do so that this >>>>>> patch >>>>>> could also be accepted into your project. >>>>>> >>>>>> The bug is here: http://bugs.mysql.com/bug.php?**id=59449<http://bugs.mysql.com/bug.php?id=59449> >>>>>> The "[11 Jun 3:16] Jeremy Thomerson" comment will be the most useful. >>>>>> >>>>> >>>> >> >> Sergei> But if Oracle will refuse to fix this bug, we will look at your >> >>> patch, >>>> >>> Sergei> certainly. >> >>> >>>> To ensure we don't forget about this, if you are not sure that Oracle >>>> has fixed the bug within one month, ping us and we will look at >>>> adding this into MariaDB at once. >>>> >>>> >> Jeremy> Thanks a bunch! I was away a lot in July and August and didn't >> get to >> Jeremy> follow up on this. Looking at the original bug report, I have >> seen >> Jeremy> absolutely no activity. I'm not even sure why they have not >> touched / >> Jeremy> accepted it. If anyone here knows a way to get it into one (or >> both) >> Jeremy> projects, I'd appreciate any help you can offer. >> >> The patch you suggested was: >> >> === modified file 'sql/sql_base.cc' >> --- sql/sql_base.cc 2011-04-12 10:59:59 +0000 >> +++ sql/sql_base.cc 2011-06-10 20:11:52 +0000 >> @@ -6975,6 +6975,28 @@ >> *counter= i; >> *resolution= RESOLVED_IGNORING_ALIAS; >> break; >> + } >> + else if (find->type() == Item::FUNC_ITEM&& ((Item_func*) >> find)->functype() == Item_func::COLLATE_FUNC&& ((Item_func*) >> find)->arg_count == 2) >> >> + { >> + /* >> + If someone uses "ORDER BY column_name COLLATE utf8_spanish_ci", >> it is a function >> + that we are trying to find, and not a real column. Bug #59449 >> exposes that this >> + failed when column_name is actually referring to the name of an >> alias, whether >> + that alias is for a real column or for a function result (such >> as coalesce). This >> + block of code finds the real item, which is the "column" >> referred to by the >> + collation function for this scenario. >> + */ >> + Item *collated_column = ((Item_func*) find)->arguments()[0]; >> + int is_alias = item->type() == Item::FUNC_ITEM || >> + ((item->type() == Item::FIELD_ITEM || item->type() >> == Item::REF_ITEM)&& >> + my_strcasecmp(system_charset_**info, >> ((Item_ident*) item)->field_name, item->name)); >> + if (is_alias&& collated_column->name&& item->name&& >> !my_strcasecmp(system_charset_**info, item->name, >> collated_column->name)) >> >> + { >> + found =&collated_column; >> + *counter = i; >> + *resolution = RESOLVED_BEHIND_ALIAS; >> + break; >> + } >> } >> } >> else if (table_name&& item->type() == Item::REF_ITEM&& >> >> What I don't like with the patch is that we make COLLATE a special >> case. I think that all usage of alias names in ORDER BY should be >> treated the same. >> >> In other words: >> >> ORDER BY LanguageName >> ORDER BY LanguageName COLLATE utf8_spanish_ci >> ORDER BY concat(LanguageName,"aa") >> >> should all use either the column or the alias in the same manner. >> >> As we normally prefer alias for a column name in the ORDER BY, >> I think we should fix this so that we always prefer the alias name in >> all ORDER BY context. >> >> The other option would be to give an error in this case as it's not >> clear what the user wants. This is hower likely to break many old >> applictions. >> >> After talking with Sergei, he concluded: >> >> "yes, I believe now that according to the SQL:2003 (ISO/IEC >> 9075-2:2003, part 2 (foundations), 14.1<declare cursor>, Syntax >> Rules, item 18) aliases from the select list take precedence over >> column names from the table" >> >> The standard is very vague in the area, but it seams to support the >> idea that alias'es should resolve first). >> >> This means that we should fix things to always first prefer alias. >> The fix in this case needs to be done in item::fix_fields() and not in >> sql_base.cc >> >> I will ask Timour to see if he can provide a fix for this. >> (As he knows the fix_fields code quite well and he needs something to >> bite into now when he is back from his vacation). >> >> Timour, can you handle this? >> >> Regards, >> Monty >> >

1 0

[Maria-developers] WL#240 New (by Mdcallag): client-side compressed types
by worklog-noreply＠askmonty.org 03 Oct '11

03 Oct '11

----------------------------------------------------------------------- WORKLOG TASK -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- TASK...........: client-side compressed types CREATION DATE..: Fri, 30 Sep 2011, 20:31 SUPERVISOR.....: IMPLEMENTOR....: COPIES TO......: CATEGORY.......: Client-BackLog TASK ID........: 240 (http://askmonty.org/worklog/?tid=240) VERSION........: WorkLog-4.0 STATUS.........: Un-Assigned PRIORITY.......: 60 WORKED HOURS...: 0 ESTIMATE.......: 0 (hours remain) ORIG. ESTIMATE.: 0 PROGRESS NOTES: DESCRIPTION: Is it possible to add a variant of the blob types that is compressed on the client side? By this I mean that the client library handles compression and decompression so that the client code does not have to deal with it. The benefit from this is reduced load. There will be less network traffic between client/server and master/slave. This uses less space in the binlog, relay log, innodb transaction log and database files. The alternative is to use compression between client/server and master/slave but that doesn't help with the size of the binlog file nor with the innodb datafiles or log file. We have too many cases where huge text/blob values store uncompressed JSON and the bloat from that hurts performance. ESTIMATED WORK TIME ESTIMATED COMPLETION DATE ----------------------------------------------------------------------- WorkLog (v4.0.0)

4 4

[Maria-developers] WL#239 New (by Jaequery): ability to have multiple TIMESTAMP columns with CURRENT_TIMESTAMP / on_update TIMESTAMP
by worklog-noreply＠askmonty.org 28 Sep '11

28 Sep '11

----------------------------------------------------------------------- WORKLOG TASK -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- TASK...........: ability to have multiple TIMESTAMP columns with CURRENT_TIMESTAMP / on_update TIMESTAMP CREATION DATE..: Wed, 28 Sep 2011, 19:37 SUPERVISOR.....: IMPLEMENTOR....: COPIES TO......: CATEGORY.......: Client-BackLog TASK ID........: 239 (http://askmonty.org/worklog/?tid=239) VERSION........: Benchmarks-3.0 STATUS.........: Un-Assigned PRIORITY.......: 60 WORKED HOURS...: 0 ESTIMATE.......: 0 (hours remain) ORIG. ESTIMATE.: 0 PROGRESS NOTES: DESCRIPTION: Let's say I have two columns: 'created' and 'updated' Currently I can only let 'created' use the CURRENT_TIMESTAMP attribute. But ideally, I'd like 'updated' to also use CURRENT_TIMESTAMP as well as on_update TIMESTAMP. This makes a lot more sense for operations like this when a developer would have 'created', 'updated', or even 'deleted', columns. Only way we can do this right now is via triggers or (shudder) setting them from the app server. I understand there would be a underlying change and would really appreciate some kind of change for this particular behavior. ESTIMATED WORK TIME ESTIMATED COMPLETION DATE ----------------------------------------------------------------------- WorkLog (v4.0.0)

1 0

Re: [Maria-developers] Please review: [Commits] Rev 3199: Fix bug lp:858148. in file:///home/tsk/mprog/src/5.3/
by Igor Babaev 28 Sep '11

28 Sep '11

OK to push. Regards, Igor. On 09/28/2011 07:23 AM, Timour Katchaounov wrote: > Hi Igor, > > Could you please review the following fix. > It is very much related to a bug you fixed: > https://bugs.launchpad.net/bugs/702301 > > Thanks, > Timour > > -------- Original Message -------- > Return-Path: <commits-bounces(a)mariadb.org> > X-Original-To: timour(a)montyprogram.com > Delivered-To: timour(a)montyprogram.com > Received: from localhost (localhost.localdomain [127.0.0.1]) by > hasky.askmonty.org (Postfix) with ESMTP id DFD38D0DCB; Wed, 28 Sep > 2011 17:20:59 +0300 (EEST) > X-Virus-Scanned: Debian amavisd-new at mail.askmonty.org > Received: from hasky.askmonty.org ([127.0.0.1]) by localhost > (mail.askmonty.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP > id lpHLfsC2eNw9; Wed, 28 Sep 2011 17:20:59 +0300 (EEST) > Received: from hasky.askmonty.org (localhost.localdomain [127.0.0.1]) > by hasky.askmonty.org (Postfix) with ESMTP id BE79DC8236; Wed, 28 Sep > 2011 17:20:59 +0300 (EEST) > Received: by hasky.askmonty.org (Postfix) id 38FC0C8234; Wed, 28 Sep > 2011 17:20:58 +0300 (EEST) > Delivered-To: commits(a)mariadb.org > Received: from localhost (localhost.localdomain [127.0.0.1]) by > hasky.askmonty.org (Postfix) with ESMTP id 31622C8236 for > <commits(a)mariadb.org>; Wed, 28 Sep 2011 17:20:58 +0300 (EEST) > X-Virus-Scanned: Debian amavisd-new at mail.askmonty.org > Received: from hasky.askmonty.org ([127.0.0.1]) by localhost > (mail.askmonty.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP > id BvGNnoY8SHUx for <commits(a)mariadb.org>; Wed, 28 Sep 2011 17:20:52 > +0300 (EEST) > Received: from localhost6.localdomain6 (unknown [212.91.171.126]) > (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No > client certificate requested) by hasky.askmonty.org (Postfix) with > ESMTPSA id 24886C8234 for <commits(a)mariadb.org>; Wed, 28 Sep 2011 > 17:20:52 +0300 (EEST) > Content-Type: multipart/mixed; > boundary="===============0198351398646356273==" > MIME-Version: 1.0 > From: <timour(a)askmonty.org> > User-Agent: bzr/2.3.4 > To: <commits(a)mariadb.org> > Message-Id: <20110928142052.24886C8234(a)hasky.askmonty.org> > Date: Wed, 28 Sep 2011 17:20:52 +0300 (EEST) > Subject: [Commits] Rev 3199: Fix bug lp:858148. in > file:///home/tsk/mprog/src/5.3/ > X-BeenThere: commits(a)mariadb.org > X-Mailman-Version: 2.1.9 > Precedence: list > Reply-To: maria-developers(a)lists.launchpad.net > List-Id: MariaDB Commits List <commits.mariadb.org> > List-Unsubscribe: > <https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits>, > <mailto:commits-request@mariadb.org?subject=unsubscribe> > List-Archive: <http://lists.askmonty.org/pipermail/commits> > List-Post: <mailto:commits@mariadb.org> > List-Help: <mailto:commits-request@mariadb.org?subject=help> > List-Subscribe: > <https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits>, > <mailto:commits-request@mariadb.org?subject=subscribe> > Sender: commits-bounces(a)mariadb.org > Errors-To: commits-bounces(a)mariadb.org > > At file:///home/tsk/mprog/src/5.3/ > > ------------------------------------------------------------ > revno: 3199 > revision-id: timour(a)askmonty.org-20110928142043-b88ob3qytbroxdks > parent: psergey(a)askmonty.org-20110928090147-izz3ebqbcrnakndu > fixes bug(s): https://launchpad.net/bugs/858148 > committer: timour(a)askmonty.org > branch nick: 5.3 > timestamp: Wed 2011-09-28 17:20:43 +0300 > message: > Fix bug lp:858148. > > Analysis: > The crash is a result of the same cause as all similar > bugs (lp:827416, lp:718763, lp:778413, lp:806943, > lp:611690). The general pattern is that some optimization > requires the evaluation of some condition (e.g. the WHERE > clause), and this condition contains a subquery, such that > the subquery itself requires a temporary table for its > execution. During the subquery execution the original > tables in the FROM clause are replaced by the temporary > table needed for the final GROUP or ORDER operation. All > this happens during optimization of the outer query. Later > when EXPLAIN is run for the subquery, explain attempts to > print the name of the tables in the FROM clause, but it > finds there a temporary table without a corresponding > TABLE_LIST object. The attempt to print the name of a > NULL table list results in a crash. > > Solution: > This patch extends the fix to bug lp:702301, and dissalows > constant substitution of aggregate functions if the filter > condition used to check MIN/MAX keys is an expensive condition.

1 0

[Maria-developers] WL#238 New (by Snabb): Implement NOW() default value for DATETIME columns
by worklog-noreply＠askmonty.org 27 Sep '11

27 Sep '11

----------------------------------------------------------------------- WORKLOG TASK -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- TASK...........: Implement NOW() default value for DATETIME columns CREATION DATE..: Tue, 27 Sep 2011, 14:05 SUPERVISOR.....: IMPLEMENTOR....: COPIES TO......: CATEGORY.......: Server-RawIdeaBin TASK ID........: 238 (http://askmonty.org/worklog/?tid=238) VERSION........: Server-9.x STATUS.........: Un-Assigned PRIORITY.......: 60 WORKED HOURS...: 0 ESTIMATE.......: 0 (hours remain) ORIG. ESTIMATE.: 0 PROGRESS NOTES: DESCRIPTION: Have a look at MySQL bug #27645: http://bugs.mysql.com/bug.php?id=27645 Currently it is possible to set the default value of a TIMESTAMP column to CURRENT_TIME but this is not possible with DATETIME columns. Looking at that bug report it looks like I am not the only one who wishes to use DATETIME columns and have a way of setting their default value to the time when the row was created. Also TIMESTAMP DEFAULT CURRENT_TIME can occur only once in a table. It should be possible to have several "DATETIME DEFAULT NOW()" columns (as this is useful for having automatically initialized created_time and modified_time columns for a row, for example). Looks like MariaDB might gain some supporters by implementing this much-wanted feature. Currently the only decent work-around is to create a trigger which populates DATETIME columns before insert. (I hope this is the proper way to file a feature request.) ESTIMATED WORK TIME ESTIMATED COMPLETION DATE ----------------------------------------------------------------------- WorkLog (v4.0.0)

1 0

Re: [Maria-developers] [Commits] Rev 3192: Added new options to KILL. New syntax is KILL [HARD|SOFT] [CONNECTION|QUERY] [ID | USER user_name] in lp:maria/5.3
by Sergei Golubchik 26 Sep '11

26 Sep '11

Hi, Michael! On Sep 23, Michael Widenius wrote: > revno: 3192 > revision-id: monty(a)askmonty.org-20110922221338-wxnyyd5grsn689dy > parent: sergii(a)pisem.net-20110922090400-jkxe2vagjakr4lbi > committer: Michael Widenius <monty(a)askmonty.org> > branch nick: maria-5.3 > timestamp: Fri 2011-09-23 01:13:38 +0300 > message: > Added new options to KILL. New syntax is > KILL [HARD|SOFT] [CONNECTION|QUERY] [ID | USER user_name] > - If USER is given, all threads for that user is signaled > - If SOFT is used then the KILL will not be sent to the handler. Hm. So, this is the definition? "not sent to the handler"? I liked the older version of it "not interrupt an operation that leaves the table corrupted or unusable". This definition is easier to understand for the user, who does not necessarily know or care how every particular statement is split between the server and the engine (especially, as this is different for different engines). So, the point is - no matter how this SOFT is internally implemented (by not sending to the handler or whatever), what is the official definition of this feature? What should the manual say? Currently is says "will not send the signal to storage engines" which - see above - is suboptimal, in my opinion. > This can be used to not interrupt critical things in the handler > like 'REPAIR'. > > Internally added more kill signals. This gives us more information > of why a query/connection was killed. > - KILL_SERVER is used when server is going down. In this case the > users gets ER_SHUTDOWN as the reason connection was killed. > - Changed signals to number in correct order, which makes it easier > to test how the signal should affect the code. > - New error message ER_CONNECTION_KILLED if connection was killed by > 'KILL CONNECTION'. Before we got error ER_SHUTDOWN. > > Changed names of not used parameters KILL_QUERY & KILL_CONNCTION to > mysql_kill() to not conflict with defines in the server > === modified file 'include/mysql.h.pp' > --- a/include/mysql.h.pp 2011-07-21 12:50:25 +0000 > +++ b/include/mysql.h.pp 2011-09-22 22:13:38 +0000 > @@ -67,8 +67,8 @@ enum mysql_enum_shutdown_level { > SHUTDOWN_WAIT_UPDATES= (unsigned char)(1 << 3), > SHUTDOWN_WAIT_ALL_BUFFERS= ((unsigned char)(1 << 3) << 1), > SHUTDOWN_WAIT_CRITICAL_BUFFERS= ((unsigned char)(1 << 3) << 1) + 1, > - KILL_QUERY= 254, > - KILL_CONNECTION= 255 > + SHUTDOWN_KILL_QUERY= 254, > + SHUTDOWN_KILL_CONNECTION= 255 Why wouldn't you completely remove KILL_QUERY and KILL_CONNECTION from this enum ? Strictly speaking they are not "shutdown levels", they make no sense as arguments to mysql_shutdown(). And, as you've noticed, shutdown levels are completely ignored at the moment anyway - so nobody should miss these constants if you remove them. > }; > enum enum_cursor_type > { > > === modified file 'mysql-test/t/kill.test' > --- a/mysql-test/t/kill.test 2009-12-03 11:19:05 +0000 > +++ b/mysql-test/t/kill.test 2011-09-22 22:13:38 +0000 > @@ -337,5 +337,35 @@ SELECT 1; > > ########################################################################### > > +--echo # > +--echo # Test kill USER > +--echo # > + > +grant ALL on test.* to test@localhost; > +grant ALL on test.* to test2@localhost; > +connect (con3, localhost, test,,); > +connect (con4, localhost, test2,,); > +connection default; > +--enable_info > +kill hard query user test2@nohost; > +kill soft query user test@localhost; > +kill hard query user test@localhost; > +kill soft connection user test2; > +kill hard connection user test@localhost; What about real tests? When you kill soft actually kills (and doesn't kill) a statement. You can ask Philip to write them, I suppose. > === modified file 'sql/sql_parse.cc' > --- a/sql/sql_parse.cc 2011-09-10 15:01:27 +0000 > +++ b/sql/sql_parse.cc 2011-09-22 22:13:38 +0000 > @@ -7231,6 +7240,76 @@ uint kill_one_thread(THD *thd, ulong id, > } ... > +static uint kill_threads_for_user(THD *thd, LEX_USER *user, > + killed_state kill_signal, ha_rows *rows) > +{ > + THD *tmp; > + List<THD> threads_to_kill; > + DBUG_ENTER("kill_threads_for_user"); > + > + *rows= 0; > + > + if (thd->is_fatal_error) // If we run out of memory > + DBUG_RETURN(ER_OUT_OF_RESOURCES); > + > + DBUG_PRINT("enter", ("user: %s signal: %u", user->user.str, > + (uint) kill_signal)); > + > + VOID(pthread_mutex_lock(&LOCK_thread_count)); // For unlink from list > + I_List_iterator<THD> it(threads); > + while ((tmp=it++)) > + { > + if (tmp->command == COM_DAEMON) > + continue; > + /* > + Check that hostname (if given) and user name matches. > + > + host.str[0] == '%' means that host name was not given. See sql_yacc.yy > + */ > + if (((user->host.str[0] == '%' && !user->host.str[1]) || > + !strcmp(tmp->security_ctx->host, user->host.str)) && > + !strcmp(tmp->security_ctx->user, user->user.str)) > + { > + if (!(thd->security_ctx->master_access & SUPER_ACL) && > + !thd->security_ctx->user_matches(tmp->security_ctx)) So, without SUPER a user can only kill his own connections with KILL USER. A question. Should "KILL USER my_name" also kill the current connection, or it should only kill all other connections? > + { > + VOID(pthread_mutex_unlock(&LOCK_thread_count)); > + DBUG_RETURN(ER_KILL_DENIED_ERROR); > + } > + if (!threads_to_kill.push_back(tmp, tmp->mem_root)) > + pthread_mutex_lock(&tmp->LOCK_thd_data); // Lock from delete > + } > + } > + VOID(pthread_mutex_unlock(&LOCK_thread_count)); > + if (!threads_to_kill.is_empty()) > + { > + List_iterator_fast<THD> it(threads_to_kill); > + THD *ptr; > + while ((ptr= it++)) > + { > + ptr->awake(kill_signal); > + pthread_mutex_unlock(&ptr->LOCK_thd_data); > + (*rows)++; > + } > + } It may be a bit problematic. You lock thd->LOCK_thd_data for many threads. This defines a specific locking order. Which sounds quite fragile - I see no guarantee that this order cannot be violated by another thread. It can be even violated by some other feature that we'll implement in the future - and we won't notice it, even your deadlock detector won't catch it, because you create the dependency only during KILL USER, and only for specific THD's. So, if we'll ever have this locking order violated, it will show up as very random lockups in the undefined future :( I would suggest to rewrite this code (if possible) to avoid locking all thd->LOCK_thd_data's at once. Note that this code doesn't have to be fast. > + DBUG_RETURN(0); > +} > + > @@ -7241,16 +7320,33 @@ uint kill_one_thread(THD *thd, ulong id, > +void sql_kill_user(THD *thd, LEX_USER *user, killed_state state) > +{ > + uint error; > + ha_rows rows; > + if (!(error= kill_threads_for_user(thd, user, state, &rows))) I don't really understand why you've created a separate kill_threads_for_user() instead of putting the code here. But ok, as you like. > + my_ok(thd, rows); > + else > + { > + /* > + This is probably ER_OUT_OF_RESOURCES, but in the future we may > + want to write the name of the user we tried to kill > + */ > + my_error(error, MYF(0), user->host.str, user->user.str); > + } > +} > + > === modified file 'storage/archive/ha_archive.cc' > --- a/storage/archive/ha_archive.cc 2011-07-05 19:46:53 +0000 > +++ b/storage/archive/ha_archive.cc 2011-09-22 22:13:38 +0000 > @@ -17,6 +17,7 @@ > #pragma implementation // gcc: Class implementation > #endif > > +#define MYSQL_SERVER 1 why? > #include "mysql_priv.h" > #include <myisam.h> > Regards, Sergei

2 2

Re: [Maria-developers] CloudLinux patches for MariaDB
by Michael Widenius 26 Sep '11

26 Sep '11

Hi! >>>>> "Sergei" == Sergei Golubchik <serg(a)askmonty.org> writes: Sergei> Hi, Michael! Sergei> On Sep 23, Michael Widenius wrote: >> === modified file 'mysql-test/r/user_limits.result' >> --- mysql-test/r/user_limits.result 2005-05-02 18:45:06 +0000 >> +++ mysql-test/r/user_limits.result 2011-09-23 11:50:40 +0000 >> @@ -65,6 +65,16 @@ select * from t1; >> connect(localhost,mysqltest_1,,test,MYSQL_PORT,MYSQL_SOCK); >> ERROR 42000: User 'mysqltest_1' has exceeded the 'max_user_connections' resource (current value: 3) >> +grant usage on *.* to mysqltest_1@localhost with max_user_connections -1; Sergei> I still think that -1 (and 0 too) as special values look like a very Sergei> unprofessional hack. I'd rather see keywords there like Sergei> with max_user_connections UNLIMITED Sergei> with max_user_connections NONE That would be harder to document and understand when you take into account how the global variable max_user_connections work. I prefer to have these work identical. Sergei> as for the SET, SHOW VARIABLES, and command-line options - we can easily Sergei> support keywords if we declare max_user_connections to be a string Sergei> variable, not numeric, and will parse it manually. That I see would be an even great hack. The 'proper' way for the future would be to change the default value of 'max_user_connections' to MAX_INT and in the future threat 0 to mean zero connections. >> === modified file 'scripts/mysql_system_tables_fix.sql' >> --- scripts/mysql_system_tables_fix.sql 2011-05-28 02:11:32 +0000 >> +++ scripts/mysql_system_tables_fix.sql 2011-09-22 21:41:45 +0000 >> @@ -326,8 +326,11 @@ UPDATE host SET Create_routine_priv=Crea >> >> # >> # Add max_user_connections resource limit >> +# this is signed in MariaDB so that if one sets it's to -1 then the user >> +# can't connect anymore. Sergei> You need one special value (-1) and to get it you cut the number of Sergei> allowed values in half. Instead, you could've kept max_user_connections Sergei> as unsigned and only reserve (uint)~0 as "cannot connect". It's a maxint; No one will *ever* have 2~31 connections. This is ok. >> switch (var->show_type()) >> { >> - case SHOW_INT: get_sys_var_safe (uint); >> + case SHOW_INT: get_sys_var_safe (int); >> case SHOW_LONG: get_sys_var_safe (ulong); >> case SHOW_LONGLONG: get_sys_var_safe (ulonglong); >> case SHOW_HA_ROWS: get_sys_var_safe (ha_rows); Sergei> This is a bit inconsistent. INT is signed, but LONG/LONGLONG are Sergei> unsigned. Do I understand correctly that there will be no way to create Sergei> an unsigned INT variable after your patch? If you want unsigned int, you just use 'long' (Most of our variables are 'long' anyway; We have only a handful of int variables and the range of MAX_INT is good enough for these). When we ever need unsigned int, we will add a SHOW_UINT. I wanted to keep the patch minimal and as no-intrusive as possible and thats why I only fixed SHOW_INT and not SHOW_LONG and SHOW_LONGLONG. It's actually a bug that SHOW_INT didn't take negative numbers before; It's supposed to work the same way as GET_INT works in my_getopt.c, which it didn't do. >> === modified file 'sql/sql_acl.cc' >> - /* Don't allow the user to connect if he has done too many queries */ >> - if ((acl_user->user_resource.questions || acl_user->user_resource.updates || >> - acl_user->user_resource.conn_per_hour || >> - acl_user->user_resource.user_conn || max_user_connections) && >> - get_or_create_user_conn(thd, >> - (opt_old_style_user_limits ? sctx->user : sctx->priv_user), >> - (opt_old_style_user_limits ? sctx->host_or_ip : sctx->priv_host), >> - &acl_user->user_resource)) >> + /* >> + Don't allow the user to connect if he has done too many queries >> + We always have to allocated this as someone may change >> + max_user_connections any time. Sergei> What do you mean? That the original code was wrong. We can *not* do the allocation based on the original 'if' as it's assumed that max_user_connections would never change. My comments explain that the we have always to call get_or_create_user_conn() because max_user_connections depends on this structure to exists. >> + */ >> + if (get_or_create_user_conn(thd, >> + (opt_old_style_user_limits ? sctx->user : >> + sctx->priv_user), >> + (opt_old_style_user_limits ? sctx->host_or_ip >> + : sctx->priv_host), >> + &acl_user->user_resource)) >> DBUG_RETURN(1); // The error is set by get_or_create_user_conn() >> } >> else >> >> === modified file 'sql/sql_connect.cc' >> --- sql/sql_connect.cc 2011-09-22 22:13:38 +0000 >> +++ sql/sql_connect.cc 2011-09-22 22:58:07 +0000 >> @@ -113,8 +113,11 @@ int check_for_max_user_connections(THD * >> DBUG_ENTER("check_for_max_user_connections"); >> >> (void) pthread_mutex_lock(&LOCK_user_conn); >> + >> + /* Root is not affected by the value of max_user_connections */ Sergei> It's an incompatible change. Why did you do it? If the point was to Sergei> allow SUPER connections when max_user_connections==-1, then it would be Sergei> enough to make only this value (-1) to be ignored by the SUPER user, and Sergei> any other value limit should still apply. root users should not be affected by the global variables max_user_connections or max_connections. Yes, it's incompatible, but ok and necessary in this case as otherwise no one could login if we set global.max_user_connections=1 which would not let you maintain the server at all. Sergei> SUPER user typically ignores features that prevent him from connecting Sergei> to the server. E.g incorrect init-connect option may do it. Or a normal Sergei> user may use up all max_connections limit and prevent SUPER from Sergei> connecting. That's why SUPER ignores init-connect and is allowed one Sergei> connection above max_connections. Sergei> But max_user_connections can not prevent SUPER from connecting (unless Sergei> it is -1). If it is set to, for example, 5, SUPER can have five Sergei> simultaneous connections and no normal user can prevent him from doing Sergei> it. This is why SUPER does *not* ignore max_user_connections. Sergei> So, by this logic, only the value max_user_connections==-1 should be Sergei> ignored by SUPER, and not any other value. I think it's better to not regard any value as special. It's easier to document it the current way; Doing it your way would make it hard to document and understand and that usually means it's a bad idea. (For example, what happens if you set max_user_connections=-1 and then later change it to 1; What limits apply to the root user that is now logged in twice). If you want to constrain the root user, you can do that with GRANT root ... max_user_connections=# Regards, Monty

1 0