MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Sergei Golubchik Date:March 14 2008 8:02pm
Subject:Re: bk commit into 6.0 tree (antony:1.2573) WL#3771
View as plain text  
Hi!

On Feb 29, Antony T Curtis wrote:
> On 27 Feb, 2008, at 12:52, Sergei Golubchik wrote:
> 
> >>--- /dev/null	Wed Dec 31 16:00:00 196900
> >>+++ b/include/mysql/plugin_audit.h	2008-02-22 08:56:22 -08:00
> >>@@ -0,0 +1,78 @@
> >.....
> >>+#define MYSQL_AUDIT_CLASS_MASK_SIZE 1
> >>+typedef unsigned long long  
> >>mysql_audit_mask_t[MYSQL_AUDIT_CLASS_MASK_SIZE];
> >>+
> >>+struct st_mysql_audit
> >>+{
> >>+  int interface_version;
> >>+  const mysql_audit_mask_t *class_mask;
> >
> >sorry, I don't understand - a pointer to an array ?
> >You had a pointer to ulonglong, I suggested to change a pointer to
> >an array. But now you have a pointer to an array. One dereference  
> >too much.
> 
> Pointer to an array, same amount of dereferencing as original pointer  
> to ulonglong except that array is now not unbounded.

okay, but why not to have this array in the st_mysql_audit structure ?
 
> >>+  void (*release_thd)(MYSQL_THD);
> >>+  void (*event_notify)(MYSQL_THD, const struct mysql_event *);
> >>+};
> >>+
> >>diff -Nrup a/sql/mysqld.cc b/sql/mysqld.cc
> >>--- a/sql/mysqld.cc	2008-02-14 01:00:48 -08:00
> >>+++ b/sql/mysqld.cc	2008-02-22 08:56:20 -08:00
> >>@@ -1239,6 +1240,7 @@ extern "C" void unireg_abort(int exit_co
> >>  clean_up(!opt_help && (exit_code || !opt_bootstrap)); /* purecov: 
> inspected */
> >>  DBUG_PRINT("quit",("done with cleanup in unireg_abort"));
> >>  wait_for_signal_thread_to_end();
> >>+  mysql_audit_finalize();
> >
> >why did you move it from clean_up_mutexes() ?
> >
> >>  clean_up_mutexes();
> >>  my_end(opt_endinfo ? MY_CHECK_ERROR | MY_GIVE_INFO : 0);
> >>  exit(exit_code); /* purecov: inspected */
> >>@@ -4313,6 +4325,7 @@ int main(int argc, char **argv)
> >>#endif
> >>  clean_up(1);
> >>  wait_for_signal_thread_to_end();
> >>+  mysql_audit_finalize();
> >
> >Cannot you have mysql_audit_finalize() in one place ?
> 
> It seemed to interfere with my aesthetic sense having it in  
> clean_up_mutexes() because all the other lines in that function are  
> pthread_mutex_destroy() calls.

okay, I can see that.

Could you try to find a place for this call to have it only once and
still not interfere with your aesthetic sense ?
 
> >>  clean_up_mutexes();
> >>  my_end(opt_endinfo ? MY_CHECK_ERROR | MY_GIVE_INFO : 0);
> >>
> >>diff -Nrup a/sql/scheduler.cc b/sql/scheduler.cc
> >>--- a/sql/scheduler.cc	2007-12-13 09:17:25 -08:00
> >>+++ b/sql/scheduler.cc	2008-02-22 08:56:20 -08:00
> >>@@ -666,6 +667,8 @@ static bool libevent_needs_immediate_pro
> >>static void libevent_thd_add(THD* thd)
> >>{
> >>  char c=0;
> >>+  /* release any audit resources, this thd is going to sleep */
> >>+  mysql_audit_release(thd);
> >
> >Why do you need it here ?
> >1. Why not to do it in do_command(), at the end of the statement
> >processing, that is ?
> >2. Having it in libevent_thd_add implies that it depends on the
> >scheduler.  We'll need to patch every new scheduler that could be
> >created, then ?
> 
> minor optimization, don't bother releasing audit plugins when you're  
> busy executing statements. releasing is deferred until thd is idle.  
> reduces overall overhead, especially in a busy machine.

When will you release audit plugins in the old thread-per-connection
scheduler ?
 
> >>  pthread_mutex_lock(&LOCK_thd_add);
> >>  /* queue for libevent */
> >>  thds_need_adding= list_add(thds_need_adding, &thd->scheduler.list);

Regards / Mit vielen Grüssen,
Sergei

-- 
   __  ___     ___ ____  __
  /  |/  /_ __/ __/ __ \/ /   Sergei Golubchik <serg@stripped>
 / /|_/ / // /\ \/ /_/ / /__  Principal Software Developer/Server Architect
/_/  /_/\_, /___/\___\_\___/  MySQL GmbH, Dachauer Str. 37, D-80335 München
       <___/                  Geschäftsführer: Kaj Arnö - HRB
München 162140
Thread
bk commit into 6.0 tree (antony:1.2573) WL#3771antony22 Feb
  • Re: bk commit into 6.0 tree (antony:1.2573) WL#3771Sergei Golubchik27 Feb
    • Re: bk commit into 6.0 tree (antony:1.2573) WL#3771Antony T Curtis29 Feb
      • Re: bk commit into 6.0 tree (antony:1.2573) WL#3771Sergei Golubchik14 Mar
        • Re: bk commit into 6.0 tree (antony:1.2573) WL#3771Antony T Curtis14 Mar
          • Re: bk commit into 6.0 tree (antony:1.2573) WL#3771Sergei Golubchik17 Mar