diff options
author | Kaloian Manassiev <kaloian.manassiev@mongodb.com> | 2017-07-26 15:17:46 -0400 |
---|---|---|
committer | Kaloian Manassiev <kaloian.manassiev@mongodb.com> | 2017-07-26 15:23:08 -0400 |
commit | 39b009d422cb0a71001bdc0bd87e5cfd0e6c5b3f (patch) | |
tree | 991cb013a71d5de522ec51bb41afec840b994481 | |
parent | 044bf07073d0d45cbb9b61b843fa33074599971f (diff) | |
download | mongo-39b009d422cb0a71001bdc0bd87e5cfd0e6c5b3f.tar.gz |
Revert "SERVER-28752 Get rid of BatchedInsertRequest::getIndexTargetingNS"
This reverts commit 86defa9d193f34275af2c4f3c783dffb046182ff.
This reverts commit 1f097a38ad68e1b362cde71b70fabfb63ea4aae7.
-rw-r--r-- | src/mongo/db/auth/user_cache_invalidator_job.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/ops/write_ops.h | 6 | ||||
-rw-r--r-- | src/mongo/db/ops/write_ops_parsers.cpp | 45 | ||||
-rw-r--r-- | src/mongo/s/catalog/sharding_catalog_enable_sharding_test.cpp | 2 | ||||
-rw-r--r-- | src/mongo/s/commands/cluster_write.cpp | 6 | ||||
-rw-r--r-- | src/mongo/s/write_ops/batched_command_request.cpp | 36 | ||||
-rw-r--r-- | src/mongo/s/write_ops/batched_command_request.h | 3 | ||||
-rw-r--r-- | src/mongo/s/write_ops/batched_insert_request.cpp | 22 | ||||
-rw-r--r-- | src/mongo/s/write_ops/batched_insert_request.h | 9 |
9 files changed, 80 insertions, 51 deletions
diff --git a/src/mongo/db/auth/user_cache_invalidator_job.cpp b/src/mongo/db/auth/user_cache_invalidator_job.cpp index 81602d77a14..5c9b7c30d8c 100644 --- a/src/mongo/db/auth/user_cache_invalidator_job.cpp +++ b/src/mongo/db/auth/user_cache_invalidator_job.cpp @@ -94,7 +94,7 @@ public: StatusWith<OID> getCurrentCacheGeneration(OperationContext* opCtx) { try { BSONObjBuilder result; - const bool ok = Grid::get(opCtx)->catalogClient()->runUserManagementReadCommand( + const bool ok = grid.catalogClient()->runUserManagementReadCommand( opCtx, "admin", BSON("_getUserCacheGeneration" << 1), &result); if (!ok) { return getStatusFromCommandResult(result.obj()); diff --git a/src/mongo/db/ops/write_ops.h b/src/mongo/db/ops/write_ops.h index ec055786029..5463bb9a0e2 100644 --- a/src/mongo/db/ops/write_ops.h +++ b/src/mongo/db/ops/write_ops.h @@ -71,12 +71,6 @@ int32_t getStmtIdForWriteAt(const T& op, size_t writePos) { return getStmtIdForWriteAt(op.getWriteCommandBase(), writePos); } -/** - * Must only be called if the insert is for the "system.indexes" namespace. Returns the actual - * namespace for which the index is being created. - */ -NamespaceString extractIndexedNamespace(const Insert& insertOp); - // TODO: Delete this getter once IDL supports defaults for object and array fields template <class T> const BSONObj& collationOf(const T& opEntry) { diff --git a/src/mongo/db/ops/write_ops_parsers.cpp b/src/mongo/db/ops/write_ops_parsers.cpp index f224fdf3dca..b4e4b9157a7 100644 --- a/src/mongo/db/ops/write_ops_parsers.cpp +++ b/src/mongo/db/ops/write_ops_parsers.cpp @@ -62,30 +62,6 @@ void checkOpCountForCommand(const T& op, size_t numOps) { !stmtIds || stmtIds->size() == numOps); } -void validateInsertOp(const write_ops::Insert& insertOp) { - const auto& nss = insertOp.getNamespace(); - const auto& docs = insertOp.getDocuments(); - - if (nss.isSystemDotIndexes()) { - // This is only for consistency with sharding. - uassert(ErrorCodes::InvalidLength, - "Insert commands to system.indexes are limited to a single insert", - docs.size() == 1); - - const auto indexedNss(extractIndexedNamespace(insertOp)); - - uassert(ErrorCodes::InvalidNamespace, - str::stream() << indexedNss.ns() << " is not a valid namespace to index", - indexedNss.isValid()); - - uassert(ErrorCodes::IllegalOperation, - str::stream() << indexedNss.ns() << " is not in the target database " << nss.db(), - nss.db().compare(indexedNss.db()) == 0); - } - - checkOpCountForCommand(insertOp, docs.size()); -} - } // namespace namespace write_ops { @@ -116,21 +92,19 @@ int32_t getStmtIdForWriteAt(const WriteCommandBase& writeCommandBase, size_t wri return kFirstStmtId + writePos; } -NamespaceString extractIndexedNamespace(const Insert& insertOp) { - invariant(insertOp.getNamespace().isSystemDotIndexes()); - - const auto& documents = insertOp.getDocuments(); - invariant(documents.size() == 1); - - return NamespaceString(documents.at(0)["ns"].str()); -} - } // namespace write_ops write_ops::Insert InsertOp::parse(const OpMsgRequest& request) { auto insertOp = Insert::parse(IDLParserErrorContext("insert"), request); - validateInsertOp(insertOp); + if (insertOp.getNamespace().isSystemDotIndexes()) { + // This is only for consistency with sharding. + uassert(ErrorCodes::InvalidLength, + "Insert commands to system.indexes are limited to a single insert", + insertOp.getDocuments().size() == 1); + } + + checkOpCountForCommand(insertOp, insertOp.getDocuments().size()); return insertOp; } @@ -157,7 +131,6 @@ write_ops::Insert InsertOp::parseLegacy(const Message& msgRaw) { return documents; }()); - validateInsertOp(op); return op; } @@ -217,7 +190,7 @@ write_ops::Delete DeleteOp::parseLegacy(const Message& msgRaw) { op.setWriteCommandBase(std::move(writeCommandBase)); } - op.setDeletes([&] { + op.setDeletes([&]() { std::vector<write_ops::DeleteOpEntry> deletes; deletes.emplace_back(); diff --git a/src/mongo/s/catalog/sharding_catalog_enable_sharding_test.cpp b/src/mongo/s/catalog/sharding_catalog_enable_sharding_test.cpp index 8007c729a0a..7cccb916106 100644 --- a/src/mongo/s/catalog/sharding_catalog_enable_sharding_test.cpp +++ b/src/mongo/s/catalog/sharding_catalog_enable_sharding_test.cpp @@ -51,6 +51,8 @@ #include "mongo/s/client/shard_registry.h" #include "mongo/s/config_server_test_fixture.h" #include "mongo/s/write_ops/batched_command_response.h" +#include "mongo/s/write_ops/batched_insert_request.h" +#include "mongo/s/write_ops/batched_update_request.h" #include "mongo/stdx/future.h" #include "mongo/util/log.h" #include "mongo/util/scopeguard.h" diff --git a/src/mongo/s/commands/cluster_write.cpp b/src/mongo/s/commands/cluster_write.cpp index 8f3ca8d22d7..174d9773a0b 100644 --- a/src/mongo/s/commands/cluster_write.cpp +++ b/src/mongo/s/commands/cluster_write.cpp @@ -210,6 +210,12 @@ void ClusterWriter::write(OperationContext* opCtx, const NamespaceString& nss = request->getNS(); + std::string errMsg; + if (request->isInsertIndexRequest() && !request->isValidIndexRequest(&errMsg)) { + toBatchError(Status(ErrorCodes::InvalidOptions, errMsg), response); + return; + } + // Config writes and shard writes are done differently if (nss.db() == NamespaceString::kConfigDb || nss.db() == NamespaceString::kAdminDb) { Grid::get(opCtx)->catalogClient()->writeConfigServerDirect(opCtx, *request, response); diff --git a/src/mongo/s/write_ops/batched_command_request.cpp b/src/mongo/s/write_ops/batched_command_request.cpp index 65bdf090ce0..322efc25afd 100644 --- a/src/mongo/s/write_ops/batched_command_request.cpp +++ b/src/mongo/s/write_ops/batched_command_request.cpp @@ -86,19 +86,41 @@ BatchedDeleteRequest* BatchedCommandRequest::getDeleteRequest() const { bool BatchedCommandRequest::isInsertIndexRequest() const { if (_batchType != BatchedCommandRequest::BatchType_Insert) return false; - return getNS().isSystemDotIndexes(); } -NamespaceString BatchedCommandRequest::getTargetingNSS() const { - if (!isInsertIndexRequest()) { - return getNS(); +bool BatchedCommandRequest::isValidIndexRequest(std::string* errMsg) const { + std::string dummy; + if (!errMsg) + errMsg = &dummy; + dassert(isInsertIndexRequest()); + + if (sizeWriteOps() != 1) { + *errMsg = "invalid batch request for index creation"; + return false; + } + + const NamespaceString& targetNSS = getTargetingNSS(); + if (!targetNSS.isValid()) { + *errMsg = targetNSS.ns() + " is not a valid namespace to index"; + return false; } - const auto& documents = _insertReq->getDocuments(); - invariant(documents.size() == 1); + const NamespaceString& reqNSS = getNS(); + if (reqNSS.db().compare(targetNSS.db()) != 0) { + *errMsg = + targetNSS.ns() + " namespace is not in the request database " + reqNSS.db().toString(); + return false; + } + + return true; +} + +const NamespaceString& BatchedCommandRequest::getTargetingNSS() const { + if (!isInsertIndexRequest()) + return getNS(); - return NamespaceString(documents.at(0)["ns"].str()); + return _insertReq->getIndexTargetingNS(); } bool BatchedCommandRequest::isVerboseWC() const { diff --git a/src/mongo/s/write_ops/batched_command_request.h b/src/mongo/s/write_ops/batched_command_request.h index dd7d96bcfce..6916d051a0e 100644 --- a/src/mongo/s/write_ops/batched_command_request.h +++ b/src/mongo/s/write_ops/batched_command_request.h @@ -90,8 +90,9 @@ public: // Index creation is also an insert, but a weird one. bool isInsertIndexRequest() const; + bool isValidIndexRequest(std::string* errMsg) const; - NamespaceString getTargetingNSS() const; + const NamespaceString& getTargetingNSS() const; // // individual field accessors diff --git a/src/mongo/s/write_ops/batched_insert_request.cpp b/src/mongo/s/write_ops/batched_insert_request.cpp index 95eff792efb..23fb632d850 100644 --- a/src/mongo/s/write_ops/batched_insert_request.cpp +++ b/src/mongo/s/write_ops/batched_insert_request.cpp @@ -34,6 +34,14 @@ #include "mongo/util/mongoutils/str.h" namespace mongo { +namespace { + +void extractIndexNSS(const BSONObj& indexDesc, NamespaceString* indexNSS) { + *indexNSS = NamespaceString(indexDesc["ns"].str()); +} + +} // namespace + const BSONField<std::string> BatchedInsertRequest::collName("insert"); const BSONField<std::vector<BSONObj>> BatchedInsertRequest::documents("documents"); @@ -92,11 +100,16 @@ void BatchedInsertRequest::parseRequest(const OpMsgRequest& request) { _documents.push_back(documentEntry.getOwned()); } + if (_documents.size() >= 1) { + extractIndexNSS(_documents.at(0), &_targetNSS); + } + _isDocumentsSet = true; } void BatchedInsertRequest::clear() { _ns = NamespaceString(); + _targetNSS = NamespaceString(); _isNSSet = false; _documents.clear(); @@ -107,6 +120,7 @@ void BatchedInsertRequest::cloneTo(BatchedInsertRequest* other) const { other->clear(); other->_ns = _ns; + other->_targetNSS = _targetNSS; other->_isNSSet = _isNSSet; for (std::vector<BSONObj>::const_iterator it = _documents.begin(); it != _documents.end(); @@ -130,9 +144,17 @@ const NamespaceString& BatchedInsertRequest::getNS() const { return _ns; } +const NamespaceString& BatchedInsertRequest::getIndexTargetingNS() const { + return _targetNSS; +} + void BatchedInsertRequest::addToDocuments(const BSONObj& documents) { _documents.push_back(documents); _isDocumentsSet = true; + + if (_documents.size() == 1) { + extractIndexNSS(_documents.at(0), &_targetNSS); + } } size_t BatchedInsertRequest::sizeDocuments() const { diff --git a/src/mongo/s/write_ops/batched_insert_request.h b/src/mongo/s/write_ops/batched_insert_request.h index ad215a5ad75..a5a00de0e0c 100644 --- a/src/mongo/s/write_ops/batched_insert_request.h +++ b/src/mongo/s/write_ops/batched_insert_request.h @@ -71,6 +71,12 @@ public: void setNS(NamespaceString collName); const NamespaceString& getNS() const; + /** + * Returns the ns for the index being created. Valid only if this is a index + * insert request. + */ + const NamespaceString& getIndexTargetingNS() const; + void addToDocuments(const BSONObj& documents); std::size_t sizeDocuments() const; const std::vector<BSONObj>& getDocuments() const; @@ -87,6 +93,9 @@ private: // (M) array of documents to be inserted std::vector<BSONObj> _documents; bool _isDocumentsSet; + + // (O) cached copied of target ns + NamespaceString _targetNSS; }; } // namespace mongo |