[Maria-developers] review of mdev-4911
Hi. Good work. I only have one small comment about the code, see below. And one about the syntax.
=== modified file 'sql/sql_parse.cc' --- sql/sql_parse.cc 2013-08-14 08:48:50 +0000 +++ sql/sql_parse.cc 2013-09-11 12:29:32 +0000 @@ -7139,24 +7139,60 @@ THD *find_thread_by_id(ulong id)
/** - kill on thread. + Find a thread by query id and return it, locking it LOCK_thd_data + + @param id Identifier of the query we're looking for + + @return NULL - not found + pointer - thread found, and its LOCK_thd_data is locked. +*/ + +static THD *find_thread_by_query_id(longlong id)
why didn't you reuse find_thread_by_id()? this function is almost identical to it.
+{ + THD *tmp; + mysql_mutex_lock(&LOCK_thread_count); // For unlink from list + I_List_iterator<THD> it(threads); + while ((tmp=it++)) + { + if (tmp->get_command() == COM_DAEMON) + continue; + if (tmp->query_id == id) + { + mysql_mutex_lock(&tmp->LOCK_thd_data); // Lock from delete + break; + } + } + mysql_mutex_unlock(&LOCK_thread_count); + return tmp; +} + + === modified file 'sql/sql_yacc.yy' --- sql/sql_yacc.yy 2013-08-13 11:35:36 +0000 +++ sql/sql_yacc.yy 2013-09-11 12:29:32 +0000 @@ -12890,6 +12891,11 @@ kill_expr: Lex->users_list.push_back($2); Lex->kill_type= KILL_TYPE_USER; } + | ID_SYM expr + { + Lex->value_list.push_front($2); + Lex->kill_type= KILL_TYPE_QUERY; + }
So, you implemented KILL [ CONNECTION | QUERY ] [ ID ] expr It allows, in particular KILL CONNECTION ID 10 which looks strange, why would that mean that 10 is a query id? I originally suggested KILL [ CONNECTION | QUERY [ ID ] ] expr to allow only KILL CONNECTION expr KILL QUERY expr KILL QUERY ID expr because in this case it's quite clear "QUERY ID" and because I thought it's a bit strange to kill a connection by query_id. If you want to allow that (as I saw in the comments, Roberto didn't thought it's strange to kill a connection by query id), may be you'd better use KILL [ CONNECTION | QUERY ] [ QUERY_ID ] expr ? Regards, Sergei
Hi Sergei, thanks for your comments, answers inline. On Thu, Sep 12, 2013 at 04:27:57PM +0200, Sergei Golubchik wrote:
Hi.
Good work. I only have one small comment about the code, see below. And one about the syntax.
=== modified file 'sql/sql_parse.cc' --- sql/sql_parse.cc 2013-08-14 08:48:50 +0000 +++ sql/sql_parse.cc 2013-09-11 12:29:32 +0000 @@ -7139,24 +7139,60 @@ THD *find_thread_by_id(ulong id)
/** - kill on thread. + Find a thread by query id and return it, locking it LOCK_thd_data + + @param id Identifier of the query we're looking for + + @return NULL - not found + pointer - thread found, and its LOCK_thd_data is locked. +*/ + +static THD *find_thread_by_query_id(longlong id)
why didn't you reuse find_thread_by_id()? this function is almost identical to it. For no good reason, I'll fix it.
+{ + THD *tmp; + mysql_mutex_lock(&LOCK_thread_count); // For unlink from list + I_List_iterator<THD> it(threads); + while ((tmp=it++)) + { + if (tmp->get_command() == COM_DAEMON) + continue; + if (tmp->query_id == id) + { + mysql_mutex_lock(&tmp->LOCK_thd_data); // Lock from delete + break; + } + } + mysql_mutex_unlock(&LOCK_thread_count); + return tmp; +} + + === modified file 'sql/sql_yacc.yy' --- sql/sql_yacc.yy 2013-08-13 11:35:36 +0000 +++ sql/sql_yacc.yy 2013-09-11 12:29:32 +0000 @@ -12890,6 +12891,11 @@ kill_expr: Lex->users_list.push_back($2); Lex->kill_type= KILL_TYPE_USER; } + | ID_SYM expr + { + Lex->value_list.push_front($2); + Lex->kill_type= KILL_TYPE_QUERY; + }
So, you implemented KILL [ CONNECTION | QUERY ] [ ID ] expr It allows, in particular
KILL CONNECTION ID 10
which looks strange, why would that mean that 10 is a query id?
I originally suggested KILL [ CONNECTION | QUERY [ ID ] ] expr to allow only
KILL CONNECTION expr KILL QUERY expr KILL QUERY ID expr
because in this case it's quite clear "QUERY ID" and because I thought it's a bit strange to kill a connection by query_id.
If you want to allow that (as I saw in the comments, Roberto didn't thought it's strange to kill a connection by query id), may be you'd better use
KILL [ CONNECTION | QUERY ] [ QUERY_ID ] expr ?
Same here. I was a bit scared by amount of affected test cases, so decided to submit "early" patch. Thanks, Sergey
Hi Guys! Some mariadb's user comments :) 2013/9/12 Sergey Vojtovich <svoj@mariadb.org>
Hi Sergei,
thanks for your comments, answers inline.
On Thu, Sep 12, 2013 at 04:27:57PM +0200, Sergei Golubchik wrote:
Hi.
Good work. I only have one small comment about the code, see below. And one about the syntax.
=== modified file 'sql/sql_parse.cc' --- sql/sql_parse.cc 2013-08-14 08:48:50 +0000 +++ sql/sql_parse.cc 2013-09-11 12:29:32 +0000 @@ -7139,24 +7139,60 @@ THD *find_thread_by_id(ulong id)
/** - kill on thread. + Find a thread by query id and return it, locking it LOCK_thd_data + + @param id Identifier of the query we're looking for + + @return NULL - not found + pointer - thread found, and its LOCK_thd_data is locked. +*/ + +static THD *find_thread_by_query_id(longlong id)
why didn't you reuse find_thread_by_id()? this function is almost identical to it.
That was the patch that i sent on MDEV, just a raw idea, but worked, maybe a copy and paste hehe =)
For no good reason, I'll fix it.
+{ + THD *tmp; + mysql_mutex_lock(&LOCK_thread_count); // For unlink from list + I_List_iterator<THD> it(threads); + while ((tmp=it++)) + { + if (tmp->get_command() == COM_DAEMON) + continue; + if (tmp->query_id == id)
there was a warning in gcc about unsigned and signed when i put the patch in jira, but i didn't removed maybe "if ((longlong) (tmp->query_id)==id)" could remove the warning, but i don't know if it's ok
+ { + mysql_mutex_lock(&tmp->LOCK_thd_data); // Lock from delete + break; + } + } + mysql_mutex_unlock(&LOCK_thread_count); + return tmp; +} + + === modified file 'sql/sql_yacc.yy' --- sql/sql_yacc.yy 2013-08-13 11:35:36 +0000 +++ sql/sql_yacc.yy 2013-09-11 12:29:32 +0000 @@ -12890,6 +12891,11 @@ kill_expr: Lex->users_list.push_back($2); Lex->kill_type= KILL_TYPE_USER; } + | ID_SYM expr + { + Lex->value_list.push_front($2); + Lex->kill_type= KILL_TYPE_QUERY; + }
So, you implemented KILL [ CONNECTION | QUERY ] [ ID ] expr It allows, in particular
KILL CONNECTION ID 10
I like the QUERY_ID with underscore to know that we are talking about the query_id, not the thread_id example: KILL CONNECTION QUERY_ID 99999 KILL QUERY QUERY_ID 99999
which looks strange, why would that mean that 10 is a query id?
I originally suggested KILL [ CONNECTION | QUERY [ ID ] ] expr to allow only
KILL CONNECTION expr KILL QUERY expr KILL QUERY ID expr
because in this case it's quite clear "QUERY ID" and because I thought it's a bit strange to kill a connection by query_id.
If you want to allow that (as I saw in the comments, Roberto didn't thought it's strange to kill a connection by query id), may be you'd better use
For me, it's ok to kill connection or query using the query id or the thread id, both are nice solutions the query_id can change while we read the processlist and send the kill command, while the thread_id many times don't i want kill using the query id as parameter not the thread id, if the query isn't what i want, mariadb will send an error or a warning, that's what i expect i'm new to source code of mariadb, it's difficult for me to change bison files, i tested but was very confusing to me now (maybe in future i could change it easier) i think a nice nice syntax could be: KILL [CONNECTION | CONNECTION QUERY ID | QUERY | QUERY ID ] exp just an idea I think that QUERY_ID is easier to implement in bison than QUERY ID, but i'm a begginner :P
KILL [ CONNECTION | QUERY ] [ QUERY_ID ] expr ? Same here. I was a bit scared by amount of affected test cases, so decided to submit "early" patch.
wow! a lot of test changes! i didn't found a kill test with many ids, for example: KILL QUERY ID 1,2,3,4,5 in my patch it didn't worked :P and i don't know how to allow it i think the "KILL 1,2,3,4,5" is allowed for KILL command based on threads, arent? could be nice the same syntax for kill query_id :) since it's the "same" KILL command, but using different "WHERE" parts another question :) maybe in futures MDEV could be nice something like: KILL QUERY_ID IN (1,2,3,4,5) or KILL QUERY_ID IN (SELECT QUERY_ID FROM information_schema.PROCESSLIST WHERE ..... ) KILL THREAD_ID IN (1,2,3,4,5) or KILL THREAD_ID IN (SELECT ID FROM information_schema.PROCESSLIST WHERE ..... ) KILL CONNECTION QUERY_ID IN (1,2,3,4,5) or KILL CONNECTION QUERY_ID IN (SELECT QUERY_ID FROM information_schema.PROCESSLIST WHERE ..... ) KILL CONNECTION THREAD_ID IN (1,2,3,4,5) or KILL CONNECTION THREAD_ID IN (SELECT ID FROM information_schema.PROCESSLIST WHERE ..... ) it's another mdev, but what you think about it?
Thanks, Sergey
_______________________________________________ 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
thanks guys! you are the best! -- Roberto Spadim
Hi Roberto, comments inline. On Thu, Sep 12, 2013 at 12:15:13PM -0300, Roberto Spadim wrote:
Hi Guys! Some mariadb's user comments :)
2013/9/12 Sergey Vojtovich <svoj@mariadb.org>
Hi Sergei,
thanks for your comments, answers inline.
On Thu, Sep 12, 2013 at 04:27:57PM +0200, Sergei Golubchik wrote:
Hi.
Good work. I only have one small comment about the code, see below. And one about the syntax.
=== modified file 'sql/sql_parse.cc' --- sql/sql_parse.cc 2013-08-14 08:48:50 +0000 +++ sql/sql_parse.cc 2013-09-11 12:29:32 +0000 @@ -7139,24 +7139,60 @@ THD *find_thread_by_id(ulong id)
/** - kill on thread. + Find a thread by query id and return it, locking it LOCK_thd_data + + @param id Identifier of the query we're looking for + + @return NULL - not found + pointer - thread found, and its LOCK_thd_data is locked. +*/ + +static THD *find_thread_by_query_id(longlong id)
why didn't you reuse find_thread_by_id()? this function is almost identical to it.
That was the patch that i sent on MDEV, just a raw idea, but worked, maybe a copy and paste hehe =) Yes, I used some of your patches. Thanks for sharing them.
For no good reason, I'll fix it.
+{ + THD *tmp; + mysql_mutex_lock(&LOCK_thread_count); // For unlink from list + I_List_iterator<THD> it(threads); + while ((tmp=it++)) + { + if (tmp->get_command() == COM_DAEMON) + continue; + if (tmp->query_id == id)
there was a warning in gcc about unsigned and signed when i put the patch in jira, but i didn't removed maybe "if ((longlong) (tmp->query_id)==id)" could remove the warning, but i don't know if it's ok
In your patch `id` is ulong and `query_id` is int64. In my patch `id` is longlong, so it is not affected.
+ { + mysql_mutex_lock(&tmp->LOCK_thd_data); // Lock from delete + break; + } + } + mysql_mutex_unlock(&LOCK_thread_count); + return tmp; +} + + === modified file 'sql/sql_yacc.yy' --- sql/sql_yacc.yy 2013-08-13 11:35:36 +0000 +++ sql/sql_yacc.yy 2013-09-11 12:29:32 +0000 @@ -12890,6 +12891,11 @@ kill_expr: Lex->users_list.push_back($2); Lex->kill_type= KILL_TYPE_USER; } + | ID_SYM expr + { + Lex->value_list.push_front($2); + Lex->kill_type= KILL_TYPE_QUERY; + }
So, you implemented KILL [ CONNECTION | QUERY ] [ ID ] expr It allows, in particular
KILL CONNECTION ID 10
I like the QUERY_ID with underscore to know that we are talking about the query_id, not the thread_id example: KILL CONNECTION QUERY_ID 99999 KILL QUERY QUERY_ID 99999
I'll leave it up to you to convince Serg to use your syntax. :) My preference is to kill connection by thread_id and query by query_id, because I normally either want to stop a particular query, or stop all activity in particular connection. But it is incompatible change.
which looks strange, why would that mean that 10 is a query id?
I originally suggested KILL [ CONNECTION | QUERY [ ID ] ] expr to allow only
KILL CONNECTION expr KILL QUERY expr KILL QUERY ID expr
because in this case it's quite clear "QUERY ID" and because I thought it's a bit strange to kill a connection by query_id.
If you want to allow that (as I saw in the comments, Roberto didn't thought it's strange to kill a connection by query id), may be you'd better use
For me, it's ok to kill connection or query using the query id or the thread id, both are nice solutions the query_id can change while we read the processlist and send the kill command, while the thread_id many times don't i want kill using the query id as parameter not the thread id, if the query isn't what i want, mariadb will send an error or a warning, that's what i expect
i'm new to source code of mariadb, it's difficult for me to change bison files, i tested but was very confusing to me now (maybe in future i could change it easier) i think a nice nice syntax could be: KILL [CONNECTION | CONNECTION QUERY ID | QUERY | QUERY ID ] exp just an idea I think that QUERY_ID is easier to implement in bison than QUERY ID, but i'm a begginner :P
KILL [ CONNECTION | QUERY ] [ QUERY_ID ] expr ? Same here. I was a bit scared by amount of affected test cases, so decided to submit "early" patch.
wow! a lot of test changes!
i didn't found a kill test with many ids, for example: KILL QUERY ID 1,2,3,4,5 in my patch it didn't worked :P and i don't know how to allow it i think the "KILL 1,2,3,4,5" is allowed for KILL command based on threads, arent? could be nice the same syntax for kill query_id :) since it's the "same" KILL command, but using different "WHERE" parts
another question :) maybe in futures MDEV could be nice something like: KILL QUERY_ID IN (1,2,3,4,5) or KILL QUERY_ID IN (SELECT QUERY_ID FROM information_schema.PROCESSLIST WHERE ..... ) KILL THREAD_ID IN (1,2,3,4,5) or KILL THREAD_ID IN (SELECT ID FROM information_schema.PROCESSLIST WHERE ..... ) KILL CONNECTION QUERY_ID IN (1,2,3,4,5) or KILL CONNECTION QUERY_ID IN (SELECT QUERY_ID FROM information_schema.PROCESSLIST WHERE ..... ) KILL CONNECTION THREAD_ID IN (1,2,3,4,5) or KILL CONNECTION THREAD_ID IN (SELECT ID FROM information_schema.PROCESSLIST WHERE ..... )
it's another mdev, but what you think about it?
Heh, what about DELETE FROM INFORMATION_SCHEMA.PROCESSLIST WHERE ...? But I guess that'll be quite complex. Anyway we didn't yet implement multi-process kill. Feel free to create another MDEV. Regards, Sergey
Hi Sergey! thanks a lot showing a patch with unit test/sql test and many other things i don't understand how to do :)
That was the patch that i sent on MDEV, just a raw idea, but worked, maybe
a copy and paste hehe =) Yes, I used some of your patches. Thanks for sharing them.
MariaDB team is very very wellcome =)
there was a warning in gcc about unsigned and signed when i put the patch in jira, but i didn't removed maybe "if ((longlong) (tmp->query_id)==id)" could remove the warning, but i don't know if it's ok In your patch `id` is ulong and `query_id` is int64. In my patch `id` is longlong, so it is not affected.
hummm, i used pentium64 (xeon E31240) linux 3.6.9, gcc 4.7.2 glibc 2.16, mariadb-10.0.3 source code, maybe its different? i will test again at weekend with last mariadb source and check if i done something wrong, but if it's ok no problem =)
So, you implemented KILL [ CONNECTION | QUERY ] [ ID ] expr
It allows, in particular
KILL CONNECTION ID 10
I like the QUERY_ID with underscore to know that we are talking about the query_id, not the thread_id example: KILL CONNECTION QUERY_ID 99999 KILL QUERY QUERY_ID 99999 I'll leave it up to you to convince Serg to use your syntax. :)
i checked that "QUERY ID" is "more difficult" to implement than "QUERY_ID" at .yy files, i'm wrong? My preference is to kill connection by thread_id and query by query_id,
because I normally either want to stop a particular query, or stop all activity in particular connection. But it is incompatible change.
there's mysql work with this kind of syntax? i didn't found it at bug.mysql at mail list, Justin at Percona, talked about patchs in others forks maybe a single unique syntax is better than maridb only syntax, check message from MDEV description (copied from mail list): Justin Swanhartgreenlion@gmail.com Percona, Inc Hi, KILL THREAD THREAD_ID WITH QUERY_ID QUERY_ID and KILL QUERY THREAD_ID WITH QUERY_ID QUERY_ID and possibly KILL QUERY WITH QUERY_ID should be supported. This is a very important and missing feature which is included in other forks. I_S.PROCESS_LIST should be extended to include QUERY_ID (you can get query_id from SHOW commands but not I_S). The above KILL commands if the QUERY_ID no longer exists on the system, thus you can KILL a SELECT without worrying that it has moved on to creating a new transaction or statement. i didn't checked percona server, but since justin is reported as percona, maybe he's talking about something that exists i prefer mariadb as main mysql server, maybe doing this new syntax here we will 'export' to all others forks i know it'is time to think about what's better in parser speed/complexity and sql human readable format, right? -- other point about KILL CONNECTION QUERY_ID and KILL QUERY QUERY_ID, and why I worked in this MDEV ... i don't have problems about syntax, i have problem about allowing a kill connection based in a query_id. solving this will solve my problem My today scenario: I have at least three or four systems running in one server, and when a connection is broken (disconnect) the system (mainly php scripts) will reconnect to mariadb and continue the work, it's like a batch that can't stop, just pause, wait and continue later (10~60 seconds), or when used in web it's stoped and user must retry request if i turn off background programs, they will continue from last ok query, it's similar to a background mail queue dispatcher, that's why when connection is droped (tcp/ip or network problem) i restart the job, and when killed it wait a bit there're some queries that have a "/* -- S++ ABORTABLE -- */" or "/* -- S++ SU ABORTABLE -- */" comment, example: /* -- S++ SU ABORTABLE -- pid:27001 spd_user: rspadim SELF='/rdm-business/app/config.manutencao.spadim' ms=0 ip=186.222.25.x<186.222.25.204>xx session=mt2ite.6b305o19s3o0msb9h54cgw408gkgcc04o4 */SELECT SUM(quant),SUM(pecas),SUM(pbruto),SUM(pliq),SUM(vbruto),SUM(vliq),SUM(custo) FROM est_mov WHERE estoque_entrada_un=1000 in this case "est_mov" table have >10million rows (~14GB) and can cause server to slow down, in other words i need to kill this connection and script can't restart it, killing the query i just make script to restart the comment "/* ABORTABLE */" allow program to be canceled via "KILL command" at web php interface, but like any web interface i have some times between informations,like: (with heidisql or mysql command line i have some times too) 1) SHOW PROCESSLIST (~1 ms) 2) send processlist to web client (chrome browser) (~100ms) 3) user select what query to kill (1 ~10 seconds) 4) chrome browser send the http request to server (~20ms) 5) server execute the show processlist to know if the query can or can't be killed based on /* abortable */ comment (~2ms) 6) server kill the query (~1ms) (KILL CONNECTION xxxx) my problem isn't the program allowing a kill command since it can restart the work or stop, it's not a problem the "problem" is the boring time lost at a wrong kill command, since i use persistent connections at php, and a thread running a script can be used in another script without changing it thread_id (can be confirmed at show processlist) my problem is sending a kill command to the wrong thread since i'm using the thread_id to kill the connection and not the query_id, check i use "kill connection xxx" not "kill query xxx" i'm not using the threadpool yet and i don't know how processlist is reported with thread pool, is the id isn't unique in this case (using threadpool)? if not or yes, no problem, we will have query_id to make it unique now :), but think about killing a connection with threadpool without query_id, maybe i could kill the wrong connection, not? i don't know if "kill connection query_id xxx" add extra complexity, but in my case it's a "must have" feature to allow a better (more precise) user kill command, the kill query will not do the work, since the query is be reexecuted instead of canceled (when connection is dropped/killed) well if we could allow a "kill connection query_id xxx", this is nice to me, if not i will use "kill connection xxxx" without query_id and users will kill wrong connections (ok i can patch the server and solve this problem, but a 'native' solution is better =) ) --- thinking about threadpool... internally i know that killing the thread "isn't nice", maybe a interface to send a request to kill query_id xx to thread yyy is better (more precise) today we do something like: for(first thread to last thread) if(query_id==kill_query_id) kill thread (check that we don't send to thread the query_id parameters, since we are at server side, it's too fast to changed query_id in only one instruction but possible, right?) i think we should do something like: for(first thread to last thread) if(query_id==kill_query_id) request_thread_to_kill_query(thread_id,query_id) and inside request_thread_to_kill_query( function, check if it have the query_id at threadpool, and kill it (kill connection or kill query) i don't know if it's what mariadb/mysql should do inside code when using threadpool, i'm using only one process per connection, and don't have this kind of problem other doubt now... when we have a daemon process (plugin) there's a query id for it? query_id=0? in this case we only have an "unique query id" with thread_id+query_id? maybe we should avoid the KILL QUERY_ID = 0
KILL QUERY_ID IN (1,2,3,4,5) or KILL QUERY_ID IN (SELECT QUERY_ID FROM information_schema.PROCESSLIST WHERE ..... ) KILL THREAD_ID IN (1,2,3,4,5) or KILL THREAD_ID IN (SELECT ID FROM information_schema.PROCESSLIST WHERE ..... ) KILL CONNECTION QUERY_ID IN (1,2,3,4,5) or KILL CONNECTION QUERY_ID IN (SELECT QUERY_ID FROM information_schema.PROCESSLIST WHERE ..... ) KILL CONNECTION THREAD_ID IN (1,2,3,4,5) or KILL CONNECTION THREAD_ID IN (SELECT ID FROM information_schema.PROCESSLIST WHERE ..... )
it's another mdev, but what you think about it? Heh, what about DELETE FROM INFORMATION_SCHEMA.PROCESSLIST WHERE ...? But I guess
another question :) maybe in futures MDEV could be nice something like: that'll be quite complex.
yes i think it's complex too, and i don't know many things about mariadb source code, but i know the complexity creating a new engine just to information_schema, maybe it's a nice engine, but maybe just a lot of work again about sintax... maybe a WHERE could be added to KILL instead of a DELETE FROM INFORMATION_SCHEMA... KILL [CONNECTION | QUERY] [WHERE some_fields some_operators some_values and_no_subquery | <thread_id> | QUERY_ID <query_id>] about WHERE, we could use the same fields of show processlist: ID, USER, HOST, DB, COMMAND, TIME, STATE, INFO, TIME_MS, STAGE, MAX_STAGE, PROGRESS, MEMORY_USED, EXAMINED_ROWS, QUERY_ID Anyway we didn't yet implement multi-process kill. Feel free to create
another MDEV.
No problem, i will create it, but... I put one patch in MDEV-4917 with the IF_IDLE flag, but i think it's not nice :/ This could be done with a newer idea using KILL WHERE, and the KILL 1,2,3,4 could use it too, for example KILL WHERE thread_id IN (1,2,3,4,5) The WHERE is much more flexible and human readable, and we could kill without seeing what is happening, for example... today i MUST get information_schema.PROCESSLIST to know what is running in mysql/mariadb server A kill command with WHERE will kill the same queries but with less network traffic and less interaction, example: KILL WHERE command='sleep' will kill all queries that are Idle, instead of: SHOW PROCESSLIST read processlist with a perl/python/php/or any other program/script/human (dba), and create many queries KILL IF_IDLE QUERY_ID 1 KILL IF_IDLE QUERY_ID 2 KILL IF_IDLE QUERY_ID 3 ... i'm not using in a high production server with thread pool or many connection, but i'm thinking about it with kill 1000 idle process, you will need 1000 kill command + 1 show processlist, this can be reduced with kill where query_id in (1,2,3,4) or better with only one KILL WHERE status='idle', without a lot of network traffic that's a point to a new MDEV i know for now, the query_id could solve my problems of killing a connection/query without killing the wrong client
Regards, Sergey
sorry i wrote in poor english, it's not my main language =] thanks guys! -- Roberto Spadim
Hi, Roberto! On Sep 13, Roberto Spadim wrote:
My preference is to kill connection by thread_id and query by query_id, because I normally either want to stop a particular query, or stop all activity in particular connection. But it is incompatible change.
there's mysql work with this kind of syntax? i didn't found it at bug.mysql at mail list, Justin at Percona, talked about patchs in others forks maybe a single unique syntax is better than maridb only syntax, check message from MDEV description (copied from mail list):
Justin Swanhartgreenlion@gmail.com Percona, Inc
KILL THREAD THREAD_ID WITH QUERY_ID QUERY_ID and KILL QUERY THREAD_ID WITH QUERY_ID QUERY_ID and possibly KILL QUERY WITH QUERY_ID
should be supported. This is a very important and missing feature which is included in other forks.
It's not in MySQL 5.6 and not in Percona Server 5.5 or 5.6. I did not check Google patches, Facebook patches and other sets of MySQL patches, though.
my problem isn't the program allowing a kill command since it can restart the work or stop, it's not a problem the "problem" is the boring time lost at a wrong kill command, since i use persistent connections at php, and a thread running a script can be used in another script without changing it thread_id (can be confirmed at show processlist) my problem is sending a kill command to the wrong thread since i'm using the thread_id to kill the connection and not the query_id, check i use "kill connection xxx" not "kill query xxx"
Okay... This makes sense. If you use a connection pool that, indeed, connection id does not correspond to a logical connection. Still, while KILL CONNECTION QUERY_ID is kind of ok, KILL QUERY QUERY_ID is very silly. I'd rather allow subqueries in KILL, to support KILL CONNECTION (select thread_id from information_schema.processlist where ...) then you won't need to kill by query id or state or if_idle - you can have everything in the where clause. but this is a larger change that we cannot do 10.0, we simply don't have time for it. We could try to do it in 10.1 though, properly and generally, so that you can kill using as complex conditions as you like. Instead of creating many limited shortcut syntax variants for special cases.
i'm not using the threadpool yet and i don't know how processlist is reported with thread pool, is the id isn't unique in this case (using threadpool)?
Unique. Every connection has its own connection id, with or without thread pool. Internal scheduling implementation doesn't affect user-visible connection ids.
i don't know if it's what mariadb/mysql should do inside code when using threadpool, i'm using only one process per connection, and don't have this kind of problem
threadpool doesn't have this problem either.
other doubt now... when we have a daemon process (plugin) there's a query id for it?
No.
query_id=0? in this case we only have an "unique query id" with thread_id+query_id? maybe we should avoid the KILL QUERY_ID = 0
Right, thanks.
again about sintax... maybe a WHERE could be added to KILL instead of a DELETE FROM INFORMATION_SCHEMA...
KILL [CONNECTION | QUERY] [WHERE some_fields some_operators some_values and_no_subquery | <thread_id> | QUERY_ID <query_id>]
about WHERE, we could use the same fields of show processlist:
ID, USER, HOST, DB, COMMAND, TIME, STATE, INFO, TIME_MS, STAGE, MAX_STAGE, PROGRESS, MEMORY_USED, EXAMINED_ROWS, QUERY_ID
I'd rather allow subqueries there, it'll be much more natural. Regards, Sergei
participants (3)
-
Roberto Spadim -
Sergei Golubchik -
Sergey Vojtovich