Bug#35948153 Problem setting up events due to stale NdbApi dictionary

cache [#2]

This is second patch, solving the problem of ineffiecent cache
invalidation when invalidating a table which is known to be invalid but
unknown if it is in the cache or not.

Problem:
Currently the only way to invalidate a table in the NdbApi dictionary
cache is to open the table and then mark it as invalid. In case the
table does not exists in the cache, it will still have to be opened and
thus fetched fom NDB.

This means that in order to get the latest table definition it has to be
fetched two times, although the table definition does not already exist
in the cache. This is inefficient.

Analysis:
In order to avoid the double roundtrip there need to be a function which
marks the table as invalid only if it exists in the cache.

Fix:
Implement a NdbApi function that invalidates table by name if it exists
in the cache.
Replace the old pattern of opening table in order to invalidate it with
the new function.

The old pattern is still a valid use case for invalidating a table after
having worked with it.

Change-Id: I20f275f1fed76d991330348bea4ae72548366467

Author: blaudden

storage/ndb/include/ndbapi/Ndb.hpp

@@ -2213,6 +2213,9 @@ class Ndb {
 
   static const char *externalizeTableName(const char *internalTableName,
                                           bool fullyQualifiedNames);
+  static BaseString internalize_table_name(const char *db_name,
+                                           const char *schema,
+                                           const char *table_name);
   const BaseString internalize_table_name(const char *external_name) const;
 
   static const char *externalizeIndexName(const char *internalIndexName,

storage/ndb/include/ndbapi/NdbDictionary.hpp

@@ -2916,6 +2916,9 @@ class NdbDictionary {
     int removeIndexGlobal(const Index &ndbidx, int invalidate) const;
     int removeTableGlobal(const Table &ndbtab, int invalidate) const;
     void invalidateDbGlobal(const char *dbname);
+    // Invalidate table by name if it exist in the cache
+    void invalidateTableGlobal(const char *dbName, const char *schemaName,
+                               const char *tableName);
 #endif
 
     /*

storage/ndb/plugin/ha_ndbcluster_binlog.cc

@@ -1240,14 +1240,9 @@ static void ndbcluster_binlog_event_operation_teardown(THD *thd, Ndb *is_ndb,
       Ndb_event_data::get_event_data(pOp->getCustomData());
   NDB_SHARE *const share = event_data->share;
 
-  {
-    // Since table has been dropped or cluster connection lost the NdbApi table
-    // should be invalidated in the global dictionary cache
-    Ndb_table_guard ndbtab_g(is_ndb, share->db, share->table_name);
-    if (ndbtab_g.get_table()) {
-      ndbtab_g.invalidate();
-    }
-  }
+  // Since table has been dropped or cluster connection lost the NdbApi table
+  // should be invalidated in the global dictionary cache
+  Ndb_table_guard::invalidate_table(is_ndb, share->db, share->table_name);
 
   // Close the table in MySQL Server
   ndb_tdc_close_cached_table(thd, share->db, share->table_name);
@@ -2298,8 +2293,7 @@ class Ndb_schema_event_handler {
   void ndbapi_invalidate_table(const char *db_name,
                                const char *table_name) const {
     DBUG_TRACE;
-    Ndb_table_guard ndbtab_g(m_thd_ndb->ndb, db_name, table_name);
-    ndbtab_g.invalidate();
+    Ndb_table_guard::invalidate_table(m_thd_ndb->ndb, db_name, table_name);
   }
 
   NDB_SHARE *acquire_reference(const char *db, const char *name,

storage/ndb/plugin/ndb_dd_sync.cc

@@ -1187,11 +1187,8 @@ bool Ndb_dd_sync::synchronize_table(const char *schema_name,
                                     const char *table_name) const {
   ndb_log_verbose(1, "Synchronizing table '%s.%s'", schema_name, table_name);
 
-  {
-    // Invalidate potentially stale cached table
-    Ndb_table_guard ndbtab_g(m_thd_ndb->ndb, schema_name, table_name);
-    ndbtab_g.invalidate();
-  }
+  // Invalidate potentially stale cached table
+  Ndb_table_guard::invalidate_table(m_thd_ndb->ndb, schema_name, table_name);
 
   Ndb_table_guard ndbtab_g(m_thd_ndb->ndb, schema_name, table_name);
   const NdbDictionary::Table *ndbtab = ndbtab_g.get_table();

storage/ndb/plugin/ndb_metadata_sync.cc

@@ -912,8 +912,8 @@ bool Ndb_metadata_sync::sync_table(THD *thd, const std::string &schema_name,
                  table_name.c_str());
 
     // Invalidate the table in NdbApi
-    Ndb_table_guard ndbtab_guard(ndb, schema_name.c_str(), table_name.c_str());
-    ndbtab_guard.invalidate();
+    Ndb_table_guard::invalidate_table(ndb, schema_name.c_str(),
+                                      table_name.c_str());
     return true;
   }
 

storage/ndb/plugin/ndb_table_guard.h

@@ -186,6 +186,18 @@ class Ndb_table_guard {
   }
 
   const NdbError &getNdbError() const { return m_ndberror; }
+
+  /**
+     @brief Invalidate table in the global dictionary cache (if it exists)
+     without fetching table from NDB on cache miss, this saves one roundtrip.
+
+     This function is to be used to invalidate table when it's not already being
+     held open.
+  */
+  static void invalidate_table(Ndb *ndb, const char *dbname,
+                               const char *tabname) {
+    ndb->getDictionary()->invalidateTableGlobal(dbname, "def", tabname);
+  }
 };
 
 #endif

storage/ndb/src/ndbapi/DictCache.cpp

@@ -360,6 +360,35 @@ void GlobalDictCache::invalidateDb(const char *name, size_t len) {
   DBUG_VOID_RETURN;
 }
 
+void GlobalDictCache::invalidateTable(const BaseString &internal_name) {
+  DBUG_TRACE;
+  DBUG_PRINT("enter", ("internal_name: '%s'", internal_name.c_str()));
+  Vector<TableVersion> *vers =
+      m_tableHash.getData(internal_name.c_str(), internal_name.length());
+  if (vers == nullptr) {
+    DBUG_PRINT("exit", ("nothing cached"));
+    return;
+  }
+
+  if (vers->size() == 0) {
+    DBUG_PRINT("exit", ("no cached versions"));
+    return;
+  }
+
+  TableVersion *const ver = &vers->back();
+  if (ver->m_status == RETREIVING) {
+    DBUG_PRINT("exit", ("fresh version on the way"));
+    return;
+  }
+
+  ver->m_impl->m_status = NdbDictionary::Object::Invalid;
+  ver->m_status = DROPPED;
+  if (ver->m_refCount == 0) {
+    delete ver->m_impl;
+    vers->erase(vers->size() - 1);
+  }
+}
+
 void GlobalDictCache::release(const NdbTableImpl *tab, int invalidate) {
   DBUG_ENTER("GlobalDictCache::release");
   DBUG_PRINT("enter",

storage/ndb/src/ndbapi/DictCache.hpp

@@ -94,6 +94,7 @@ class GlobalDictCache : public NdbLockable {
   }
 
   void invalidateDb(const char *name, size_t len);
+  void invalidateTable(const BaseString &name);
 
  public:
   enum Status { OK = 0, DROPPED = 1, RETREIVING = 2 };

storage/ndb/src/ndbapi/Ndb.cpp

@@ -1733,9 +1733,21 @@ const char *Ndb::externalizeIndexName(const char *internalIndexName,
   }
 }
 
+// Format internal name from db, schema and table name
+BaseString Ndb::internalize_table_name(const char *db_name, const char *schema,
+                                       const char *table_name) {
+  BaseString internal_name;
+  // Internal table name format <db>/<schema>/<table>
+  internal_name.assfmt("%s%c%s%c%s", db_name, table_name_separator, schema,
+                       table_name_separator, table_name);
+
+  DBUG_PRINT("exit", ("internal_name: %s", internal_name.c_str()));
+  return internal_name;
+}
+
+// Format internal name using schema and db name from the Ndb object
 const BaseString Ndb::internalize_table_name(const char *external_name) const {
-  BaseString ret;
-  DBUG_ENTER("internalize_table_name");
+  DBUG_TRACE;
   DBUG_PRINT("enter", ("external_name: %s", external_name));
 
 #ifdef VM_TRACE
@@ -1744,13 +1756,8 @@ const BaseString Ndb::internalize_table_name(const char *external_name) const {
   assert(theImpl->m_schemaname.length());
 #endif
 
-  // Internal table name format <db>/<schema>/<table>
-  ret.assfmt("%s%c%s%c%s", theImpl->m_dbname.c_str(), table_name_separator,
-             theImpl->m_schemaname.c_str(), table_name_separator,
-             external_name);
-
-  DBUG_PRINT("exit", ("internal_name: %s", ret.c_str()));
-  DBUG_RETURN(ret);
+  return internalize_table_name(theImpl->m_dbname.c_str(),
+                                theImpl->m_schemaname.c_str(), external_name);
 }
 
 const BaseString Ndb::getDatabaseFromInternalName(const char *internalName) {

storage/ndb/src/ndbapi/NdbDictionary.cpp

@@ -3125,6 +3125,12 @@ void NdbDictionary::Dictionary::invalidateDbGlobal(const char *name) {
   }
 }
 
+void NdbDictionary::Dictionary::invalidateTableGlobal(const char *dbName,
+                                                      const char *schemaName,
+                                                      const char *tableName) {
+  m_impl.invalidateTableGlobal(dbName, schemaName, tableName);
+}
+
 int NdbDictionary::Dictionary::beginSchemaTrans() {
   return m_impl.beginSchemaTrans();
 }

storage/ndb/src/ndbapi/NdbDictionaryImpl.cpp

@@ -5053,6 +5053,19 @@ int NdbDictionaryImpl::dropIndexGlobal(NdbIndexImpl &impl, bool ignoreFKs) {
   ERR_RETURN(getNdbError(), ret);
 }
 
+void NdbDictionaryImpl::invalidateTableGlobal(const char *dbName,
+                                              const char *schemaName,
+                                              const char *tableName) {
+  DBUG_TRACE;
+  if (!m_globalHash) return;
+
+  BaseString internalName(
+      Ndb::internalize_table_name(dbName, schemaName, tableName));
+  m_globalHash->lock();
+  m_globalHash->invalidateTable(internalName);
+  m_globalHash->unlock();
+}
+
 int NdbDictInterface::dropIndex(const NdbTableImpl &timpl) {
   DBUG_ENTER("NdbDictInterface::dropIndex");
   DBUG_PRINT("enter",

storage/ndb/src/ndbapi/NdbDictionaryImpl.hpp

@@ -965,6 +965,8 @@ class NdbDictionaryImpl : public NdbDictionary::Dictionary {
   int dropIndexGlobal(NdbIndexImpl &, bool ignoreFKs);
   int releaseTableGlobal(const NdbTableImpl &impl, int invalidate);
   int releaseIndexGlobal(const NdbIndexImpl &impl, int invalidate);
+  void invalidateTableGlobal(const char *dbName, const char *schemaName,
+                             const char *tableName);
 
   NdbTableImpl *getTable(const char *tableName, void **data = nullptr);
   NdbTableImpl *getBlobTable(const NdbTableImpl &, uint col_no);