Merge lp:~dedzone/drizzle/query-cache-hook into lp:~drizzle-trunk/drizzle/development

[Djellel E. Difallah]

[Monty Taylor (mordred)]

One little thing:

cout << "Results are cached" << endl;

Looks like a debugging statement. Could you please remove this?

In general looks fine to me, although I have not examined codeflow through the runtime - Brian?

review: Needs Fixing

[Toru Maesaka (tmaesaka)]

Agreed. It looks fine from first glimpse but I'm also not 100% sure
whether the hook points are placed in appropriate places in terms of
code flow.

I think it would be awesome if Djelle could provide us with arguments
on why the hooks are placed where they are atm.

Cheers,
Toru

On Thu, Jul 8, 2010 at 1:41 PM, Monty Taylor <email address hidden> wrote:
> Review: Needs Fixing
> One little thing:
>
> cout << "Results are cached" << endl;
>
> Looks like a debugging statement. Could you please remove this?
>
> In general looks fine to me, although I have not examined codeflow through the runtime - Brian?
> --
> https://code.launchpad.net/~dedzone/drizzle/query-cache-hook/+merge/29359
> You are subscribed to branch lp:drizzle.
>

--
Toru Maesaka <email address hidden>

[Djellel E. Difallah (dedzone)]

Hello,

I have added a wiki page explaining the Query Cache interface and the hooks into the code base.

Can you please review it.

Thanks!

~Djellel

[Monty Taylor (mordred)]

For those following along at home, the wiki page is:

http://drizzle.org/wiki/Query_cache

[lp:~dedzone/drizzle/query-cache-hook]

1647. By Djellel E. Difallah <ded@ubuntu> on 2010-07-12

merge with trunk

[Toru Maesaka (tmaesaka)]

Fantastic!

This is exactly what I wanted to see. From engineering perspective, I
wanted to see this much earlier in the project but I'm happy to see
progress regardless :)

-Toru

On Tuesday, July 13, 2010, Monty Taylor <email address hidden> wrote:
> For those following along at home, the wiki page is:
>
> http://drizzle.org/wiki/Query_cache
> --
> https://code.launchpad.net/~dedzone/drizzle/query-cache-hook/+merge/29729
> You are subscribed to branch lp:drizzle.
>

--
Toru Maesaka <email address hidden>

[Brian Aker (brianaker)]

It would be good to see some test cases. How is the invalidation work within a transaction?

Cheers,
  -Brian

review: Needs Information

