[MDEV-4911] add KILL query id, and add query id information to processlist Created: 2013-08-17  Updated: 2013-10-03  Resolved: 2013-09-13

Status: Closed
Project: MariaDB Server
Component/s: None
Fix Version/s: 10.0.5

Type: Task Priority: Major
Reporter: roberto spadim Assignee: Sergey Vojtovich
Resolution: Fixed Votes: 0
Labels: None

Attachments: Text File diff_10_0_4_mdev4911_lex.h     Text File diff_10_0_4_mdev4911_sql_class.h     Text File diff_10_0_4_mdev4911_sql_parse.cc     File diff_10_0_4_mdev4911_sql_yacc.yy     Text File diff_10_0_4_sql_show.cc     Text File mdev-4911.patch    
Issue Links:
Relates
relates to MDEV-5096 Wrong error message on attempt to kil... Closed

 Description   

This MDEV comes from a sync problem about SHOW PROCESSLIST and KILL command, i will add two parts to this MDEV:


FIRST PART:
let me explain the problem:

there's two connections (1) and (2) and they will execute some commands:

1) SELECT * FROM A_BIG_TABLE_THAT_WILL_EXPEND_MANY_TIME
2) SHOW PROCESSLIST /* WHAT HAPPENED WITH MARIADB? USERS ARE REPORTING SYSTEM LOCKED OR SLOW!? */

THREAD ID COMMAND
10 SELECT * FROM A BIG TABLE....
11 SHOW PROCESSLIST

1) BEGIN TRANSACTION
2) KILL 10 /* WELL THE SELECT IS KILLING MY SERVER, I WILL KILL IT! */

1) NOOOOOOOOOOO! MY BEGIN TRANSACTION WAS KILLED!
2) NOOOOOOOOOOO! I SENT KILL COMMAND TO WRONG QUERY!

=] removing the last two commands, the problem here is how to kill the right query, and not the right thread...

well contacting Sergei Golubchik and maria-developer / maria-discussiong mail list, i think the best way is:

1) add a new parameter to KILL command, the new syntax is:

KILL [ CONNECTION [ QUERY [ ID ] ] | QUERY [ ID ] ] id 
or (same thing) 
KILL [ CONNECTION | CONNECTION QUERY [ ID ] | QUERY | QUERY ID ] id 

examples:
today KILL commands:

KILL QUERY 1;  -> this will kill thread id 1, but don't disconnect client
KILL 1; or KILL CONNECTION 1;  -> this will kill thread 1 and disconnect client

new KILL commands:

KILL QUERY ID 1; -> this will kill query id 1, but don't disconnect client
KILL CONNECTION QUERY ID 1; KILL CONNECTION QUERY 1; -> this will kill query id 1, and disconnect client

I was thinking about add a redundant command with thread id + query id, and asked to sergei...
since query id is a big int (or a 32 bit int) we have low probability of kill a wrong query id, since the show processlist and the kill command normally take <1 second for scripts/programs, and take <10 seconds for a dba sql interface, and considering a very busy server (1.000.000 qps) we have many time to the query id be the same, for 32 bit overflow we have more than should have 2^32 (4.294.967.296) queries, more than 10 seconds (10.000.000 queries) with busy server...
resuming... we WILL NOT add the redundant command (thread id + query id) just the query id command

2) we WILL NOT change the protocol kill command
(http://dev.mysql.com/doc/internals/en/com-process-kill.html#packet-COM_PROCESS_KILL), just change the SQL (COM_QUERY) KILL command

that's all! good job =)


SECOND PART: (done with diff_10_0_4_sql_show.cc file)
add the QUERY ID column to processlist

SELECT * FROM information_schema.PROCESSLIST

ID QUERY_ID USER HOST DB COMMAND TIME STATE INFO TIME_MS STAGE MAX_STAGE PROGRESS MEMORY_USED EXAMINED_ROWS
2 10 rspadim 179.213.3.11:30165 information_schema Query 0 executing select * from processlist 0.240 0 0 0.000 83288 0


important commend from mail list:

Justin Swanhart
greenlion@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.


from sql_class.h (10.0.4)

~ line 2304 - THD class
  /*
    Id of current query. Statement can be reused to execute several queries
    query_id is global in context of the whole MySQL server.
    ID is automatically generated from mutex-protected counter.
    It's used in handler code for various purposes: to check which columns
    from table are necessary for this select, to check if it's necessary to
    update auto-updatable fields (like auto_increment and timestamp).
  */
  query_id_t query_id;


TODO:
some things that must be done:​

1) the KILL QUERY_ID 1,2,3,4,5 don't work, that's my first patch with sql_yacc.yy maybe i done something wrong since %expect changed +8 numbers

2) the QUERY_ID isn't what i want but well worked hehehehe, maybe change the syntax and make it more beautiful

3) there's some warnings in gcc that must be checked (signed compare with unsigned), and others case) that don't use the right flag (i add a bit = 16 for KILL_QUERY command)

4) the KILL QUERY_ID <non exists query id>, return a error something like "thread id not found", must be "query id not found"

5) i don't remember if have other problem...



 Comments   
Comment by Sergei Golubchik [ 2013-08-18 ]

Actually, I was saying

KILL [ CONNECTION | QUERY [ ID ] ] id

that is, allowing only variants as below:

KILL thread_id
KILL CONNECTION thread_id
KILL QUERY thread_id
KILL QUERY ID query_id

with no variant that allows to kill a connection using a query id.

Comment by roberto spadim [ 2013-08-18 ]

hahaha sorry
i didn't saw that "id" don't have "thread/query" before it

but this don't add the
KILL CONNECTION QUERY ID query_id (or something to kill the connection with
a query id)

maybe this is better:
KILL [ CONNECTION [QUERY [ID]] | QUERY [ ID ] ] id
or (same thing)
KILL [ CONNECTION | CONNECTION QUERY [ID] | QUERY | QUERY ID ] id

Comment by roberto spadim [ 2013-08-18 ]

description rewrite to allow the new syntax

Comment by roberto spadim [ 2013-08-18 ]

there's some problem killing a connection with a query id? i'm talking about internally in source code...

Comment by roberto spadim [ 2013-08-18 ]

must test

Comment by roberto spadim [ 2013-08-18 ]

developers check that i'm adding the IF_IDLE patch before this one...
the sql_yacc.yy, have a
+%expect 186
in if_idle patch (MDEV-4916)
i didn't start the new .yy but this number will be based on MDEV-4916

Comment by roberto spadim [ 2013-08-18 ]

this patch is blocked by MDEV-4916
it expect that the last value of sql_yacc.yy is "%expect 186"

from MDEV-4916 patch:
-%expect 185
+%expect 186

Comment by roberto spadim [ 2013-08-18 ]

TODO:
add the parser... i'm not good in .yy files but i will try...

the main problem now is:
set the
lex->kill_type = KILL_TYPE_QUERY

everything else is done

Comment by roberto spadim [ 2013-08-18 ]

for now it do the job, but i don'tk now if parser is ok...
i added a QUERY_ID, instead of a beutifull language....
KILL QUERY_ID id
KILL CONNECTION QUERY_ID id
KILL QUERY QUERY_ID id

Comment by roberto spadim [ 2013-08-18 ]

kill query_id 14;
/* Erro SQL (1094): Unknown thread id: 14 */

must understand how to add a new error mensage
in this case it should give "Unknown query id: 14" (change thread to query)

Comment by roberto spadim [ 2013-08-18 ]

kill query_id 14, 15
don't work... just one query id/command

Comment by roberto spadim [ 2013-08-18 ]

well i will sleep it's 8AM now! bye

Comment by roberto spadim [ 2013-08-23 ]

Hi sergey, i`m stalled here, and don`t know how to continue and solve this problems

1) the KILL QUERY_ID 1,2,3,4,5 don't work, that's my first patch with sql_yacc.yy maybe i done something wrong since %expect changed +8 numbers

2) the QUERY_ID isn't what i want but well worked hehehehe, maybe change the syntax and make it more beautiful

3) there's some warnings in gcc that must be checked (signed compare with unsigned), and others case) that don't use the right flag (i add a bit = 16 for KILL_QUERY command), is this the right way to include new kill option?

4) the KILL QUERY_ID <non exists query id>, return a error something like "thread id not found", must be "query id not found"

5) i don't remember if have other problem... could you check?

thanks

Comment by Sergey Vojtovich [ 2013-09-11 ]

Sergei, please review patch for this task.

Comment by Sergey Vojtovich [ 2013-09-12 ]

Patch attached (original patch didn't reach commits@).

Comment by roberto spadim [ 2013-09-12 ]

nice when possible i will test it
one point that i didn't found on unit tests, did you tested the kill with many ids?
example:
kill query id 1,2,3,4,5

Comment by Sergey Vojtovich [ 2013-09-13 ]

Pushed to 10.0-base, revision-id: svoj@mariadb.org-20130913161456-g6z5rgwoczfsvdrj

Comment by roberto spadim [ 2013-09-13 ]

hi sergey patch read, just one point (considering that syntax is ok )

4) the KILL QUERY_ID <non exists query id>, return a error something like "thread id not found", must be "query id not found"

Comment by Sergey Vojtovich [ 2013-09-13 ]

Ehm, I was going to fix it, but missed somehow. Thanks for pointing it out.

Comment by roberto spadim [ 2013-09-13 ]

no problem

Generated at Thu Feb 08 07:00:09 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.