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
