[MDEV-12009] Allow to force kill user threads/query which are flagged as high priority by Galera Created: 2017-02-07  Updated: 2020-12-30  Resolved: 2020-12-30

Status: Closed
Project: MariaDB Server
Component/s: Galera
Fix Version/s: N/A

Type: Task Priority: Major
Reporter: Claudio Nanni Assignee: Jan Lindström (Inactive)
Resolution: Won't Fix Votes: 0
Labels: galera

Issue Links:
Blocks
is blocked by MDEV-18464 Port kill_one_trx fixes from 10.4 to ... Closed

 Description   

See also: https://github.com/codership/galera/issues/448

In some cases Galera can mark a user thread as high priority(TOI DDLs) making it unkillable (with misleading message at MDEV-12008)
I think it should be allowed a way to kill such thread/query under the responsibility of the DBA.
Syntax could be:
KILL [CONNECTION | QUERY] processlist_id [FORCE]

 Comments   
Comment by Jan Lindström (Inactive) [ 2018-07-19 ]

This is a feature request. bar How hard it would be to add this new FORCE keyword for kill and does it require change on storage engine API or can it be added to current structures as a additional info ?

Comment by Alexander Barkov [ 2018-07-19 ]

jplindst From a glance it looks like something not hard to do.

If I understand the code correctly, the change should be done in kill_one_thread() in this code:

    if (((thd->security_ctx->master_access & SUPER_ACL) ||
 
        thd->security_ctx->user_matches(tmp->security_ctx)) &&
 
        !wsrep_thd_is_BF(tmp, false))
 
    {
 
      tmp->awake_no_mutex(kill_signal);
 
      error=0;
 
    }
 
    else
 
      error= (type == KILL_TYPE_QUERY ? ER_KILL_QUERY_DENIED_ERROR :
 
                                        ER_KILL_DENIED_ERROR);

Most likely it is wsrep_thd_is_BF() who decides if the thread is kill-able.
But I'm not fully sure. There are no comments for wsrep_thd_is_BF() in the code.

I suggest you discuss with psergey and serg.

When adding a new FORCE flag, please consider introducing a new class Sql_cmd_kill instead of adding this flag directly to LEX, as well ass moving LEX::kill_type and LEX::kill_signal into this new class Sql_cmd_kill. I can help with this refactoring.

Comment by Sergei Golubchik [ 2018-07-19 ]

Why is this check wsrep_thd_is_BF there in the first place? It directly contradicts the comment above it, that says

    /*
 
      If we're SUPER, we can KILL anything, including system-threads.
 
      No further checks.
 
      ...
 
  */

Comment by Alexander Barkov [ 2018-07-25 ]

ralf.gebhardt@mariadb.com, it's probably a bug. See Sergei's comment. Perhaps a super user should be able to kill such threads even without this new FORCE flag. In such case it's just 5 minutes.
If we still want to add FORCE, then it is 4 hours.

Comment by Jan Lindström (Inactive) [ 2018-09-13 ]

serg Does thd->security_ctx->user_matches(tmp->security_ctx)) match with current user or superuser ?

Comment by Sergei Golubchik [ 2018-09-19 ]

thd->security_ctx->user_matches(tmp->security_ctx)) means that thd->security_ctx has the same user as tmp->security_ctx.
thd->security_ctx is the current user.
But I don't see how it's relevant for the current issue.

Comment by Jan Lindström (Inactive) [ 2019-02-04 ]

I will allow SUPER to kill both high priority transactions as well as wsrep applier threads as it should be.

Comment by Marko Mäkelä [ 2019-03-28 ]

I reverted this change, because I think that it depends on a correct fix of MDEV-18464.

Comment by Jan Lindström (Inactive) [ 2020-12-30 ]

Killing wsrep high priority threads is dangerous and could lead to cluster inconsistent state. Thus it should not be allowed.

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