SERVER-22352 do not fail startup if lockFile cannot be created in readOnly mode

12 files changed, 65 insertions, 29 deletions

diff --git a/src/mongo/db/service_context_d.cpp b/src/mongo/db/service_context_d.cpp
index 0a5965c03b8..e48d8fe743d 100644
--- a/src/mongo/db/service_context_d.cpp
+++ b/src/mongo/db/service_context_d.cpp
@@ -85,8 +85,19 @@ void ServiceContextMongoD::createLockFile() {
                 false);
     }
     bool wasUnclean = _lockFile->createdByUncleanShutdown();
-    uassertStatusOK(_lockFile->open());
+    auto openStatus = _lockFile->open();
+    if (storageGlobalParams.readOnly && openStatus == ErrorCodes::IllegalOperation) {
+        _lockFile.reset();
+    } else {
+        uassertStatusOK(openStatus);
+    }
+
     if (wasUnclean) {
+        if (storageGlobalParams.readOnly) {
+            severe() << "Attempted to open dbpath in readOnly mode, but the server was "
+                        "previously not shut down cleanly.";
+            fassertFailedNoTrace(34414);
+        }
         warning() << "Detected unclean shutdown - " << _lockFile->getFilespec() << " is not empty.";
     }
 }
@@ -95,6 +106,11 @@ void ServiceContextMongoD::initializeGlobalStorageEngine() {
     // This should be set once.
     invariant(!_storageEngine);
 
+    // We should have a _lockFile or be in read-only mode. Confusingly, we can still have a lockFile
+    // if we are in read-only mode. This can happen if the server is started in read-only mode on a
+    // writable dbpath.
+    invariant(_lockFile || storageGlobalParams.readOnly);
+
     const std::string dbpath = storageGlobalParams.dbpath;
     if (auto existingStorageEngine = StorageEngineMetadata::getStorageEngineForPath(dbpath)) {
         if (storageGlobalParams.engineSetByUser) {
@@ -150,19 +166,34 @@ void ServiceContextMongoD::initializeGlobalStorageEngine() {
 
     std::unique_ptr<StorageEngineMetadata> metadata = StorageEngineMetadata::forPath(dbpath);
 
+    if (storageGlobalParams.readOnly) {
+        uassert(34415,
+                "Server was started in read-only mode, but the storage metadata file was not"
+                " found.",
+                metadata.get());
+    }
+
     // Validate options in metadata against current startup options.
     if (metadata.get()) {
         uassertStatusOK(factory->validateMetadata(*metadata, storageGlobalParams));
     }
 
-    invariant(_lockFile);
-    ScopeGuard guard = MakeGuard(&StorageEngineLockFile::close, _lockFile.get());
-    _storageEngine = factory->create(storageGlobalParams, *_lockFile);
+    ScopeGuard guard = MakeGuard([&] {
+        if (_lockFile) {
+            _lockFile->close();
+        }
+    });
+
+    _storageEngine = factory->create(storageGlobalParams, _lockFile.get());
     _storageEngine->finishInit();
-    uassertStatusOK(_lockFile->writePid());
+
+    if (_lockFile) {
+        uassertStatusOK(_lockFile->writePid());
+    }
 
     // Write a new metadata file if it is not present.
     if (!metadata.get()) {
+        invariant(!storageGlobalParams.readOnly);
         metadata.reset(new StorageEngineMetadata(storageGlobalParams.dbpath));
         metadata->setStorageEngine(factory->getCanonicalName().toString());
         metadata->setStorageEngineOptions(factory->createMetadataOptions(storageGlobalParams));
@@ -176,9 +207,10 @@ void ServiceContextMongoD::initializeGlobalStorageEngine() {
 
 void ServiceContextMongoD::shutdownGlobalStorageEngineCleanly() {
     invariant(_storageEngine);
-    invariant(_lockFile.get());
     _storageEngine->cleanShutdown();
-    _lockFile->clearPidAndUnlock();
+    if (_lockFile) {
+        _lockFile->clearPidAndUnlock();
+    }
 }
 
 void ServiceContextMongoD::registerStorageEngine(const std::string& name,
diff --git a/src/mongo/db/storage/devnull/devnull_init.cpp b/src/mongo/db/storage/devnull/devnull_init.cpp
index e6979b7f3ca..afdfecc1457 100644
--- a/src/mongo/db/storage/devnull/devnull_init.cpp
+++ b/src/mongo/db/storage/devnull/devnull_init.cpp
@@ -42,7 +42,7 @@ namespace {
 class DevNullStorageEngineFactory : public StorageEngine::Factory {
 public:
     virtual StorageEngine* create(const StorageGlobalParams& params,
-                                  const StorageEngineLockFile& lockFile) const {
+                                  const StorageEngineLockFile* lockFile) const {
         KVStorageEngineOptions options;
         options.directoryPerDB = params.directoryperdb;
         options.forRepair = params.repair;
diff --git a/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_init.cpp b/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_init.cpp
index 5e66d5c18dc..2418819df9a 100644
--- a/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_init.cpp
+++ b/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_init.cpp
@@ -43,7 +43,7 @@ class EphemeralForTestFactory : public StorageEngine::Factory {
 public:
     virtual ~EphemeralForTestFactory() {}
     virtual StorageEngine* create(const StorageGlobalParams& params,
-                                  const StorageEngineLockFile& lockFile) const {
+                                  const StorageEngineLockFile* lockFile) const {
         KVStorageEngineOptions options;
         options.directoryPerDB = params.directoryperdb;
         options.forRepair = params.repair;
diff --git a/src/mongo/db/storage/mmap_v1/dur_journal.cpp b/src/mongo/db/storage/mmap_v1/dur_journal.cpp
index 14f329e0931..1e5df4f1f40 100644
--- a/src/mongo/db/storage/mmap_v1/dur_journal.cpp
+++ b/src/mongo/db/storage/mmap_v1/dur_journal.cpp
@@ -212,7 +212,7 @@ boost::filesystem::path Journal::getFilePathFor(int filenumber) const {
 
 /** never throws
     @param anyFiles by default we only look at j._* files. If anyFiles is true, return true
-           if there are any files in the journal directory. acquirePathLock() uses this to
+           if there are any files in the journal directory. checkForUncleanShutdown() uses this to
            make sure that the journal directory is mounted.
     @return true if journal dir is not empty
 */
diff --git a/src/mongo/db/storage/mmap_v1/dur_journal.h b/src/mongo/db/storage/mmap_v1/dur_journal.h
index e88ea7dabb5..2477eb846ce 100644
--- a/src/mongo/db/storage/mmap_v1/dur_journal.h
+++ b/src/mongo/db/storage/mmap_v1/dur_journal.h
@@ -75,7 +75,7 @@ unsigned long long journalReadLSN();
 
 /** never throws.
     @param anyFiles by default we only look at j._* files. If anyFiles is true, return true
-           if there are any files in the journal directory. acquirePathLock() uses this to
+           if there are any files in the journal directory. checkForUncleanShutdown() uses this to
            make sure that the journal directory is mounted.
     @return true if there are any journal files in the journal dir.
 */
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 160c268c6fa..55dfc010d1a 100644
--- a/src/mongo/db/storage/mmap_v1/mmap_v1_engine.cpp
+++ b/src/mongo/db/storage/mmap_v1/mmap_v1_engine.cpp
@@ -65,9 +65,9 @@ namespace {
 
 #if !defined(__sun)
 // if doingRepair is true don't consider unclean shutdown an error
-void acquirePathLock(MMAPV1Engine* storageEngine,
-                     bool doingRepair,
-                     const StorageEngineLockFile& lockFile) {
+void checkForUncleanShutdown(MMAPV1Engine* storageEngine,
+                             bool doingRepair,
+                             const StorageEngineLockFile& lockFile) {
     string name = lockFile.getFilespec();
     bool oldFile = lockFile.createdByUncleanShutdown();
 
@@ -139,9 +139,9 @@ void acquirePathLock(MMAPV1Engine* storageEngine,
     }
 }
 #else
-void acquirePathLock(MMAPV1Engine* storageEngine,
-                     bool doingRepair,
-                     const StorageEngineLockFile& lockFile) {
+void checkForUncleanShutDown(MMAPV1Engine* storageEngine,
+                             bool doingRepair,
+                             const StorageEngineLockFile& lockFile) {
     // TODO - this is very bad that the code above not running here.
 
     // Not related to lock file, but this is where we handle unclean shutdown
@@ -220,15 +220,18 @@ void clearTmpFiles() {
 }
 }  // namespace
 
-MMAPV1Engine::MMAPV1Engine(const StorageEngineLockFile& lockFile) {
+MMAPV1Engine::MMAPV1Engine(const StorageEngineLockFile* lockFile) {
     // TODO check non-journal subdirs if using directory-per-db
     checkReadAhead(storageGlobalParams.dbpath);
 
-    acquirePathLock(this, storageGlobalParams.repair, lockFile);
+    if (!storageGlobalParams.readOnly) {
+        invariant(lockFile);
+        checkForUncleanShutdown(this, storageGlobalParams.repair, *lockFile);
 
-    FileAllocator::get()->start();
+        FileAllocator::get()->start();
 
-    MONGO_ASSERT_ON_EXCEPTION_WITH_MSG(clearTmpFiles(), "clear tmp files");
+        MONGO_ASSERT_ON_EXCEPTION_WITH_MSG(clearTmpFiles(), "clear tmp files");
+    }
 }
 
 void MMAPV1Engine::finishInit() {
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 4fac787b4b0..76162ef23e3 100644
--- a/src/mongo/db/storage/mmap_v1/mmap_v1_engine.h
+++ b/src/mongo/db/storage/mmap_v1/mmap_v1_engine.h
@@ -43,7 +43,7 @@ class MMAPV1DatabaseCatalogEntry;
 
 class MMAPV1Engine : public StorageEngine {
 public:
-    MMAPV1Engine(const StorageEngineLockFile& lockFile);
+    MMAPV1Engine(const StorageEngineLockFile* lockFile);
     virtual ~MMAPV1Engine();
 
     void finishInit();
diff --git a/src/mongo/db/storage/mmap_v1/mmap_v1_init.cpp b/src/mongo/db/storage/mmap_v1/mmap_v1_init.cpp
index 832cc059ec8..b9ec10cf141 100644
--- a/src/mongo/db/storage/mmap_v1/mmap_v1_init.cpp
+++ b/src/mongo/db/storage/mmap_v1/mmap_v1_init.cpp
@@ -44,7 +44,7 @@ class MMAPV1Factory : public StorageEngine::Factory {
 public:
     virtual ~MMAPV1Factory() {}
     virtual StorageEngine* create(const StorageGlobalParams& params,
-                                  const StorageEngineLockFile& lockFile) const {
+                                  const StorageEngineLockFile* lockFile) const {
         return new MMAPV1Engine(lockFile);
     }
 
diff --git a/src/mongo/db/storage/storage_engine.h b/src/mongo/db/storage/storage_engine.h
index 9b84b32e39d..2d96b096f3c 100644
--- a/src/mongo/db/storage/storage_engine.h
+++ b/src/mongo/db/storage/storage_engine.h
@@ -67,10 +67,11 @@ public:
         virtual ~Factory() {}
 
         /**
-         * Return a new instance of the StorageEngine.  Caller owns the returned pointer.
+         * Return a new instance of the StorageEngine. The lockFile parameter may be null if
+         * params.readOnly is set. Caller owns the returned pointer.
          */
         virtual StorageEngine* create(const StorageGlobalParams& params,
-                                      const StorageEngineLockFile& lockFile) const = 0;
+                                      const StorageEngineLockFile* lockFile) const = 0;
 
         /**
          * Returns the name of the storage engine.
diff --git a/src/mongo/db/storage/storage_engine_lock_file_posix.cpp b/src/mongo/db/storage/storage_engine_lock_file_posix.cpp
index cb78d518e15..acbec3fdd8e 100644
--- a/src/mongo/db/storage/storage_engine_lock_file_posix.cpp
+++ b/src/mongo/db/storage/storage_engine_lock_file_posix.cpp
@@ -192,7 +192,7 @@ void StorageEngineLockFile::clearPidAndUnlock() {
     log() << "shutdown: removing fs lock...";
     // This ought to be an unlink(), but Eliot says the last
     // time that was attempted, there was a race condition
-    // with acquirePathLock().
+    // with StorageEngineLockFile::open().
     if (::ftruncate(_lockFileHandle->_fd, 0)) {
         int errorcode = errno;
         log() << "couldn't remove fs lock " << errnoWithDescription(errorcode);
diff --git a/src/mongo/db/storage/storage_engine_lock_file_windows.cpp b/src/mongo/db/storage/storage_engine_lock_file_windows.cpp
index e38b2752bad..b303606672d 100644
--- a/src/mongo/db/storage/storage_engine_lock_file_windows.cpp
+++ b/src/mongo/db/storage/storage_engine_lock_file_windows.cpp
@@ -190,7 +190,7 @@ void StorageEngineLockFile::clearPidAndUnlock() {
     log() << "shutdown: removing fs lock...";
     // This ought to be an unlink(), but Eliot says the last
     // time that was attempted, there was a race condition
-    // with acquirePathLock().
+    // with StorageEngineLockFile::open().
     Status status = _truncateFile(_lockFileHandle->_handle);
     if (!status.isOK()) {
         log() << "couldn't remove fs lock " << status.toString();
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_init.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_init.cpp
index c992b93f99f..f01229964d4 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_init.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_init.cpp
@@ -56,8 +56,8 @@ class WiredTigerFactory : public StorageEngine::Factory {
 public:
     virtual ~WiredTigerFactory() {}
     virtual StorageEngine* create(const StorageGlobalParams& params,
-                                  const StorageEngineLockFile& lockFile) const {
-        if (lockFile.createdByUncleanShutdown()) {
+                                  const StorageEngineLockFile* lockFile) const {
+        if (lockFile && lockFile->createdByUncleanShutdown()) {
             warning() << "Recovering data from the last clean checkpoint.";
         }