Bug fix 3.5/internal issue #622 (#9787)

* Bug fix/internal issue #622 (#9781)

* Added analyzer cache invalidation for dropped database

* Fixed jslint reported errors

* Update CHANGELOG

CHANGELOG

@@ -1,6 +1,8 @@
 v3.5.1 (XXXX-XX-XX)
 -------------------
 
+* Fixed internal issue #622: Analyzer cache is now invalidated for dropped database.
+
 * Show query string length and cacheability information in query explain output.
 
 * The AQL functions `FULLTEXT`, `NEAR`, `WITHIN` and `WITHIN_RECTANGLE` are now

arangod/IResearch/IResearchAnalyzerFeature.cpp

@@ -655,6 +655,7 @@ bool dropLegacyAnalyzersCollection(
   return lookupRes.is(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND);
 }
 
+
 void registerUpgradeTasks() {
   using namespace arangodb;
   using namespace arangodb::application_features;
@@ -1708,38 +1709,15 @@ arangodb::Result IResearchAnalyzerFeature::loadAnalyzers(
     }
 
     auto* vocbase = dbFeature->lookupDatabase(database);
-    static auto cleanupAnalyzers = []( // remove invalid analyzers
-      IResearchAnalyzerFeature& feature, // analyzer feature
-      decltype(_lastLoad)::iterator& lastLoadItr, // iterator
-      irs::string_ref const& database // database
-    )->void {
-      if (lastLoadItr == feature._lastLoad.end()) {
-        return; // nothing to do (if not in '_lastLoad' then not in '_analyzers')
-      }
-
-      // remove invalid database and analyzers
-      feature._lastLoad.erase(lastLoadItr);
-
-      // remove no longer valid analyzers (force remove)
-      for (auto itr = feature._analyzers.begin(),
-           end = feature._analyzers.end();
-           itr != end;) {
-        auto split = splitAnalyzerName(itr->first);
-
-        if (split.first == database) {
-          itr = feature._analyzers.erase(itr);
-        } else {
-          ++itr;
-        }
-      }
-    };
 
     if (!vocbase) {
       if (engine && engine->inRecovery()) {
         return arangodb::Result(); // database might not have come up yet
       }
-
-      cleanupAnalyzers(*this, itr, database); // cleanup any analyzers for 'database'
+      if (itr != _lastLoad.end()) {
+        cleanupAnalyzers(database); // cleanup any analyzers for 'database'
+        _lastLoad.erase(itr);
+      }
 
       return arangodb::Result( // result
         TRI_ERROR_ARANGO_DATABASE_NOT_FOUND, // code
@@ -1748,7 +1726,9 @@ arangodb::Result IResearchAnalyzerFeature::loadAnalyzers(
     }
 
     if (!getAnalyzerCollection(*vocbase)) {
-      cleanupAnalyzers(*this, itr, database); // cleanup any analyzers for 'database'
+      if (itr != _lastLoad.end()) {
+        cleanupAnalyzers(database); // cleanup any analyzers for 'database'
+      }
       _lastLoad[databaseKey] = currentTimestamp; // update timestamp
 
       return arangodb::Result(); // no collection means nothing to load
@@ -2472,6 +2452,33 @@ bool IResearchAnalyzerFeature::visit( // visit analyzers
   return true;
 }
 
+void IResearchAnalyzerFeature::cleanupAnalyzers(irs::string_ref const& database) {
+  if (ADB_UNLIKELY(database.empty())) {
+    TRI_ASSERT(FALSE);
+    return;
+  }
+  for (auto itr = _analyzers.begin(), end = _analyzers.end();  itr != end;) {
+    auto split = splitAnalyzerName(itr->first);
+    if (split.first == database) {
+      itr = _analyzers.erase(itr);
+    } else {
+      ++itr;
+    }
+  }
+}
+
+void IResearchAnalyzerFeature::invalidate(const TRI_vocbase_t& vocbase) {
+  WriteMutex mutex(_mutex);
+  SCOPED_LOCK(mutex); 
+  auto database = irs::string_ref(vocbase.name());
+  auto itr = _lastLoad.find(
+      irs::make_hashed_ref(database, std::hash<irs::string_ref>())); 
+  if (itr != _lastLoad.end()) {
+    cleanupAnalyzers(database);
+    _lastLoad.erase(itr);
+  }
+}
+
 }  // namespace iresearch
 }  // namespace arangodb
 

arangod/IResearch/IResearchAnalyzerFeature.h

@@ -251,6 +251,12 @@ class IResearchAnalyzerFeature final : public arangodb::application_features::Ap
   bool visit(std::function<bool(AnalyzerPool::ptr const& analyzer)> const& visitor,
              TRI_vocbase_t const* vocbase) const;
 
+  ///////////////////////////////////////////////////////////////////////////////
+  // @brief removes analyzers for specified database from cache
+  // @param vocbase  database to invalidate analyzers
+  ///////////////////////////////////////////////////////////////////////////////
+  void invalidate(const TRI_vocbase_t& vocbase);
+
  private:
   // map of caches of irs::analysis::analyzer pools indexed by analyzer name and
   // their associated metas
@@ -314,6 +320,12 @@ class IResearchAnalyzerFeature final : public arangodb::application_features::Ap
       irs::string_ref const& database = irs::string_ref::NIL // database to load
   );
 
+  ////////////////////////////////////////////////////////////////////////////////
+  /// removes analyzers for database from feature cache
+  /// Write lock must be acquired by caller
+  /// @param database the database to cleanup analyzers for 
+  void cleanupAnalyzers(irs::string_ref const& database);
+
   //////////////////////////////////////////////////////////////////////////////
   /// @brief store the definition for the speicifed pool in the corresponding
   ///        vocbase

arangod/RestServer/DatabaseFeature.cpp

@@ -61,6 +61,7 @@
 #include "VocBase/LogicalCollection.h"
 #include "VocBase/ticks.h"
 #include "VocBase/vocbase.h"
+#include "IResearch/IResearchAnalyzerFeature.h"
 
 #include <velocypack/velocypack-aliases.h>
 
@@ -816,6 +817,12 @@ int DatabaseFeature::dropDatabase(std::string const& name, bool waitForDeletion,
 #endif
     arangodb::aql::QueryCache::instance()->invalidate(vocbase);
 
+    auto* analyzers = 
+      arangodb::application_features::ApplicationServer::lookupFeature<arangodb::iresearch::IResearchAnalyzerFeature>();
+    if (analyzers != nullptr) {
+      analyzers->invalidate(*vocbase);
+    }
+
     engine->prepareDropDatabase(*vocbase, !engine->inRecovery(), res);
   }
   // must not use the database after here, as it may now be

tests/js/common/aql/aql-view-arangosearch-ddl-cluster.js

@@ -1,5 +1,5 @@
 /*jshint globalstrict:false, strict:false, maxlen: 500 */
-/*global fail, assertUndefined, assertEqual, assertNotEqual, assertTrue, assertFalse*/
+/*global fail, assertUndefined, assertEqual, assertNotEqual, assertTrue, assertFalse, assertNull*/
 
 ////////////////////////////////////////////////////////////////////////////////
 /// DISCLAIMER
@@ -1094,6 +1094,27 @@ function IResearchFeatureDDLTestSuite () {
       assertTrue(String === properties.links.col2.analyzers[0].constructor);
       assertEqual("identity", properties.links.col2.analyzers[0]);
       
+      db._useDatabase("_system");
+      db._dropDatabase(dbName);
+    },
+    testLeftAnalyzerInDroppedDatabase: function () {
+      const dbName = "TestNameDroppedDB";
+      const analyzerName = "TestAnalyzer";
+      db._useDatabase("_system");
+      try { db._dropDatabase(dbName); } catch (e) {}
+      db._createDatabase(dbName);
+      db._useDatabase(dbName);
+      analyzers.save(analyzerName, "identity");
+      // recreating database
+      db._useDatabase("_system");
+      db._dropDatabase(dbName);
+      db._createDatabase(dbName);
+      db._useDatabase(dbName);
+
+      assertNull(analyzers.analyzer(analyzerName));
+      // this should be no name conflict
+      analyzers.save(analyzerName, "text", {"stopwords" : [], "locale":"en"});
+     
       db._useDatabase("_system");
       db._dropDatabase(dbName);
     }

tests/js/common/aql/aql-view-arangosearch-ddl-noncluster.js

@@ -1,5 +1,5 @@
 /*jshint globalstrict:false, strict:false, maxlen: 500 */
-/*global fail, assertUndefined, assertEqual, assertNotEqual, assertTrue, assertFalse*/
+/*global fail, assertUndefined, assertEqual, assertNotEqual, assertTrue, assertFalse, assertNull*/
 
 ////////////////////////////////////////////////////////////////////////////////
 /// DISCLAIMER
@@ -1094,6 +1094,27 @@ function IResearchFeatureDDLTestSuite () {
       assertTrue(String === properties.links.col2.analyzers[0].constructor);
       assertEqual("identity", properties.links.col2.analyzers[0]);
       
+      db._useDatabase("_system");
+      db._dropDatabase(dbName);
+    },
+    testLeftAnalyzerInDroppedDatabase: function () {
+      const dbName = "TestNameDroppedDB";
+      const analyzerName = "TestAnalyzer";
+      db._useDatabase("_system");
+      try { db._dropDatabase(dbName); } catch (e) {}
+      db._createDatabase(dbName);
+      db._useDatabase(dbName);
+      analyzers.save(analyzerName, "identity");
+      // recreating database
+      db._useDatabase("_system");
+      db._dropDatabase(dbName);
+      db._createDatabase(dbName);
+      db._useDatabase(dbName);
+
+      assertNull(analyzers.analyzer(analyzerName));
+      // this should be no name conflict
+      analyzers.save(analyzerName, "text", {"stopwords" : [], "locale":"en"});
+     
       db._useDatabase("_system");
       db._dropDatabase(dbName);
     }