SERVER-15262 Remove syncDataAndTruncateJournal from the public interface

15 files changed, 22 insertions, 75 deletions

diff --git a/src/mongo/db/catalog/database.cpp b/src/mongo/db/catalog/database.cpp
index a12d8f4a0fd..e22b4111f5d 100644
--- a/src/mongo/db/catalog/database.cpp
+++ b/src/mongo/db/catalog/database.cpp
@@ -527,15 +527,6 @@ namespace mongo {
 
         audit::logDropDatabase( currentClient.get(), name );
 
-        // Not sure we need this here, so removed.  If we do, we need to move it down
-        // within other calls both (1) as they could be called from elsewhere and
-        // (2) to keep the lock order right - groupcommitmutex must be locked before
-        // mmmutex (if both are locked).
-        //
-        //  RWLockRecursive::Exclusive lk(MongoFile::mmmutex);
-
-        txn->recoveryUnit()->syncDataAndTruncateJournal();
-
         dbHolder().close( txn, name );
         db = NULL; // d is now deleted
 
diff --git a/src/mongo/db/catalog/database_holder.cpp b/src/mongo/db/catalog/database_holder.cpp
index 16aa4b9e668..a00ae4258e1 100644
--- a/src/mongo/db/catalog/database_holder.cpp
+++ b/src/mongo/db/catalog/database_holder.cpp
@@ -38,7 +38,6 @@
 #include "mongo/db/catalog/database_holder.h"
 #include "mongo/db/global_environment_experiment.h"
 #include "mongo/db/operation_context.h"
-#include "mongo/db/storage/mmap_v1/dur.h"
 #include "mongo/db/storage/storage_engine.h"
 #include "mongo/util/file_allocator.h"
 #include "mongo/util/log.h"
diff --git a/src/mongo/db/dbcommands.cpp b/src/mongo/db/dbcommands.cpp
index 3287d917648..61e20701ea8 100644
--- a/src/mongo/db/dbcommands.cpp
+++ b/src/mongo/db/dbcommands.cpp
@@ -193,9 +193,7 @@ namespace mongo {
             }
 
             {
-
-                // this is suboptimal but syncDataAndTruncateJournal is called from dropDatabase,
-                // and that may need a global lock.
+                // TODO: SERVER-4328 Don't lock globally
                 Lock::GlobalWrite lk(txn->lockState());
                 Client::Context context(txn, dbname);
 
@@ -276,8 +274,7 @@ namespace mongo {
                 return false;
             }
 
-            // SERVER-4328 todo don't lock globally. currently syncDataAndTruncateJournal is being
-            // called within, and that requires a global lock i believe.
+            // TODO: SERVER-4328 Don't lock globally
             Lock::GlobalWrite lk(txn->lockState());
             Client::Context context(txn,  dbname );
 
diff --git a/src/mongo/db/storage/README.md b/src/mongo/db/storage/README.md
index f256e23a104..ceab8e8b4d8 100644
--- a/src/mongo/db/storage/README.md
+++ b/src/mongo/db/storage/README.md
@@ -137,20 +137,6 @@ acts on that information.
 
 RecoveryUnit
 ------------
-**Q**: Should RecoveryUnit::syncDataAndTruncateJournal be static or in a different class? I am not
-sure why they need to effect the instance of RecoveryUnit?
-**A**: These methods are effectively static in that they will only modify global
-state. However, these methods are virtual, which is why we aren’t declaring them
-as static. That being said, we will likely remove this methods from the public
-API and move them into our mmapv1 storage engine implementation in the near
-future.
-
-**Q**: RecoveryUnit::syncDataAndTruncateJournal sounds like a checkpoint. That can
-take a long time, is this expected to block until that is done?  
-**A**: Yes. Note that this is only called externally when we drop a database or when
-the user explicitly requests a sync via the fsync command.  We may rename this
-method as part of a naming sweep. 
-
 **Q**: As documented I don’t understand the point of the RecoverUnit::endUnitOfWork
 nesting behavior. Can you explain where it is used or will be used?  
 **A**: The RecoveryUnit interface and the mmapv1 (current mongodb storage engine)
diff --git a/src/mongo/db/storage/heap1/heap1_recovery_unit.h b/src/mongo/db/storage/heap1/heap1_recovery_unit.h
index d4f375405cd..89213570de4 100644
--- a/src/mongo/db/storage/heap1/heap1_recovery_unit.h
+++ b/src/mongo/db/storage/heap1/heap1_recovery_unit.h
@@ -62,8 +62,6 @@ namespace mongo {
             invariant(!"don't call writingPtr");
         }
 
-        virtual void syncDataAndTruncateJournal() {}
-
     private:
         typedef boost::shared_ptr<Change> ChangePtr;
         typedef std::vector<ChangePtr> Changes;
diff --git a/src/mongo/db/storage/mmap_v1/dur.h b/src/mongo/db/storage/mmap_v1/dur.h
index 786926db14c..e5d818d9e25 100644
--- a/src/mongo/db/storage/mmap_v1/dur.h
+++ b/src/mongo/db/storage/mmap_v1/dur.h
@@ -160,12 +160,16 @@ namespace mongo {
                 return (T*) writingPtr(x, sizeof(T));
             }
 
-            /** Commits pending changes, flushes all changes to main data
-                files, then removes the journal.
-                
-                This is useful as a "barrier" to ensure that writes before this
-                call will never go through recovery and be applied to files
-                that have had changes made after this call applied.
+            /**
+             * Commits pending changes, flushes all changes to main data files, then removes the
+             * journal.
+             *
+             * WARNING: Data *must* be in a crash-recoverable state when this is called and must
+             *          not be inside of a write unit of work.
+             *
+             * This is useful as a "barrier" to ensure that writes before this call will never go
+             * through recovery and be applied to files that have had changes made after this call
+             * applied.
              */
             virtual void syncDataAndTruncateJournal(OperationContext* txn) = 0;
 
diff --git a/src/mongo/db/storage/mmap_v1/dur_recovery_unit.cpp b/src/mongo/db/storage/mmap_v1/dur_recovery_unit.cpp
index b7865de710f..ca9b7c682de 100644
--- a/src/mongo/db/storage/mmap_v1/dur_recovery_unit.cpp
+++ b/src/mongo/db/storage/mmap_v1/dur_recovery_unit.cpp
@@ -193,13 +193,6 @@ namespace mongo {
 #endif
     }
 
-    void DurRecoveryUnit::syncDataAndTruncateJournal() {
-#if ROLLBACK_ENABLED
-        publishChanges();
-#endif
-        return getDur().syncDataAndTruncateJournal(_txn);
-    }
-
     void MemoryWrite::commit() {
         // TODO don't go through getDur() interface.
         if (getDur().isDurable()) {
diff --git a/src/mongo/db/storage/mmap_v1/dur_recovery_unit.h b/src/mongo/db/storage/mmap_v1/dur_recovery_unit.h
index 1ff3da892c2..9e0fb9251eb 100644
--- a/src/mongo/db/storage/mmap_v1/dur_recovery_unit.h
+++ b/src/mongo/db/storage/mmap_v1/dur_recovery_unit.h
@@ -60,8 +60,6 @@ namespace mongo {
         //  The recovery unit takes ownership of change.
         virtual void registerChange(Change* change);
 
-        virtual void syncDataAndTruncateJournal();
-
     private:
         void recordPreimage(char* data, size_t len);
         void publishChanges();
diff --git a/src/mongo/db/storage/mmap_v1/heap_record_store_btree.h b/src/mongo/db/storage/mmap_v1/heap_record_store_btree.h
index 65a101c761e..92dc8bf7bab 100644
--- a/src/mongo/db/storage/mmap_v1/heap_record_store_btree.h
+++ b/src/mongo/db/storage/mmap_v1/heap_record_store_btree.h
@@ -199,8 +199,6 @@ namespace mongo {
 
         virtual void* writingPtr(void* data, size_t len);
 
-        virtual void syncDataAndTruncateJournal() {}
-
         // -----------------------
 
         void notifyInsert( HeapRecordStoreBtree* rs, const DiskLoc& loc );
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 586ce052d06..fff33e208a5 100644
--- a/src/mongo/db/storage/mmap_v1/mmap_v1_engine.cpp
+++ b/src/mongo/db/storage/mmap_v1/mmap_v1_engine.cpp
@@ -339,7 +339,7 @@ namespace {
         // Before the files are closed, flush any potentially outstanding changes, which might
         // reference this database. Otherwise we will assert when subsequent applications of the
         // global journal entries occur, which happen to have write intents for the removed files.
-        getDur().commitNow(txn);
+        getDur().syncDataAndTruncateJournal(txn);
 
         boost::mutex::scoped_lock lk( _entryMapMutex );
         MMAPV1DatabaseCatalogEntry* entry = _entryMap[db.toString()];
diff --git a/src/mongo/db/storage/mmap_v1/repair_database.cpp b/src/mongo/db/storage/mmap_v1/repair_database.cpp
index 9fce4a4eca2..7779744ff53 100644
--- a/src/mongo/db/storage/mmap_v1/repair_database.cpp
+++ b/src/mongo/db/storage/mmap_v1/repair_database.cpp
@@ -44,6 +44,7 @@
 #include "mongo/db/client.h"
 #include "mongo/db/index/index_descriptor.h"
 #include "mongo/db/storage/mmap_v1/mmap_v1_database_catalog_entry.h"
+#include "mongo/db/storage/mmap_v1/dur.h"
 #include "mongo/util/file.h"
 #include "mongo/util/file_allocator.h"
 #include "mongo/util/log.h"
@@ -240,7 +241,8 @@ namespace mongo {
                   << "db: " << _dbName << " path: " << _pathString;
 
             try {
-                _txn->recoveryUnit()->syncDataAndTruncateJournal();
+                getDur().syncDataAndTruncateJournal(_txn);
+
                 // need both in case journaling is disabled
                 MongoFile::flushAll(true);
 
@@ -279,7 +281,8 @@ namespace mongo {
 
         BackgroundOperation::assertNoBgOpInProgForDb(dbName);
 
-        txn->recoveryUnit()->syncDataAndTruncateJournal(); // Must be done before and after repair
+        // Must be done before and after repair
+        getDur().syncDataAndTruncateJournal(txn);
 
         intmax_t totalSize = dbSize( dbName );
         intmax_t freeSize = File::freeSpace(storageGlobalParams.repairpath);
@@ -314,9 +317,8 @@ namespace mongo {
             scoped_ptr<MMAPV1DatabaseCatalogEntry> dbEntry;
             scoped_ptr<Database> tempDatabase;
 
-            // Must syncDataAndTruncateJournal before closing files as done by
-            // MMAPV1DatabaseCatalogEntry's destructor.
-            ON_BLOCK_EXIT(&RecoveryUnit::syncDataAndTruncateJournal, txn->recoveryUnit());
+            // Must call this before MMAPV1DatabaseCatalogEntry's destructor closes the DB files
+            ON_BLOCK_EXIT(&dur::DurableInterface::syncDataAndTruncateJournal, &getDur(), txn);
 
             {
                 dbEntry.reset(new MMAPV1DatabaseCatalogEntry(txn,
@@ -431,7 +433,8 @@ namespace mongo {
 
             }
 
-            txn->recoveryUnit()->syncDataAndTruncateJournal();
+            getDur().syncDataAndTruncateJournal(txn);
+
             // need both in case journaling is disabled
             MongoFile::flushAll(true);
 
diff --git a/src/mongo/db/storage/recovery_unit.h b/src/mongo/db/storage/recovery_unit.h
index 931451c8235..4139eca1a2f 100644
--- a/src/mongo/db/storage/recovery_unit.h
+++ b/src/mongo/db/storage/recovery_unit.h
@@ -131,18 +131,6 @@ namespace mongo {
          */
         virtual void* writingPtr(void* data, size_t len) = 0;
 
-        /**
-         * Commits pending changes, flushes all changes to main data files, then removes the
-         * journal.
-         *
-         * WARNING: Data *must* be in a crash-recoverable state when this is called.
-         *
-         * This is useful as a "barrier" to ensure that writes before this call will never go
-         * through recovery and be applied to files that have had changes made after this call
-         * applied.
-         */
-        virtual void syncDataAndTruncateJournal() = 0;
-
         //
         // Syntactic sugar
         //
diff --git a/src/mongo/db/storage/recovery_unit_noop.h b/src/mongo/db/storage/recovery_unit_noop.h
index 3c6fa76adc4..1e07b1f9c0b 100644
--- a/src/mongo/db/storage/recovery_unit_noop.h
+++ b/src/mongo/db/storage/recovery_unit_noop.h
@@ -53,8 +53,6 @@ namespace mongo {
         virtual void* writingPtr(void* data, size_t len) {
             return data;
         }
-
-        virtual void syncDataAndTruncateJournal() { }
     };
 
 }  // namespace mongo
diff --git a/src/mongo/db/storage/rocks/rocks_recovery_unit.cpp b/src/mongo/db/storage/rocks/rocks_recovery_unit.cpp
index 27c36fcb733..84df8f687b9 100644
--- a/src/mongo/db/storage/rocks/rocks_recovery_unit.cpp
+++ b/src/mongo/db/storage/rocks/rocks_recovery_unit.cpp
@@ -113,10 +113,6 @@ namespace mongo {
         return data;
     }
 
-    void RocksRecoveryUnit::syncDataAndTruncateJournal() {
-        log() << "RocksRecoveryUnit::syncDataAndTruncateJournal() does nothing";
-    }
-
     // lazily initialized because Recovery Units are sometimes initialized just for reading,
     // which does not require write batches
     rocksdb::WriteBatchWithIndex* RocksRecoveryUnit::writeBatch() {
diff --git a/src/mongo/db/storage/rocks/rocks_recovery_unit.h b/src/mongo/db/storage/rocks/rocks_recovery_unit.h
index faaa4bc5425..70e41e93e0f 100644
--- a/src/mongo/db/storage/rocks/rocks_recovery_unit.h
+++ b/src/mongo/db/storage/rocks/rocks_recovery_unit.h
@@ -75,8 +75,6 @@ namespace mongo {
 
         virtual void* writingPtr(void* data, size_t len);
 
-        virtual void syncDataAndTruncateJournal();
-
         virtual void registerChange(Change* change);
 
         // local api