← Back to team overview

maria-developers team mailing list archive

Re: cbfa5f6: MDEV-11177 mysqlbinlog exits silently without error when another

 

Hi, Alexey!

On Mar 13, Alexey Botchkov wrote:
> revision-id: cbfa5f6a6d48b3b755462eb0df5e34c63de78f98 (mariadb-10.2.4-46-gcbfa5f6)
> parent(s): eded6243bc4796ab44e70403edd059d32225f589
> committer: Alexey Botchkov
> timestamp: 2017-03-13 18:26:37 +0400
> message:
> 
> MDEV-11177 mysqlbinlog exits silently without error when another
> instance connects to server.
> 
>         New thread kill status added KILL_SLAVE_SAME_ID, and the related
>         error message.
> 
> diff --git a/sql/sql_class.h b/sql/sql_class.h
> index 22895d7..986cb1c 100644
> --- a/sql/sql_class.h
> +++ b/sql/sql_class.h
> @@ -478,16 +478,22 @@ enum killed_state
>    KILL_TIMEOUT= 8,
>    KILL_TIMEOUT_HARD= 9,
>    /*
> +    When binlog reading thread connects to the server it kills
> +    all the binlog threads with the same ID.
> +  */
> +  KILL_SLAVE_SAME_ID= 10,
> +  /*
>      All of the following killed states will kill the connection
>      KILL_CONNECTION must be the first of these and it must start with
>      an even number (becasue of HARD bit)!
>    */
> -  KILL_CONNECTION= 10,
> -  KILL_CONNECTION_HARD= 11,
> -  KILL_SYSTEM_THREAD= 12,
> -  KILL_SYSTEM_THREAD_HARD= 13,
> -  KILL_SERVER= 14,
> -  KILL_SERVER_HARD= 15
> +  KILL_CONNECTION= 12,
> +  KILL_CONNECTION_HARD= 13,
> +  KILL_SYSTEM_THREAD= 14,
> +  KILL_SYSTEM_THREAD_HARD= 15,
> +  KILL_SERVER= 16,
> +  KILL_SERVER_HARD= 17,

Okay. I don't really like it, because KILL_CONNECTION/KILL_QUERY is
how a user can specify what to kill, and KILL_SLAVE_SAME_ID is not that.
But looking at the current enum killed_state, I see that its values
already tell
  * _what_ to kill: KILL_CONNECTION, KILL_QUERY
  * _how_ to kill: KILL_HARD
  * _why_ to kill: KILL_TIMEOUT, KILL_SERVER, etc
all in one enum.

Your KILL_SLAVE_SAME_ID is another one in the "why to kill" list, it
adds to this mess, but it doesn't make it much worse, I'd say.

This needs to be cleaned up, though.

A question - are you sure there enum values are not visible to clients,
not part of the API? If you've checked that and they are not - ok to
push.

If you'd like to clean up KILL_* values (in a separate commit, of
course), a very simple change would be to make it into a bitmap. Like
first bit is "how to kill" (hard/not hard). Then one bit for "what to
kill" - connection/query. The rest is for why. There'll be helpers like

#define is_hard_kill(X)         ((X) & HARD_KILL_BIT)
#define is_kill_query(X)        ((X) & KILL_QUERY_BIT)
#define is_kill_connection(X)   (!is_kill_query(X))
#define kill_reason(X)          ((enum kill_reason)((X) >> 2))

Something like that. This doesn't change much internally, only the
presentation and the way of thinking about it changes.

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx