Merge lp:~fallenpegasus/drizzle/logbugfix into lp:~drizzle-trunk/drizzle/development

[Mark Atwood]

[Mark Atwood (fallenpegasus)]

fixes a logging bug
added PCRE matching to log-to-file
makes the log-to-file plugin be more C++'ish

[Brian Aker (brianaker)]

Hi!

One of your comments mentions "unloading". Plugins unload during deinit, so you can fix the cleanup issue there.

Also, when is the file descriptor closed? You removed that during the deinit.

Cheers,
   -Brian

review: Needs Information

[lp:~fallenpegasus/drizzle/logbugfix]

1046. By Mark Atwood on 2009-06-04

merge from main

1047. By Mark Atwood on 2009-06-05

add destructor to logging_query, to close filehandle and clean up PCRE stuff

[Mark Atwood (fallenpegasus)]

Done. The deinit calls the destructor, and the destructore closes the file descriptor and cleans up the PCRE allocation.s

[lp:~fallenpegasus/drizzle/logbugfix]

1048. By Mark Atwood on 2009-06-05

make logging_syslog be more C++'ish

1049. By Mark Atwood on 2009-06-05

make logging_gearman plugin be more C++'ish

=== modified file 'drizzled/logging.cc'
--- drizzled/logging.cc	2009-05-11 17:50:22 +0000
+++ drizzled/logging.cc	2009-05-27 20:05:51 +0000
@@ -112,7 +112,7 @@
   /* Use find_if instead of foreach so that we can collect return codes */
   vector<Logging_handler *>::iterator iter=
     find_if(all_loggers.begin(), all_loggers.end(),
-            LoggingPreIterate(session)); 
+            LoggingPostIterate(session)); 
   /* If iter is == end() here, that means that all of the plugins returned
    * false, which in this case means they all succeeded. Since we want to 
    * return false on success, we return the value of the two being != 

=== modified file 'plugin/logging_gearman/logging_gearman.cc'
--- plugin/logging_gearman/logging_gearman.cc	2009-05-11 17:50:22 +0000
+++ plugin/logging_gearman/logging_gearman.cc	2009-05-27 20:41:17 +0000
@@ -202,7 +202,8 @@
     msgbuf_len=
       snprintf(msgbuf, MAX_MSG_LEN,
                "%"PRIu64",%"PRIu64",%"PRIu64",\"%.*s\",\"%s\",\"%.*s\","
-               "%"PRIu64",%"PRIu64",%"PRIu64",%"PRIu64",%"PRIu64"",
+               "%"PRIu64",%"PRIu64",%"PRIu64",%"PRIu64",%"PRIu64
+               "%"PRIu32",%"PRIu32"",
                t_mark,
                session->thread_id,
                session->query_id,
@@ -220,7 +221,9 @@
                (t_mark - session->start_utime),
                (t_mark - session->utime_after_lock),
                session->sent_row_count,
-               session->examined_row_count);
+               session->examined_row_count,
+               session->tmp_table,
+               session->total_warn_count);
   
     char job_handle[GEARMAN_JOB_HANDLE_SIZE];
   

=== modified file 'plugin/logging_query/logging_query.cc'
--- plugin/logging_query/logging_query.cc	2009-05-11 17:50:22 +0000
+++ plugin/logging_query/logging_query.cc	2009-05-27 23:16:02 +0000
@@ -21,19 +21,19 @@
 #include <drizzled/plugin/logging_handler.h>
 #include <drizzled/gettext.h>
 #include <drizzled/session.h>
+#include <pcre.h>
 
 /* TODO make this dynamic as needed */
 static const int MAX_MSG_LEN= 32*1024;
 
 static bool sysvar_logging_query_enable= false;
 static char* sysvar_logging_query_filename= NULL;
-/* TODO fix these to not be unsigned long one we have sensible sys_var system */
+static char* sysvar_logging_query_pcre= NULL;
+/* TODO fix these to not be unsigned long once we have sensible sys_var system */
 static unsigned long sysvar_logging_query_threshold_slow= 0;
 static unsigned long sysvar_logging_query_threshold_big_resultset= 0;
 static unsigned long sysvar_logging_query_threshold_big_examined= 0;
 
