SERVER-22956 abstract ExtentManager construction

10 files changed, 114 insertions, 41 deletions

diff --git a/src/mongo/db/storage/mmap_v1/extent_manager.h b/src/mongo/db/storage/mmap_v1/extent_manager.h
index 052634d639b..4ce623ffc8a 100644
--- a/src/mongo/db/storage/mmap_v1/extent_manager.h
+++ b/src/mongo/db/storage/mmap_v1/extent_manager.h
@@ -41,6 +41,7 @@
 namespace mongo {
 
 class DataFile;
+class DataFileVersion;
 class MmapV1RecordHeader;
 class RecordFetcher;
 class OperationContext;
@@ -66,6 +67,14 @@ class ExtentManager {
 public:
     ExtentManager() {}
 
+    class Factory {
+    public:
+        virtual ~Factory() = default;
+        virtual std::unique_ptr<ExtentManager> create(StringData dbname,
+                                                      StringData path,
+                                                      bool directoryPerDB) = 0;
+    };
+
     virtual ~ExtentManager() {}
 
     /**
@@ -176,5 +185,11 @@ public:
      * Caller takes owernship of CacheHint
      */
     virtual CacheHint* cacheHint(const DiskLoc& extentLoc, const HintType& hint) = 0;
+
+    virtual DataFileVersion getFileFormat(OperationContext* txn) const = 0;
+    virtual void setFileFormat(OperationContext* txn, DataFileVersion newVersion) = 0;
+
+    virtual const DataFile* getOpenFile(int n) const = 0;
 };
-}
+
+}  // namespace mongo
diff --git a/src/mongo/db/storage/mmap_v1/mmap_v1_database_catalog_entry.cpp b/src/mongo/db/storage/mmap_v1/mmap_v1_database_catalog_entry.cpp
index dc1e6bd3af1..4f97ed08f4c 100644
--- a/src/mongo/db/storage/mmap_v1/mmap_v1_database_catalog_entry.cpp
+++ b/src/mongo/db/storage/mmap_v1/mmap_v1_database_catalog_entry.cpp
@@ -152,12 +152,16 @@ private:
     Entry* const _cachedEntry;
 };
 
-MMAPV1DatabaseCatalogEntry::MMAPV1DatabaseCatalogEntry(
-    OperationContext* txn, StringData name, StringData path, bool directoryPerDB, bool transient)
+MMAPV1DatabaseCatalogEntry::MMAPV1DatabaseCatalogEntry(OperationContext* txn,
+                                                       StringData name,
+                                                       StringData path,
+                                                       bool directoryPerDB,
+                                                       bool transient,
+                                                       std::unique_ptr<ExtentManager> extentManager)
     : DatabaseCatalogEntry(name),
       _path(path.toString()),
       _namespaceIndex(_path, name.toString()),
