← Back to team overview

maria-developers team mailing list archive

Re: [Commits] 7f7cbb1: MDEV-6756: map a linux pid (child pid) to a connection id shown in the output of SHOW PROCESSLIST

 

Hi, Sanja!

There's an issue with cmake detection and with tests.
But the code looks fine, thanks!

On Sep 14, sanja@xxxxxxxxxxx wrote:
> revision-id: 7f7cbb132fa0a40687e4b25a9c9e163c93ec6d15 (mariadb-10.1.6-12-g7f7cbb1)
> parent(s): 86a3613d4e981c341e38291c9eeec5dc9f836fae
> committer: Oleksandr Byelkin
> timestamp: 2015-09-14 12:47:57 +0200
> message:
> 
> MDEV-6756: map a linux pid (child pid) to a connection id shown in the
> output of SHOW PROCESSLIST
> 
> Added tid (thread ID) for system where it is present.
> 
>  ps -eL -o tid,pid,command
> 
> shows the thread on Linux
> 
> diff --git a/mysql-test/r/information_schema.result b/mysql-test/r/information_schema.result
> index 2a918ad..ef889c7 100644
> --- a/mysql-test/r/information_schema.result
> +++ b/mysql-test/r/information_schema.result
> @@ -2066,3 +2066,7 @@ Variable_name	Value
>  Opened_tables	3
>  drop database mysqltest;
>  drop database db1;
> +set @TID= (SELECT tid FROM information_schema.processlist);
> +select (SELECT tid FROM information_schema.processlist) = @TID;
> +(SELECT tid FROM information_schema.processlist) = @TID
> +1

That seems pretty useless as a test :)

> diff --git a/sql/sql_class.cc b/sql/sql_class.cc
> index bf161b2..0478b33 100644
> --- a/sql/sql_class.cc
> +++ b/sql/sql_class.cc
> @@ -69,6 +69,10 @@
>  #include "sql_connect.h"
>  #include "my_atomic.h"
>  
> +#ifdef HAVE_SYS_SYSCALL_H

Pardon me? Did you see whether your code actually work?
You check here whether HAVE_SYS_SYSCALL_H is defined, but you don't have

  CHECK_INCLUDE_FILES(sys/syscall.h HAVE_SYS_SYSCALL_H)

in configure.cmake and HAVE_SYS_SYSCALL_H in config.h.cmake. So, I don't
see how your code can possibly work as HAVE_SYS_SYSCALL_H should be
never defined in your patch.

Please, when fixing it, make sure that your test *fails* without TID
support (I suspect you'll have to enable it only on linux).

And then you could remove that useless test above.

> +#include <sys/syscall.h>
> +#endif

Regards,
Sergei


Follow ups