← 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, Oleksandr!

On Sep 15, Oleksandr Byelkin wrote:
> >> 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 :)
> What else I can check? I have an idea that in diferent threads the TIDs 
> are different, but it will fail on platforms where it is not supported 
> and so equal to zero.

Yes, you can move it to a linux-specific test file.
But a test that succeeds with a feature disabled is not of much use.

> >> 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.
> I checked and it works, to make it working I added:
> 
> +#cmakedefine HAVE_SYS_SYSCALL_H 1
> 
> to  config.h.cmake

Right, but you still need that CHECK_INCLUDE_FILES() line.
Currently it works only because tokudb checks for sys/syscall.h.
If you do a clean build without tokudb, you code won't work again.

Regards,
Sergei



References