SERVER-17312 collmod command now handles parsing of all arguments

For the two currently supported engine-specific options, the collmod command
will handle parsing the arguments and tell the CollectionCatalogEntry to
update it's flags option.

This removes the ability of storage engines to have custom options that can
be changed after the collection is created. There were issues related to
argument validation and replication of changes (including for initial sync).
A correct solution will be designed as SERVER-17320.

jstests/core/collmod.js

@@ -9,6 +9,8 @@ var coll = "collModTest";
 var t = db.getCollection( coll );
 t.drop();
 
+var isMongos = ("isdbgrid" == db.runCommand("ismaster").msg);
+
 db.createCollection( coll );
 
 function findTTL( key, expireAfterSeconds ) {
@@ -19,6 +21,16 @@ function findTTL( key, expireAfterSeconds ) {
     return all.length == 1;
 }
 
+function findCollectionInfo() {
+    var all = db.getCollectionInfos();
+    all = all.filter( function(z) { return z.name == t.getName(); } );
+    assert.eq(all.length, 1);
+    return all[0];
+}
+
+// ensure we fail with gibberish options
+assert.commandFailed(t.runCommand('collmod', {NotARealOption:1}));
+
 // add a TTL index
 t.ensureIndex( {a : 1}, { "expireAfterSeconds": 50 } )
 assert( findTTL( { a : 1 }, 50 ), "TTL index not added" );
@@ -64,3 +76,24 @@ var res = db.runCommand( { "collMod" : coll ,
 debug( res );
 assert( findTTL( {a:1}, 100), "TTL index should be 100 now" );
 
+// Clear usePowerOf2Sizes and enable noPadding. Make sure collection options.flags is updated.
+var res = db.runCommand( { "collMod" : coll ,
+                           "usePowerOf2Sizes" : false,
+                           "noPadding" : true} )
+debug( res );
+assert.commandWorked(res);
+var info = findCollectionInfo();
+assert.eq(info.options.flags, 2, tojson(info)); // 2 is CollectionOptions::Flag_NoPadding
+
+// Clear noPadding and check results object and options.flags.
+var res = db.runCommand( { "collMod" : coll ,
+                           "noPadding" : false} )
+debug( res );
+assert.commandWorked(res);
+if (!isMongos) {
+    // don't check this for sharding passthrough since mongos has a different output format.
+    assert.eq(res.noPadding_old, true, tojson(res));
+    assert.eq(res.noPadding_new, false, tojson(res));
+}
+var info = findCollectionInfo();
+assert.eq(info.options.flags, 0, tojson(info));

jstests/mmap_v1/use_power_of_2_a.js

@@ -10,20 +10,23 @@ function test(defaultMode) {
     db.createCollection('b', {usePowerOf2Sizes: false});
     assert.eq(db.b.stats().userFlags & 1, 0);
 
-    // capped should be 0
+    // Capped collections now behave like regular collections in terms of userFlags. Previously they
+    // were always 0, unless collmod was used.
+
+    // capped should obey default (even though it is ignored)
     db.c.drop();
     db.createCollection('c', {capped:true, size: 10});
-    assert.eq(db.c.stats().userFlags & 1, 0);
+    assert.eq(db.c.stats().userFlags & 1, defaultMode);
 
-    // capped should be 0
+    // capped explicitly off should be 0
     db.d.drop();
     db.createCollection('d', {capped:true, size: 10, usePowerOf2Sizes: false});
     assert.eq(db.d.stats().userFlags & 1, 0);
 
-    // capped and ask explicitly for powerOf2 should be 0
+    // capped and ask explicitly for powerOf2 should be 1
     db.e.drop();
     db.createCollection('e', {capped:true, size: 10, usePowerOf2Sizes: true});
-    assert.eq(db.e.stats().userFlags & 1, 0);
+    assert.eq(db.e.stats().userFlags & 1, 1);
 }
 
 assert.eq(db.adminCommand({getParameter:1,

jstests/tool/dumprestoreWithNoOptions.js

@@ -23,13 +23,7 @@ dbname2 = "NOT_"+dbname;
 
 db.dropDatabase();
 
-// MMapV1 always sets newcollectionsusepowerof2sizes, WT does not
-defaultFlags = { "flags" : 1 }
-var ss = db.serverStatus();
-
-if (ss.storageEngine.name != "mmapv1") {
-    defaultFlags = {};
-}
+var defaultFlags = {}
 
 var options = { capped: true, size: 4096, autoIndexId: true };
 db.createCollection('capped', options);

src/mongo/db/catalog/collection_catalog_entry.h

@@ -98,6 +98,13 @@ namespace mongo {
         virtual void updateTTLSetting( OperationContext* txn,
                                        StringData idxName,
                                        long long newExpireSeconds ) = 0;
+
+        /**
+         * Sets the flags field of CollectionOptions to newValue.
+         * Subsequent calls to getCollectionOptions should have flags==newValue and flagsSet==true.
+         */
+        virtual void updateFlags(OperationContext* txn, int newValue) = 0;
+
     private:
         NamespaceString _ns;
     };

src/mongo/db/catalog/collection_options.cpp

@@ -57,7 +57,9 @@ namespace mongo {
         initialNumExtents = 0;
         initialExtentSizes.clear();
         autoIndexId = DEFAULT;
-        flags = 0;
+        // For compatibility with previous versions if the user sets no flags,
+        // we set Flag_UsePowerOf2Sizes in case the user downgrades.
+        flags = Flag_UsePowerOf2Sizes;
         flagsSet = false;
         temp = false;
         storageEngine = BSONObj();

src/mongo/db/catalog/collection_options.h

@@ -76,7 +76,11 @@ namespace mongo {
         } autoIndexId;
 
         // user flags
-        int flags;
+        enum UserFlags {
+            Flag_UsePowerOf2Sizes = 1 << 0,
+            Flag_NoPadding = 1 << 1,
+        };
+        int flags; // a bitvector of UserFlags
         bool flagsSet;
 
         bool temp;

src/mongo/db/dbcommands.cpp

@@ -1125,17 +1125,39 @@ namespace mongo {
                     }
                 }
                 else {
-                    Status s = coll->getRecordStore()->setCustomOption( txn, e, &result );
-                    if ( s.isOK() ) {
-                        // no-op
-                    }
-                    else if ( s.code() == ErrorCodes::InvalidOptions ) {
-                        errmsg = str::stream() << "unknown option to collMod: " << e.fieldName();
+                    // As of SERVER-17312 we only support these two options. When SERVER-17320 is
+                    // resolved this will need to be enhanced to handle other options.
+                    typedef CollectionOptions CO;
+                    const StringData name = e.fieldNameStringData();
+                    const int flag = (name == "usePowerOf2Sizes") ? CO::Flag_UsePowerOf2Sizes :
+                                     (name == "noPadding") ? CO::Flag_NoPadding :
+                                     0;
+                    if (!flag) {
+                        errmsg = str::stream() << "unknown option to collMod: " << name;
                         ok = false;
+                        continue;
                     }
-                    else {
-                        return appendCommandStatus( result, s );
-                    }
+
+                    CollectionCatalogEntry* cce = coll->getCatalogEntry();
+
+                    const int oldFlags = cce->getCollectionOptions(txn).flags;
+                    const bool oldSetting = oldFlags & flag;
+                    const bool newSetting = e.trueValue();
+
+                    result.appendBool( name.toString() + "_old", oldSetting );
+                    result.appendBool( name.toString() + "_new", newSetting );
+
+                    const int newFlags = newSetting
+                                       ? (oldFlags | flag) // set flag
+                                       : (oldFlags & ~flag); // clear flag
+
+                    // NOTE we do this unconditionally to ensure that we note that the user has
+                    // explicitly set flags, even if they are just setting the default.
+                    cce->updateFlags(txn, newFlags);
+
+                    const CollectionOptions newOptions = cce->getCollectionOptions(txn);
+                    invariant(newOptions.flags == newFlags);
+                    invariant(newOptions.flagsSet);
                 }
             }
 

src/mongo/db/storage/SConscript

@@ -60,7 +60,6 @@ env.Library(
 env.Library(
     target='record_store_test_harness',
     source=[
-        'record_store_test_customoption.cpp',
         'record_store_test_datafor.cpp',
         'record_store_test_datasize.cpp',
         'record_store_test_deleterecord.cpp',

src/mongo/db/storage/devnull/devnull_kv_engine.cpp

@@ -159,12 +159,6 @@ namespace mongo {
             return Status::OK();
         }
 
-        virtual Status setCustomOption( OperationContext* txn,
-                                        const BSONElement& option,
-                                        BSONObjBuilder* info = NULL ) {
-            return Status::OK();
-        }
-
         virtual void updateStatsAfterRepair(OperationContext* txn,
                                             long long numRecords,
                                             long long dataSize) {

src/mongo/db/storage/in_memory/in_memory_record_store.cpp

@@ -459,20 +459,6 @@ namespace mongo {
         return Status::OK();
     }
 
-    Status InMemoryRecordStore::setCustomOption(
-                OperationContext* txn, const BSONElement& option, BSONObjBuilder* info) {
-        StringData name = option.fieldName();
-        if ( name == "usePowerOf2Sizes" ) {
-            // we ignore, so just say ok
-            return Status::OK();
-        }
-
-        return Status( ErrorCodes::InvalidOptions,
-                       mongoutils::str::stream()
-                       << "unknown custom option to InMemoryRecordStore: "
-                       << name );
-    }
-
     void InMemoryRecordStore::increaseStorageSize(OperationContext* txn,
                                                   int size, bool enforceQuota) {
         // unclear what this would mean for this class. For now, just error if called.

src/mongo/db/storage/in_memory/in_memory_record_store.h

@@ -111,10 +111,6 @@ namespace mongo {
 
         virtual Status touch( OperationContext* txn, BSONObjBuilder* output ) const;
 
-        virtual Status setCustomOption( OperationContext* txn,
-                                        const BSONElement& option,
-                                        BSONObjBuilder* info = NULL );
-
         virtual void increaseStorageSize( OperationContext* txn,  int size, bool enforceQuota );
 
         virtual int64_t storageSize( OperationContext* txn,

src/mongo/db/storage/kv/kv_collection_catalog_entry.cpp

@@ -171,6 +171,13 @@ namespace mongo {
         _catalog->putMetaData( txn, ns().toString(), md );
     }
 
+    void KVCollectionCatalogEntry::updateFlags(OperationContext* txn, int newValue) {
+        MetaData md = _getMetaData( txn );
+        md.options.flags = newValue;
+        md.options.flagsSet = true;
+        _catalog->putMetaData( txn, ns().toString(), md );
+    }
+
     BSONCollectionCatalogEntry::MetaData KVCollectionCatalogEntry::_getMetaData( OperationContext* txn ) const {
         return _catalog->getMetaData( txn, ns().toString() );
     }

src/mongo/db/storage/kv/kv_collection_catalog_entry.h

@@ -76,6 +76,8 @@ namespace mongo {
                                        StringData idxName,
                                        long long newExpireSeconds );
 
+        virtual void updateFlags(OperationContext* txn, int newValue);
+
         RecordStore* getRecordStore() { return _recordStore.get(); }
         const RecordStore* getRecordStore() const { return _recordStore.get(); }
 

src/mongo/db/storage/mmap_v1/catalog/namespace_details_collection_entry.cpp

@@ -28,30 +28,53 @@
 *    it in the license file.
 */
 
+#define MONGO_LOG_DEFAULT_COMPONENT ::mongo::logger::LogComponent::kStorage
+
 #include "mongo/db/storage/mmap_v1/catalog/namespace_details_collection_entry.h"
 
 #include "mongo/db/index/index_descriptor.h"
+#include "mongo/db/ops/update.h"
 #include "mongo/db/storage/mmap_v1/catalog/namespace_details.h"
+#include "mongo/db/storage/mmap_v1/catalog/namespace_details_rsv1_metadata.h"
 #include "mongo/db/storage/mmap_v1/mmap_v1_database_catalog_entry.h"
 #include "mongo/db/storage/record_store.h"
+#include "mongo/util/log.h"
 #include "mongo/util/startup_test.h"
 
 namespace mongo {
 
     using std::string;
 
-    NamespaceDetailsCollectionCatalogEntry::NamespaceDetailsCollectionCatalogEntry( StringData ns,
-                                                                                    NamespaceDetails* details,
-                                                                                    RecordStore* indexRecordStore,
-                                                                                    MMAPV1DatabaseCatalogEntry* db )
+    NamespaceDetailsCollectionCatalogEntry::NamespaceDetailsCollectionCatalogEntry(
+            StringData ns,
+            NamespaceDetails* details,
+            RecordStore* namespacesRecordStore,
+            RecordStore* indexRecordStore,
+            MMAPV1DatabaseCatalogEntry* db )
         : CollectionCatalogEntry( ns ),
           _details( details ),
+          _namespacesRecordStore(namespacesRecordStore),
           _indexRecordStore( indexRecordStore ),
           _db( db ) {
     }
 
     CollectionOptions NamespaceDetailsCollectionCatalogEntry::getCollectionOptions(OperationContext* txn) const {
-        return _db->getCollectionOptions( txn, ns().ns() );
+        CollectionOptions options = _db->getCollectionOptions( txn, ns().ns() );
+
+        if (options.flagsSet) {
+            if (options.flags != _details->userFlags) {
+                warning() << "system.namespaces and NamespaceDetails disagree about userFlags."
+                          << " system.namespaces: " << options.flags
+                          << " NamespaceDetails: " << _details->userFlags;
+                dassert(options.flags == _details->userFlags);
+            }
+        }
+
+        // Fill in the actual flags from the NamespaceDetails.
+        // Leaving flagsSet alone since it indicates whether the user actively set the flags.
+        options.flags = _details->userFlags;
+
+        return options;
     }
 
     int NamespaceDetailsCollectionCatalogEntry::getTotalIndexCount( OperationContext* txn ) const {
@@ -334,5 +357,40 @@ namespace mongo {
         }
     }
 
+    void NamespaceDetailsCollectionCatalogEntry::updateFlags(OperationContext* txn, int newValue) {
+        NamespaceDetailsRSV1MetaData md(ns().ns(), _details);
+        md.replaceUserFlags(txn, newValue);
+
+        if ( !_namespacesRecordStore )
+            return;
+
+        boost::scoped_ptr<RecordIterator> iterator( _namespacesRecordStore->getIterator(txn) );
+        while ( !iterator->isEOF() ) {
+            RecordId loc = iterator->getNext();
+
+            BSONObj oldEntry = iterator->dataFor( loc ).toBson();
+            BSONElement e = oldEntry["name"];
+            if ( e.type() != String )
+                continue;
+
+            if ( e.String() != ns().ns() )
+                continue;
+
+            BSONObj newEntry =
+                applyUpdateOperators( oldEntry,
+                                      BSON( "$set" << BSON( "options.flags" << newValue) ) );
+
+            StatusWith<RecordId> result = _namespacesRecordStore->updateRecord(txn,
+                                                                               loc,
+                                                                               newEntry.objdata(),
+                                                                               newEntry.objsize(),
+                                                                               false,
+                                                                               NULL);
+            fassert( 17486, result.isOK() );
+            return;
+        }
+
+        fassertFailed( 17488 );
+    }
 
 }

src/mongo/db/storage/mmap_v1/catalog/namespace_details_collection_entry.h

@@ -47,6 +47,7 @@ namespace mongo {
     public:
         NamespaceDetailsCollectionCatalogEntry( StringData ns,
                                                 NamespaceDetails* details,
+                                                RecordStore* namespacesRecordStore,
                                                 RecordStore* indexRecordStore,
                                                 MMAPV1DatabaseCatalogEntry* db );
 
@@ -100,12 +101,15 @@ namespace mongo {
                                        StringData idxName,
                                        long long newExpireSeconds );
 
+        virtual void updateFlags(OperationContext* txn, int newValue);
+
         // not part of interface, but available to my storage engine
 
         int _findIndexNumber( OperationContext* txn, StringData indexName) const;
 
     private:
         NamespaceDetails* _details;
+        RecordStore* _namespacesRecordStore;
         RecordStore* _indexRecordStore;
         MMAPV1DatabaseCatalogEntry* _db;