-static int fd= -1;
-
 /* stolen from mysys/my_getsystime
    until the Session has a good utime "now" we can use
    will have to use this instead */
@@ -164,17 +164,50 @@
   return dst;
 }
 
+/* TODO, write a matcng destructor.  But since plugins are never unlaoded, necessary? */
 
-/* we could just not have a pre entrypoint at all,
-   and have logging_pre == NULL
-   but we have this here for the sake of being an example */
 class Logging_query: public Logging_handler
 {
+  int fd;
+  pcre *re;
+  pcre_extra *pe;
+
 public:
-  Logging_query() : Logging_handler("Logging_query") {}
+
+  Logging_query() : Logging_handler("Logging_query"), fd(-1), re(NULL), pe(NULL)
+  {
+
+    /* if there is no destination filename, dont bother doing anything */
+    if (sysvar_logging_query_filename == NULL)
+      return;
+
+    fd= open(sysvar_logging_query_filename,
+             O_WRONLY | O_APPEND | O_CREAT,
+             S_IRUSR|S_IWUSR);
+    if (fd < 0)
+    {
+      errmsg_printf(ERRMSG_LVL_ERROR, _("fail open() fn=%s er=%s\n"),
+                    sysvar_logging_query_filename,
+                    strerror(errno));
+      return;
+    }
+
+    if (sysvar_logging_query_pcre != NULL)
+    {
+      const char *this_pcre_error;
+      int this_pcre_erroffset;
+      re= pcre_compile(sysvar_logging_query_pcre, 0, &this_pcre_error,
+                       &this_pcre_erroffset, NULL);
+      pe= pcre_study(re, 0, &this_pcre_error);
+      /* TODO emit error messages if there is a problem */
+    }
+  }
 
   virtual bool pre (Session *)
   {
+    /* we could just not have a pre entrypoint at all,
+       and have logging_pre == NULL
+       but we have this here for the sake of being an example */
     return false;
   }
 
@@ -215,7 +248,15 @@
   
     if ((t_mark - session->start_utime) < (sysvar_logging_query_threshold_slow))
       return false;
-  
+
+    if (re)
+    {
+      int this_pcre_rc;
+      this_pcre_rc = pcre_exec(re, pe, session->query, session->query_length, 0, 0, NULL, 0);
+      if (this_pcre_rc < 0)
+        return false;
+    }
+
     // buffer to quotify the query
     unsigned char qs[255];
   
@@ -228,7 +269,8 @@
     msgbuf_len=
       snprintf(msgbuf, MAX_MSG_LEN,
                "%"PRIu64",%"PRIu64",%"PRIu64",\"%.*s\",\"%s\",\"%.*s\","
-               "%"PRIu64",%"PRIu64",%"PRIu64",%"PRIu64",%"PRIu64"\n",
+               "%"PRIu64",%"PRIu64",%"PRIu64",%"PRIu64",%"PRIu64
+               "%"PRIu32",%"PRIu32"\n",
                t_mark,
                session->thread_id,
                session->query_id,
@@ -246,8 +288,9 @@
                (t_mark - session->start_utime),
                (t_mark - session->utime_after_lock),
                session->sent_row_count,
-               session->examined_row_count);
-  
+               session->examined_row_count,
+               session->tmp_table,
+               session->total_warn_count);
   
     // a single write has a kernel thread lock, thus no need mutex guard this
     wrv= write(fd, msgbuf, msgbuf_len);
