maria-developers team mailing list archive

[Oleksandr Byelkin <sanja@xxxxxxxxxxxxxxxx>]

Hi!

On 12.09.15 21:35, Sergei Golubchik wrote:
> Hi, Sanja!
>
> On Aug 12, sanja@xxxxxxxxxxx wrote:
> > revision-id: 6811945eaf6a6e979d92739399b084bf4e00ae2d (mariadb-10.1.6-12-g6811945e)
> > parent(s): 86a3613d4e981c341e38291c9eeec5dc9f836fae
> > committer: Oleksandr Byelkin
> > timestamp: 2015-08-12 23:09:48 +0200
> > message:
> >
> > MDEV-6756: map a linux pid (child pid) to a connection id shown in the output of SHOW PROCESSLIST
> >
> > Added tid & pid for Linux.
> >
> > diff --git a/mysql-test/r/information_schema.result b/mysql-test/r/information_schema.result
> > index 2a918ad..5b3370b 100644
> > --- a/mysql-test/r/information_schema.result
> > +++ b/mysql-test/r/information_schema.result
> > @@ -2066,3 +2066,18 @@ Variable_name	Value
> >   Opened_tables	3
> >   drop database mysqltest;
> >   drop database db1;
> > +SHOW PROCESSLIST;
> > +Id	User	Host	db	Command	Time	State	Info	Progress
> > +#	root	#	NULL	Query	#	#	SHOW PROCESSLIST	0.000
> > +set @@old_mode="";
> > +SHOW PROCESSLIST;
> Eh, no, please, don't change SHOW PROCESSLIST.
> Only add new columns to I_S.PROCESSLIST. And without OLD_MODE.
>
> > diff --git a/sql/CMakeLists.txt b/sql/CMakeLists.txt
> > index 033a0e0..d1fbaa3 100644
> > --- a/sql/CMakeLists.txt
> > +++ b/sql/CMakeLists.txt
> > @@ -143,6 +143,10 @@ SET (SQL_SOURCE
> >                  ${MYSYS_LIBWRAP_SOURCE}
> >   			   )
> >     
> > +IF (CMAKE_SYSTEM_NAME MATCHES "Linux")
> > +  ADD_DEFINITIONS(-DHAVE_TID_SYSCALL)
> > +ENDIF()
> Don't match OS name please, better use
>
>    #ifdef HAVE_SYS_SYSCALL_H
>    #include <sys/syscall.h>
>    #endif
>    ...
>    #ifdef __NR_gettid
>    ...
>    #endif
Are there way to check that it has thread ID syscall particulary (I am 
afraid that most OSes has syscall)?
> > +
> >   IF (CMAKE_SYSTEM_NAME MATCHES "Linux" OR
> >       CMAKE_SYSTEM_NAME MATCHES "Windows" OR
> >       CMAKE_SYSTEM_NAME MATCHES "SunOS" OR
> > diff --git a/sql/sql_class.cc b/sql/sql_class.cc
> > index bf161b2..20c03fe 100644
> > --- a/sql/sql_class.cc
> > +++ b/sql/sql_class.cc
> > @@ -2062,6 +2068,13 @@ bool THD::store_globals()
> >       This allows us to move THD to different threads if needed.
> >     */
> >     mysys_var->id= thread_id;
> > +#ifdef HAVE_TID_SYSCALL
> > +  os_pid= getpid();
> why would you want to have pid there? isn't it the same for all threads?
No, it (PID) differs from thread on 1 and it is what actually shown in 
the process list.

My understanding is that each thread group has own PID, and now we have 
threads each in its own group, so they have both PID and thread ID 
different and unique.

Thread IDs are really unique ID for thread, but I was not able to make 
ps show it. It shows PID in TID column at least on my computer, so I 
decided to have them both is the best variant.
> > +  os_thread_id= syscall(__NR_gettid);
> > +#else
> > +  os_pid= 0;
> > +  os_thread_id= 0;
> > +#endif
> >     real_id= pthread_self();                      // For debugging
> >     mysys_var->stack_ends_here= thread_stack +    // for consistency, see libevent_thread_proc
> >                                 STACK_DIRECTION * (long)my_thread_stack_size;
> I didn't review the rest of the patch as without old_mode and SHOW PROCESSLIST
> changes most of it will disappear.
>
> Regards,
> Sergei