[Padraig O'Sullivan (posulliv)]

Hey Brian,

We were planning on doing invalidation using the replication API for this project. Would you mind expanding on what you mean here?

Thanks,
Padraig

[Toru Maesaka (tmaesaka)]

Hi Brian,

Regarding test cases for Djellel's code, his changeset only includes
hooks and no real implementation of the plugin. So, I'm wondering how
we could write tests for it at this point. Would a "dummy" plugin
work? for example a module called "query-cache-test" or something
along that line.

As for invalidation in transactions, it shouldn't be a problem if we
use the Replication API since the transaction should have succeeded
when the query cache invalidator is called.

Cheers,
Toru

On Tue, Jul 20, 2010 at 10:17 AM, Brian Aker <email address hidden> wrote:
> Review: Needs Information
> It would be good to see some test cases. How is the invalidation work within a transaction?
>
> Cheers,
>  -Brian
>
>
> --
> https://code.launchpad.net/~dedzone/drizzle/query-cache-hook/+merge/29729
> You are subscribed to branch lp:drizzle.
>

--
Toru Maesaka <email address hidden>

=== modified file 'drizzled/plugin/query_cache.cc'
--- drizzled/plugin/query_cache.cc	2010-05-15 18:23:34 +0000
+++ drizzled/plugin/query_cache.cc	2010-07-12 19:31:41 +0000
@@ -2,6 +2,7 @@
  *  vim:expandtab:shiftwidth=2:tabstop=2:smarttab:
  *
  *  Copyright (C) 2008 Sun Microsystems
+ *  Copyright (C) 2010 Djellel Eddine Difallah
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -37,122 +38,144 @@
 
 /* Namespaces are here to prevent global symbol clashes with these classes */
 
-class TryFetchAndSendIterate
- : public unary_function<plugin::QueryCache *, bool>
-{
-  Session *session;
-  bool is_transactional;
-public:
-  TryFetchAndSendIterate(Session *session_arg, bool is_transactional_arg) :
-    unary_function<plugin::QueryCache *, bool>(),
-    session(session_arg), is_transactional(is_transactional_arg) { }
-
-  inline result_type operator()(argument_type handler)
-  {
-    if (handler->tryFetchAndSend(session, is_transactional))
-    {
-      errmsg_printf(ERRMSG_LVL_ERROR,
-                    _("qcache plugin '%s' try_fetch_and_send() failed"),
-                    handler->getName().c_str());
-      return true;
-    }
-    return false;
-  }
-};
-
-class SetIterate
- : public unary_function<plugin::QueryCache *, bool>
-{
-  Session *session;
-  bool is_transactional;
-public:
-  SetIterate(Session *session_arg, bool is_transactional_arg) :
-    unary_function<plugin::QueryCache *, bool>(),
-    session(session_arg), is_transactional(is_transactional_arg) { }
-
-  inline result_type operator()(argument_type handler)
-  {
-
-    if (handler->set(session, is_transactional))
-    {
-      errmsg_printf(ERRMSG_LVL_ERROR, _("qcache plugin '%s' set() failed"),
-                    handler->getName().c_str());
-      return true;
-    }
-    return false;
-  }
-};
-
-class InvalidateTableIterate
- : public unary_function<plugin::QueryCache *, bool>
-{
-  Session *session;
-  bool is_transactional;
-public:
-  InvalidateTableIterate(Session *session_arg, bool is_transactional_arg) :
-    unary_function<plugin::QueryCache *, bool>(),
-    session(session_arg), is_transactional(is_transactional_arg) { }
-
-  inline result_type operator()(argument_type handler)
-  {
-
-    if (handler->invalidateTable(session, is_transactional))
-    {
-      errmsg_printf(ERRMSG_LVL_ERROR,
-                    _("qcache plugin '%s' invalidateTable() failed"),
-                    handler->getName().c_str());
-      return true;
-    }
-    return false;
-  }
-};
-
-
-class InvalidateDbIterate
- : public unary_function<plugin::QueryCache *, bool>
-{
-  Session *session;
-  const char *dbname;
-  bool is_transactional;
-public:
-  InvalidateDbIterate(Session *session_arg, const char *dbname_arg,
-                      bool is_transactional_arg) :
-    unary_function<plugin::QueryCache *, bool>(),
-    session(session_arg), dbname(dbname_arg),
-    is_transactional(is_transactional_arg) { }
-
-  inline result_type operator()(argument_type handler)
-  {
-    if (handler->invalidateDb(session, dbname, is_transactional))
-    {
-      errmsg_printf(ERRMSG_LVL_ERROR,
-                    _("qcache plugin '%s' invalidateDb() failed"),
-                    handler->getName().c_str());
-      return true;
-    }
-    return false;
-  }
-};
-
-class FlushIterate
- : public unary_function<plugin::QueryCache *, bool>
-{
-  Session *session;
-public:
-  FlushIterate(Session *session_arg) :
+class IsCachedIterate
+ : public unary_function<plugin::QueryCache *, bool>
+{
+  Session *session;
+public:
+  IsCachedIterate(Session* session_arg) :
+    unary_function<plugin::QueryCache *, bool>(),
+    session(session_arg) { }
+
+  inline result_type operator()(argument_type handler)
+  {
+    return handler->doIsCached(session);
+  }
+};
+
+bool plugin::QueryCache::isCached(Session *session)
+{
+  vector<plugin::QueryCache *>::iterator iter=
+    find_if(all_query_cache.begin(), all_query_cache.end(),
+            IsCachedIterate(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 != 
+   */
+  return iter != all_query_cache.end();
+}
+
+
+class SendCachedResultsetIterate
+ : public unary_function<plugin::QueryCache *, bool>
+{
+  Session *session;
+public:
+  SendCachedResultsetIterate(Session *session_arg) :
+    unary_function<plugin::QueryCache *, bool>(),
+    session(session_arg) { }
+
+  inline result_type operator()(argument_type handler)
+  {
+    if (handler->doSendCachedResultset(session))
+      return false;
+    return true;
+  }
+};
+bool plugin::QueryCache::sendCachedResultset(Session *session)
+{
+  vector<plugin::QueryCache *>::iterator iter=
+    find_if(all_query_cache.begin(), all_query_cache.end(),
+            SendCachedResultsetIterate(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 != 
+   */
+  return iter != all_query_cache.end();
+}
+
+class PrepareResultsetIterate
+ : public unary_function<plugin::QueryCache *, bool>
+{
+  Session *session;
+public:
+  PrepareResultsetIterate(Session *session_arg) :
     unary_function<plugin::QueryCache *, bool>(), session(session_arg) { }
 
   inline result_type operator()(argument_type handler)
   {
-    if (handler->flush(session))
-    {
-      errmsg_printf(ERRMSG_LVL_ERROR, _("qcache plugin '%s' flush() failed"),
-                    handler->getName().c_str());
-      return true;
-    }
-    return false;
-  }
-};
+    return handler->doPrepareResultset(session);
+  }
+};
+bool plugin::QueryCache::prepareResultset(Session *session)
+{
+  vector<plugin::QueryCache *>::iterator iter=
+    find_if(all_query_cache.begin(), all_query_cache.end(),
+            PrepareResultsetIterate(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 != 
+   */
+  return iter != all_query_cache.end();
+}
+
+class SetResultsetIterate
+ : public unary_function<plugin::QueryCache *, bool>
+{
+  Session *session;
+public:
+  SetResultsetIterate(Session *session_arg) :
+    unary_function<plugin::QueryCache *, bool>(),
+    session(session_arg) { }
+
+  inline result_type operator()(argument_type handler)
+  {
+    return handler->doSetResultset(session);
+  }
+};
+
+bool plugin::QueryCache::setResultset(Session *session)
+{
+  /* Use find_if instead of foreach so that we can collect return codes */
+  vector<plugin::QueryCache *>::iterator iter=
+    find_if(all_query_cache.begin(), all_query_cache.end(),
+            SetResultsetIterate(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 != 
+   */
+  return iter != all_query_cache.end();
+}
+
+class InsertRecordIterate
+ : public unary_function<plugin::QueryCache *, bool>
+{
+  Session *session;
+  List<Item> &item;
+public:
+  InsertRecordIterate(Session *session_arg, List<Item> &item_arg) :
+    unary_function<plugin::QueryCache *, bool>(),
+    session(session_arg), item(item_arg) { }
+
+  inline result_type operator()(argument_type handler)
+  {
+    return handler->doInsertRecord(session, item);
+  }
+};
+bool plugin::QueryCache::insertRecord(Session *session, List<Item> &items)
+{
+  /* Use find_if instead of foreach so that we can collect return codes */
+  vector<plugin::QueryCache *>::iterator iter=
+    find_if(all_query_cache.begin(), all_query_cache.end(),
+            InsertRecordIterate(session, items));
+  /* 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 != 
+   */
+  return iter != all_query_cache.end();
+}
+
+
 
 bool plugin::QueryCache::addPlugin(plugin::QueryCache *handler)
 {
@@ -166,73 +189,4 @@
                         handler));
 }
 
-
-bool plugin::QueryCache::tryFetchAndSendDo(Session *session,
-                                           bool transactional)
-{
-  /* Use find_if instead of foreach so that we can collect return codes */
-  vector<plugin::QueryCache *>::iterator iter=
-    find_if(all_query_cache.begin(), all_query_cache.end(),
-            TryFetchAndSendIterate(session, transactional));
-  /* 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 != 
-   */
-  return iter != all_query_cache.end();
-}
-
-bool plugin::QueryCache::setDo(Session *session, bool transactional)
-{
-  /* Use find_if instead of foreach so that we can collect return codes */
-  vector<plugin::QueryCache *>::iterator iter=
-    find_if(all_query_cache.begin(), all_query_cache.end(),
-            SetIterate(session, transactional));
-  /* 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 != 
-   */
-  return iter != all_query_cache.end();
-}
-
-bool plugin::QueryCache::invalidateTableDo(Session *session,
-                                         bool transactional)
-{
-  /* Use find_if instead of foreach so that we can collect return codes */
-  vector<plugin::QueryCache *>::iterator iter=
-    find_if(all_query_cache.begin(), all_query_cache.end(),
-            InvalidateTableIterate(session, transactional));
-  /* 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 != 
-   */
-  return iter != all_query_cache.end();
-}
-
-bool plugin::QueryCache::invalidateDbDo(Session *session, const char *dbname,
-                                        bool transactional)
-{
-  /* Use find_if instead of foreach so that we can collect return codes */
-  vector<plugin::QueryCache *>::iterator iter=
-    find_if(all_query_cache.begin(), all_query_cache.end(),
-            InvalidateDbIterate(session, dbname, transactional));
-  /* 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 != 
-   */
-  return iter != all_query_cache.end();
-}
-
-bool plugin::QueryCache::flushDo(Session *session)
-{
-  /* Use find_if instead of foreach so that we can collect return codes */
-  vector<plugin::QueryCache *>::iterator iter=
-    find_if(all_query_cache.begin(), all_query_cache.end(),
-            FlushIterate(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 != 
-   */
-  return iter != all_query_cache.end();
-}
-
 } /* namespace drizzled */

=== modified file 'drizzled/plugin/query_cache.h'
--- drizzled/plugin/query_cache.h	2010-03-06 02:08:13 +0000
+++ drizzled/plugin/query_cache.h	2010-07-12 19:31:41 +0000
@@ -4,6 +4,7 @@
  *  Definitions required for Query Cache plugin
  *
  *  Copyright (C) 2008 Sun Microsystems, Toru Maesaka
+ *  Copyright (C) 2010 Djellel Eddine Difallah
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -22,55 +23,60 @@
 #ifndef DRIZZLED_PLUGIN_QUERY_CACHE_H
 #define DRIZZLED_PLUGIN_QUERY_CACHE_H
 
+#include "drizzled/plugin.h"
 #include "drizzled/plugin/plugin.h"
+#include <drizzled/sql_list.h>
 
 namespace drizzled
 {
 class Session;
+class select_result;
 
 namespace plugin
 {
 
 /* 
   This is the API that a qcache plugin must implement.
-  it should implement each of these function pointers.
-  if a function pointer is NULL (not loaded), that's ok.
-
-  Return:
-    false = success
-    true  = failure
 */
+
 class QueryCache : public Plugin
 {
+private:  
+  
   QueryCache();
   QueryCache(const QueryCache &);
   QueryCache& operator=(const QueryCache &);
-public:
+
+public:  
+
   explicit QueryCache(std::string name_arg)
     : Plugin(name_arg, "QueryCache")
   {}
 
   virtual ~QueryCache() {}
-  /* Lookup the cache and transmit the data back to the client */
-  virtual bool tryFetchAndSend(Session *session,
-                               bool is_transactional)= 0;
-
-  virtual bool set(Session *session, bool is_transactional)= 0;
-  virtual bool invalidateTable(Session *session, bool is_transactional)= 0;
-  virtual bool invalidateDb(Session *session, const char *db_name,
-                            bool transactional)= 0;
-  virtual bool flush(Session *session)= 0;
+
+  /* these are the Query Cache interface functions */
+
+  /* Lookup the cache and transmit the data back to the client */
+  virtual bool doIsCached(Session* session)= 0;  
+  /* Lookup the cache and transmit the data back to the client */
+  virtual bool doSendCachedResultset(Session *session)= 0;
+  /* Send the current Resultset to the cache */
+  virtual bool doSetResultset(Session *session)= 0;
+  /* initiate a new Resultset (header) */
+  virtual bool doPrepareResultset(Session *session)= 0;
+  /* push a record to the current Resultset */
+  virtual bool doInsertRecord(Session *session, List<Item> &item)= 0;
 
   static bool addPlugin(QueryCache *handler);
   static void removePlugin(QueryCache *handler);
 
   /* These are the functions called by the rest of the Drizzle server */
-  static bool tryFetchAndSendDo(Session *session, bool transactional);
-  static bool setDo(Session *session, bool transactional);
-  static bool invalidateTableDo(Session *session, bool transactional);
-  static bool invalidateDbDo(Session *session, const char *db_name,
-                            bool transactional);
-  static bool flushDo(Session *session);
+  static bool isCached(Session *session);
+  static bool sendCachedResultset(Session *session);
+  static bool prepareResultset(Session *session);
+  static bool setResultset(Session *session);
+  static bool insertRecord(Session *session, List<Item> &item);
 };
 
 } /* namespace plugin */

=== modified file 'drizzled/select_send.h'
--- drizzled/select_send.h	2010-02-14 19:27:57 +0000
+++ drizzled/select_send.h	2010-07-12 19:31:41 +0000
@@ -22,6 +22,7 @@
 #define DRIZZLED_SELECT_SEND_H
 
 #include <drizzled/plugin/client.h>
+#include <drizzled/plugin/query_cache.h>
 #include <drizzled/plugin/transactional_storage_engine.h>
 
 namespace drizzled
@@ -111,6 +112,10 @@
         break;
       }
     }
+    /* Insert this record to the Resultset into the cache */
+    if (session->query_cache_key != "" && session->getResultsetMessage() != NULL)
+      plugin::QueryCache::insertRecord(session, items);
+
     session->sent_row_count++;
     if (session->is_error())
       return true;

=== modified file 'drizzled/session.cc'
--- drizzled/session.cc	2010-07-07 17:51:44 +0000
+++ drizzled/session.cc	2010-07-12 19:31:41 +0000
@@ -229,6 +229,9 @@
   cleanup_done= abort_on_warning= no_warnings_for_error= false;
   pthread_mutex_init(&LOCK_delete, MY_MUTEX_INIT_FAST);
 
+  /* query_cache init */
+  query_cache_key= "";
+  resultset= NULL;
   /* Variables with default values */
   proc_info="login";
   where= Session::DEFAULT_WHERE;
@@ -1464,6 +1467,8 @@
 {
   /* Cleanup SQL processing state to reuse this statement in next query. */
   lex_end(lex);
+  query_cache_key= ""; // reset the cache key
+  resultset= NULL;
 }
 
 bool Session::copy_db_to(char **p_db, size_t *p_db_length)

=== modified file 'drizzled/session.h'
--- drizzled/session.h	2010-07-07 17:51:44 +0000
+++ drizzled/session.h	2010-07-12 19:31:41 +0000
@@ -67,6 +67,7 @@
 {
 class Transaction;
 class Statement;
+class Resultset;
 }
 namespace internal
 {
@@ -368,6 +369,8 @@
   */
   std::string db;
 
+  /* current cache key */
+  std::string query_cache_key;
   /**
     Constant for Session::where initialization in the beginning of every query.
 
@@ -1175,7 +1178,15 @@
   {
     return statement_message;
   }
-
+  
+  /**
+   * Returns a pointer to the current Resulset message for this
+   * Session, or NULL if no active message.
+   */
+  message::Resultset *getResultsetMessage() const
+  {
+    return resultset;
+  }
   /**
    * Sets the active transaction message used by the ReplicationServices
    * component.
@@ -1197,10 +1208,24 @@
   {
     statement_message= in_message;
   }
+
+  /**
+   * Sets the active Resultset message used by the Query Cache
+   * plugin.
+   *
+   * @param[in] Pointer to the message
+   */
+  void setResultsetMessage(message::Resultset *in_message)
+  {
+    resultset= in_message;
+  }
+
 private:
   /** Pointers to memory managed by the ReplicationServices component */
   message::Transaction *transaction_message;
   message::Statement *statement_message;
+  /* Pointer to the current resultset of Select query */
+  message::Resultset *resultset;
   plugin::EventObserverList *session_event_observers;
   
   /* Schema observers are mapped to databases. */

=== modified file 'drizzled/sql_parse.cc'
--- drizzled/sql_parse.cc	2010-07-07 17:51:44 +0000
+++ drizzled/sql_parse.cc	2010-07-12 19:31:41 +0000
@@ -44,6 +44,7 @@
 
 #include "drizzled/plugin/logging.h"
 #include "drizzled/plugin/query_rewrite.h"
+#include "drizzled/plugin/query_cache.h"
 #include "drizzled/plugin/authorization.h"
 #include "drizzled/optimizer/explain_plan.h"
 #include "drizzled/pthread_globals.h"
@@ -476,9 +477,7 @@
 
   /* now we are ready to execute the statement */
   res= lex->statement->execute();
-
   session->set_proc_info("query end");
-
   /*
     The return value for ROW_COUNT() is "implementation dependent" if the
     statement is not DELETE, INSERT or UPDATE, but -1 is what JDBC and ODBC
@@ -492,7 +491,6 @@
 
   return (res || session->is_error());
 }
-
 bool execute_sqlcom_select(Session *session, TableList *all_tables)
 {
   LEX	*lex= session->lex;
@@ -540,7 +538,13 @@
     {
       if (!result && !(result= new select_send()))
         return true;
+
+      /* Init the Query Cache plugin */
+      plugin::QueryCache::prepareResultset(session); 
       res= handle_select(session, lex, result, 0);
+      /* Send the Resultset to the cache */
+      plugin::QueryCache::setResultset(session); 
+
       if (result != lex->result)
         delete result;
     }
@@ -715,13 +719,25 @@
   uint64_t start_time= my_getsystime();
   lex_start(session);
   session->reset_for_next_command();
-
+  /* Check if the Query is Cached if and return true if yes
+   * TODO the plugin has to make sure that the query is cacheble
+   * by setting the query_safe_cache param to TRUE
+   */
+  bool res= true;
+  if (plugin::QueryCache::isCached(session))
+  {
+    res= plugin::QueryCache::sendCachedResultset(session);
+  }
+  if (not res)
+  {
+#if defined(DEBUG)
+    errmsg_printf(ERRMSG_LVL_DBUG,_("Results retrieved from cache"));
+#endif /* DEBUG */
+    return;
+  }
   LEX *lex= session->lex;
-
   Lex_input_stream lip(session, inBuf, length);
-
   bool err= parse_sql(session, &lip);
-
   if (!err)
   {
     {
@@ -731,9 +747,9 @@
                                  session->thread_id,
                                  const_cast<const char *>(session->db.empty() ? "" : session->db.c_str()));
         // Implement Views here --Brian
-
         /* Actually execute the query */
-        try {
+        try 
+        {
           mysql_execute_command(session);
         }
         catch (...)
@@ -749,7 +765,6 @@
   {
     assert(session->is_error());
   }
-
   lex->unit.cleanup();
   session->set_proc_info("freeing items");
   session->end_statement();