10000 Bug fix/internal issue #622 by Dronplane · Pull Request #9781 · arangodb/arangodb · GitHub
[go: up one dir, main page]

Skip to content

Bug fix/internal issue #622 #9781

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 35 additions & 28 deletions arangod/IResearch/IResearchAnalyzerFeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,7 @@ bool dropLegacyAnalyzersCollection(
return lookupRes.is(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND);
}


void registerUpgradeTasks() {
using namespace arangodb;
using namespace arangodb::application_features;
Expand Down Expand Up @@ -1820,38 +1821,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
Expand All @@ -1860,7 +1838,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
Expand Down Expand Up @@ -2584,6 +2564,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

Expand Down
12 changes: 12 additions & 0 deletions arangod/IResearch/IResearchAnalyzerFeature.h
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,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
Expand Down Expand Up @@ -328,6 +334,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
Expand Down
7 changes: 7 additions & 0 deletions arangod/RestServer/DatabaseFeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
#include "VocBase/LogicalCollection.h"
#include "VocBase/ticks.h"
#include "VocBase/vocbase.h"
#include "IResearch/IResearchAnalyzerFeature.h"

#include <velocypack/velocypack-aliases.h>

Expand Down Expand Up @@ -823,6 +824,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
Expand Down
23 changes: 22 additions & 1 deletion tests/js/common/aql/aql-view-arangosearch-ddl-cluster.js
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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);
}
Expand Down
23 changes: 22 additions & 1 deletion tests/js/common/aql/aql-view-arangosearch-ddl-noncluster.js
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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);
}
Expand Down
0