maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #08901
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