Bug#24930129: REDUCE NUMBER OF DICTIONARY CACHE LOOKUPS

Move acquire() calls higher in call hierarchy to reduce
number of acquire() calls. Pass const dd object references or
pointers instead of object name strings.

Part 1: Do this for schemas, events and stored routines.

Author: jhauglid

mysql-test/r/dd_debug.result

@@ -61,7 +61,7 @@ SET GLOBAL DEBUG= '-d, fail_while_acquiring_dd_object';
 # 1.3b No schema found during drop.
 SET DEBUG= '+d, pretend_no_schema_in_drop_schema';
 DROP SCHEMA s1;
-ERROR 42000: Unknown database 's1'
+ERROR HY000: Can't drop database 's1'; database doesn't exist
 SET DEBUG= '-d, pretend_no_schema_in_drop_schema';
 # 1.3c Fail while dropping dd object during drop.
 SET DEBUG= '+d, fail_while_dropping_dd_object';

mysql-test/t/dd_debug.test

@@ -93,7 +93,7 @@ SET GLOBAL DEBUG= '-d, fail_while_acquiring_dd_object';
 --echo # 1.3b No schema found during drop.
 
 SET DEBUG= '+d, pretend_no_schema_in_drop_schema';
---error ER_BAD_DB_ERROR
+--error ER_DB_DROP_EXISTS
 DROP SCHEMA s1;
 SET DEBUG= '-d, pretend_no_schema_in_drop_schema';
 

sql/dd/cache/dictionary_client.h

@@ -648,6 +648,42 @@ class Dictionary_client
                const typename T::cache_partition_type** object);
 
 
