SERVER-35782 Repair should move aside unsalvagable data files and create empty ones in their place.

(cherry picked from commit 60ed4a2dbfa1a80c9bb8da87a6d2fa55b55dffa4)

Conflicts:
    src/mongo/db/repair_database.cpp

10 files changed, 328 insertions, 115 deletions

diff --git a/src/mongo/db/catalog/database_impl.cpp b/src/mongo/db/catalog/database_impl.cpp
index 30b25cc4ff8..1da150acc36 100644
--- a/src/mongo/db/catalog/database_impl.cpp
+++ b/src/mongo/db/catalog/database_impl.cpp
@@ -220,10 +220,11 @@ Collection* DatabaseImpl::_getOrCreateCollectionInstance(OperationContext* opCtx
     auto uuid = cce->getCollectionOptions(opCtx).uuid;
 
     unique_ptr<RecordStore> rs(_dbEntry->getRecordStore(nss.ns()));
-    invariant(
-        rs.get(),
-        str::stream() << "Record store did not exist. Collection: " << nss.ns() << " UUID: "
-                      << (uuid ? uuid->toString() : "none"));  // if cce exists, so should this
+    if (rs.get() == nullptr) {
+        severe() << "Record store did not exist. Collection: " << nss.ns() << " UUID: "
+                 << (uuid ? uuid->toString() : "none");  // if cce exists, so should this
+        fassertFailedNoTrace(50936);
+    }
 
     // Not registering AddCollectionChange since this is for collections that already exist.
     Collection* coll = new Collection(opCtx, nss.ns(), uuid, cce.release(), rs.release(), _dbEntry);
diff --git a/src/mongo/db/repair_database.cpp b/src/mongo/db/repair_database.cpp
index df89ce310c6..11eb61b33a8 100644
--- a/src/mongo/db/repair_database.cpp
+++ b/src/mongo/db/repair_database.cpp
@@ -230,6 +230,40 @@ Status rebuildIndexesOnCollection(OperationContext* opCtx,
     return Status::OK();
 }
 
+namespace {
+Status repairCollections(OperationContext* opCtx,
+                         StorageEngine* engine,
+                         const std::string& dbName) {
+
+    DatabaseCatalogEntry* dbce = engine->getDatabaseCatalogEntry(opCtx, dbName);
+
+    std::list<std::string> colls;
+    dbce->getCollectionNamespaces(&colls);
+
+    for (std::list<std::string>::const_iterator it = colls.begin(); it != colls.end(); ++it) {
+        // Don't check for interrupt after starting to repair a collection otherwise we can
+        // leave data in an inconsistent state. Interrupting between collections is ok, however.
+        opCtx->checkForInterrupt();
+
+        log() << "Repairing collection " << *it;
+
+        Status status = engine->repairRecordStore(opCtx, *it);
+        if (!status.isOK())
+            return status;
+
+        CollectionCatalogEntry* cce = dbce->getCollectionCatalogEntry(*it);
+        auto swIndexNameObjs = getIndexNameObjs(opCtx, dbce, cce);
+        if (!swIndexNameObjs.isOK())
+            return swIndexNameObjs.getStatus();
+
+        status = rebuildIndexesOnCollection(opCtx, dbce, cce, swIndexNameObjs.getValue());
+        if (!status.isOK())
+            return status;
+    }
+    return Status::OK();
+}
+}  // namespace
+
 Status repairDatabase(OperationContext* opCtx,
                       StorageEngine* engine,
                       const std::string& dbName,
@@ -292,33 +326,10 @@ Status repairDatabase(OperationContext* opCtx,
         }
     });
 