@@ -261,34 +304,6 @@
 
 static int logging_query_plugin_init(PluginRegistry &registry)
 {
-
-  if (sysvar_logging_query_filename == NULL)
-  {
-    /* no destination filename was specified via system variables
-       return now, dont set the callback pointers
-    */
-    return 0;
-  }
-
-  fd= open(sysvar_logging_query_filename,
-           O_WRONLY | O_APPEND | O_CREAT,
-           S_IRUSR|S_IWUSR);
-  if (fd < 0)
-  {
-    errmsg_printf(ERRMSG_LVL_ERROR, _("fail open() fn=%s er=%s\n"),
-                  sysvar_logging_query_filename,
-                  strerror(errno));
-
-    /* TODO
-       we should return an error here, so the plugin doesnt load
-       but this causes Drizzle to crash
-       so until that is fixed,
-       just return a success,
-       but leave the function pointers as NULL and the fd as -1
-    */
-    return 0;
-  }
-
   handler= new Logging_query();
   registry.add(handler);
 
@@ -297,13 +312,6 @@
 
 static int logging_query_plugin_deinit(PluginRegistry &registry)
 {
-
-  if (fd >= 0)
-  {
-    close(fd);
-    fd= -1;
-  }
-
   registry.remove(handler);
   delete handler;
 
@@ -328,6 +336,15 @@
   NULL, /* update func*/
   NULL /* default */);
 
+static DRIZZLE_SYSVAR_STR(
+  pcre,
+  sysvar_logging_query_pcre,
+  PLUGIN_VAR_READONLY,
+  N_("PCRE to match the query against"),
+  NULL, /* check func */
+  NULL, /* update func*/
+  NULL /* default */);
+
 static DRIZZLE_SYSVAR_ULONG(
   threshold_slow,
   sysvar_logging_query_threshold_slow,
@@ -367,6 +384,7 @@
 static struct st_mysql_sys_var* logging_query_system_variables[]= {
   DRIZZLE_SYSVAR(enable),
   DRIZZLE_SYSVAR(filename),
+  DRIZZLE_SYSVAR(pcre),
   DRIZZLE_SYSVAR(threshold_slow),
   DRIZZLE_SYSVAR(threshold_big_resultset),
   DRIZZLE_SYSVAR(threshold_big_examined),

=== modified file 'plugin/logging_syslog/logging_syslog.cc'
--- plugin/logging_syslog/logging_syslog.cc	2009-05-11 17:50:22 +0000
+++ plugin/logging_syslog/logging_syslog.cc	2009-05-27 20:56:47 +0000
@@ -110,7 +110,8 @@
            " query=\"%.*s\""
            " command=\"%.*s\""
            " t_connect=%lld t_start=%lld t_lock=%lld"
-           " rows_sent=%ld rows_examined=%ld\n",
+           " rows_sent=%ld rows_examined=%ld"
+           " tmp_table=%ld total_warn_count=%ld\n",
            (unsigned long) session->thread_id,
            (unsigned long) session->query_id,
            dbl, dbs,
@@ -121,30 +122,9 @@
            (unsigned long long) (t_mark - session->start_utime),
            (unsigned long long) (t_mark - session->utime_after_lock),
            (unsigned long) session->sent_row_count,
-           (unsigned long) session->examined_row_count);
-  
-#if 0
-    syslog(syslog_priority,
-           "thread_id=%ld query_id=%ld"
-           " db=\"%.*s\""
-           " query=\".*%s\""
-           " command=%.*s"
-           " t_connect=%lld t_start=%lld t_lock=%lld"
-           " rows_sent=%ld rows_examined=%ld\n",
-           (unsigned long) session->thread_id,
-           (unsigned long) session->query_id,
-           session->db_length, session->db,
-           // dont need to quote the query, because syslog does it itself
-           session->query_length, session->query,
-           (int) command_name[session->command].length,
-           command_name[session->command].str,
-           (unsigned long long) (t_mark - session->connect_utime),
-           (unsigned long long) (t_mark - session->start_utime),
-           (unsigned long long) (t_mark - session->utime_after_lock),
-           (unsigned long) session->sent_row_count,
-           (unsigned long) session->examined_row_count);
-  
-#endif
+           (unsigned long) session->examined_row_count,
+           (unsigned long) session->tmp_table,
+           (unsigned long) session->total_warn_count);
   
     return false;
   }