+  /**
+    Retrieve an object by its schema- and object name.
+
+    This function returns a cloned object that can be modified.
+
+    @note We will acquire an IX-lock on the schema name unless we already
+          have one. This is needed for proper synchronization with schema
+          DDL in cases where the table does not exist, and where the
+          indirect synchronization based on table names therefore will not
+          apply.
+
+    @note This is a variant of the method above asking for an object of type
+          T, and hence using T's functions for updating name keys etc.
+          This function, however, returns the instance pointed to as type
+          T::cache_partition_type to ease handling of various subtypes
+          of the same base type.
+
+    @todo TODO: We should change the MDL acquisition (see above) for a more
+          long term solution.
+
+    @tparam       T             Dictionary object type.
+    @param        schema_name   Name of the schema containing the object.
+    @param        object_name   Name of the object.
+    @param [out]  object        Dictionary object, if present; otherwise NULL.
+
+    @retval       false   No error.
+    @retval       true    Error (from handling a cache miss, or from
+                                 failing to get an MDL lock).
+  */
+
+  template <typename T>
+  bool acquire_for_modification(const String_type &schema_name,
+                                const String_type &object_name,
+                                typename T::cache_partition_type** object);
+
+
   /**
     Retrieve a table object by its se private id.
 

sql/dd/dd_event.cc

@@ -1,4 +1,4 @@
-/* Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+/* Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
 
    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
@@ -32,7 +32,6 @@
 #include "sql_lex.h"
 #include "sql_string.h"
 #include "system_variables.h"
-#include "transaction.h"        // trans_commit
 #include "tztime.h"             // Time_zone
 
 
@@ -258,29 +257,6 @@ static Event::enum_interval_field get_enum_interval_field(
 }
 
 
-bool
-event_exists(dd::cache::Dictionary_client *dd_client,
-             const String_type &schema_name,
-             const String_type &event_name,
-             bool *exists)
-{
-  DBUG_ENTER("dd::event_exists");
-  DBUG_ASSERT(exists);
-
-  const dd::Event *event_ptr= nullptr;
-  dd::cache::Dictionary_client::Auto_releaser releaser(dd_client);
-  if (dd_client->acquire(schema_name, event_name, &event_ptr))
-  {
-    // Error is reported by the dictionary subsystem.
-    DBUG_RETURN(true);
-  }
-
-  *exists= event_ptr != nullptr;
-
-  DBUG_RETURN(false);
-}
-
-
 /**
   Set Event attributes.
 
@@ -387,7 +363,7 @@ static void set_event_attributes(THD *thd, Event *event,
 
 
 bool create_event(THD *thd,
-                  const String_type &schema_name,
+                  const Schema &schema,
                   const String_type &event_name,
                   const String_type &event_body,
                   const String_type &event_body_utf8,
@@ -396,143 +372,55 @@ bool create_event(THD *thd,
 {
   DBUG_ENTER("dd::create_event");
 
-  dd::cache::Dictionary_client *client= thd->dd_client();
-  dd::cache::Dictionary_client::Auto_releaser releaser(client);
-  const dd::Schema *sch_obj= nullptr;
-
-  // Acquire schema object.
-  if (client->acquire(schema_name, &sch_obj))
-  {
-    // Error is reported by the dictionary subsystem.
-    DBUG_RETURN(true);
-  }
-  if (sch_obj == nullptr)
-  {
-    my_error(ER_BAD_DB_ERROR, MYF(0), schema_name.c_str());
-    DBUG_RETURN(true);
-  }
-
-  std::unique_ptr<dd::Event> event(sch_obj->create_event(thd));
+  std::unique_ptr<dd::Event> event(schema.create_event(thd));
 
   // Set Event attributes.
   set_event_attributes(thd, event.get(), event_name, event_body,
                        event_body_utf8, definer, event_data, false);
 
-  if (client->store(event.get()))
-  {
-    trans_rollback_stmt(thd);
-    // Full rollback we have THD::transaction_rollback_request.
-    trans_rollback(thd);
-    DBUG_RETURN(true);
-  }
-
-  DBUG_RETURN(trans_commit_stmt(thd) || trans_commit(thd));
+  DBUG_RETURN(thd->dd_client()->store(event.get()));
 }
 
 
-bool update_event(THD *thd, const Event *event,
-                  const String_type &new_db_name,
+bool update_event(THD *thd, Event *event,
+                  const dd::Schema *new_schema,
                   const String_type &new_event_name,
                   const String_type &new_event_body,
                   const String_type &new_event_body_utf8,
                   const LEX_USER *definer,
                   Event_parse_data *event_data)
 {
   DBUG_ENTER("dd::update_event");
-  DBUG_ASSERT(event != nullptr);
-
-  dd::cache::Dictionary_client *client= thd->dd_client();
-  dd::cache::Dictionary_client::Auto_releaser releaser(client);
-
-  Event *new_event= nullptr;
-  if (client->acquire_for_modification(event->id(), &new_event))
-    DBUG_RETURN(true);
 
   // Check whether alter event was given dates that are in the past.
-  if (event_data->check_dates(thd, static_cast<int>(new_event->on_completion())))
+  if (event_data->check_dates(thd, static_cast<int>(event->on_completion())))
     DBUG_RETURN(true);
 
   // Update Schema Id if there is a dbname change.
-  if (new_db_name != "")
-  {
-    const dd::Schema *to_sch_ptr;
-    if (client->acquire(new_db_name, &to_sch_ptr))
-      DBUG_RETURN(true);
-
-    if (to_sch_ptr == nullptr)
-    {
-      my_error(ER_BAD_DB_ERROR, MYF(0), new_db_name.c_str());
-      DBUG_RETURN(true);
-    }
-
-    new_event->set_schema_id(to_sch_ptr->id());
-  }
+  if (new_schema != nullptr)
+    event->set_schema_id(new_schema->id());
 
   // Set the altered event attributes.
-  set_event_attributes(thd, new_event,
+  set_event_attributes(thd, event,
                        new_event_name != "" ? new_event_name : event->name(),
                        new_event_body, new_event_body_utf8, definer,
                        event_data, true);
 
-  if (client->update(new_event))
-  {
-    trans_rollback_stmt(thd);
-    // Full rollback we have THD::transaction_rollback_request.
-    trans_rollback(thd);
-    DBUG_RETURN(true);
-  }
-
-  DBUG_RETURN(trans_commit_stmt(thd) || trans_commit(thd));
+  DBUG_RETURN(thd->dd_client()->update(event));
 }
 
-bool update_event_time_and_status(THD *thd, const Event *event,
-                                  my_time_t last_executed, ulonglong status)
-{
-  DBUG_ENTER("dd::update_event_time_fields");
-
-  DBUG_ASSERT(event != nullptr);
-
-  dd::cache::Dictionary_client *client= thd->dd_client();
-  dd::cache::Dictionary_client::Auto_releaser releaser(client);
-
-  Event *new_event= nullptr;
-  if (client->acquire_for_modification(event->id(), &new_event))
-    DBUG_RETURN(true);
-
-  new_event->set_event_status_null(false);
-  new_event->set_event_status(get_enum_event_status(status));
-  new_event->set_last_executed_null(false);
-  new_event->set_last_executed(last_executed);
 
-  if (client->update(new_event))
-  {
-    trans_rollback_stmt(thd);
-    // Full rollback in case we have THD::transaction_rollback_request.
-    trans_rollback(thd);
-    DBUG_RETURN(true);
-  }
-
-  DBUG_RETURN(trans_commit_stmt(thd) || trans_commit(thd));
-}
-
-
-bool drop_event(THD *thd, const Event *event)
+bool update_event_time_and_status(THD *thd, Event *event,
+                                  my_time_t last_executed, ulonglong status)
 {
-  DBUG_ENTER("dd::drop_event");
-
-  DBUG_ASSERT(event != nullptr);
-
-  Disable_gtid_state_update_guard disabler(thd);
+  DBUG_ENTER("dd::update_event_time_and_status");
 
-  if (thd->dd_client()->drop(event))
-  {
-    trans_rollback_stmt(thd);
-    // Full rollback in case we have THD::transaction_rollback_request.
-    trans_rollback(thd);
-    DBUG_RETURN(true);
-  }
+  event->set_event_status_null(false);
+  event->set_event_status(get_enum_event_status(status));
+  event->set_last_executed_null(false);
+  event->set_last_executed(last_executed);
 
-  DBUG_RETURN(trans_commit_stmt(thd) || trans_commit(thd));
+  DBUG_RETURN(thd->dd_client()->update(event));
 }
 
 } // namespace dd

sql/dd/dd_event.h

@@ -31,6 +31,7 @@ using sql_mode_t= ulonglong;
 
 namespace dd
 {
+  class Schema;
 namespace cache
 {
   class Dictionary_client;
@@ -67,27 +68,11 @@ int get_old_on_completion(Event::enum_on_completion on_completion);
 */
 interval_type get_old_interval_type(Event::enum_interval_field interval_field);
 