-    DatabaseCatalogEntry* dbce = engine->getDatabaseCatalogEntry(opCtx, dbName);
-
-    std::list<std::string> colls;
-    dbce->getCollectionNamespaces(&colls);
-
-    for (std::list<std::string>::const_iterator it = colls.begin(); it != colls.end(); ++it) {
-        // Don't check for interrupt after starting to repair a collection otherwise we can
-        // leave data in an inconsistent state. Interrupting between collections is ok, however.
-        opCtx->checkForInterrupt();
-
-        log() << "Repairing collection " << *it;
-
-        Status status = engine->repairRecordStore(opCtx, *it);
-        if (!status.isOK())
-            return status;
-
-        CollectionCatalogEntry* cce = dbce->getCollectionCatalogEntry(*it);
-        auto swIndexNameObjs = getIndexNameObjs(opCtx, dbce, cce);
-        if (!swIndexNameObjs.isOK())
-            return swIndexNameObjs.getStatus();
-
-        status = rebuildIndexesOnCollection(opCtx, dbce, cce, swIndexNameObjs.getValue());
-        if (!status.isOK())
-            return status;
-
-        // TODO: uncomment once SERVER-16869
-        // engine->flushAllFiles(true);
+    auto status = repairCollections(opCtx, engine, dbName);
+    if (!status.isOK()) {
+        severe() << "Failed to repair database " << dbName << ": " << status.reason();
+        return status;
     }
 
     return Status::OK();
diff --git a/src/mongo/db/storage/SConscript b/src/mongo/db/storage/SConscript
index bcc77a44af4..6d03a95d06e 100644
--- a/src/mongo/db/storage/SConscript
+++ b/src/mongo/db/storage/SConscript
@@ -254,6 +254,16 @@ env.Library(
 )
 
 env.Library(
+    target='storage_file_util',
+    source=[
+        'storage_file_util.cpp',
+    ],
+    LIBDEPS_PRIVATE=[
+        '$BUILD_DIR/mongo/base',
+    ],
+)
+
+env.Library(
     target='storage_repair_observer',
     source=[
         'storage_repair_observer.cpp',
@@ -263,6 +273,7 @@ env.Library(
         '$BUILD_DIR/mongo/db/dbhelpers',
         '$BUILD_DIR/mongo/db/service_context',
         '$BUILD_DIR/mongo/db/repl/replica_set_messages',
+        'storage_file_util',
     ],
 )
 
diff --git a/src/mongo/db/storage/storage_file_util.cpp b/src/mongo/db/storage/storage_file_util.cpp
new file mode 100644
index 00000000000..6ffa979213f
--- /dev/null
+++ b/src/mongo/db/storage/storage_file_util.cpp
@@ -0,0 +1,126 @@
+/**
+ *    Copyright (C) 2018 MongoDB Inc.
+ *
+ *    This program is free software: you can redistribute it and/or modify
+ *    it under the terms of the GNU Affero General Public License, version 3,
+ *    as published by the Free Software Foundation.
+ *
+ *    This program is distributed in the hope that it will be useful,
+ *    but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *    GNU Affero General Public License for more details.
+ *
+ *    You should have received a copy of the GNU Affero General Public License
+ *    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ *    As a special exception, the copyright holders give permission to link the
+ *    code of portions of this program with the OpenSSL library under certain
+ *    conditions as described in each individual source file and distribute
+ *    linked combinations including the program with the OpenSSL library. You
+ *    must comply with the GNU Affero General Public License in all respects for
+ *    all of the code used other than as permitted herein. If you modify file(s)
+ *    with this exception, you may extend this exception to your version of the
+ *    file(s), but you are not obligated to do so. If you do not wish to do so,
+ *    delete this exception statement from your version. If you delete this
+ *    exception statement from all source files in the program, then also delete
+ *    it in the license file.
+ */
+
+#define MONGO_LOG_DEFAULT_COMPONENT ::mongo::logger::LogComponent::kStorage
+
+#include "mongo/db/storage/storage_file_util.h"
+
+#include <cerrno>
+#include <cstring>
+
+#ifdef __linux__
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#endif
+
+#include <boost/filesystem/path.hpp>
+
+#include "mongo/util/file.h"
+#include "mongo/util/log.h"
+
+namespace mongo {
+
+Status fsyncFile(const boost::filesystem::path& path) {
+    File file;
+    file.open(path.string().c_str(), /*read-only*/ false, /*direct-io*/ false);
+    if (!file.is_open()) {
+        return {ErrorCodes::FileOpenFailed,
+                str::stream() << "Failed to open file " << path.string()};
+    }
+    file.fsync();
+    return Status::OK();
+}
+
+Status fsyncParentDirectory(const boost::filesystem::path& file) {
+#ifdef __linux__  // this isn't needed elsewhere
+    if (!file.has_parent_path()) {
+        return {ErrorCodes::InvalidPath,
+                str::stream() << "Couldn't find parent directory for file: " << file.string()};
+    }
+
+    boost::filesystem::path dir = file.parent_path();
+
+    LOG(1) << "flushing directory " << dir.string();
+
+    int fd = ::open(dir.string().c_str(), O_RDONLY);
+    if (fd < 0) {
+        return {ErrorCodes::FileOpenFailed,
+                str::stream() << "Failed to open directory " << dir.string() << " for flushing: "
+                              << errnoWithDescription()};
+    }
+    if (fsync(fd) != 0) {
+        int e = errno;
+        if (e == EINVAL) {
+            warning() << "Could not fsync directory because this file system is not supported.";
+        } else {
+            close(fd);
+            return {ErrorCodes::OperationFailed,
+                    str::stream() << "Failed to fsync directory '" << dir.string() << "': "
+                                  << errnoWithDescription(e)};
+        }
+    }
+    close(fd);
+#endif
+    return Status::OK();
+}
+
+Status fsyncRename(const boost::filesystem::path& source, const boost::filesystem::path& dest) {
+    if (boost::filesystem::exists(dest)) {
+        return {ErrorCodes::FileRenameFailed,
+                "Attempted to rename file to an existing file: " + dest.string()};
+    }
+
+    boost::system::error_code ec;
+    boost::filesystem::rename(source, dest, ec);
+    if (ec) {
+        return {ErrorCodes::FileRenameFailed,
+                str::stream() << "Error renaming data file from " << source.string() << " to "
+                              << dest.string()
+                              << ": "
+                              << ec.message()};
+    }
+    auto status = fsyncFile(dest);
+    if (!status.isOK()) {
+        return status;
+    }
+
+    status = fsyncParentDirectory(source);
+    if (!status.isOK()) {
+        return status;
+    }
+
+    status = fsyncParentDirectory(dest);
+    if (!status.isOK()) {
+        return status;
+    }
+    return Status::OK();
+}
+
+
+}  // namespace mongo
diff --git a/src/mongo/db/storage/storage_file_util.h b/src/mongo/db/storage/storage_file_util.h
new file mode 100644
index 00000000000..909e78d8959
--- /dev/null
+++ b/src/mongo/db/storage/storage_file_util.h
@@ -0,0 +1,57 @@
+/**
+ *    Copyright (C) 2018 MongoDB Inc.
+ *
+ *    This program is free software: you can redistribute it and/or modify
+ *    it under the terms of the GNU Affero General Public License, version 3,
+ *    as published by the Free Software Foundation.
+ *
+ *    This program is distributed in the hope that it will be useful,
+ *    but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *    GNU Affero General Public License for more details.
+ *
+ *    You should have received a copy of the GNU Affero General Public License
+ *    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ *    As a special exception, the copyright holders give permission to link the
+ *    code of portions of this program with the OpenSSL library under certain
+ *    conditions as described in each individual source file and distribute
+ *    linked combinations including the program with the OpenSSL library. You
+ *    must comply with the GNU Affero General Public License in all respects for
+ *    all of the code used other than as permitted herein. If you modify file(s)
+ *    with this exception, you may extend this exception to your version of the
+ *    file(s), but you are not obligated to do so. If you do not wish to do so,
+ *    delete this exception statement from your version. If you delete this
+ *    exception statement from all source files in the program, then also delete
+ *    it in the license file.
+ */
+
+#pragma once
+
+#include "mongo/platform/basic.h"
+
+#include <boost/filesystem.hpp>
+
+#include "mongo/base/status.h"
+
+namespace mongo {
+
+/**
+ * Perform an fsync on the file.
+ */
+Status fsyncFile(const boost::filesystem::path& path);
+
+/**
+ * Perform an fsync on the parent directory of 'file'.
+ */
+Status fsyncParentDirectory(const boost::filesystem::path& file);
+
+/**
+ * Perform a filesystem rename from 'source' to 'dest'. Performs an fsync on the destination file
+ * and the parent directories of both 'source' and 'dest'.
+ *
+ * Returns a FileRenameFailed error if the destination file already exists.
+ */
+Status fsyncRename(const boost::filesystem::path& source, const boost::filesystem::path& dest);
+
+}  // namespace mongo
diff --git a/src/mongo/db/storage/storage_repair_observer.cpp b/src/mongo/db/storage/storage_repair_observer.cpp
index 94bd398a168..dd0d699d8b7 100644
--- a/src/mongo/db/storage/storage_repair_observer.cpp
+++ b/src/mongo/db/storage/storage_repair_observer.cpp
@@ -45,6 +45,7 @@
 #include "mongo/db/operation_context.h"
 #include "mongo/db/repl/repl_set_config.h"
 #include "mongo/db/service_context.h"
+#include "mongo/db/storage/storage_file_util.h"
 #include "mongo/db/storage/storage_options.h"
 #include "mongo/util/file.h"
 #include "mongo/util/log.h"
@@ -58,50 +59,6 @@ static const std::string kRepairIncompleteFileName = "_repair_incomplete";
 const auto getRepairObserver =
     ServiceContext::declareDecoration<std::unique_ptr<StorageRepairObserver>>();
 
-Status fsyncFile(const boost::filesystem::path& path) {
-    File file;
-    file.open(path.string().c_str(), /*read-only*/ false, /*direct-io*/ false);
-    if (!file.is_open()) {
-        return {ErrorCodes::FileOpenFailed,
-                str::stream() << "Failed to open file " << path.string()};
-    }
-    file.fsync();
-    return Status::OK();
-}
-
-Status fsyncParentDirectory(const boost::filesystem::path& file) {
-#ifdef __linux__  // this isn't needed elsewhere
-    if (!file.has_parent_path()) {
-        return {ErrorCodes::InvalidPath,
-                str::stream() << "Couldn't find parent directory for file: " << file.string()};
-    }
-
-    boost::filesystem::path dir = file.parent_path();
-
-    LOG(1) << "flushing directory " << dir.string();
-
-    int fd = ::open(dir.string().c_str(), O_RDONLY);
-    if (fd < 0) {
-        return {ErrorCodes::FileOpenFailed,
-                str::stream() << "Failed to open directory " << dir.string() << " for flushing: "
-                              << errnoWithDescription()};
-    }
-    if (fsync(fd) != 0) {
-        int e = errno;
-        if (e == EINVAL) {
-            warning() << "Could not fsync directory because this file system is not supported.";
-        } else {
-            close(fd);
-            return {ErrorCodes::OperationFailed,
-                    str::stream() << "Failed to fsync directory '" << dir.string() << "': "
-                                  << errnoWithDescription(e)};
-        }
-    }
-    close(fd);
-#endif
-    return Status::OK();
-}
-
 }  // namespace
 
 StorageRepairObserver::StorageRepairObserver(const std::string& dbpath) {
diff --git a/src/mongo/db/storage/wiredtiger/SConscript b/src/mongo/db/storage/wiredtiger/SConscript
index 25bd55e1bb1..97cfaecb3af 100644
--- a/src/mongo/db/storage/wiredtiger/SConscript
+++ b/src/mongo/db/storage/wiredtiger/SConscript
@@ -70,6 +70,7 @@ if wiredtiger:
             '$BUILD_DIR/mongo/db/storage/key_string',
             '$BUILD_DIR/mongo/db/storage/kv/kv_prefix',
             '$BUILD_DIR/mongo/db/storage/oplog_hack',
+            '$BUILD_DIR/mongo/db/storage/storage_file_util',
             '$BUILD_DIR/mongo/db/storage/storage_options',
             '$BUILD_DIR/mongo/util/concurrency/ticketholder',
             '$BUILD_DIR/mongo/util/elapsed_tracker',
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.cpp
index 15fb33453c8..f7947eeb404 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.cpp
@@ -68,6 +68,7 @@
 #include "mongo/db/server_parameters.h"
 #include "mongo/db/service_context.h"
 #include "mongo/db/storage/journal_listener.h"
+#include "mongo/db/storage/storage_file_util.h"
 #include "mongo/db/storage/storage_options.h"
 #include "mongo/db/storage/wiredtiger/wiredtiger_customization_hooks.h"
 #include "mongo/db/storage/wiredtiger/wiredtiger_extensions.h"
@@ -757,35 +758,65 @@ Status WiredTigerKVEngine::_salvageIfNeeded(const char* uri) {
         warning() << "Data file is missing for " << uri
                   << ". Attempting to drop and re-create the collection.";
 
-        auto swMetadata = WiredTigerUtil::getMetadataRaw(session, uri);
-        if (!swMetadata.isOK()) {
-            error() << "Failed to get metadata for " << uri;
-            return swMetadata.getStatus();
-        }
-
-        rc = session->drop(session, uri, NULL);
-        if (rc != 0) {
-            error() << "Failed to drop " << uri;
-            return wtRCToStatus(rc);
-        }
-
-        rc = session->create(session, uri, swMetadata.getValue().c_str());
-        if (rc != 0) {
-            error() << "Failed to create " << uri << " with config: " << swMetadata.getValue();
-            return wtRCToStatus(rc);
-        }
-        log() << "Successfully re-created " << uri << ".";
-        return {ErrorCodes::DataModifiedByRepair,
-                str::stream() << "Re-created empty data file for " << uri};
+        return _rebuildIdent(session, uri);
     }
 
-    // TODO need to cleanup the sizeStorer cache after salvaging.
     log() << "Verify failed on uri " << uri << ". Running a salvage operation.";
     auto status = wtRCToStatus(session->salvage(session, uri, NULL), "Salvage failed:");
     if (status.isOK()) {
         return {ErrorCodes::DataModifiedByRepair, str::stream() << "Salvaged data for " << uri};
     }
-    return status;
+
+    warning() << "Salvage failed for uri " << uri << ": " << status.reason()
+              << ". The file will be moved out of the way and a new ident will be created.";
+
+    //  If the data is unsalvageable, we should completely rebuild the ident.
+    return _rebuildIdent(session, uri);
+}
+
+Status WiredTigerKVEngine::_rebuildIdent(WT_SESSION* session, const char* uri) {
+    invariant(_inRepairMode);
+
+    static const char tablePrefix[] = "table:";
+    invariant(std::string(uri).find(tablePrefix) == 0);
+
+    const std::string identName(uri + sizeof(tablePrefix) - 1);
+    auto filePath = getDataFilePathForIdent(identName);
+    if (filePath) {
+        const boost::filesystem::path corruptFile(filePath->string() + ".corrupt");
+        warning() << "Moving data file " << filePath->string() << " to backup as "
+                  << corruptFile.string();
+
+        auto status = fsyncRename(filePath.get(), corruptFile);
+        if (!status.isOK()) {
+            return status;
+        }
+    }
+
+    warning() << "Rebuilding ident " << identName;
+
+    // This is safe to call after moving the file because it only reads from the metadata, and not
+    // the data file itself.
+    auto swMetadata = WiredTigerUtil::getMetadataRaw(session, uri);
+    if (!swMetadata.isOK()) {
+        error() << "Failed to get metadata for " << uri;
+        return swMetadata.getStatus();
+    }
+
+    int rc = session->drop(session, uri, NULL);
+    if (rc != 0) {
+        error() << "Failed to drop " << uri;
+        return wtRCToStatus(rc);
+    }
+
+    rc = session->create(session, uri, swMetadata.getValue().c_str());
+    if (rc != 0) {
+        error() << "Failed to create " << uri << " with config: " << swMetadata.getValue();
+        return wtRCToStatus(rc);
+    }
+    log() << "Successfully re-created " << uri << ".";
+    return {ErrorCodes::DataModifiedByRepair,
+            str::stream() << "Re-created empty data file for " << uri};
 }
 
 int WiredTigerKVEngine::flushAllFiles(OperationContext* opCtx, bool sync) {
@@ -895,24 +926,18 @@ Status WiredTigerKVEngine::recoverOrphanedIdent(OperationContext* opCtx,
 
     boost::filesystem::path tmpFile{*identFilePath};
     tmpFile += ".tmp";
-    if (boost::filesystem::exists(tmpFile, ec)) {
-        return {ErrorCodes::FileRenameFailed,
-                "Attempted to rename data file to an existing temporary file: " + tmpFile.string()};
-    }
 
     log() << "Renaming data file " + identFilePath->string() + " to temporary file " +
             tmpFile.string();
-
-    boost::filesystem::rename(*identFilePath, tmpFile, ec);
-    if (ec) {
-        return {ErrorCodes::FileRenameFailed,
-                "Error renaming data file to temporary file: " + ec.message()};
+    auto status = fsyncRename(identFilePath.get(), tmpFile);
+    if (!status.isOK()) {
+        return status;
     }
 
     log() << "Creating new RecordStore for collection " + ns + " with UUID: " +
             (options.uuid ? options.uuid->toString() : "none");
 
-    auto status = createGroupedRecordStore(opCtx, ns, ident, options, KVPrefix::kNotPrefixed);
+    status = createGroupedRecordStore(opCtx, ns, ident, options, KVPrefix::kNotPrefixed);
     if (!status.isOK()) {
         return status;
     }
@@ -923,11 +948,14 @@ Status WiredTigerKVEngine::recoverOrphanedIdent(OperationContext* opCtx,
     if (ec) {
         return {ErrorCodes::UnknownError, "Error deleting empty data file: " + ec.message()};
     }
+    status = fsyncParentDirectory(*identFilePath);
+    if (!status.isOK()) {
+        return status;
+    }
 
-    boost::filesystem::rename(tmpFile, *identFilePath, ec);
-    if (ec) {
-        return {ErrorCodes::FileRenameFailed,
-                "Error renaming data file back from temporary file: " + ec.message()};
+    status = fsyncRename(tmpFile, identFilePath.get());
+    if (!status.isOK()) {
+        return status;
     }
 
     log() << "Salvaging ident " + ident;
@@ -939,7 +967,10 @@ Status WiredTigerKVEngine::recoverOrphanedIdent(OperationContext* opCtx,
         return {ErrorCodes::DataModifiedByRepair,
                 str::stream() << "Salvaged data for ident " << ident};
     }
-    return status;
+    warning() << "Could not salvage data. Rebuilding ident: " << status.reason();
+
+    //  If the data is unsalvageable, we should completely rebuild the ident.
+    return _rebuildIdent(session, _uri(ident).c_str());
 #endif
 }
 
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.h b/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.h
index 71470c5aa82..e7b7940bf00 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.h
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.h
@@ -290,6 +290,15 @@ private:
     Status _salvageIfNeeded(const char* uri);
     void _ensureIdentPath(StringData ident);
 
+    /**
+     * Recreates a WiredTiger ident from the provided URI by dropping and recreating the ident.
+     * This moves aside the existing data file, if one exists, with an added ".corrupt" suffix.
+     *
+     * Returns DataModifiedByRepair if the rebuild was successful, and any other error on failure.
+     * This will never return Status::OK().
+     */
+    Status _rebuildIdent(WT_SESSION* session, const char* uri);
+
     bool _hasUri(WT_SESSION* session, const std::string& uri) const;
 
     std::string _uri(StringData ident) const;
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine_test.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine_test.cpp
index ab167ff8b8a..6800f1606fb 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine_test.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine_test.cpp
@@ -190,7 +190,7 @@ TEST_F(WiredTigerKVEngineRepairTest, OrphanedDataFilesCanBeRecovered) {
 #endif
 }
 
-TEST_F(WiredTigerKVEngineRepairTest, UnrecoverableOrphanedDataFilesFailGracefully) {
+TEST_F(WiredTigerKVEngineRepairTest, UnrecoverableOrphanedDataFilesAreRebuilt) {
     auto opCtxPtr = makeOperationContext();
 
     std::string ns = "a.b";
@@ -238,8 +238,17 @@ TEST_F(WiredTigerKVEngineRepairTest, UnrecoverableOrphanedDataFilesFailGracefull
 
     ASSERT(boost::filesystem::exists(*dataFilePath));
 
-    // This should fail gracefully and not cause any crashing.
-    ASSERT_NOT_OK(_engine->recoverOrphanedIdent(opCtxPtr.get(), ns, ident, options));
+    // This should recreate an empty data file successfully and move the old one to a name that ends
+    // in ".corrupt".
+    auto status = _engine->recoverOrphanedIdent(opCtxPtr.get(), ns, ident, options);
+    ASSERT_EQ(ErrorCodes::DataModifiedByRepair, status.code()) << status.reason();
+
+    boost::filesystem::path corruptFile = (dataFilePath->string() + ".corrupt");
+    ASSERT(boost::filesystem::exists(corruptFile));
+
+    rs = _engine->getRecordStore(opCtxPtr.get(), ns, ident, options);
+    RecordData data;
+    ASSERT_FALSE(rs->findRecord(opCtxPtr.get(), loc, &data));
 #endif
 }