SERVER-7186 Fixed the case where a no-op turned into an object replace on the secondary.

2 files changed, 59 insertions, 8 deletions

diff --git a/src/mongo/db/ops/update.cpp b/src/mongo/db/ops/update.cpp
index 30e57e38e43..4324ee28701 100644
--- a/src/mongo/db/ops/update.cpp
+++ b/src/mongo/db/ops/update.cpp
@@ -100,12 +100,20 @@ namespace mongo {
 
             if ( logop ) {
                 DEV verify( mods->size() );
-
                 BSONObj pattern = patternOrig;
-                DEBUGUPDATE( "\t rewrite update: " << mss->getOpLogRewrite() );
-                logOp("u", ns, mss->getOpLogRewrite() , &pattern, 0, fromMigrate );
+                BSONObj logObj = mss->getOpLogRewrite();
+                DEBUGUPDATE( "\t rewrite update: " << logObj );
+
+                // It is possible that the entire mod set was a no-op over this document.  We
+                // would have an empty log record in that case. If we call logOp, with an empty
+                // record, that would be replicated as "clear this record", which is not what
+                // we want. Therefore, to get a no-op in the replica, we simply don't log.
+                if ( logObj.nFields() ) {
+                    logOp("u", ns, logObj, &pattern, 0, fromMigrate );
+                }
             }
             return UpdateResult( 1 , 1 , 1 , BSONObj() );
+
         } // end $operator update
 
         // regular update
@@ -323,13 +331,11 @@ namespace mongo {
                     const BSONObj& onDisk = loc.obj();
 
                     ModSet* useMods = mods.get();
-                    //bool forceRewrite = false;
 
                     auto_ptr<ModSet> mymodset;
                     if ( details.hasElemMatchKey() && mods->hasDynamicArray() ) {
                         useMods = mods->fixDynamicArray( details.elemMatchKey() );
                         mymodset.reset( useMods );
-                        //forceRewrite = true;
                     }
 
                     auto_ptr<ModSetState> mss = useMods->prepare( onDisk );
@@ -381,9 +387,17 @@ namespace mongo {
 
                     if ( logop ) {
                         DEV verify( mods->size() );
-
-                        DEBUGUPDATE( "\t rewrite update: " << mss->getOpLogRewrite() );
-                        logOp("u", ns, mss->getOpLogRewrite() , &pattern, 0, fromMigrate );
+                        BSONObj logObj = mss->getOpLogRewrite();
+                        DEBUGUPDATE( "\t rewrite update: " << logObj );
+
+                        // It is possible that the entire mod set was a no-op over this
+                        // document.  We would have an empty log record in that case. If we
+                        // call logOp, with an empty record, that would be replicated as "clear
+                        // this record", which is not what we want. Therefore, to get a no-op
+                        // in the replica, we simply don't log.
+                        if ( logObj.nFields() ) {
+                            logOp("u", ns, logObj , &pattern, 0, fromMigrate );
+                        }
                     }
                     numModded++;
                     if ( ! multi )
diff --git a/src/mongo/dbtests/repltests.cpp b/src/mongo/dbtests/repltests.cpp
index 7896f4ef8ad..836670240a6 100644
--- a/src/mongo/dbtests/repltests.cpp
+++ b/src/mongo/dbtests/repltests.cpp
@@ -1027,6 +1027,41 @@ namespace ReplTests {
             }
         };
 
+        class SingletonNoRename : public Base {
+        public:
+            void doIt() const {
+                client()->update( ns(), BSONObj(), fromjson("{$rename:{a:'b'}}" ) );
+
+            }
+            using ReplTests::Base::check;
+            void check() const {
+                ASSERT_EQUALS( 1, count() );
+                check( fromjson( "{_id:0,z:1}" ), one(fromjson("{'_id':0}" ) ) );
+            }
+            void reset() const {
+                deleteAll( ns() );
+                insert( fromjson( "{'_id':0,z:1}" ) );
+            }
+        };
+
+        class IndexedSingletonNoRename : public Base {
+        public:
+            void doIt() const {
+                client()->update( ns(), BSONObj(), fromjson("{$rename:{a:'b'}}" ) );
+            }
+            using ReplTests::Base::check;
+            void check() const {
+                ASSERT_EQUALS( 1, count() );
+                check( fromjson( "{_id:0,z:1}" ), one(fromjson("{'_id':0}" ) ) );
+            }
+            void reset() const {
+                deleteAll( ns() );
+                // Add an index on 'a'.  This prevents the update from running 'in place'.
+                client()->ensureIndex( ns(), BSON( "a" << 1 ) );
+                insert( fromjson( "{'_id':0,z:1}" ) );
+            }
+        };
+
         class AddToSetEmptyMissing : public Base {
         public:
             void doIt() const {
@@ -1266,6 +1301,8 @@ namespace ReplTests {
             add< Idempotence::RenameOverwrite >();
             add< Idempotence::NoRename >();
             add< Idempotence::NestedNoRename >();
+            add< Idempotence::SingletonNoRename >();
+            add< Idempotence::IndexedSingletonNoRename >();
             add< Idempotence::AddToSetEmptyMissing >();
             add< DeleteOpIsIdBased >();
             add< DatabaseIgnorerBasic >();