-/**
-   Check if an event exists under a schema.
-
-   @param       dd_client   Dictionary client
-   @param       schema_name Schema name of the event object name.
-   @param       name        The event name to search for.
-   @param [out] exists      Value set to true if the object is found else false.
-
-   @retval      true         Failure (error has been reported).
-   @retval      false        Success.
-*/
-bool event_exists(dd::cache::Dictionary_client *dd_client,
-                  const String_type &schema_name,
-                  const String_type &name,
-                  bool *exists);
-
 /**
    Create an event object and commit it to DD Table Events.
 
    @param thd              Thread handle
-   @param schema_name      Database name
+   @param schema           Schema object.
    @param event_name       Event name
    @param event_body       Event body.
    @param event_body_utf8  Event body in utf8 format.
@@ -97,7 +82,7 @@ bool event_exists(dd::cache::Dictionary_client *dd_client,
    @retval true  Event creation failed.
    @retval false Event creation succeeded.
 */
-bool create_event(THD *thd, const String_type &schema_name,
+bool create_event(THD *thd, const Schema &schema,
                   const String_type &event_name, const String_type &event_body,
                   const String_type &event_body_utf8, const LEX_USER *definer,
                   Event_parse_data *event_data);
@@ -108,7 +93,7 @@ bool create_event(THD *thd, const String_type &schema_name,
 
   @param thd                 Thread handle
   @param event               Event to update.
-  @param new_db_name         Updated db name.
+  @param new_schema          New Schema or nullptr if the schema does not change.
   @param new_event_name      Updated Event name.
   @param new_event_body      Updated Event body.
   @param new_event_body_utf8 Updated Event body in utf8 format.
@@ -118,8 +103,8 @@ bool create_event(THD *thd, const String_type &schema_name,
   @retval true  Event updation failed.
   @retval false Event updation succeeded.
 */
-bool update_event(THD *thd, const Event *event,
-                  const String_type &new_db_name,
+bool update_event(THD *thd, Event *event,
+                  const dd::Schema *new_schema,
                   const String_type &new_event_name,
                   const String_type &new_event_body,
                   const String_type &new_event_body_utf8,
@@ -137,19 +122,9 @@ bool update_event(THD *thd, const Event *event,
   @retval true  true if update failed.
   @retval false false if update succeeded.
 */
-bool update_event_time_and_status(THD *thd, const Event *event,
+bool update_event_time_and_status(THD *thd, Event *event,
                                   my_time_t last_executed,
                                   ulonglong status);
 
-/**
-  Drop an Event from event metadata table.
-
-  @param thd            Thread handle.
-  @param event          Event to be droppped.
-
-  @retval true if event drop failed.
-  @retval false if event drop succeeded.
-*/
-bool drop_event(THD *thd, const Event *event);
 } // namespace dd
 #endif // DD_EVENT_INCLUDED