From 37627eca43dcb19def199e50fa7673ab8710d7a4 Mon Sep 17 00:00:00 2001 From: Robert Guo Date: Mon, 12 Dec 2016 09:55:44 -0500 Subject: SERVER-25004 isolate parsing logic in collmod (cherry-picked from commit 948f978a782167ddb6efec9f9adf5ad317eb4d0d) --- src/mongo/db/catalog/coll_mod.cpp | 243 ++++++++++++++++++++++-------------- src/mongo/db/catalog/coll_mod.h | 9 ++ src/mongo/db/catalog/collection.cpp | 12 +- src/mongo/db/catalog/collection.h | 22 ++-- 4 files changed, 175 insertions(+), 111 deletions(-) diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp index 0f2304a6f49..df3e2e203e1 100644 --- a/src/mongo/db/catalog/coll_mod.cpp +++ b/src/mongo/db/catalog/coll_mod.cpp @@ -41,6 +41,99 @@ #include "mongo/db/service_context.h" namespace mongo { + +struct CollModRequest { + const IndexDescriptor* idx = nullptr; + BSONElement indexExpireAfterSeconds = {}; + BSONElement collValidator = {}; + std::string collValidationAction = {}; + std::string collValidationLevel = {}; + BSONElement usePowerOf2Sizes = {}; + BSONElement noPadding = {}; +}; + +StatusWith parseCollModRequest(OperationContext* txn, + const NamespaceString& nss, + Collection* coll, + const BSONObj& cmdObj) { + CollModRequest cmr; + + BSONForEach(e, cmdObj) { + if (str::equals("collMod", e.fieldName())) { + // no-op + } else if (str::startsWith(e.fieldName(), "$")) { + // no-op ignore top-level fields prefixed with $. They are for the command processor + } else if (LiteParsedQuery::cmdOptionMaxTimeMS == e.fieldNameStringData()) { + // no-op + } else if (str::equals("index", e.fieldName())) { + BSONObj indexObj = e.Obj(); + BSONObj keyPattern = indexObj.getObjectField("keyPattern"); + + if (keyPattern.isEmpty()) { + return Status(ErrorCodes::InvalidOptions, "no keyPattern specified"); + } + cmr.indexExpireAfterSeconds = indexObj["expireAfterSeconds"]; + if (cmr.indexExpireAfterSeconds.eoo()) { + return Status(ErrorCodes::InvalidOptions, "no expireAfterSeconds field"); + } + if (!cmr.indexExpireAfterSeconds.isNumber()) { + return Status(ErrorCodes::InvalidOptions, + "expireAfterSeconds field must be a number"); + } + + const IndexDescriptor* idx = + coll->getIndexCatalog()->findIndexByKeyPattern(txn, keyPattern); + if (idx == NULL) { + return Status(ErrorCodes::InvalidOptions, + str::stream() << "cannot find index " << keyPattern << " for ns " + << nss.ns()); + } + cmr.idx = idx; + + BSONElement oldExpireSecs = cmr.idx->infoObj().getField("expireAfterSeconds"); + + if (oldExpireSecs.eoo()) { + return Status(ErrorCodes::InvalidOptions, "no expireAfterSeconds field to update"); + } + if (!oldExpireSecs.isNumber()) { + return Status(ErrorCodes::InvalidOptions, + "existing expireAfterSeconds field is not a number"); + } + } else if (str::equals("validator", e.fieldName())) { + auto statusW = coll->parseValidator(e.Obj()); + if (!statusW.isOK()) + return statusW.getStatus(); + + cmr.collValidator = e; + } else if (str::equals("validationLevel", e.fieldName())) { + auto statusW = coll->parseValidationLevel(e.String()); + if (!statusW.isOK()) + return statusW.getStatus(); + + cmr.collValidationLevel = e.String(); + } else if (str::equals("validationAction", e.fieldName())) { + auto statusW = coll->parseValidationAction(e.String()); + if (!statusW.isOK()) + return statusW.getStatus(); + + cmr.collValidationAction = e.String(); + } else { + // 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. + const StringData name = e.fieldNameStringData(); + if (name == "usePowerOf2Sizes") + cmr.usePowerOf2Sizes = e; + else if (name == "noPadding") + cmr.noPadding = e; + else + return Status(ErrorCodes::InvalidOptions, + str::stream() << "unknown option to collMod: " << name); + } + } + + return {std::move(cmr)}; +} + Status collMod(OperationContext* txn, const NamespaceString& nss, const BSONObj& cmdObj, @@ -49,7 +142,7 @@ Status collMod(OperationContext* txn, ScopedTransaction transaction(txn, MODE_IX); AutoGetDb autoDb(txn, dbName, MODE_X); Database* const db = autoDb.getDb(); - Collection* coll = db ? db->getCollection(nss) : NULL; + Collection* coll = db ? db->getCollection(nss) : nullptr; // This can kill all cursors so don't allow running it while a background operation is in // progress. @@ -71,122 +164,80 @@ Status collMod(OperationContext* txn, << nss.ns()); } + auto statusW = parseCollModRequest(txn, nss, coll, cmdObj); + if (!statusW.isOK()) { + return statusW.getStatus(); + } + + CollModRequest cmr = statusW.getValue(); + WriteUnitOfWork wunit(txn); - Status errorStatus = Status::OK(); + if (!cmr.indexExpireAfterSeconds.eoo()) { + BSONElement& newExpireSecs = cmr.indexExpireAfterSeconds; + BSONElement oldExpireSecs = cmr.idx->infoObj().getField("expireAfterSeconds"); + + if (oldExpireSecs != newExpireSecs) { + result->appendAs(oldExpireSecs, "expireAfterSeconds_old"); + // Change the value of "expireAfterSeconds" on disk. + coll->getCatalogEntry()->updateTTLSetting( + txn, cmr.idx->indexName(), newExpireSecs.safeNumberLong()); + // Notify the index catalog that the definition of this index changed. + cmr.idx = coll->getIndexCatalog()->refreshEntry(txn, cmr.idx); + result->appendAs(newExpireSecs, "expireAfterSeconds_new"); + } + } - BSONForEach(e, cmdObj) { - if (str::equals("collMod", e.fieldName())) { - // no-op - } else if (str::startsWith(e.fieldName(), "$")) { - // no-op ignore top-level fields prefixed with $. They are for the command processor - } else if (LiteParsedQuery::cmdOptionMaxTimeMS == e.fieldNameStringData()) { - // no-op - } else if (str::equals("index", e.fieldName())) { - BSONObj indexObj = e.Obj(); - BSONObj keyPattern = indexObj.getObjectField("keyPattern"); + if (!cmr.collValidator.eoo()) + coll->setValidator(txn, cmr.collValidator.Obj()); - if (keyPattern.isEmpty()) { - errorStatus = Status(ErrorCodes::InvalidOptions, "no keyPattern specified"); - continue; - } + if (!cmr.collValidationAction.empty()) + coll->setValidationAction(txn, cmr.collValidationAction); - BSONElement newExpireSecs = indexObj["expireAfterSeconds"]; - if (newExpireSecs.eoo()) { - errorStatus = Status(ErrorCodes::InvalidOptions, "no expireAfterSeconds field"); - continue; - } - if (!newExpireSecs.isNumber()) { - errorStatus = - Status(ErrorCodes::InvalidOptions, "expireAfterSeconds field must be a number"); - continue; - } + if (!cmr.collValidationLevel.empty()) + coll->setValidationLevel(txn, cmr.collValidationLevel); - const IndexDescriptor* idx = - coll->getIndexCatalog()->findIndexByKeyPattern(txn, keyPattern); - if (idx == NULL) { - errorStatus = Status(ErrorCodes::InvalidOptions, - str::stream() << "cannot find index " << keyPattern - << " for ns " << nss.ns()); - continue; - } - BSONElement oldExpireSecs = idx->infoObj().getField("expireAfterSeconds"); - if (oldExpireSecs.eoo()) { - errorStatus = - Status(ErrorCodes::InvalidOptions, "no expireAfterSeconds field to update"); - continue; - } - if (!oldExpireSecs.isNumber()) { - errorStatus = Status(ErrorCodes::InvalidOptions, - "existing expireAfterSeconds field is not a number"); - continue; - } + auto setCollectionOption = [&](BSONElement& COElement) { + typedef CollectionOptions CO; + const StringData name = COElement.fieldNameStringData(); - if (oldExpireSecs != newExpireSecs) { - result->appendAs(oldExpireSecs, "expireAfterSeconds_old"); - // Change the value of "expireAfterSeconds" on disk. - coll->getCatalogEntry()->updateTTLSetting( - txn, idx->indexName(), newExpireSecs.numberLong()); - // Notify the index catalog that the definition of this index changed. - idx = coll->getIndexCatalog()->refreshEntry(txn, idx); - result->appendAs(newExpireSecs, "expireAfterSeconds_new"); - } - } else if (str::equals("validator", e.fieldName())) { - auto status = coll->setValidator(txn, e.Obj()); - if (!status.isOK()) - errorStatus = std::move(status); - } else if (str::equals("validationLevel", e.fieldName())) { - auto status = coll->setValidationLevel(txn, e.String()); - if (!status.isOK()) - errorStatus = std::move(status); - } else if (str::equals("validationAction", e.fieldName())) { - auto status = coll->setValidationAction(txn, e.String()); - if (!status.isOK()) - errorStatus = std::move(status); - } else { - // 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) { - errorStatus = Status(ErrorCodes::InvalidOptions, - str::stream() << "unknown option to collMod: " << name); - continue; - } + int flag = (name == "usePowerOf2Sizes") ? CO::Flag_UsePowerOf2Sizes + : (name == "noPadding") ? CO::Flag_NoPadding : 0; - CollectionCatalogEntry* cce = coll->getCatalogEntry(); + CollectionCatalogEntry* cce = coll->getCatalogEntry(); - const int oldFlags = cce->getCollectionOptions(txn).flags; - const bool oldSetting = oldFlags & flag; - const bool newSetting = e.trueValue(); + const int oldFlags = cce->getCollectionOptions(txn).flags; + const bool oldSetting = oldFlags & flag; + const bool newSetting = COElement.trueValue(); - result->appendBool(name.toString() + "_old", oldSetting); - result->appendBool(name.toString() + "_new", newSetting); + result->appendBool(name.toString() + "_old", oldSetting); + result->appendBool(name.toString() + "_new", newSetting); - const int newFlags = newSetting ? (oldFlags | flag) // set flag - : (oldFlags & ~flag); // clear flag + 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); + // 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); - } + const CollectionOptions newOptions = cce->getCollectionOptions(txn); + invariant(newOptions.flags == newFlags); + invariant(newOptions.flagsSet); + }; + + if (!cmr.usePowerOf2Sizes.eoo()) { + setCollectionOption(cmr.usePowerOf2Sizes); } - if (!errorStatus.isOK()) { - return errorStatus; + if (!cmr.noPadding.eoo()) { + setCollectionOption(cmr.noPadding); } getGlobalServiceContext()->getOpObserver()->onCollMod( txn, (dbName.toString() + ".$cmd").c_str(), cmdObj); wunit.commit(); + return Status::OK(); } } // namespace mongo diff --git a/src/mongo/db/catalog/coll_mod.h b/src/mongo/db/catalog/coll_mod.h index eb8644b74d1..bb86bbc50d1 100644 --- a/src/mongo/db/catalog/coll_mod.h +++ b/src/mongo/db/catalog/coll_mod.h @@ -27,13 +27,22 @@ */ #include "mongo/base/status.h" +#include "mongo/base/status_with.h" namespace mongo { class BSONObj; class BSONObjBuilder; +class Collection; class NamespaceString; class OperationContext; +struct CollModRequest; + +StatusWith parseCollModRequest(OperationContext* txn, + const NamespaceString& nss, + Collection* coll, + const BSONObj& cmdObj); + /** * Performs the collection modification described in "cmdObj" on the collection "ns". */ diff --git a/src/mongo/db/catalog/collection.cpp b/src/mongo/db/catalog/collection.cpp index 0043b20a4c5..42fc0ea7fee 100644 --- a/src/mongo/db/catalog/collection.cpp +++ b/src/mongo/db/catalog/collection.cpp @@ -180,9 +180,9 @@ Collection::Collection(OperationContext* txn, _validatorDoc(_details->getCollectionOptions(txn).validator.getOwned()), _validator(uassertStatusOK(parseValidator(_validatorDoc))), _validationAction(uassertStatusOK( - _parseValidationAction(_details->getCollectionOptions(txn).validationAction))), + parseValidationAction(_details->getCollectionOptions(txn).validationAction))), _validationLevel(uassertStatusOK( - _parseValidationLevel(_details->getCollectionOptions(txn).validationLevel))), + parseValidationLevel(_details->getCollectionOptions(txn).validationLevel))), _cursorManager(fullNS), _cappedNotifier(_recordStore->isCapped() ? new CappedInsertNotifier() : nullptr), _mustTakeCappedLockOnInsert(isCapped() && !_ns.isSystemDotProfile() && !_ns.isOplog()) { @@ -823,7 +823,7 @@ Status Collection::setValidator(OperationContext* txn, BSONObj validatorDoc) { return Status::OK(); } -StatusWith Collection::_parseValidationLevel(StringData newLevel) { +StatusWith Collection::parseValidationLevel(StringData newLevel) { if (newLevel == "") { // default return STRICT_V; @@ -839,7 +839,7 @@ StatusWith Collection::_parseValidationLevel(String } } -StatusWith Collection::_parseValidationAction(StringData newAction) { +StatusWith Collection::parseValidationAction(StringData newAction) { if (newAction == "") { // default return ERROR_V; @@ -878,7 +878,7 @@ StringData Collection::getValidationAction() const { Status Collection::setValidationLevel(OperationContext* txn, StringData newLevel) { invariant(txn->lockState()->isCollectionLockedForMode(ns().toString(), MODE_X)); - StatusWith status = _parseValidationLevel(newLevel); + StatusWith status = parseValidationLevel(newLevel); if (!status.isOK()) { return status.getStatus(); } @@ -893,7 +893,7 @@ Status Collection::setValidationLevel(OperationContext* txn, StringData newLevel Status Collection::setValidationAction(OperationContext* txn, StringData newAction) { invariant(txn->lockState()->isCollectionLockedForMode(ns().toString(), MODE_X)); - StatusWith status = _parseValidationAction(newAction); + StatusWith status = parseValidationAction(newAction); if (!status.isOK()) { return status.getStatus(); } diff --git a/src/mongo/db/catalog/collection.h b/src/mongo/db/catalog/collection.h index 6823045b952..becfe366ab3 100644 --- a/src/mongo/db/catalog/collection.h +++ b/src/mongo/db/catalog/collection.h @@ -38,6 +38,7 @@ #include "mongo/base/status_with.h" #include "mongo/base/string_data.h" #include "mongo/bson/mutable/damage_vector.h" +#include "mongo/db/catalog/coll_mod.h" #include "mongo/db/catalog/collection_info_cache.h" #include "mongo/db/catalog/cursor_manager.h" #include "mongo/db/catalog/index_catalog.h" @@ -362,6 +363,16 @@ public: */ void temp_cappedTruncateAfter(OperationContext* txn, RecordId end, bool inclusive); + enum ValidationAction { WARN, ERROR_V }; + enum ValidationLevel { OFF, MODERATE, STRICT_V }; + + /** + * Returns a non-ok Status if validator is not legal for this collection. + */ + StatusWithMatchExpression parseValidator(const BSONObj& validator) const; + + static StatusWith parseValidationLevel(StringData); + static StatusWith parseValidationAction(StringData); /** * Sets the validator for this collection. * @@ -428,11 +439,6 @@ private: */ Status checkValidation(OperationContext* txn, const BSONObj& document) const; - /** - * Returns a non-ok Status if validator is not legal for this collection. - */ - StatusWithMatchExpression parseValidator(const BSONObj& validator) const; - Status recordStoreGoingToMove(OperationContext* txn, const RecordId& oldLocation, const char* oldBuffer, @@ -470,11 +476,9 @@ private: BSONObj _validatorDoc; // Points into _validatorDoc. Null means no filter. std::unique_ptr _validator; - enum ValidationAction { WARN, ERROR_V } _validationAction; - enum ValidationLevel { OFF, MODERATE, STRICT_V } _validationLevel; - static StatusWith _parseValidationLevel(StringData); - static StatusWith _parseValidationAction(StringData); + ValidationAction _validationAction; + ValidationLevel _validationLevel; // this is mutable because read only users of the Collection class // use it keep state. This seems valid as const correctness of Collection -- cgit v1.2.1