-      _extentManager(name, path, directoryPerDB) {
+      _extentManager(std::move(extentManager)) {
     invariant(txn->lockState()->isDbLockedForMode(name, MODE_X));
 
     try {
@@ -168,9 +172,9 @@ MMAPV1DatabaseCatalogEntry::MMAPV1DatabaseCatalogEntry(
         // Initialize the extent manager. This will create the first data file (.0) if needed
         // and if this fails we would leak the .ns file above. Leaking the .ns or .0 file is
         // acceptable, because subsequent openDB calls will exercise the code path again.
-        Status s = _extentManager.init(txn);
+        Status s = _extentManager->init(txn);
         if (!s.isOK()) {
-            msgasserted(16966, str::stream() << "_extentManager.init failed: " << s.toString());
+            msgasserted(16966, str::stream() << "_extentManager->init failed: " << s.toString());
         }
 
         // This is the actual loading of the on-disk structures into cache.
@@ -231,7 +235,7 @@ Status MMAPV1DatabaseCatalogEntry::dropCollection(OperationContext* txn, StringD
 
     // free extents
     if (!details->firstExtent.isNull()) {
-        _extentManager.freeExtents(txn, details->firstExtent, details->lastExtent);
+        _extentManager->freeExtents(txn, details->firstExtent, details->lastExtent);
         *txn->recoveryUnit()->writing(&details->firstExtent) = DiskLoc().setInvalid();
         *txn->recoveryUnit()->writing(&details->lastExtent) = DiskLoc().setInvalid();
     }
@@ -384,13 +388,13 @@ void MMAPV1DatabaseCatalogEntry::appendExtraStats(OperationContext* opCtx,
     if (isEmpty()) {
         output->appendNumber("fileSize", 0);
     } else {
-        output->appendNumber("fileSize", _extentManager.fileSize() / scale);
+        output->appendNumber("fileSize", _extentManager->fileSize() / scale);
         output->appendNumber("nsSizeMB",
                              static_cast<int>(_namespaceIndex.fileLength() / (1024 * 1024)));
 
         int freeListSize = 0;
         int64_t freeListSpace = 0;
-        _extentManager.freeListStats(opCtx, &freeListSize, &freeListSpace);
+        _extentManager->freeListStats(opCtx, &freeListSize, &freeListSpace);
 
         BSONObjBuilder extentFreeList(output->subobjStart("extentFreeList"));
         extentFreeList.append("num", freeListSize);
@@ -398,7 +402,7 @@ void MMAPV1DatabaseCatalogEntry::appendExtraStats(OperationContext* opCtx,
         extentFreeList.done();
 
         {
-            const DataFileVersion version = _extentManager.getFileFormat(opCtx);
+            const DataFileVersion version = _extentManager->getFileFormat(opCtx);
 
             BSONObjBuilder dataFileVersion(output->subobjStart("dataFileVersion"));
             dataFileVersion.append("major", version.majorRaw());
@@ -409,10 +413,10 @@ void MMAPV1DatabaseCatalogEntry::appendExtraStats(OperationContext* opCtx,
 }
 
 bool MMAPV1DatabaseCatalogEntry::isOlderThan24(OperationContext* opCtx) const {
-    if (_extentManager.numFiles() == 0)
+    if (_extentManager->numFiles() == 0)
         return false;
 
-    const DataFileVersion version = _extentManager.getFileFormat(opCtx);
+    const DataFileVersion version = _extentManager->getFileFormat(opCtx);
 
     invariant(version.isCompatibleWithCurrentCode());
 
@@ -420,10 +424,10 @@ bool MMAPV1DatabaseCatalogEntry::isOlderThan24(OperationContext* opCtx) const {
 }
 
 void MMAPV1DatabaseCatalogEntry::markIndexSafe24AndUp(OperationContext* opCtx) {
-    if (_extentManager.numFiles() == 0)
+    if (_extentManager->numFiles() == 0)
         return;
 
-    DataFileVersion version = _extentManager.getFileFormat(opCtx);
+    DataFileVersion version = _extentManager->getFileFormat(opCtx);
 
     invariant(version.isCompatibleWithCurrentCode());
 
@@ -431,14 +435,14 @@ void MMAPV1DatabaseCatalogEntry::markIndexSafe24AndUp(OperationContext* opCtx) {
         return;  // nothing to do
 
     version.setIs24IndexClean();
-    _extentManager.setFileFormat(opCtx, version);
+    _extentManager->setFileFormat(opCtx, version);
 }
 
 bool MMAPV1DatabaseCatalogEntry::currentFilesCompatible(OperationContext* opCtx) const {
-    if (_extentManager.numFiles() == 0)
+    if (_extentManager->numFiles() == 0)
         return true;
 
-    return _extentManager.getOpenFile(0)->getHeader()->version.isCompatibleWithCurrentCode();
+    return _extentManager->getOpenFile(0)->getHeader()->version.isCompatibleWithCurrentCode();
 }
 
 void MMAPV1DatabaseCatalogEntry::getCollectionNamespaces(std::list<std::string>* tofill) const {
@@ -477,14 +481,14 @@ void MMAPV1DatabaseCatalogEntry::_init(OperationContext* txn) {
         }
 
         if (!freeListDetails->firstExtent.isNull()) {
-            _extentManager.freeExtents(
+            _extentManager->freeExtents(
                 txn, freeListDetails->firstExtent, freeListDetails->lastExtent);
         }
 
         _namespaceIndex.kill_ns(txn, oldFreeList.ns());
     }
 
-    DataFileVersion version = _extentManager.getFileFormat(txn);
+    DataFileVersion version = _extentManager->getFileFormat(txn);
     if (version.isCompatibleWithCurrentCode() && !version.mayHave28Freelist()) {
         if (storageGlobalParams.readOnly) {
             severe() << "Legacy storage format detected, but server was started with the "
@@ -494,7 +498,7 @@ void MMAPV1DatabaseCatalogEntry::_init(OperationContext* txn) {
 
         // Any DB that can be opened and written to gets this flag set.
         version.setMayHave28Freelist();
-        _extentManager.setFileFormat(txn, version);
+        _extentManager->setFileFormat(txn, version);
     }
 
     const NamespaceString nsi(name(), "system.indexes");
@@ -532,7 +536,7 @@ void MMAPV1DatabaseCatalogEntry::_init(OperationContext* txn) {
         NamespaceDetailsRSV1MetaData* md =
             new NamespaceDetailsRSV1MetaData(nsn.toString(), nsDetails);
         nsEntry->recordStore.reset(
-            new SimpleRecordStoreV1(txn, nsn.toString(), md, &_extentManager, false));
+            new SimpleRecordStoreV1(txn, nsn.toString(), md, _extentManager.get(), false));
     }
 
     if (!indexEntry) {
@@ -542,7 +546,7 @@ void MMAPV1DatabaseCatalogEntry::_init(OperationContext* txn) {
             new NamespaceDetailsRSV1MetaData(nsi.toString(), indexDetails);
 
         indexEntry->recordStore.reset(
-            new SimpleRecordStoreV1(txn, nsi.toString(), md, &_extentManager, true));
+            new SimpleRecordStoreV1(txn, nsi.toString(), md, _extentManager.get(), true));
     }
 
     RecordId indexNamespaceId;
@@ -632,14 +636,14 @@ Status MMAPV1DatabaseCatalogEntry::createCollection(OperationContext* txn,
     if (allocateDefaultSpace) {
         RecordStoreV1Base* rs = _getRecordStore(ns);
         if (options.initialNumExtents > 0) {
-            int size = _massageExtentSize(&_extentManager, options.cappedSize);
+            int size = _massageExtentSize(_extentManager.get(), options.cappedSize);
             for (int i = 0; i < options.initialNumExtents; i++) {
                 rs->increaseStorageSize(txn, size, false);
             }
         } else if (!options.initialExtentSizes.empty()) {
             for (size_t i = 0; i < options.initialExtentSizes.size(); i++) {
                 int size = options.initialExtentSizes[i];
-                size = _massageExtentSize(&_extentManager, size);
+                size = _massageExtentSize(_extentManager.get(), size);
                 rs->increaseStorageSize(txn, size, false);
             }
         } else if (options.capped) {
@@ -647,13 +651,13 @@ Status MMAPV1DatabaseCatalogEntry::createCollection(OperationContext* txn,
             do {
                 // Must do this at least once, otherwise we leave the collection with no
                 // extents, which is invalid.
-                int sz =
-                    _massageExtentSize(&_extentManager, options.cappedSize - rs->storageSize(txn));
+                int sz = _massageExtentSize(_extentManager.get(),
+                                            options.cappedSize - rs->storageSize(txn));
                 sz &= 0xffffff00;
                 rs->increaseStorageSize(txn, sz, false);
             } while (rs->storageSize(txn) < options.cappedSize);
         } else {
-            rs->increaseStorageSize(txn, _extentManager.initialSize(128), false);
+            rs->increaseStorageSize(txn, _extentManager->initialSize(128), false);
         }
     }
 
@@ -700,10 +704,10 @@ void MMAPV1DatabaseCatalogEntry::_insertInCache(OperationContext* txn,
 
     if (details->isCapped) {
         entry->recordStore.reset(new CappedRecordStoreV1(
-            txn, NULL, ns, md.release(), &_extentManager, nss.coll() == "system.indexes"));
+            txn, NULL, ns, md.release(), _extentManager.get(), nss.coll() == "system.indexes"));
     } else {
         entry->recordStore.reset(new SimpleRecordStoreV1(
-            txn, ns, md.release(), &_extentManager, nss.coll() == "system.indexes"));
+            txn, ns, md.release(), _extentManager.get(), nss.coll() == "system.indexes"));
     }
 }
 
diff --git a/src/mongo/db/storage/mmap_v1/mmap_v1_database_catalog_entry.h b/src/mongo/db/storage/mmap_v1/mmap_v1_database_catalog_entry.h
index 4211e26096a..c907ba2b2ab 100644
--- a/src/mongo/db/storage/mmap_v1/mmap_v1_database_catalog_entry.h
+++ b/src/mongo/db/storage/mmap_v1/mmap_v1_database_catalog_entry.h
@@ -57,7 +57,8 @@ public:
                                StringData name,
                                StringData path,
                                bool directoryperdb,
-                               bool transient);
+                               bool transient,
+                               std::unique_ptr<ExtentManager> extentManager);
 
     virtual ~MMAPV1DatabaseCatalogEntry();
 
@@ -110,11 +111,11 @@ public:
                                 const CollectionCatalogEntry* collection,
                                 IndexCatalogEntry* index);
 
-    const MmapV1ExtentManager* getExtentManager() const {
-        return &_extentManager;
+    const ExtentManager* getExtentManager() const {
+        return _extentManager.get();
     }
-    MmapV1ExtentManager* getExtentManager() {
-        return &_extentManager;
+    ExtentManager* getExtentManager() {
+        return _extentManager.get();
     }
 
     CollectionOptions getCollectionOptions(OperationContext* txn, StringData ns) const;
@@ -186,7 +187,7 @@ private:
     const std::string _path;
 
     NamespaceIndex _namespaceIndex;
-    MmapV1ExtentManager _extentManager;
+    std::unique_ptr<ExtentManager> _extentManager;
     CollectionMap _collections;
 };
 }
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 4f29d366b38..a448ee579ad 100644
--- a/src/mongo/db/storage/mmap_v1/mmap_v1_engine.cpp
+++ b/src/mongo/db/storage/mmap_v1/mmap_v1_engine.cpp
@@ -220,7 +220,12 @@ void clearTmpFiles() {
 }
 }  // namespace
 
-MMAPV1Engine::MMAPV1Engine(const StorageEngineLockFile* lockFile) {
+MMAPV1Engine::MMAPV1Engine(const StorageEngineLockFile* lockFile)
+    : MMAPV1Engine(lockFile, stdx::make_unique<MmapV1ExtentManager::Factory>()) {}
+
+MMAPV1Engine::MMAPV1Engine(const StorageEngineLockFile* lockFile,
+                           std::unique_ptr<ExtentManager::Factory> extentManagerFactory)
+    : _extentManagerFactory(std::move(extentManagerFactory)) {
     // TODO check non-journal subdirs if using directory-per-db
     checkReadAhead(storageGlobalParams.dbpath);
 
@@ -272,7 +277,13 @@ DatabaseCatalogEntry* MMAPV1Engine::getDatabaseCatalogEntry(OperationContext* op
     // can be creating the same database concurrenty. We need to create the database outside of
     // the _entryMapMutex so we do not deadlock (see SERVER-15880).
     MMAPV1DatabaseCatalogEntry* entry = new MMAPV1DatabaseCatalogEntry(
-        opCtx, db, storageGlobalParams.dbpath, storageGlobalParams.directoryperdb, false);
+        opCtx,
+        db,
+        storageGlobalParams.dbpath,
+        storageGlobalParams.directoryperdb,
+        false,
+        _extentManagerFactory->create(
+            db, storageGlobalParams.dbpath, storageGlobalParams.directoryperdb));
 
     stdx::lock_guard<stdx::mutex> lk(_entryMapMutex);
 
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 00f2f655f98..83704c8093b 100644
--- a/src/mongo/db/storage/mmap_v1/mmap_v1_engine.h
+++ b/src/mongo/db/storage/mmap_v1/mmap_v1_engine.h
@@ -33,6 +33,7 @@
 #include <map>
 
 #include "mongo/db/storage/mmap_v1/record_access_tracker.h"
+#include "mongo/db/storage/mmap_v1/extent_manager.h"
 #include "mongo/db/storage/storage_engine.h"
 #include "mongo/stdx/mutex.h"
 
@@ -44,6 +45,8 @@ class MMAPV1DatabaseCatalogEntry;
 class MMAPV1Engine : public StorageEngine {
 public:
     MMAPV1Engine(const StorageEngineLockFile* lockFile);
+    MMAPV1Engine(const StorageEngineLockFile* lockFile,
+                 std::unique_ptr<ExtentManager::Factory> extentManagerFactory);
     virtual ~MMAPV1Engine();
 
     void finishInit();
@@ -109,6 +112,8 @@ private:
     // addresses. It is used when higher layers (e.g. the query system) need to ask
     // the storage engine whether data is likely in physical memory.
     RecordAccessTracker _recordAccessTracker;
+
+    std::unique_ptr<ExtentManager::Factory> _extentManagerFactory;
 };
 
 void _deleteDataFiles(const std::string& database);
diff --git a/src/mongo/db/storage/mmap_v1/mmap_v1_extent_manager.cpp b/src/mongo/db/storage/mmap_v1/mmap_v1_extent_manager.cpp
index d7c44aabfab..2076ca868b1 100644
--- a/src/mongo/db/storage/mmap_v1/mmap_v1_extent_manager.cpp
+++ b/src/mongo/db/storage/mmap_v1/mmap_v1_extent_manager.cpp
@@ -119,6 +119,13 @@ MmapV1ExtentManager::MmapV1ExtentManager(StringData dbname, StringData path, boo
     _recordAccessTracker = &mmapEngine->getRecordAccessTracker();
 }
 
+std::unique_ptr<ExtentManager> MmapV1ExtentManager::Factory::create(StringData dbname,
+                                                                    StringData path,
+                                                                    bool directoryPerDB) {
+    return stdx::make_unique<MmapV1ExtentManager>(
+        std::move(dbname), std::move(path), directoryPerDB);
+}
+
 boost::filesystem::path MmapV1ExtentManager::_fileName(int n) const {
     stringstream ss;
     ss << _dbname << '.' << n;
diff --git a/src/mongo/db/storage/mmap_v1/mmap_v1_extent_manager.h b/src/mongo/db/storage/mmap_v1/mmap_v1_extent_manager.h
index a2f2931e1b4..a66a19f8fb3 100644
--- a/src/mongo/db/storage/mmap_v1/mmap_v1_extent_manager.h
+++ b/src/mongo/db/storage/mmap_v1/mmap_v1_extent_manager.h
@@ -76,6 +76,12 @@ class MmapV1ExtentManager : public ExtentManager {
     MONGO_DISALLOW_COPYING(MmapV1ExtentManager);
 
 public:
+    class Factory : public ExtentManager::Factory {
+        virtual std::unique_ptr<ExtentManager> create(StringData dbname,
+                                                      StringData path,
+                                                      bool directoryPerDB) final;
+    };
+
     /**
      * @param freeListDetails this is a reference into the .ns file
      *        while a bit odd, this is not a layer violation as extents
@@ -141,10 +147,10 @@ public:
     /**
      * Not thread safe, requires a database exclusive lock
      */
-    DataFileVersion getFileFormat(OperationContext* txn) const;
-    void setFileFormat(OperationContext* txn, DataFileVersion newVersion);
+    DataFileVersion getFileFormat(OperationContext* txn) const final;
+    void setFileFormat(OperationContext* txn, DataFileVersion newVersion) final;
 
-    const DataFile* getOpenFile(int n) const {
+    const DataFile* getOpenFile(int n) const final {
         return _getOpenFile(n);
     }
 
diff --git a/src/mongo/db/storage/mmap_v1/record_store_v1_test_help.cpp b/src/mongo/db/storage/mmap_v1/record_store_v1_test_help.cpp
index 12801124b95..234acf8695e 100644
--- a/src/mongo/db/storage/mmap_v1/record_store_v1_test_help.cpp
+++ b/src/mongo/db/storage/mmap_v1/record_store_v1_test_help.cpp
@@ -295,6 +295,16 @@ DummyExtentManager::CacheHint* DummyExtentManager::cacheHint(const DiskLoc& exte
     return new CacheHint();
 }
 
+DataFileVersion DummyExtentManager::getFileFormat(OperationContext* txn) const {
+    return DataFileVersion::defaultForNewFiles();
+}
+
+void DummyExtentManager::setFileFormat(OperationContext* txn, DataFileVersion newVersion) {}
+
+const DataFile* DummyExtentManager::getOpenFile(int n) const {
+    return nullptr;
+}
+
 namespace {
 void accumulateExtentSizeRequirements(const LocAndSize* las, std::map<int, size_t>* sizes) {
     if (!las)
diff --git a/src/mongo/db/storage/mmap_v1/record_store_v1_test_help.h b/src/mongo/db/storage/mmap_v1/record_store_v1_test_help.h
index 0a038f9e9f3..1f29b59334c 100644
--- a/src/mongo/db/storage/mmap_v1/record_store_v1_test_help.h
+++ b/src/mongo/db/storage/mmap_v1/record_store_v1_test_help.h
@@ -32,6 +32,7 @@
 
 #include <vector>
 
+#include "mongo/db/storage/mmap_v1/data_file.h"
 #include "mongo/db/storage/mmap_v1/extent_manager.h"
 #include "mongo/db/storage/mmap_v1/record_store_v1_base.h"
 
@@ -141,6 +142,13 @@ public:
 
     virtual CacheHint* cacheHint(const DiskLoc& extentLoc, const HintType& hint);
 
+    DataFileVersion getFileFormat(OperationContext* txn) const final;
+
+    virtual void setFileFormat(OperationContext* txn, DataFileVersion newVersion) final;
+
+    const DataFile* getOpenFile(int n) const final;
+
+
 protected:
     struct ExtentInfo {
         char* data;
diff --git a/src/mongo/db/storage/mmap_v1/repair_database.cpp b/src/mongo/db/storage/mmap_v1/repair_database.cpp
index a6b31ba784f..b73d346a708 100644
--- a/src/mongo/db/storage/mmap_v1/repair_database.cpp
+++ b/src/mongo/db/storage/mmap_v1/repair_database.cpp
@@ -322,7 +322,13 @@ Status MMAPV1Engine::repairDatabase(OperationContext* txn,
 
         {
             dbEntry.reset(new MMAPV1DatabaseCatalogEntry(
-                txn, dbName, reservedPathString, storageGlobalParams.directoryperdb, true));
+                txn,
+                dbName,
+                reservedPathString,
+                storageGlobalParams.directoryperdb,
+                true,
+                _extentManagerFactory->create(
+                    dbName, reservedPathString, storageGlobalParams.directoryperdb)));
             tempDatabase.reset(new Database(txn, dbName, dbEntry.get()));
         }