- developers - lists.mariadb.org

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 27 Sep '11

27 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

[Maria-developers] SOCKET_SIZE_TYPE question on accept() call
by Steve Ellcey 24 Sep '11

24 Sep '11

Whild building mariadb on HP-UX, I ran into a problem with a call to accept(). The mariadb configure script uses MYSQL_TYPE_ACCEPT to determine if the third argument to accept should be type socklen_t or type int, and sets SOCKET_SIZE_TYPE accordingly, but in socket_accept (plugin/handler_socket/libhsclient/socket.cpp), the third argument to the accept call (addrlen_r) is hardcoded to be type socklen_t instead of using SOCKET_SIZE_TYPE. I think that the addrlen_r argument to socket_accept should be type SOCKET_SIZE_TYPE instead of socklent_t. I think this also means that the addrlen field in the socket_args structure (plugin/handler_socket/libhsclient/socket.hpp) should be type SOCKET_SIZE_TYPE instead of socklen_t. Does that sound right? Steve Ellcey sje(a)cup.hp.com

2 4

[Maria-developers] WL#237 New (by DonQuichote): more IF EXISTS clauses
by worklog-noreply＠askmonty.org 23 Sep '11

23 Sep '11

----------------------------------------------------------------------- WORKLOG TASK -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- TASK...........: more IF EXISTS clauses CREATION DATE..: Fri, 23 Sep 2011, 14:10 SUPERVISOR.....: IMPLEMENTOR....: COPIES TO......: CATEGORY.......: Server-RawIdeaBin TASK ID........: 237 (http://askmonty.org/worklog/?tid=237) VERSION........: WorkLog-4.0 STATUS.........: Un-Assigned PRIORITY.......: 60 WORKED HOURS...: 0 ESTIMATE.......: 0 (hours remain) ORIG. ESTIMATE.: 0 PROGRESS NOTES: DESCRIPTION: It would be nice if more statements were repeatable for use in scripts (See http://www.howtoforge.com/node/4833 for the use of this). I would like to see the IF (NOT) EXISTS clauses added to the RENAME command and it would really be nice if such a clause could be added to the various operations of the ALTER TABLE command. For example: RENAME TABLE IF EXISTS WrongName TO RightName; -- when placed in an update script, it will rename the table if it still has the wrong name, but it will not fail if it already has the right name. ALTER TABLE SomeTable ADD COLUMN IF NOT EXISTS ExtraColumn VARCHAR(255) DEFAULT NULL; ESTIMATED WORK TIME ESTIMATED COMPLETION DATE ----------------------------------------------------------------------- WorkLog (v4.0.0)

1 0

[Maria-developers] MariaDB 5.2.9
by Daniel Bartholomew 23 Sep '11

23 Sep '11

All, Monty asked me to prep the MariaDB 5.2.9 release. He said to use the build of revision 3033 for the release: http://buildbot.askmonty.org/buildbot/grid?branch=5.2&category=package http://bazaar.launchpad.net/~maria-captains/maria/5.2/revision/3033 ...and I've uploaded the files from that build to the primary mirror. They should be mirrored worldwide by tomorrow. But before I can finish the release and announce it I need a couple of things: First, I need one of the developers to tag the release in the source tree: http://bazaar.launchpad.net/~maria-captains/maria/5.2/changes Second, I need to know if there is anything that I should be sure to mention in the release notes. One thing I can think of which hasn't been mentioned in the past few release notes is which version of MySQL that this version of MariaDB has merged in. The current versions of other things, like PBXT and XtraDB would also be helpful. Thanks. -- Daniel Bartholomew MariaDB - http://mariadb.org Monty Program - http://montyprogram.com AskMonty Knowledgebase - http://kb.askmonty.org

2 2

[Maria-developers] Building MariaDB 3.1 Beta on HP-UX
by Steve Ellcey 22 Sep '11

22 Sep '11

Hi, I have downloaded the MariaDB 3.1 Beta sources and am trying to build MariaDB on HP-UX 11.31 using the HP compilers and ran into a few problems that I wanted to ask about. The first problem I ran into was with anonymous unions (and structs). Anonymous unions are part of the C++ standard, but not C (though the GNU C compiler allows them). The header file include/ma_dyncol.h has a structure, st_dynamic_column_value, which includes both anonymous unions and anonymous structures and this file is included in both C files (mysys/ma_dyncol.c and unittest/mysys/ma_dyncol-t.c) and C++ files (sql/item_strfunc.cc). This is causing the HP C compiler problems since it doesn't allow anonymous unions (or structs) by default. Now I can use a GNU compatibility mode in the compiler (-Agcc) to allow this to compile, but that creates other problems, specifically it causes the MariaDB configure script to think I am using GCC (because __GNUC__ is defined in this mode) and then it runs the ABI check which fails because the HP compiler doesn't generate preprocessed source that exactly matches the expected GNU compiler output due to minor changes in the output. So I was wondering if this use of anonymous unions in C code was intentional or unintentional? I was also wondering about the use of the anonymous structs in the anonymous union as that is not part of the C or C++ standards, though the GNU compilers only complain about this when you use the -pedantic flag. What solution to this issue would people recommend? We could use the GNU compatibility mode with the HP compilers but would have to modify the configure script to not run the ABI check. Or we could remove the use of anonymous unions (and structs) from the C code in MariaDB. I prefer the second option because it makes the C code more consistent with the C99 standard. Steve Ellcey sje(a)cup.hp.com

2 3

[Maria-developers] MySQL Bug 59449 - Accepted by MariaDB?
by Jeremy Thomerson 21 Sep '11

21 Sep '11

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 The "[11 Jun 3:16] Jeremy Thomerson" comment will be the most useful. Please CC me on any replies to this thread because I am not subscribed to this mailing list. Best Regards, Jeremy Thomerson

3 4

[Maria-developers] Microseconds support in MariaDB 5.3.
by Peter Laursen 21 Sep '11

21 Sep '11

I noticed: http://askmonty.org/blog/the-2-year-old-mariadb/ "The support for microseconds in TIMESTAMP, DATETIME, and TIME". I fully agree the 'full seconds support only' has been a sever limitation in MySQL. But I have two concerns with ths: 1) 'noisyness'. On not very fast systems the last digits of a TIME in microseconds should not be considered too much important. It depends on what else the system is doing at the time. I think for lots of users milliseconds and not microseconds would be better/more relevant. It is not always a lot of digits can be proven true. When we went to school we were all taught not to present results in natural sciences more accurate than the accuracy of data sources and measurements. Providing too many digits in a resuslt actually caused a lower score in my highschool! 2) Compability. There is a problem with UPDATES. A client/application written for MySQL may assume 'full seconds support only' and may generate the current MySQL format byte format in the WHERE clause (typically if the client reserves a 14 byte buffer for TIMERSTAMPS etc). In SQL: UPDATE table SET column = 'something' WHERE time_column = '01:01:01': .. this update will fail if the currently stored value is '01:01:01:010001'; and the rigth query in the context would be. UPDATE table SET column = 'something' WHERE time_column = '01:01:01:010001'; Ideally I think there should be a configuration parameter and a global variable to SET time_accuracy = full_second|millisecond|microsecond. Did somebody have considerations about same? I definitely think that lots of applications using a TIME|TIMESTAMP|DATETIME column in the WHERE-clause of an UPDATE statement will need some rewrite to be upgradable to MariaDB 5.3 -- Peter

4 5

Re: [Maria-developers] [Commits] Rev 3020: MWL#192: Non-blocking client API for libmysqlclient. in http://bazaar.launchpad.net/~maria-captains/maria/5.2
by Kristian Nielsen 20 Sep '11

20 Sep '11

Hi Monty, Please find included the patch for MWL#192, non-blocking client API, for review. This patch is against MariaDB 5.2.8, though I expect the feature will eventually be merged and pushed to MariaDB 5.5. One thing in particular I would like to ask for advice on is if there are ways to improve the integration with libmysql/NET/Vio, as you know this code much better than me. - Kristian.

2 4

[Maria-developers] strange lines in test_if_skip_sort_order()
by Sergei Golubchik 14 Sep '11

14 Sep '11

Hi. At the end of test_if_skip_sort_order() you put (in 5.3): 17959 /* 17960 orig_cond is a part of pre_idx_push_cond, 17961 no need to restore it. 17962 */ 17963 orig_cond= 0; 17964 orig_cond_saved= false; 17965 if (orig_cond_saved && !changed_key) 17966 tab->set_cond(orig_cond); Besides incorrect indentation, you set orig_cond_saved to false, and in the next line writes an if() statement that can only work if orig_cond_saved is true. If orig_cond_saved=false is correct, then this if() can be completely removed. Or, may be, orig_cond_saved should not be set to false? Regards, Sergei

1 0