From 568f6de65c6e9a88a26aa2e10b12cb68ae0f1b06 Mon Sep 17 00:00:00 2001 From: Eliot Horowitz Date: Mon, 4 Aug 2014 09:35:48 -0400 Subject: SERVER-13635: change ownership of DatabaseCatalogEntry, now StorageEngine owns --- src/mongo/db/catalog/database.cpp | 2 +- src/mongo/db/catalog/database.h | 4 ++-- src/mongo/db/catalog/database_holder.cpp | 4 ++++ src/mongo/db/storage/heap1/heap1_engine.cpp | 20 ++++++++++++------ src/mongo/db/storage/heap1/heap1_engine.h | 7 +++--- src/mongo/db/storage/mmap_v1/mmap_v1_engine.cpp | 27 +++++++++++++++++++----- src/mongo/db/storage/mmap_v1/mmap_v1_engine.h | 12 +++++++++++ src/mongo/db/storage/mmap_v1/repair_database.cpp | 16 +++++++------- src/mongo/db/storage/storage_engine.h | 8 ++++++- 9 files changed, 73 insertions(+), 27 deletions(-) diff --git a/src/mongo/db/catalog/database.cpp b/src/mongo/db/catalog/database.cpp index f3c8179b9e7..fa0b78ad859 100644 --- a/src/mongo/db/catalog/database.cpp +++ b/src/mongo/db/catalog/database.cpp @@ -524,7 +524,7 @@ namespace mongo { } const DatabaseCatalogEntry* Database::getDatabaseCatalogEntry() const { - return _dbEntry.get(); + return _dbEntry; } void dropAllDatabasesExceptLocal(OperationContext* txn) { diff --git a/src/mongo/db/catalog/database.h b/src/mongo/db/catalog/database.h index 2b080f063de..40c6903ea5c 100644 --- a/src/mongo/db/catalog/database.h +++ b/src/mongo/db/catalog/database.h @@ -59,7 +59,7 @@ namespace mongo { // you probably need to be in dbHolderMutex when constructing this Database(OperationContext* txn, const StringData& name, - DatabaseCatalogEntry* dbEntry ); + DatabaseCatalogEntry* dbEntry ); // not owner here // must call close first ~Database(); @@ -144,7 +144,7 @@ namespace mongo { const std::string _name; // "alleyinsider" - boost::scoped_ptr _dbEntry; + DatabaseCatalogEntry* _dbEntry; // not owned here const std::string _profileName; // "alleyinsider.system.profile" const std::string _indexesName; // "alleyinsider.system.indexes" diff --git a/src/mongo/db/catalog/database_holder.cpp b/src/mongo/db/catalog/database_holder.cpp index 8b494631d67..6d129153add 100644 --- a/src/mongo/db/catalog/database_holder.cpp +++ b/src/mongo/db/catalog/database_holder.cpp @@ -135,6 +135,8 @@ namespace mongo { it->second->close( txn ); delete it->second; _dbs.erase( db ); + + getGlobalEnvironment()->getGlobalStorageEngine()->closeDatabase( txn, db.toString() ); } bool DatabaseHolder::closeAll(OperationContext* txn, @@ -173,6 +175,8 @@ namespace mongo { _dbs.erase( name ); + getGlobalEnvironment()->getGlobalStorageEngine()->closeDatabase( txn, name ); + bb.append( name ); } diff --git a/src/mongo/db/storage/heap1/heap1_engine.cpp b/src/mongo/db/storage/heap1/heap1_engine.cpp index 3c229390e87..5efe7e9b2ff 100644 --- a/src/mongo/db/storage/heap1/heap1_engine.cpp +++ b/src/mongo/db/storage/heap1/heap1_engine.cpp @@ -35,14 +35,21 @@ namespace mongo { + Heap1Engine::~Heap1Engine() { + for ( DBMap::const_iterator it = _dbs.begin(); it != _dbs.end(); ++it ) { + delete it->second; + } + _dbs.clear(); + } + RecoveryUnit* Heap1Engine::newRecoveryUnit( OperationContext* opCtx ) { return new Heap1RecoveryUnit(); } void Heap1Engine::listDatabases( std::vector* out ) const { boost::mutex::scoped_lock lk( _dbLock ); - for ( DBMap::const_iterator i = _dbs.begin(); i != _dbs.end(); ++i ) { - out->push_back( *i ); + for ( DBMap::const_iterator it = _dbs.begin(); it != _dbs.end(); ++it ) { + out->push_back( it->first ); } } @@ -50,15 +57,14 @@ namespace mongo { const StringData& dbName ) { boost::mutex::scoped_lock lk( _dbLock ); - // THIS is temporary I think - _dbs.insert( dbName.toString() ); - return new Heap1DatabaseCatalogEntry( dbName ); - /* Heap1DatabaseCatalogEntry*& db = _dbs[dbName.toString()]; if ( !db ) db = new Heap1DatabaseCatalogEntry( dbName ); return db; - */ } + Status Heap1Engine::closeDatabase(OperationContext* txn, const StringData& db ) { + // no-op as not file handles + return Status::OK(); + } } diff --git a/src/mongo/db/storage/heap1/heap1_engine.h b/src/mongo/db/storage/heap1/heap1_engine.h index 727a18fb6ea..ce624bf4184 100644 --- a/src/mongo/db/storage/heap1/heap1_engine.h +++ b/src/mongo/db/storage/heap1/heap1_engine.h @@ -30,7 +30,6 @@ #pragma once -#include #include #include @@ -44,7 +43,7 @@ namespace mongo { class Heap1Engine : public StorageEngine { public: - virtual ~Heap1Engine() {} + virtual ~Heap1Engine(); virtual RecoveryUnit* newRecoveryUnit( OperationContext* opCtx ); @@ -53,6 +52,8 @@ namespace mongo { virtual DatabaseCatalogEntry* getDatabaseCatalogEntry( OperationContext* opCtx, const StringData& db ); + virtual Status closeDatabase(OperationContext* txn, const StringData& db ); + /** * @return number of files flushed */ @@ -65,7 +66,7 @@ namespace mongo { private: mutable boost::mutex _dbLock; - typedef std::set DBMap; + typedef std::map DBMap; DBMap _dbs; }; diff --git a/src/mongo/db/storage/mmap_v1/mmap_v1_engine.cpp b/src/mongo/db/storage/mmap_v1/mmap_v1_engine.cpp index e8ef1e285c5..c34a0b9b7a2 100644 --- a/src/mongo/db/storage/mmap_v1/mmap_v1_engine.cpp +++ b/src/mongo/db/storage/mmap_v1/mmap_v1_engine.cpp @@ -301,6 +301,10 @@ namespace { } MMAPV1Engine::~MMAPV1Engine() { + for ( EntryMap::const_iterator it = _entryMap.begin(); it != _entryMap.end(); ++it ) { + delete it->second; + } + _entryMap.clear(); } RecoveryUnit* MMAPV1Engine::newRecoveryUnit( OperationContext* opCtx ) { @@ -313,11 +317,24 @@ namespace { DatabaseCatalogEntry* MMAPV1Engine::getDatabaseCatalogEntry( OperationContext* opCtx, const StringData& db ) { - return new MMAPV1DatabaseCatalogEntry( opCtx, - db, - storageGlobalParams.dbpath, - storageGlobalParams.directoryperdb, - false ); + boost::mutex::scoped_lock lk( _entryMapMutex ); + MMAPV1DatabaseCatalogEntry*& entry = _entryMap[db.toString()]; + if ( !entry ) { + entry = new MMAPV1DatabaseCatalogEntry( opCtx, + db, + storageGlobalParams.dbpath, + storageGlobalParams.directoryperdb, + false ); + } + return entry; + } + + Status MMAPV1Engine::closeDatabase( OperationContext* txn, const StringData& db ) { + boost::mutex::scoped_lock lk( _entryMapMutex ); + MMAPV1DatabaseCatalogEntry* entry = _entryMap[db.toString()]; + delete entry; + _entryMap.erase( db.toString() ); + return Status::OK(); } void MMAPV1Engine::_listDatabases( const std::string& directory, diff --git a/src/mongo/db/storage/mmap_v1/mmap_v1_engine.h b/src/mongo/db/storage/mmap_v1/mmap_v1_engine.h index ca021729171..09f09b2c366 100644 --- a/src/mongo/db/storage/mmap_v1/mmap_v1_engine.h +++ b/src/mongo/db/storage/mmap_v1/mmap_v1_engine.h @@ -30,10 +30,16 @@ #pragma once +#include + +#include + #include "mongo/db/storage/storage_engine.h" namespace mongo { + class MMAPV1DatabaseCatalogEntry; + class MMAPV1Engine : public StorageEngine { public: MMAPV1Engine(); @@ -51,10 +57,16 @@ namespace mongo { DatabaseCatalogEntry* getDatabaseCatalogEntry( OperationContext* opCtx, const StringData& db ); + Status closeDatabase(OperationContext* txn, const StringData& db ); + void cleanShutdown(OperationContext* txn); private: static void _listDatabases( const std::string& directory, std::vector* out ); + + boost::mutex _entryMapMutex; + typedef std::map EntryMap; + EntryMap _entryMap; }; } diff --git a/src/mongo/db/storage/mmap_v1/repair_database.cpp b/src/mongo/db/storage/mmap_v1/repair_database.cpp index 9a6775e7e9b..7d7bf4239b4 100644 --- a/src/mongo/db/storage/mmap_v1/repair_database.cpp +++ b/src/mongo/db/storage/mmap_v1/repair_database.cpp @@ -324,18 +324,18 @@ namespace mongo { return Status(ErrorCodes::NamespaceNotFound, "database does not exist to repair"); } + scoped_ptr dbEntry; scoped_ptr tempDatabase; { - MMAPV1DatabaseCatalogEntry* entry = - new MMAPV1DatabaseCatalogEntry( txn, - dbName, - reservedPathString, - storageGlobalParams.directoryperdb, - true ); - invariant( !entry->exists() ); + dbEntry.reset( new MMAPV1DatabaseCatalogEntry( txn, + dbName, + reservedPathString, + storageGlobalParams.directoryperdb, + true ) ); + invariant( !dbEntry->exists() ); tempDatabase.reset( new Database( txn, dbName, - entry ) ); + dbEntry.get() ) ); } diff --git a/src/mongo/db/storage/storage_engine.h b/src/mongo/db/storage/storage_engine.h index f1ac7295cd4..2015deeec59 100644 --- a/src/mongo/db/storage/storage_engine.h +++ b/src/mongo/db/storage/storage_engine.h @@ -78,11 +78,17 @@ namespace mongo { /** * Return the DatabaseCatalogEntry that describes the database indicated by 'db'. * - * Caller owns pointer. + * StorageEngine owns returned pointer. + * It should not be deleted by any caller. */ virtual DatabaseCatalogEntry* getDatabaseCatalogEntry( OperationContext* opCtx, const StringData& db ) = 0; + /** + * Closes all file handles associated with a database. + */ + virtual Status closeDatabase( OperationContext* txn, const StringData& db ) = 0; + /** * @return number of files flushed */ -- cgit v1.2.1