Hawkin
/
arangodb
mirror of https://gitee.com/bigwinds/arangodb
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Use shared_ptr for LogicalCollection (#7220)

This commit is contained in:
Simon 2018-11-05 18:31:57 +01:00 committed by Jan
parent 311fb717f2
commit 0eb5142df8
16 changed files with 114 additions and 128 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
arangod/ClusterEngine/ClusterTransactionCollection.cpp
View File

@ -168,9 +168,8 @@ int ClusterTransactionCollection::use(int nestingLevel) {
} }
try { try {
_sharedCollection = ci->getCollection(_transaction->vocbase().name(), std::to_string(_cid)); _collection = ci->getCollection(_transaction->vocbase().name(), std::to_string(_cid));
if (_sharedCollection) { if (_collection) {
_collection = _sharedCollection.get();
if (!_transaction->hasHint(transaction::Hints::Hint::LOCK_NEVER) && if (!_transaction->hasHint(transaction::Hints::Hint::LOCK_NEVER) &&
!_transaction->hasHint(transaction::Hints::Hint::NO_USAGE_LOCK)) { !_transaction->hasHint(transaction::Hints::Hint::NO_USAGE_LOCK)) {
// use and usage-lock // use and usage-lock
@ -224,7 +223,7 @@ void ClusterTransactionCollection::release() {
LOG_TRX(_transaction, 0) << "unusing collection " << _cid; LOG_TRX(_transaction, 0) << "unusing collection " << _cid;
if (_usageLocked) { if (_usageLocked) {
_transaction->vocbase().releaseCollection(_collection); _transaction->vocbase().releaseCollection(_collection.get());
_usageLocked = false; _usageLocked = false;
} }
_collection = nullptr; _collection = nullptr;
@ -257,9 +256,7 @@ int ClusterTransactionCollection::doLock(AccessMode::Type type,
TRI_ASSERT(!isLocked()); TRI_ASSERT(!isLocked());
LogicalCollection* collection = _collection; TRI_ASSERT(_collection);
TRI_ASSERT(collection != nullptr);
LOG_TRX(_transaction, nestingLevel) << "write-locking collection " << _cid; LOG_TRX(_transaction, nestingLevel) << "write-locking collection " << _cid;
_lockType = type; _lockType = type;
@ -310,8 +307,7 @@ int ClusterTransactionCollection::doUnlock(AccessMode::Type type,
return TRI_ERROR_INTERNAL; return TRI_ERROR_INTERNAL;
} }
LogicalCollection* collection = _collection; TRI_ASSERT(_collection);
TRI_ASSERT(collection != nullptr);
_lockType = AccessMode::Type::NONE; _lockType = AccessMode::Type::NONE;

2
arangod/ClusterEngine/ClusterTransactionCollection.h
View File

@ -89,8 +89,6 @@ class ClusterTransactionCollection final : public TransactionCollection {
AccessMode::Type _lockType; // collection lock type, used for exclusive locks AccessMode::Type _lockType; // collection lock type, used for exclusive locks
int _nestingLevel; // the transaction level that added this collection int _nestingLevel; // the transaction level that added this collection
bool _usageLocked; // is this already locked bool _usageLocked; // is this already locked
/// @brief shared ptr to the collection so we can safely use _collection
std::shared_ptr<LogicalCollection> _sharedCollection;
}; };
} }

18
arangod/MMFiles/MMFilesTransactionCollection.cpp
View File

@ -218,7 +218,7 @@ int MMFilesTransactionCollection::use(int nestingLevel) {
} }
} else { } else {
// use without usage-lock (lock already set externally) // use without usage-lock (lock already set externally)
_collection = _transaction->vocbase().lookupCollection(_cid).get(); _collection = _transaction->vocbase().lookupCollection(_cid);
if (_collection == nullptr) { if (_collection == nullptr) {
return TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND; return TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND;
@ -300,7 +300,7 @@ void MMFilesTransactionCollection::release() {
// unuse collection, remove usage-lock // unuse collection, remove usage-lock
LOG_TRX(_transaction, 0) << "unusing collection " << _cid; LOG_TRX(_transaction, 0) << "unusing collection " << _cid;
_transaction->vocbase().releaseCollection(_collection); _transaction->vocbase().releaseCollection(_collection.get());
_collection = nullptr; _collection = nullptr;
} }
} }
@ -323,11 +323,9 @@ int MMFilesTransactionCollection::doLock(AccessMode::Type type, int nestingLevel
} }
TRI_ASSERT(!isLocked()); TRI_ASSERT(!isLocked());
TRI_ASSERT(_collection);
LogicalCollection* collection = _collection;
TRI_ASSERT(collection != nullptr); auto physical = static_cast<MMFilesCollection*>(_collection->getPhysical());
auto physical = static_cast<MMFilesCollection*>(collection->getPhysical());
TRI_ASSERT(physical != nullptr); TRI_ASSERT(physical != nullptr);
double timeout = _transaction->timeout(); double timeout = _transaction->timeout();
@ -400,10 +398,8 @@ int MMFilesTransactionCollection::doUnlock(AccessMode::Type type, int nestingLev
bool const useDeadlockDetector = (!_transaction->hasHint(transaction::Hints::Hint::SINGLE_OPERATION) && bool const useDeadlockDetector = (!_transaction->hasHint(transaction::Hints::Hint::SINGLE_OPERATION) &&
!_transaction->hasHint(transaction::Hints::Hint::NO_DLD)); !_transaction->hasHint(transaction::Hints::Hint::NO_DLD));
LogicalCollection* collection = _collection; TRI_ASSERT(_collection);
TRI_ASSERT(collection != nullptr); auto physical = static_cast<MMFilesCollection*>(_collection->getPhysical());
auto physical = static_cast<MMFilesCollection*>(collection->getPhysical());
TRI_ASSERT(physical != nullptr); TRI_ASSERT(physical != nullptr);
if (!AccessMode::isWriteOrExclusive(_lockType)) { if (!AccessMode::isWriteOrExclusive(_lockType)) {

50
arangod/MMFiles/MMFilesWalRecoverState.cpp
View File

@ -109,8 +109,8 @@ void MMFilesWalRecoverState::releaseResources() {
// release all collections // release all collections
for (auto it = openedCollections.begin(); it != openedCollections.end(); for (auto it = openedCollections.begin(); it != openedCollections.end();
++it) { ++it) {
arangodb::LogicalCollection* collection = it->second; std::shared_ptr<arangodb::LogicalCollection>& collection = it->second;
collection->vocbase().releaseCollection(collection); collection->vocbase().releaseCollection(collection.get());
} }
openedCollections.clear(); openedCollections.clear();
@ -158,14 +158,14 @@ TRI_vocbase_t* MMFilesWalRecoverState::releaseDatabase(
auto it2 = openedCollections.begin(); auto it2 = openedCollections.begin();
while (it2 != openedCollections.end()) { while (it2 != openedCollections.end()) {
arangodb::LogicalCollection* collection = it2->second; std::shared_ptr<arangodb::LogicalCollection>& collection = it2->second;
TRI_ASSERT(collection != nullptr); TRI_ASSERT(collection != nullptr);
if (collection->vocbase().id() == databaseId) { if (collection->vocbase().id() == databaseId) {
// correct database, now release the collection // correct database, now release the collection
TRI_ASSERT(vocbase == &(collection->vocbase())); TRI_ASSERT(vocbase == &(collection->vocbase()));
vocbase->releaseCollection(collection); vocbase->releaseCollection(collection.get());
// get new iterator position // get new iterator position
it2 = openedCollections.erase(it2); it2 = openedCollections.erase(it2);
} else { } else {
@ -181,7 +181,7 @@ TRI_vocbase_t* MMFilesWalRecoverState::releaseDatabase(
} }
/// @brief release a collection (so it can be dropped) /// @brief release a collection (so it can be dropped)
arangodb::LogicalCollection* MMFilesWalRecoverState::releaseCollection( std::shared_ptr<arangodb::LogicalCollection> MMFilesWalRecoverState::releaseCollection(
TRI_voc_cid_t collectionId) { TRI_voc_cid_t collectionId) {
auto it = openedCollections.find(collectionId); auto it = openedCollections.find(collectionId);
@ -189,10 +189,9 @@ arangodb::LogicalCollection* MMFilesWalRecoverState::releaseCollection(
return nullptr; return nullptr;
} }
arangodb::LogicalCollection* collection = it->second; std::shared_ptr<arangodb::LogicalCollection>& collection = it->second;
TRI_ASSERT(collection != nullptr); TRI_ASSERT(collection != nullptr);
collection->vocbase().releaseCollection(collection); collection->vocbase().releaseCollection(collection.get());
openedCollections.erase(collectionId); openedCollections.erase(collectionId);
return collection; return collection;
@ -205,12 +204,12 @@ arangodb::LogicalCollection* MMFilesWalRecoverState::useCollection(
if (it != openedCollections.end()) { if (it != openedCollections.end()) {
res = TRI_ERROR_NO_ERROR; res = TRI_ERROR_NO_ERROR;
return (*it).second; return (*it).second.get();
} }
TRI_set_errno(TRI_ERROR_NO_ERROR); TRI_set_errno(TRI_ERROR_NO_ERROR);
TRI_vocbase_col_status_e status; // ignored here TRI_vocbase_col_status_e status; // ignored here
arangodb::LogicalCollection* collection = std::shared_ptr<arangodb::LogicalCollection> collection =
vocbase->useCollection(collectionId, status); vocbase->useCollection(collectionId, status);
if (collection == nullptr) { if (collection == nullptr) {
@ -232,7 +231,7 @@ arangodb::LogicalCollection* MMFilesWalRecoverState::useCollection(
openedCollections.emplace(collectionId, collection); openedCollections.emplace(collectionId, collection);
res = TRI_ERROR_NO_ERROR; res = TRI_ERROR_NO_ERROR;
return collection; return collection.get();
} }
/// @brief looks up a collection /// @brief looks up a collection
@ -672,11 +671,9 @@ bool MMFilesWalRecoverState::ReplayMarker(MMFilesMarker const* marker,
return true; return true;
} }
arangodb::LogicalCollection* collection = auto collection = state->releaseCollection(collectionId);
state->releaseCollection(collectionId); if (!collection) {
collection = vocbase->lookupCollection(collectionId);
if (collection == nullptr) {
collection = vocbase->lookupCollection(collectionId).get();
} }
if (collection == nullptr) { if (collection == nullptr) {
@ -995,11 +992,9 @@ bool MMFilesWalRecoverState::ReplayMarker(MMFilesMarker const* marker,
return true; return true;
} }
arangodb::LogicalCollection* collection = auto collection = state->releaseCollection(collectionId);
state->releaseCollection(collectionId); if (!collection) {
collection = vocbase->lookupCollection(collectionId);
if (collection == nullptr) {
collection = vocbase->lookupCollection(collectionId).get();
} }
if (collection != nullptr) { if (collection != nullptr) {
@ -1017,7 +1012,7 @@ bool MMFilesWalRecoverState::ReplayMarker(MMFilesMarker const* marker,
if (nameSlice.isString()) { if (nameSlice.isString()) {
name = nameSlice.copyString(); name = nameSlice.copyString();
collection = vocbase->lookupCollection(name).get(); collection = vocbase->lookupCollection(name);
if (collection != nullptr) { if (collection != nullptr) {
auto otherCid = collection->id(); auto otherCid = collection->id();
@ -1058,10 +1053,10 @@ bool MMFilesWalRecoverState::ReplayMarker(MMFilesMarker const* marker,
// restore the old behavior afterwards // restore the old behavior afterwards
TRI_DEFER(state->databaseFeature->forceSyncProperties(oldSync)); TRI_DEFER(state->databaseFeature->forceSyncProperties(oldSync));
collection = vocbase->createCollection(b2.slice()).get(); collection = vocbase->createCollection(b2.slice());
} else { } else {
// collection will be kept // collection will be kept
collection = vocbase->createCollection(b2.slice()).get(); collection = vocbase->createCollection(b2.slice());
} }
TRI_ASSERT(collection != nullptr); TRI_ASSERT(collection != nullptr);
} catch (basics::Exception const& ex) { } catch (basics::Exception const& ex) {
@ -1335,11 +1330,10 @@ bool MMFilesWalRecoverState::ReplayMarker(MMFilesMarker const* marker,
} }
// ignore any potential error returned by this call // ignore any potential error returned by this call
arangodb::LogicalCollection* collection = auto collection = state->releaseCollection(collectionId);
state->releaseCollection(collectionId);
if (collection == nullptr) { if (collection == nullptr) {
collection = vocbase->lookupCollection(collectionId).get(); collection = vocbase->lookupCollection(collectionId);
} }
if (collection != nullptr) { if (collection != nullptr) {
@ -1547,7 +1541,7 @@ int MMFilesWalRecoverState::fillIndexes() {
// release all collections // release all collections
for (auto it = openedCollections.begin(); it != openedCollections.end(); for (auto it = openedCollections.begin(); it != openedCollections.end();
++it) { ++it) {
arangodb::LogicalCollection* collection = (*it).second; std::shared_ptr<arangodb::LogicalCollection>& collection = (*it).second;
auto physical = static_cast<MMFilesCollection*>(collection->getPhysical()); auto physical = static_cast<MMFilesCollection*>(collection->getPhysical());
TRI_ASSERT(physical != nullptr); TRI_ASSERT(physical != nullptr);

5
arangod/MMFiles/MMFilesWalRecoverState.h
View File

@ -125,7 +125,7 @@ struct MMFilesWalRecoverState {
TRI_vocbase_t* releaseDatabase(TRI_voc_tick_t); TRI_vocbase_t* releaseDatabase(TRI_voc_tick_t);
/// @brief release a collection (so it can be dropped) /// @brief release a collection (so it can be dropped)
arangodb::LogicalCollection* releaseCollection(TRI_voc_cid_t); std::shared_ptr<arangodb::LogicalCollection> releaseCollection(TRI_voc_cid_t);
/// @brief gets a collection (and inserts it into the cache if not in it) /// @brief gets a collection (and inserts it into the cache if not in it)
arangodb::LogicalCollection* useCollection(TRI_vocbase_t*, TRI_voc_cid_t, int&); arangodb::LogicalCollection* useCollection(TRI_vocbase_t*, TRI_voc_cid_t, int&);
@ -178,7 +178,8 @@ struct MMFilesWalRecoverState {
TRI_voc_tick_t lastTick; TRI_voc_tick_t lastTick;
std::vector<MMFilesWalLogfile*> logfilesToProcess; std::vector<MMFilesWalLogfile*> logfilesToProcess;
std::unordered_map<TRI_voc_cid_t, arangodb::LogicalCollection*> openedCollections; std::unordered_map<TRI_voc_cid_t,
std::shared_ptr<arangodb::LogicalCollection>> openedCollections;
std::unordered_map<TRI_voc_tick_t, TRI_vocbase_t*> openedDatabases; std::unordered_map<TRI_voc_tick_t, TRI_vocbase_t*> openedDatabases;
std::vector<std::string> emptyLogfiles; std::vector<std::string> emptyLogfiles;

4
arangod/Replication/Syncer.cpp
View File

@ -766,9 +766,9 @@ Result Syncer::dropIndex(arangodb::velocypack::Slice const& slice) {
return Result(TRI_ERROR_ARANGO_DATABASE_NOT_FOUND); return Result(TRI_ERROR_ARANGO_DATABASE_NOT_FOUND);
} }
auto* col = resolveCollection(*vocbase, slice).get(); auto col = resolveCollection(*vocbase, slice);
if (col == nullptr) { if (!col) {
return Result(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND); return Result(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND);
} }

4
arangod/Replication/TailingSyncer.cpp
View File

@ -678,9 +678,9 @@ Result TailingSyncer::changeCollection(VPackSlice const& slice) {
return Result(TRI_ERROR_ARANGO_DATABASE_NOT_FOUND); return Result(TRI_ERROR_ARANGO_DATABASE_NOT_FOUND);
} }
auto* col = resolveCollection(*vocbase, slice).get(); auto col = resolveCollection(*vocbase, slice);
if (col == nullptr) { if (!col) {
if (isDeleted) { if (isDeleted) {
// not a problem if a collection that is going to be deleted anyway // not a problem if a collection that is going to be deleted anyway
// does not exist on slave // does not exist on slave

32
arangod/RocksDBEngine/RocksDBReplicationContext.cpp
View File

@ -105,7 +105,7 @@ void RocksDBReplicationContext::removeVocbase(TRI_vocbase_t& vocbase) {
auto it = _iterators.begin(); auto it = _iterators.begin();
while (it != _iterators.end()) { while (it != _iterators.end()) {
if (it->second->dbGuard.database().id() == vocbase.id()) { if (it->second->vocbase.id() == vocbase.id()) {
if (it->second->isUsed()) { if (it->second->isUsed()) {
LOG_TOPIC(ERR, Logger::REPLICATION) << "trying to delete used context"; LOG_TOPIC(ERR, Logger::REPLICATION) << "trying to delete used context";
} else { } else {
@ -736,7 +736,7 @@ void RocksDBReplicationContext::use(double ttl) {
// make sure the WAL files are not deleted // make sure the WAL files are not deleted
std::set<TRI_vocbase_t*> dbs; std::set<TRI_vocbase_t*> dbs;
for (auto& pair : _iterators) { for (auto& pair : _iterators) {
dbs.emplace(&pair.second->dbGuard.database()); dbs.emplace(&pair.second->vocbase);
} }
for (TRI_vocbase_t* vocbase : dbs) { for (TRI_vocbase_t* vocbase : dbs) {
vocbase->updateReplicationClient(replicationClientId(), _snapshotTick, ttl); vocbase->updateReplicationClient(replicationClientId(), _snapshotTick, ttl);
@ -754,7 +754,7 @@ void RocksDBReplicationContext::release() {
// make sure the WAL files are not deleted immediately // make sure the WAL files are not deleted immediately
std::set<TRI_vocbase_t*> dbs; std::set<TRI_vocbase_t*> dbs;
for (auto& pair : _iterators) { for (auto& pair : _iterators) {
dbs.emplace(&pair.second->dbGuard.database()); dbs.emplace(&pair.second->vocbase);
} }
for (TRI_vocbase_t* vocbase : dbs) { for (TRI_vocbase_t* vocbase : dbs) {
vocbase->updateReplicationClient(replicationClientId(), _snapshotTick, ttl); vocbase->updateReplicationClient(replicationClientId(), _snapshotTick, ttl);
@ -782,7 +782,7 @@ RocksDBReplicationContext::CollectionIterator::CollectionIterator(
TRI_vocbase_t& vocbase, TRI_vocbase_t& vocbase,
std::shared_ptr<LogicalCollection> const& coll, std::shared_ptr<LogicalCollection> const& coll,
bool sorted, rocksdb::Snapshot const* snapshot) bool sorted, rocksdb::Snapshot const* snapshot)
: dbGuard(vocbase), : vocbase(vocbase),
logical{coll}, logical{coll},
iter{nullptr}, iter{nullptr},
bounds{RocksDBKeyBounds::Empty()}, bounds{RocksDBKeyBounds::Empty()},
@ -797,17 +797,33 @@ RocksDBReplicationContext::CollectionIterator::CollectionIterator(
_isUsed{false}, _isUsed{false},
_sortedIterator{!sorted} // this makes sure that setSorted works _sortedIterator{!sorted} // this makes sure that setSorted works
{ {
TRI_ASSERT(snapshot != nullptr); TRI_ASSERT(snapshot != nullptr && coll);
_readOptions.snapshot = snapshot; _readOptions.snapshot = snapshot;
_readOptions.verify_checksums = false; _readOptions.verify_checksums = false;
_readOptions.fill_cache = false; _readOptions.fill_cache = false;
_readOptions.prefix_same_as_start = true; _readOptions.prefix_same_as_start = true;
if (!vocbase.use()) { // false if vobase was deleted
THROW_ARANGO_EXCEPTION(TRI_ERROR_ARANGO_DATABASE_NOT_FOUND);
}
TRI_vocbase_col_status_e ignore;
int res = vocbase.useCollection(logical.get(), ignore);
if (res != TRI_ERROR_NO_ERROR) { // collection was deleted
THROW_ARANGO_EXCEPTION(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND);
}
_cTypeHandler.reset(transaction::Context::createCustomTypeHandler(vocbase, _resolver)); _cTypeHandler.reset(transaction::Context::createCustomTypeHandler(vocbase, _resolver));
vpackOptions.customTypeHandler = _cTypeHandler.get(); vpackOptions.customTypeHandler = _cTypeHandler.get();
setSorted(sorted); setSorted(sorted);
} }
RocksDBReplicationContext::CollectionIterator::~CollectionIterator() {
TRI_ASSERT(!vocbase.isDangling());
vocbase.releaseCollection(logical.get());
logical.reset();
vocbase.release();
}
void RocksDBReplicationContext::CollectionIterator::setSorted(bool sorted) { void RocksDBReplicationContext::CollectionIterator::setSorted(bool sorted) {
if (_sortedIterator != sorted) { if (_sortedIterator != sorted) {
iter.reset(); iter.reset();
@ -904,7 +920,7 @@ RocksDBReplicationContext::getCollectionIterator(TRI_vocbase_t& vocbase,
} }
if (cIter) { if (cIter) {
TRI_ASSERT(cIter->dbGuard.database().id() == vocbase.id()); TRI_ASSERT(cIter->vocbase.id() == vocbase.id());
cIter->use(); cIter->use();
if (allowCreate && cIter->sorted() != sorted) { if (allowCreate && cIter->sorted() != sorted) {
@ -915,7 +931,7 @@ RocksDBReplicationContext::getCollectionIterator(TRI_vocbase_t& vocbase,
// for initial synchronization. the inventory request and collection // for initial synchronization. the inventory request and collection
// dump requests will all happen after the batch creation, so the // dump requests will all happen after the batch creation, so the
// current tick value here is good // current tick value here is good
cIter->dbGuard.database().updateReplicationClient(replicationClientId(), _snapshotTick, _ttl); cIter->vocbase.updateReplicationClient(replicationClientId(), _snapshotTick, _ttl);
} }
return cIter; return cIter;
@ -925,7 +941,7 @@ void RocksDBReplicationContext::releaseDumpIterator(CollectionIterator* it) {
if (it) { if (it) {
TRI_ASSERT(it->isUsed()); TRI_ASSERT(it->isUsed());
if (!it->hasMore()) { if (!it->hasMore()) {
it->dbGuard.database().updateReplicationClient(replicationClientId(), _snapshotTick, _ttl); it->vocbase.updateReplicationClient(replicationClientId(), _snapshotTick, _ttl);
MUTEX_LOCKER(locker, _contextLock); MUTEX_LOCKER(locker, _contextLock);
_iterators.erase(it->logical->id()); _iterators.erase(it->logical->id());
} else { // Context::release() will update the replication client } else { // Context::release() will update the replication client

5
arangod/RocksDBEngine/RocksDBReplicationContext.h
View File

@ -32,7 +32,6 @@
#include "RocksDBEngine/RocksDBReplicationCommon.h" #include "RocksDBEngine/RocksDBReplicationCommon.h"
#include "Transaction/Methods.h" #include "Transaction/Methods.h"
#include "Utils/CollectionNameResolver.h" #include "Utils/CollectionNameResolver.h"
#include "Utils/DatabaseGuard.h"
#include "VocBase/vocbase.h" #include "VocBase/vocbase.h"
#include <rocksdb/options.h> #include <rocksdb/options.h>
@ -49,7 +48,6 @@ namespace rocksdb {
} }
namespace arangodb { namespace arangodb {
class DatabaseGuard;
class RocksDBReplicationContext { class RocksDBReplicationContext {
private: private:
@ -62,8 +60,9 @@ class RocksDBReplicationContext {
CollectionIterator(TRI_vocbase_t&, CollectionIterator(TRI_vocbase_t&,
std::shared_ptr<LogicalCollection> const&, std::shared_ptr<LogicalCollection> const&,
bool sorted, rocksdb::Snapshot const*); bool sorted, rocksdb::Snapshot const*);
~CollectionIterator();
DatabaseGuard dbGuard; TRI_vocbase_t& vocbase;
std::shared_ptr<LogicalCollection> logical; std::shared_ptr<LogicalCollection> logical;
/// Iterator over primary index or documents /// Iterator over primary index or documents

7
arangod/RocksDBEngine/RocksDBReplicationTailing.cpp
View File

@ -594,11 +594,10 @@ class WALParser final : public rocksdb::WriteBatch::Handler {
return it->second.collection(); return it->second.collection();
} }
auto* collection = _vocbase->lookupCollection(cid).get(); auto collection = _vocbase->lookupCollection(cid);
if (collection) {
if (collection != nullptr) {
_collectionCache.emplace(cid, CollectionGuard(_vocbase, collection)); _collectionCache.emplace(cid, CollectionGuard(_vocbase, collection));
return collection; return collection.get();
} }
} }

14
arangod/RocksDBEngine/RocksDBTransactionCollection.cpp
View File

@ -194,7 +194,7 @@ int RocksDBTransactionCollection::use(int nestingLevel) {
_usageLocked = true; _usageLocked = true;
} else { } else {
// use without usage-lock (lock already set externally) // use without usage-lock (lock already set externally)
_collection = _transaction->vocbase().lookupCollection(_cid).get(); _collection = _transaction->vocbase().lookupCollection(_cid);
if (_collection == nullptr) { if (_collection == nullptr) {
return TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND; return TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND;
@ -246,7 +246,7 @@ void RocksDBTransactionCollection::release() {
LOG_TRX(_transaction, 0) << "unusing collection " << _cid; LOG_TRX(_transaction, 0) << "unusing collection " << _cid;
if (_usageLocked) { if (_usageLocked) {
_transaction->vocbase().releaseCollection(_collection); _transaction->vocbase().releaseCollection(_collection.get());
_usageLocked = false; _usageLocked = false;
} }
@ -390,10 +390,7 @@ int RocksDBTransactionCollection::doLock(AccessMode::Type type,
TRI_ASSERT(!isLocked()); TRI_ASSERT(!isLocked());
LogicalCollection* collection = _collection; auto physical = static_cast<RocksDBCollection*>(_collection->getPhysical());
TRI_ASSERT(collection != nullptr);
auto physical = static_cast<RocksDBCollection*>(collection->getPhysical());
TRI_ASSERT(physical != nullptr); TRI_ASSERT(physical != nullptr);
double timeout = _transaction->timeout(); double timeout = _transaction->timeout();
@ -473,10 +470,9 @@ int RocksDBTransactionCollection::doUnlock(AccessMode::Type type,
return TRI_ERROR_INTERNAL; return TRI_ERROR_INTERNAL;
} }
LogicalCollection* collection = _collection; TRI_ASSERT(_collection);
TRI_ASSERT(collection != nullptr);
auto physical = static_cast<RocksDBCollection*>(collection->getPhysical()); auto physical = static_cast<RocksDBCollection*>(_collection->getPhysical());
TRI_ASSERT(physical != nullptr); TRI_ASSERT(physical != nullptr);
LOG_TRX(_transaction, nestingLevel) << "write-unlocking collection " << _cid; LOG_TRX(_transaction, nestingLevel) << "write-unlocking collection " << _cid;

10
arangod/StorageEngine/TransactionCollection.h
View File

@ -50,7 +50,7 @@ class TransactionCollection {
inline TRI_voc_cid_t id() const { return _cid; } inline TRI_voc_cid_t id() const { return _cid; }
LogicalCollection* collection() const { LogicalCollection* collection() const {
return _collection; // vocbase collection pointer return _collection.get(); // vocbase collection pointer
} }
std::string collectionName() const; std::string collectionName() const;
@ -91,10 +91,10 @@ class TransactionCollection {
virtual void release() = 0; virtual void release() = 0;
protected: protected:
TransactionState* _transaction; // the transaction state TransactionState* _transaction; // the transaction state
TRI_voc_cid_t const _cid; // collection id TRI_voc_cid_t const _cid; // collection id
LogicalCollection* _collection; // vocbase collection pointer std::shared_ptr<LogicalCollection> _collection; // vocbase collection pointer
AccessMode::Type _accessType; // access type (read|write) AccessMode::Type _accessType; // access type (read|write)
}; };
} }

23
arangod/Utils/CollectionGuard.h
View File

@ -47,13 +47,13 @@ class CollectionGuard {
} }
/// @brief create the guard, using a collection id /// @brief create the guard, using a collection id
CollectionGuard(TRI_vocbase_t* vocbase, TRI_voc_cid_t id, CollectionGuard(TRI_vocbase_t* vocbase, TRI_voc_cid_t cid,
bool restoreOriginalStatus = false) bool restoreOriginalStatus = false)
: _vocbase(vocbase), : _vocbase(vocbase),
_collection(nullptr), _collection(nullptr),
_originalStatus(TRI_VOC_COL_STATUS_CORRUPTED), _originalStatus(TRI_VOC_COL_STATUS_CORRUPTED),
_restoreOriginalStatus(restoreOriginalStatus) { _restoreOriginalStatus(restoreOriginalStatus) {
_collection = _vocbase->useCollection(id, _originalStatus); _collection = _vocbase->useCollection(cid, _originalStatus);
if (_collection == nullptr) { if (_collection == nullptr) {
THROW_ARANGO_EXCEPTION(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND); THROW_ARANGO_EXCEPTION(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND);
@ -67,11 +67,9 @@ class CollectionGuard {
_originalStatus(TRI_VOC_COL_STATUS_CORRUPTED), _originalStatus(TRI_VOC_COL_STATUS_CORRUPTED),
_restoreOriginalStatus(false) { _restoreOriginalStatus(false) {
_collection = _vocbase->useCollection(id, _originalStatus); _collection = _vocbase->useCollection(id, _originalStatus);
if (!_collection && !name.empty()) {
if (_collection == nullptr && !name.empty()) {
_collection = _vocbase->useCollection(name, _originalStatus); _collection = _vocbase->useCollection(name, _originalStatus);
} }
if (_collection == nullptr) { if (_collection == nullptr) {
THROW_ARANGO_EXCEPTION(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND); THROW_ARANGO_EXCEPTION(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND);
} }
@ -96,12 +94,13 @@ class CollectionGuard {
} }
} }
CollectionGuard(TRI_vocbase_t* vocbase, LogicalCollection* collection) CollectionGuard(TRI_vocbase_t* vocbase,
std::shared_ptr<LogicalCollection> const& collection)
: _vocbase(vocbase), : _vocbase(vocbase),
_collection(collection), _collection(collection),
_originalStatus(TRI_VOC_COL_STATUS_CORRUPTED), _originalStatus(TRI_VOC_COL_STATUS_CORRUPTED),
_restoreOriginalStatus(false) { _restoreOriginalStatus(false) {
int res = _vocbase->useCollection(collection, _originalStatus); int res = _vocbase->useCollection(collection.get(), _originalStatus);
if (res != TRI_ERROR_NO_ERROR) { if (res != TRI_ERROR_NO_ERROR) {
THROW_ARANGO_EXCEPTION(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND); THROW_ARANGO_EXCEPTION(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND);
} }
@ -110,13 +109,13 @@ class CollectionGuard {
/// @brief destroy the guard /// @brief destroy the guard
~CollectionGuard() { ~CollectionGuard() {
if (_collection != nullptr) { if (_collection != nullptr) {
_vocbase->releaseCollection(_collection); _vocbase->releaseCollection(_collection.get());
if (_restoreOriginalStatus && if (_restoreOriginalStatus &&
(_originalStatus == TRI_VOC_COL_STATUS_UNLOADING || (_originalStatus == TRI_VOC_COL_STATUS_UNLOADING ||
_originalStatus == TRI_VOC_COL_STATUS_UNLOADED)) { _originalStatus == TRI_VOC_COL_STATUS_UNLOADED)) {
// re-unload the collection // re-unload the collection
_vocbase->unloadCollection(_collection, false); _vocbase->unloadCollection(_collection.get(), false);
} }
} }
} }
@ -125,13 +124,13 @@ class CollectionGuard {
/// @brief prematurely release the usage lock /// @brief prematurely release the usage lock
void release() { void release() {
if (_collection != nullptr) { if (_collection != nullptr) {
_vocbase->releaseCollection(_collection); _vocbase->releaseCollection(_collection.get());
_collection = nullptr; _collection = nullptr;
} }
} }
/// @brief return the collection pointer /// @brief return the collection pointer
inline arangodb::LogicalCollection* collection() const { return _collection; } inline arangodb::LogicalCollection* collection() const { return _collection.get(); }
/// @brief return the status of the collection at the time of using the guard /// @brief return the status of the collection at the time of using the guard
inline TRI_vocbase_col_status_e originalStatus() const { inline TRI_vocbase_col_status_e originalStatus() const {
@ -143,7 +142,7 @@ class CollectionGuard {
TRI_vocbase_t* _vocbase; TRI_vocbase_t* _vocbase;
/// @brief pointer to collection /// @brief pointer to collection
arangodb::LogicalCollection* _collection; std::shared_ptr<arangodb::LogicalCollection> _collection;
/// @brief status of collection when invoking the guard /// @brief status of collection when invoking the guard
TRI_vocbase_col_status_e _originalStatus; TRI_vocbase_col_status_e _originalStatus;

34
arangod/VocBase/vocbase.cpp
View File

@ -1552,56 +1552,48 @@ int TRI_vocbase_t::useCollection(arangodb::LogicalCollection* collection,
} }
/// @brief locks a (document) collection for usage by id /// @brief locks a (document) collection for usage by id
arangodb::LogicalCollection* TRI_vocbase_t::useCollection( std::shared_ptr<arangodb::LogicalCollection> TRI_vocbase_t::useCollection(
TRI_voc_cid_t cid, TRI_vocbase_col_status_e& status) { TRI_voc_cid_t cid, TRI_vocbase_col_status_e& status) {
auto collection = lookupCollection(cid); return useCollectionInternal(lookupCollection(cid), status);
return useCollectionInternal(collection.get(), status);
} }
/// @brief locks a collection for usage by name /// @brief locks a collection for usage by name
arangodb::LogicalCollection* TRI_vocbase_t::useCollection( std::shared_ptr<arangodb::LogicalCollection> TRI_vocbase_t::useCollection(
std::string const& name, TRI_vocbase_col_status_e& status) { std::string const& name, TRI_vocbase_col_status_e& status) {
// check that we have an existing name // check that we have an existing name
arangodb::LogicalCollection* collection = nullptr; std::shared_ptr<arangodb::LogicalCollection> collection;
{ {
RECURSIVE_READ_LOCKER(_dataSourceLock, _dataSourceLockWriteOwner); RECURSIVE_READ_LOCKER(_dataSourceLock, _dataSourceLockWriteOwner);
auto it = _dataSourceByName.find(name); auto it = _dataSourceByName.find(name);
if (it != _dataSourceByName.end() if (it != _dataSourceByName.end()
&& it->second->category() == LogicalCollection::category()) { && it->second->category() == LogicalCollection::category()) {
TRI_ASSERT(std::dynamic_pointer_cast<LogicalCollection>(it->second)); TRI_ASSERT(std::dynamic_pointer_cast<LogicalCollection>(it->second));
collection = static_cast<LogicalCollection*>(it->second.get()); collection = std::static_pointer_cast<LogicalCollection>(it->second);
} }
} }
return useCollectionInternal(collection, status); return useCollectionInternal(std::move(collection), status);
} }
/// @brief locks a collection for usage by name /// @brief locks a collection for usage by name
arangodb::LogicalCollection* TRI_vocbase_t::useCollectionByUuid( std::shared_ptr<arangodb::LogicalCollection> TRI_vocbase_t::useCollectionByUuid(
std::string const& uuid, TRI_vocbase_col_status_e& status) { std::string const& uuid, TRI_vocbase_col_status_e& status) {
auto collection = lookupCollectionByUuid(uuid); return useCollectionInternal(lookupCollectionByUuid(uuid), status);
return useCollectionInternal(collection.get(), status);
} }
arangodb::LogicalCollection* TRI_vocbase_t::useCollectionInternal( std::shared_ptr<arangodb::LogicalCollection> TRI_vocbase_t::useCollectionInternal(
arangodb::LogicalCollection* collection, TRI_vocbase_col_status_e& status) { std::shared_ptr<arangodb::LogicalCollection> coll, TRI_vocbase_col_status_e& status) {
if (collection == nullptr) { if (!coll) {
TRI_set_errno(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND); TRI_set_errno(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND);
return nullptr; return nullptr;
} }
// try to load the collection // try to load the collection
int res = loadCollection(collection, status); int res = loadCollection(coll.get(), status);
if (res == TRI_ERROR_NO_ERROR) { if (res == TRI_ERROR_NO_ERROR) {
return collection; return coll;
} }
TRI_set_errno(res); TRI_set_errno(res);
return nullptr; return nullptr;
} }

18
arangod/VocBase/vocbase.h
View File

@ -370,22 +370,22 @@ struct TRI_vocbase_t {
/// Note that this will READ lock the collection you have to release the /// Note that this will READ lock the collection you have to release the
/// collection lock by yourself and call @ref TRI_ReleaseCollectionVocBase /// collection lock by yourself and call @ref TRI_ReleaseCollectionVocBase
/// when you are done with the collection. /// when you are done with the collection.
arangodb::LogicalCollection* useCollection(TRI_voc_cid_t cid, std::shared_ptr<arangodb::LogicalCollection> useCollection(TRI_voc_cid_t cid,
TRI_vocbase_col_status_e&); TRI_vocbase_col_status_e&);
/// @brief locks a collection for usage by name /// @brief locks a collection for usage by name
/// Note that this will READ lock the collection you have to release the /// Note that this will READ lock the collection you have to release the
/// collection lock by yourself and call @ref TRI_ReleaseCollectionVocBase /// collection lock by yourself and call @ref TRI_ReleaseCollectionVocBase
/// when you are done with the collection. /// when you are done with the collection.
arangodb::LogicalCollection* useCollection(std::string const& name, std::shared_ptr<arangodb::LogicalCollection> useCollection(std::string const& name,
TRI_vocbase_col_status_e&); TRI_vocbase_col_status_e&);
/// @brief locks a collection for usage by uuid /// @brief locks a collection for usage by uuid
/// Note that this will READ lock the collection you have to release the /// Note that this will READ lock the collection you have to release the
/// collection lock by yourself and call @ref TRI_ReleaseCollectionVocBase /// collection lock by yourself and call @ref TRI_ReleaseCollectionVocBase
/// when you are done with the collection. /// when you are done with the collection.
arangodb::LogicalCollection* useCollectionByUuid(std::string const& uuid, std::shared_ptr<arangodb::LogicalCollection> useCollectionByUuid(std::string const& uuid,
TRI_vocbase_col_status_e&); TRI_vocbase_col_status_e&);
/// @brief releases a collection from usage /// @brief releases a collection from usage
void releaseCollection(arangodb::LogicalCollection* collection); void releaseCollection(arangodb::LogicalCollection* collection);
@ -405,8 +405,8 @@ struct TRI_vocbase_t {
/// @brief check some invariants on the various lists of collections /// @brief check some invariants on the various lists of collections
void checkCollectionInvariants() const; void checkCollectionInvariants() const;
arangodb::LogicalCollection* useCollectionInternal( std::shared_ptr<arangodb::LogicalCollection> useCollectionInternal(
arangodb::LogicalCollection* collection, TRI_vocbase_col_status_e& status); std::shared_ptr<arangodb::LogicalCollection>, TRI_vocbase_col_status_e& status);
int loadCollection(arangodb::LogicalCollection* collection, int loadCollection(arangodb::LogicalCollection* collection,
TRI_vocbase_col_status_e& status, bool setStatus = true); TRI_vocbase_col_status_e& status, bool setStatus = true);
@ -455,4 +455,4 @@ void TRI_SanitizeObject(arangodb::velocypack::Slice const slice,
void TRI_SanitizeObjectWithEdges(arangodb::velocypack::Slice const slice, void TRI_SanitizeObjectWithEdges(arangodb::velocypack::Slice const slice,
arangodb::velocypack::Builder& builder); arangodb::velocypack::Builder& builder);
#endif #endif

2
tests/IResearch/StorageEngineMock.cpp
View File

@ -1447,7 +1447,7 @@ int TransactionCollectionMock::lockRecursive(arangodb::AccessMode::Type type, in
void TransactionCollectionMock::release() { void TransactionCollectionMock::release() {
if (_collection) { if (_collection) {
_transaction->vocbase().releaseCollection(_collection); _transaction->vocbase().releaseCollection(_collection.get());
_collection = nullptr; _collection = nullptr;
} }
} }