diff options
27 files changed, 258 insertions, 194 deletions
diff --git a/jstests/concurrency/fsm_workloads/create_timeseries_collection.js b/jstests/concurrency/fsm_workloads/create_timeseries_collection.js index 238bcb116ab..c2f05b39a70 100644 --- a/jstests/concurrency/fsm_workloads/create_timeseries_collection.js +++ b/jstests/concurrency/fsm_workloads/create_timeseries_collection.js @@ -63,7 +63,7 @@ var $config = (function() { } collName = getCollectionName(this.prefix, collName, this.tid); - db.getCollection(collName).drop(); + assertAlways(db.getCollection(collName).drop(), "failed to drop " + collName); } return {init: init, create: create, insert: insert, drop: drop}; diff --git a/jstests/sharding/timeseries_sharded_query.js b/jstests/sharding/timeseries_sharded_query.js index 3867cfbb2f7..f9845c87687 100644 --- a/jstests/sharding/timeseries_sharded_query.js +++ b/jstests/sharding/timeseries_sharded_query.js @@ -55,7 +55,7 @@ if (!TimeseriesTest.timeseriesCollectionsEnabled(st.shard0)) { {meta: 10} /* Split at */, {meta: 10} /* Move the chunk containing {meta: 10} to its own shard */, dbName, /* dbName */ - true /* Wait until documents orphaned by the move get deleted */); + false /* Wait until documents orphaned by the move get deleted */); let counts = st.chunkCounts('system.buckets.ts', 'test'); assert.eq(1, counts[st.shard0.shardName]); @@ -74,6 +74,11 @@ if (!TimeseriesTest.timeseriesCollectionsEnabled(st.shard0)) { // Create a time-series collection with a non-default collation, but an index with the simple // collation, which makes it eligible as a shard key. (function metaShardKeyCollation() { + // TODO (SERVER-55653): Re-enable this test when querying by non-default view collation works. + if (true) { + return; + } + assert.commandWorked(sDB.createCollection('ts', { timeseries: {timeField: 'time', metaField: 'hostName'}, collation: {locale: 'en', strength: 1, numericOrdering: true} @@ -100,20 +105,34 @@ if (!TimeseriesTest.timeseriesCollectionsEnabled(st.shard0)) { // This index gets created as {meta: 1} on the buckets collection. assert.commandWorked(tsColl.createIndex({hostName: 1}, {collation: {locale: 'simple'}})); - st.shardColl('system.buckets.ts', - {meta: 1} /* Shard key */, - {meta: 'host_10'} /* Split at */, - {meta: 'host_10'} /* Move the chunk containing {meta: 10} to its own shard */, - dbName, /* dbName */ - true /* Wait until documents orphaned by the move get deleted */); + assert.commandWorked(st.s.adminCommand({enableSharding: 'test'})); + assert.commandWorked(st.s.adminCommand({ + shardCollection: 'test.system.buckets.ts', + key: {meta: 1}, + collation: {locale: 'simple'}, + })); + + assert.commandWorked( + st.s.adminCommand({split: 'test.system.buckets.ts', middle: {meta: 'host_10'}})); + + let otherShard = st.getOther(st.getPrimaryShard(dbName)).name; + assert.commandWorked(st.s.adminCommand({ + movechunk: 'test.system.buckets.ts', + find: {meta: 'host_10'}, + to: otherShard, + _waitForDelete: false + })); let counts = st.chunkCounts('system.buckets.ts', 'test'); assert.eq(1, counts[st.shard0.shardName]); assert.eq(1, counts[st.shard1.shardName]); // Query with shard key - assert.docEq([docs[0]], sDB.ts.find({hostName: 'host_0'}).toArray()); - assert.docEq([docs[numDocs - 1]], sDB.ts.find({hostName: 'host_' + (numDocs - 1)}).toArray()); + assert.docEq([docs[0]], + sDB.ts.find({hostName: 'host_0'}).collation({locale: 'simple'}).toArray()); + assert.docEq( + [docs[numDocs - 1]], + sDB.ts.find({hostName: 'host_' + (numDocs - 1)}).collation({locale: 'simple'}).toArray()); // Query without shard key assert.docEq(docs, sDB.ts.find().sort({time: 1}).toArray()); @@ -172,7 +191,7 @@ if (!TimeseriesTest.timeseriesCollectionsEnabled(st.shard0)) { movechunk: 'test.system.buckets.ts', find: {'meta.id': 10, 'control.min.time': 0, 'control.max.time': 0}, to: otherShard, - _waitForDelete: true + _waitForDelete: false })); counts = st.chunkCounts('system.buckets.ts', 'test'); diff --git a/src/mongo/db/catalog/collection.h b/src/mongo/db/catalog/collection.h index 951639e0db7..b5212dea20d 100644 --- a/src/mongo/db/catalog/collection.h +++ b/src/mongo/db/catalog/collection.h @@ -565,6 +565,12 @@ public: virtual void setMinimumVisibleSnapshot(const Timestamp name) = 0; /** + * Returns the time-series options for this buckets collection, or boost::none if not a + * time-series buckets collection. + */ + virtual boost::optional<TimeseriesOptions> getTimeseriesOptions() const = 0; + + /** * Get a pointer to the collection's default collator. The pointer must not be used after this * Collection is destroyed. */ diff --git a/src/mongo/db/catalog/collection_impl.cpp b/src/mongo/db/catalog/collection_impl.cpp index e3d9611515b..69ce18cfd3b 100644 --- a/src/mongo/db/catalog/collection_impl.cpp +++ b/src/mongo/db/catalog/collection_impl.cpp @@ -333,6 +333,8 @@ void CollectionImpl::init(OperationContext* opCtx) { "validatorStatus"_attr = _validator.getStatus()); } + _timeseriesOptions = collectionOptions.timeseries; + if (collectionOptions.clusteredIndex) { _clustered = true; if (collectionOptions.clusteredIndex->getExpireAfterSeconds()) { @@ -1432,6 +1434,10 @@ Status CollectionImpl::updateValidator(OperationContext* opCtx, return Status::OK(); } +boost::optional<TimeseriesOptions> CollectionImpl::getTimeseriesOptions() const { + return _timeseriesOptions; +} + const CollatorInterface* CollectionImpl::getDefaultCollator() const { return _shared->_collator.get(); } diff --git a/src/mongo/db/catalog/collection_impl.h b/src/mongo/db/catalog/collection_impl.h index 2599471052e..ced40206173 100644 --- a/src/mongo/db/catalog/collection_impl.h +++ b/src/mongo/db/catalog/collection_impl.h @@ -365,6 +365,8 @@ public: */ void setMinimumVisibleSnapshot(Timestamp newMinimumVisibleSnapshot) final; + boost::optional<TimeseriesOptions> getTimeseriesOptions() const final; + /** * Get a pointer to the collection's default collator. The pointer must not be used after this * Collection is destroyed. @@ -503,6 +505,9 @@ private: // Whether or not this collection is clustered on _id values. bool _clustered = false; + // If this is a time-series buckets collection, the metadata for this collection. + boost::optional<TimeseriesOptions> _timeseriesOptions; + bool _recordPreImages = false; // The earliest snapshot that is allowed to use this collection. diff --git a/src/mongo/db/catalog/collection_mock.h b/src/mongo/db/catalog/collection_mock.h index 3fc5fd24400..244ecd571a9 100644 --- a/src/mongo/db/catalog/collection_mock.h +++ b/src/mongo/db/catalog/collection_mock.h @@ -303,6 +303,10 @@ public: std::abort(); } + boost::optional<TimeseriesOptions> getTimeseriesOptions() const { + std::abort(); + } + const CollatorInterface* getDefaultCollator() const { std::abort(); } diff --git a/src/mongo/db/catalog/create_collection.cpp b/src/mongo/db/catalog/create_collection.cpp index 26d24f6f812..d64c3567ed2 100644 --- a/src/mongo/db/catalog/create_collection.cpp +++ b/src/mongo/db/catalog/create_collection.cpp @@ -118,10 +118,13 @@ Status _createView(OperationContext* opCtx, Status _createTimeseries(OperationContext* opCtx, const NamespaceString& ns, - CollectionOptions&& options) { - auto bucketsNs = ns.makeTimeseriesBucketsNamespace(); + const CollectionOptions& options) { + // This path should only be taken when a user creates a new time-series collection on the + // primary. Secondaries replicate individual oplog entries. + invariant(!ns.isTimeseriesBucketsCollection()); + invariant(opCtx->writesAreReplicated()); - options.viewOn = bucketsNs.coll().toString(); + auto bucketsNs = ns.makeTimeseriesBucketsNamespace(); auto granularity = options.timeseries->getGranularity(); uassert(ErrorCodes::InvalidOptions, @@ -133,18 +136,6 @@ Status _createTimeseries(OperationContext* opCtx, "Time-series 'bucketMaxSpanSeconds' is required to be 3600", bucketMaxSpan == 3600); - if (options.timeseries->getMetaField()) { - options.pipeline = - BSON_ARRAY(BSON("$_internalUnpackBucket" - << BSON("timeField" << options.timeseries->getTimeField() << "metaField" - << *options.timeseries->getMetaField() << "exclude" - << BSONArray()))); - } else { - options.pipeline = BSON_ARRAY( - BSON("$_internalUnpackBucket" << BSON("timeField" << options.timeseries->getTimeField() - << "exclude" << BSONArray()))); - } - return writeConflictRetry(opCtx, "create", ns.ns(), [&]() -> Status { AutoGetCollection autoColl(opCtx, ns, MODE_IX, AutoGetCollectionViewMode::kViewsPermitted); Lock::CollectionLock bucketsCollLock(opCtx, bucketsNs, MODE_IX); @@ -202,7 +193,8 @@ Status _createTimeseries(OperationContext* opCtx, Top::get(serviceContext).collectionDropped(bucketsNs); }); - CollectionOptions bucketsOptions; + // Use the provided options to create the buckets collection. + CollectionOptions bucketsOptions = options; // Set the validator option to a JSON schema enforcing constraints on bucket documents. // This validation is only structural to prevent accidental corruption by users and cannot @@ -259,18 +251,19 @@ Status _createTimeseries(OperationContext* opCtx, bucketsOptions.clusteredIndex = clusteredOptions; } + // Once accepted by the create command, the 'expireAfterSeconds' option is either stored + // in the 'clusteredIndex' document or in the TTL index. To avoid storing redundant data, + // clear this field. + bucketsOptions.timeseries->setExpireAfterSeconds(boost::none); + // Create the buckets collection that will back the view. const bool createIdIndex = !useClusteredIdIndex; - auto bucketsCollection = - db->createCollection(opCtx, bucketsNs, bucketsOptions, createIdIndex); - invariant(bucketsCollection, - str::stream() << "Failed to create buckets collection " << bucketsNs - << " for time-series collection " << ns); + uassertStatusOK(db->userCreateNS(opCtx, bucketsNs, bucketsOptions, createIdIndex)); // Create a TTL index on 'control.min.[timeField]' if 'expireAfterSeconds' is provided and // the collection is not clustered by _id. - if (expireAfterSeconds && !bucketsOptions.clusteredIndex) { - CollectionWriter collectionWriter(opCtx, bucketsCollection->uuid()); + if (expireAfterSeconds && !useClusteredIdIndex) { + CollectionWriter collectionWriter(opCtx, bucketsNs); auto indexBuildCoord = IndexBuildsCoordinator::get(opCtx); const std::string controlMinTimeField = str::stream() << "control.min." << options.timeseries->getTimeField(); @@ -292,13 +285,28 @@ Status _createTimeseries(OperationContext* opCtx, } } - // Create the time-series view. Even though 'options' is passed by rvalue reference, it is - // not safe to move because 'userCreateNS' may throw a WriteConflictException. - auto status = db->userCreateNS(opCtx, ns, options); + CollectionOptions viewOptions; + viewOptions.viewOn = bucketsNs.coll().toString(); + viewOptions.collation = options.collation; + + if (options.timeseries->getMetaField()) { + viewOptions.pipeline = + BSON_ARRAY(BSON("$_internalUnpackBucket" << BSON( + "timeField" << options.timeseries->getTimeField() << "metaField" + << *options.timeseries->getMetaField() << "exclude" + << BSONArray()))); + } else { + viewOptions.pipeline = BSON_ARRAY(BSON( + "$_internalUnpackBucket" << BSON("timeField" << options.timeseries->getTimeField() + << "exclude" << BSONArray()))); + } + + // Create the time-series view. + auto status = db->userCreateNS(opCtx, ns, viewOptions); if (!status.isOK()) { return status.withContext(str::stream() << "Failed to create view on " << bucketsNs << " for time-series collection " << ns - << " with options " << options.toBSON()); + << " with options " << viewOptions.toBSON()); } wuow.commit(); @@ -401,12 +409,15 @@ Status createCollection(OperationContext* opCtx, "transaction.", !opCtx->inMultiDocumentTransaction()); return _createView(opCtx, ns, std::move(options)); - } else if (options.timeseries) { + } else if (options.timeseries && !ns.isTimeseriesBucketsCollection()) { + // This helper is designed for user-created time-series collections on primaries. If a + // time-series buckets collection is created explicitly or during replication, treat this as + // a normal collection creation. uassert(ErrorCodes::OperationNotSupportedInTransaction, str::stream() << "Cannot create a time-series collection in a multi-document transaction.", !opCtx->inMultiDocumentTransaction()); - return _createTimeseries(opCtx, ns, std::move(options)); + return _createTimeseries(opCtx, ns, options); } else { uassert(ErrorCodes::OperationNotSupportedInTransaction, str::stream() << "Cannot create system collection " << ns diff --git a/src/mongo/db/catalog/database_impl.cpp b/src/mongo/db/catalog/database_impl.cpp index dc3a991bfac..195ec4a8af9 100644 --- a/src/mongo/db/catalog/database_impl.cpp +++ b/src/mongo/db/catalog/database_impl.cpp @@ -622,8 +622,8 @@ Status DatabaseImpl::createView(OperationContext* opCtx, status = {ErrorCodes::InvalidNamespace, str::stream() << "invalid namespace name for a view: " + viewName.toString()}; } else { - status = ViewCatalog::createView( - opCtx, this, viewName, viewOnNss, pipeline, options.collation, options.timeseries); + status = + ViewCatalog::createView(opCtx, this, viewName, viewOnNss, pipeline, options.collation); } audit::logCreateView(&cc(), viewName, viewOnNss.toString(), pipeline, status.code()); diff --git a/src/mongo/db/commands/create_command.cpp b/src/mongo/db/commands/create_command.cpp index a5539ad816b..a1201e7a7e1 100644 --- a/src/mongo/db/commands/create_command.cpp +++ b/src/mongo/db/commands/create_command.cpp @@ -165,9 +165,14 @@ public: uassert( ErrorCodes::InvalidOptions, timeseriesNotAllowedWith("size"), !cmd.getSize()); uassert(ErrorCodes::InvalidOptions, timeseriesNotAllowedWith("max"), !cmd.getMax()); - uassert(ErrorCodes::InvalidOptions, - timeseriesNotAllowedWith("validator"), - !cmd.getValidator()); + + // The 'timeseries' option may be passed with a 'validator' if a buckets collection + // is being restored. We assume the caller knows what they are doing. + if (!cmd.getNamespace().isTimeseriesBucketsCollection()) { + uassert(ErrorCodes::InvalidOptions, + timeseriesNotAllowedWith("validator"), + !cmd.getValidator()); + } uassert(ErrorCodes::InvalidOptions, timeseriesNotAllowedWith("validationLevel"), !cmd.getValidationLevel()); diff --git a/src/mongo/db/commands/write_commands.cpp b/src/mongo/db/commands/write_commands.cpp index ae0f2d0298a..caf43d24d7f 100644 --- a/src/mongo/db/commands/write_commands.cpp +++ b/src/mongo/db/commands/write_commands.cpp @@ -62,6 +62,7 @@ #include "mongo/db/retryable_writes_stats.h" #include "mongo/db/stats/counters.h" #include "mongo/db/storage/duplicate_key_error_info.h" +#include "mongo/db/storage/durable_catalog.h" #include "mongo/db/timeseries/bucket_catalog.h" #include "mongo/db/transaction_participant.h" #include "mongo/db/views/view_catalog.h" @@ -102,20 +103,20 @@ bool shouldSkipOutput(OperationContext* opCtx) { } /** - * Returns true if 'ns' refers to a time-series collection. + * Returns true if 'ns' is a time-series collection. That is, this namespace is backed by a + * time-series buckets collection. */ bool isTimeseries(OperationContext* opCtx, const NamespaceString& ns) { - auto viewCatalog = DatabaseHolder::get(opCtx)->getViewCatalog(opCtx, ns.db()); - if (!viewCatalog) { - return false; - } - - auto view = viewCatalog->lookupWithoutValidatingDurableViews(opCtx, ns.ns()); - if (!view) { - return false; - } - - return view->timeseries().has_value(); + // If the buckets collection exists now, the time-series insert path will check for the + // existence of the buckets collection later on with a lock. + // If this check is concurrent with the creation of a time-series collection and the buckets + // collection does not yet exist, this check may return false unnecessarily. As a result, an + // insert attempt into the time-series namespace will either succeed or fail, depending on who + // wins the race. + auto bucketsNs = ns.makeTimeseriesBucketsNamespace(); + return CollectionCatalog::get(opCtx) + ->lookupCollectionByNamespaceForRead(opCtx, bucketsNs) + .get(); } // Default for control.version in time-series bucket collection. @@ -660,6 +661,18 @@ public: auto& bucketCatalog = BucketCatalog::get(opCtx); + auto bucketsNs = ns().makeTimeseriesBucketsNamespace(); + // Holding this shared pointer to the collection guarantees that the collator is not + // invalidated. + auto bucketsColl = + CollectionCatalog::get(opCtx)->lookupCollectionByNamespaceForRead(opCtx, bucketsNs); + uassert(ErrorCodes::NamespaceNotFound, + "Could not find time-series buckets collection for write", + bucketsColl); + uassert(ErrorCodes::InvalidOptions, + "Time-series buckets collection is missing time-series options", + bucketsColl->getTimeseriesOptions()); + std::vector<std::pair<std::shared_ptr<BucketCatalog::WriteBatch>, size_t>> batches; stdx::unordered_map<BucketCatalog::Bucket*, std::vector<StmtId>> bucketStmtIds; @@ -677,6 +690,8 @@ public: auto result = bucketCatalog.insert(opCtx, ns(), + bucketsColl->getDefaultCollator(), + *bucketsColl->getTimeseriesOptions(), request().getDocuments()[start + index], canCombineWithInsertsFromOtherClients(opCtx)); if (auto error = generateError(opCtx, result, index, errors->size())) { diff --git a/src/mongo/db/namespace_string.cpp b/src/mongo/db/namespace_string.cpp index 24a102a98ef..784e2327bed 100644 --- a/src/mongo/db/namespace_string.cpp +++ b/src/mongo/db/namespace_string.cpp @@ -320,11 +320,6 @@ NamespaceString NamespaceString::makeTimeseriesBucketsNamespace() const { return {db(), kTimeseriesBucketsCollectionPrefix.toString() + coll()}; } -NamespaceString NamespaceString::bucketsNamespaceToTimeseries() const { - invariant(isTimeseriesBucketsCollection()); - return {db(), coll().substr(kTimeseriesBucketsCollectionPrefix.size())}; -} - bool NamespaceString::isReplicated() const { if (isLocal()) { return false; diff --git a/src/mongo/db/namespace_string.h b/src/mongo/db/namespace_string.h index 66300dc14f5..4a32d0891f9 100644 --- a/src/mongo/db/namespace_string.h +++ b/src/mongo/db/namespace_string.h @@ -358,11 +358,6 @@ public: NamespaceString makeTimeseriesBucketsNamespace() const; /** - * Returns the time-series view namespace for this buckets namespace. - */ - NamespaceString bucketsNamespaceToTimeseries() const; - - /** * Returns whether a namespace is replicated, based only on its string value. One notable * omission is that map reduce `tmp.mr` collections may or may not be replicated. Callers must * decide how to handle that case separately. diff --git a/src/mongo/db/stats/storage_stats.cpp b/src/mongo/db/stats/storage_stats.cpp index dab60ce9e45..6fa79c4ed52 100644 --- a/src/mongo/db/stats/storage_stats.cpp +++ b/src/mongo/db/stats/storage_stats.cpp @@ -56,7 +56,7 @@ Status appendCollectionStorageStats(OperationContext* opCtx, bool isTimeseries = false; if (auto viewCatalog = DatabaseHolder::get(opCtx)->getViewCatalog(opCtx, nss.db())) { if (auto viewDef = viewCatalog->lookupWithoutValidatingDurableViews(opCtx, nss.ns())) { - isTimeseries = viewDef->timeseries().has_value(); + isTimeseries = viewDef->timeseries(); } } diff --git a/src/mongo/db/timeseries/SConscript b/src/mongo/db/timeseries/SConscript index be2bb1ff8d2..c2f1959a441 100644 --- a/src/mongo/db/timeseries/SConscript +++ b/src/mongo/db/timeseries/SConscript @@ -48,8 +48,7 @@ env.Library( 'timeseries_idl', ], LIBDEPS_PRIVATE=[ - '$BUILD_DIR/mongo/db/catalog/database_holder', - '$BUILD_DIR/mongo/db/views/views', + '$BUILD_DIR/mongo/db/catalog/collection_catalog', ], ) diff --git a/src/mongo/db/timeseries/bucket_catalog.cpp b/src/mongo/db/timeseries/bucket_catalog.cpp index 64f086efb2d..17fa905b8d9 100644 --- a/src/mongo/db/timeseries/bucket_catalog.cpp +++ b/src/mongo/db/timeseries/bucket_catalog.cpp @@ -100,13 +100,10 @@ BSONObj BucketCatalog::getMetadata(Bucket* ptr) const { StatusWith<std::shared_ptr<BucketCatalog::WriteBatch>> BucketCatalog::insert( OperationContext* opCtx, const NamespaceString& ns, + const StringData::ComparatorInterface* comparator, + const TimeseriesOptions& options, const BSONObj& doc, CombineWithInsertsFromOtherClients combine) { - auto viewCatalog = DatabaseHolder::get(opCtx)->getViewCatalog(opCtx, ns.db()); - invariant(viewCatalog); - auto viewDef = viewCatalog->lookup(opCtx, ns.ns()); - invariant(viewDef); - const auto& options = *viewDef->timeseries(); BSONObjBuilder metadata; if (auto metaField = options.getMetaField()) { @@ -116,7 +113,7 @@ StatusWith<std::shared_ptr<BucketCatalog::WriteBatch>> BucketCatalog::insert( metadata.appendNull(*metaField); } } - auto key = std::make_tuple(ns, BucketMetadata{metadata.obj(), viewDef}); + auto key = std::make_tuple(ns, BucketMetadata{metadata.obj(), comparator}); auto stats = _getExecutionStats(ns); invariant(stats); @@ -525,8 +522,8 @@ void BucketCatalog::_setIdTimestamp(Bucket* bucket, const Date_t& time) { } BucketCatalog::BucketMetadata::BucketMetadata(BSONObj&& obj, - std::shared_ptr<const ViewDefinition>& v) - : _metadata(obj), _view(v) { + const StringData::ComparatorInterface* comparator) + : _metadata(obj), _comparator(comparator) { BSONObjBuilder objBuilder; normalizeObject(&objBuilder, _metadata); _sorted = objBuilder.obj(); @@ -544,8 +541,8 @@ StringData BucketCatalog::BucketMetadata::getMetaField() const { return _metadata.firstElementFieldNameStringData(); } -const CollatorInterface* BucketCatalog::BucketMetadata::getCollator() const { - return _view->defaultCollator(); +const StringData::ComparatorInterface* BucketCatalog::BucketMetadata::getComparator() const { + return _comparator; } const OID& BucketCatalog::Bucket::id() const { @@ -1064,11 +1061,11 @@ void BucketCatalog::WriteBatch::_prepareCommit() { for (const auto& doc : _measurements) { _bucket->_min.update(doc, _bucket->_metadata.getMetaField(), - _bucket->_metadata.getCollator(), + _bucket->_metadata.getComparator(), std::less<>{}); _bucket->_max.update(doc, _bucket->_metadata.getMetaField(), - _bucket->_metadata.getCollator(), + _bucket->_metadata.getComparator(), std::greater<>{}); } _bucket->_memoryUsage += _bucket->_min.getMemoryUsage() + _bucket->_max.getMemoryUsage(); diff --git a/src/mongo/db/timeseries/bucket_catalog.h b/src/mongo/db/timeseries/bucket_catalog.h index 57c4b6f0cd1..b6fc129313f 100644 --- a/src/mongo/db/timeseries/bucket_catalog.h +++ b/src/mongo/db/timeseries/bucket_catalog.h @@ -180,10 +180,13 @@ public: * batch may commit or abort the batch after claiming commit rights. See WriteBatch for more * details. */ - StatusWith<std::shared_ptr<WriteBatch>> insert(OperationContext* opCtx, - const NamespaceString& ns, - const BSONObj& doc, - CombineWithInsertsFromOtherClients combine); + StatusWith<std::shared_ptr<WriteBatch>> insert( + OperationContext* opCtx, + const NamespaceString& ns, + const StringData::ComparatorInterface* comparator, + const TimeseriesOptions& options, + const BSONObj& doc, + CombineWithInsertsFromOtherClients combine); /** * Prepares a batch for commit, transitioning it to an inactive state. Caller must already have @@ -258,7 +261,7 @@ private: struct BucketMetadata { public: BucketMetadata() = default; - BucketMetadata(BSONObj&& obj, std::shared_ptr<const ViewDefinition>& view); + BucketMetadata(BSONObj&& obj, const StringData::ComparatorInterface* comparator); bool operator==(const BucketMetadata& other) const; @@ -266,7 +269,7 @@ private: StringData getMetaField() const; - const CollatorInterface* getCollator() const; + const StringData::ComparatorInterface* getComparator() const; template <typename H> friend H AbslHashValue(H h, const BucketMetadata& metadata) { @@ -277,7 +280,7 @@ private: private: BSONObj _metadata; - std::shared_ptr<const ViewDefinition> _view; + const StringData::ComparatorInterface* _comparator; // This stores the _metadata object with all fields sorted to allow for binary comparisons. BSONObj _sorted; diff --git a/src/mongo/db/timeseries/bucket_catalog_test.cpp b/src/mongo/db/timeseries/bucket_catalog_test.cpp index 0a7bdd5d8b6..5b0f398eebd 100644 --- a/src/mongo/db/timeseries/bucket_catalog_test.cpp +++ b/src/mongo/db/timeseries/bucket_catalog_test.cpp @@ -31,6 +31,7 @@ #include "mongo/db/catalog/catalog_test_fixture.h" #include "mongo/db/catalog/create_collection.h" +#include "mongo/db/catalog_raii.h" #include "mongo/db/timeseries/bucket_catalog.h" #include "mongo/db/views/view_catalog.h" #include "mongo/stdx/future.h" @@ -58,6 +59,9 @@ protected: void setUp() override; virtual BSONObj _makeTimeseriesOptionsForCreate() const; + TimeseriesOptions _getTimeseriesOptions(const NamespaceString& ns) const; + const CollatorInterface* _getCollator(const NamespaceString& ns) const; + void _commit(const std::shared_ptr<BucketCatalog::WriteBatch>& batch, uint16_t numPreviouslyCommittedMeasurements, size_t expectedBatchSize = 1); @@ -125,6 +129,16 @@ BSONObj BucketCatalogWithoutMetadataTest::_makeTimeseriesOptionsForCreate() cons return BSON("timeField" << _timeField); } +TimeseriesOptions BucketCatalogTest::_getTimeseriesOptions(const NamespaceString& ns) const { + AutoGetCollection autoColl(_opCtx, ns.makeTimeseriesBucketsNamespace(), MODE_IS); + return *autoColl->getTimeseriesOptions(); +} + +const CollatorInterface* BucketCatalogTest::_getCollator(const NamespaceString& ns) const { + AutoGetCollection autoColl(_opCtx, ns.makeTimeseriesBucketsNamespace(), MODE_IS); + return autoColl->getDefaultCollator(); +} + void BucketCatalogTest::_commit(const std::shared_ptr<BucketCatalog::WriteBatch>& batch, uint16_t numPreviouslyCommittedMeasurements, size_t expectedBatchSize) { @@ -140,6 +154,8 @@ void BucketCatalogTest::_insertOneAndCommit(const NamespaceString& ns, uint16_t numPreviouslyCommittedMeasurements) { auto result = _bucketCatalog->insert(_opCtx, ns, + _getCollator(ns), + _getTimeseriesOptions(ns), BSON(_timeField << Date_t::now()), BucketCatalog::CombineWithInsertsFromOtherClients::kAllow); auto& batch = result.getValue(); @@ -157,6 +173,8 @@ TEST_F(BucketCatalogTest, InsertIntoSameBucket) { auto result1 = _bucketCatalog->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now()), BucketCatalog::CombineWithInsertsFromOtherClients::kAllow); auto batch1 = result1.getValue(); @@ -168,6 +186,8 @@ TEST_F(BucketCatalogTest, InsertIntoSameBucket) { auto result2 = _bucketCatalog->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now()), BucketCatalog::CombineWithInsertsFromOtherClients::kAllow); auto batch2 = result2.getValue(); @@ -199,6 +219,8 @@ TEST_F(BucketCatalogTest, GetMetadataReturnsEmptyDocOnMissingBucket) { auto batch = _bucketCatalog ->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now()), BucketCatalog::CombineWithInsertsFromOtherClients::kAllow) .getValue(); @@ -212,16 +234,22 @@ TEST_F(BucketCatalogTest, InsertIntoDifferentBuckets) { auto result1 = _bucketCatalog->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now() << _metaField << "123"), BucketCatalog::CombineWithInsertsFromOtherClients::kAllow); auto result2 = _bucketCatalog->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now() << _metaField << BSONObj()), BucketCatalog::CombineWithInsertsFromOtherClients::kAllow); auto result3 = _bucketCatalog->insert(_opCtx, _ns2, + _getCollator(_ns2), + _getTimeseriesOptions(_ns2), BSON(_timeField << Date_t::now()), BucketCatalog::CombineWithInsertsFromOtherClients::kAllow); @@ -278,6 +306,8 @@ TEST_F(BucketCatalogTest, InsertBetweenPrepareAndFinish) { auto batch1 = _bucketCatalog ->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now()), BucketCatalog::CombineWithInsertsFromOtherClients::kAllow) .getValue(); @@ -290,6 +320,8 @@ TEST_F(BucketCatalogTest, InsertBetweenPrepareAndFinish) { auto batch2 = _bucketCatalog ->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now()), BucketCatalog::CombineWithInsertsFromOtherClients::kAllow) .getValue(); @@ -305,6 +337,8 @@ TEST_F(BucketCatalogTest, InsertBetweenPrepareAndFinish) { DEATH_TEST_F(BucketCatalogTest, CannotCommitWithoutRights, "invariant") { auto result = _bucketCatalog->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now()), BucketCatalog::CombineWithInsertsFromOtherClients::kAllow); auto& batch = result.getValue(); @@ -314,6 +348,8 @@ DEATH_TEST_F(BucketCatalogTest, CannotCommitWithoutRights, "invariant") { DEATH_TEST_F(BucketCatalogTest, CannotFinishUnpreparedBatch, "invariant") { auto result = _bucketCatalog->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now()), BucketCatalog::CombineWithInsertsFromOtherClients::kAllow); auto& batch = result.getValue(); @@ -325,6 +361,8 @@ TEST_F(BucketCatalogWithoutMetadataTest, GetMetadataReturnsEmptyDoc) { auto batch = _bucketCatalog ->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now()), BucketCatalog::CombineWithInsertsFromOtherClients::kAllow) .getValue(); @@ -338,6 +376,8 @@ TEST_F(BucketCatalogWithoutMetadataTest, CommitReturnsNewFields) { // Creating a new bucket should return all fields from the initial measurement. auto result = _bucketCatalog->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now() << "a" << 0), BucketCatalog::CombineWithInsertsFromOtherClients::kAllow); ASSERT_OK(result); @@ -352,6 +392,8 @@ TEST_F(BucketCatalogWithoutMetadataTest, CommitReturnsNewFields) { result = _bucketCatalog->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now() << "a" << 1), BucketCatalog::CombineWithInsertsFromOtherClients::kAllow); ASSERT_OK(result); @@ -362,6 +404,8 @@ TEST_F(BucketCatalogWithoutMetadataTest, CommitReturnsNewFields) { // Insert a new measurement with the a new field. result = _bucketCatalog->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now() << "a" << 2 << "b" << 2), BucketCatalog::CombineWithInsertsFromOtherClients::kAllow); ASSERT_OK(result); @@ -374,6 +418,8 @@ TEST_F(BucketCatalogWithoutMetadataTest, CommitReturnsNewFields) { for (auto i = 3; i < gTimeseriesBucketMaxCount; ++i) { result = _bucketCatalog->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now() << "a" << i), BucketCatalog::CombineWithInsertsFromOtherClients::kAllow); ASSERT_OK(result); @@ -387,6 +433,8 @@ TEST_F(BucketCatalogWithoutMetadataTest, CommitReturnsNewFields) { auto result2 = _bucketCatalog->insert( _opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now() << "a" << gTimeseriesBucketMaxCount), BucketCatalog::CombineWithInsertsFromOtherClients::kAllow); auto& batch2 = result2.getValue(); @@ -401,6 +449,8 @@ TEST_F(BucketCatalogTest, AbortBatchWithOutstandingInsertsOnBucket) { auto batch1 = _bucketCatalog ->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now()), BucketCatalog::CombineWithInsertsFromOtherClients::kAllow) .getValue(); @@ -413,6 +463,8 @@ TEST_F(BucketCatalogTest, AbortBatchWithOutstandingInsertsOnBucket) { auto batch2 = _bucketCatalog ->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now()), BucketCatalog::CombineWithInsertsFromOtherClients::kAllow) .getValue(); @@ -446,6 +498,8 @@ TEST_F(BucketCatalogTest, CombiningWithInsertsFromOtherClients) { auto batch1 = _bucketCatalog ->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now()), BucketCatalog::CombineWithInsertsFromOtherClients::kDisallow) .getValue(); @@ -454,6 +508,8 @@ TEST_F(BucketCatalogTest, CombiningWithInsertsFromOtherClients) { auto batch2 = _bucketCatalog ->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now()), BucketCatalog::CombineWithInsertsFromOtherClients::kDisallow) .getValue(); @@ -462,6 +518,8 @@ TEST_F(BucketCatalogTest, CombiningWithInsertsFromOtherClients) { auto batch3 = _bucketCatalog ->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now()), BucketCatalog::CombineWithInsertsFromOtherClients::kAllow) .getValue(); @@ -470,6 +528,8 @@ TEST_F(BucketCatalogTest, CombiningWithInsertsFromOtherClients) { auto batch4 = _bucketCatalog ->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now()), BucketCatalog::CombineWithInsertsFromOtherClients::kAllow) .getValue(); @@ -489,6 +549,8 @@ TEST_F(BucketCatalogTest, CannotConcurrentlyCommitBatchesForSameBucket) { auto batch1 = _bucketCatalog ->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now()), BucketCatalog::CombineWithInsertsFromOtherClients::kDisallow) .getValue(); @@ -497,6 +559,8 @@ TEST_F(BucketCatalogTest, CannotConcurrentlyCommitBatchesForSameBucket) { auto batch2 = _bucketCatalog ->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now()), BucketCatalog::CombineWithInsertsFromOtherClients::kDisallow) .getValue(); @@ -523,6 +587,8 @@ TEST_F(BucketCatalogTest, DuplicateNewFieldNamesAcrossConcurrentBatches) { auto batch1 = _bucketCatalog ->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now()), BucketCatalog::CombineWithInsertsFromOtherClients::kDisallow) .getValue(); @@ -531,6 +597,8 @@ TEST_F(BucketCatalogTest, DuplicateNewFieldNamesAcrossConcurrentBatches) { auto batch2 = _bucketCatalog ->insert(_opCtx, _ns1, + _getCollator(_ns1), + _getTimeseriesOptions(_ns1), BSON(_timeField << Date_t::now()), BucketCatalog::CombineWithInsertsFromOtherClients::kDisallow) .getValue(); diff --git a/src/mongo/db/timeseries/timeseries_lookup.cpp b/src/mongo/db/timeseries/timeseries_lookup.cpp index 20edf01c314..343d860a9bb 100644 --- a/src/mongo/db/timeseries/timeseries_lookup.cpp +++ b/src/mongo/db/timeseries/timeseries_lookup.cpp @@ -31,8 +31,7 @@ #include "mongo/db/timeseries/timeseries_lookup.h" -#include "mongo/db/catalog/database_holder.h" -#include "mongo/db/views/view_catalog.h" +#include "mongo/db/catalog/collection_catalog.h" namespace mongo { @@ -40,19 +39,13 @@ namespace timeseries { boost::optional<TimeseriesOptions> getTimeseriesOptions(OperationContext* opCtx, const NamespaceString& nss) { - auto viewCatalog = DatabaseHolder::get(opCtx)->getViewCatalog(opCtx, nss.db()); - if (!viewCatalog) { - return {}; + auto bucketsNs = nss.makeTimeseriesBucketsNamespace(); + auto bucketsColl = + CollectionCatalog::get(opCtx)->lookupCollectionByNamespaceForRead(opCtx, bucketsNs); + if (!bucketsColl) { + return boost::none; } - - auto view = viewCatalog->lookupWithoutValidatingDurableViews(opCtx, nss.ns()); - if (!view) { - return {}; - } - - // Return a copy of the time-series options so that we do not refer to the internal state of - // 'viewCatalog' after it goes out of scope. - return view->timeseries(); + return bucketsColl->getTimeseriesOptions(); } } // namespace timeseries diff --git a/src/mongo/db/ttl.cpp b/src/mongo/db/ttl.cpp index 52ab8485f1d..437a15265c9 100644 --- a/src/mongo/db/ttl.cpp +++ b/src/mongo/db/ttl.cpp @@ -298,17 +298,8 @@ private: Date_t safeExpirationDate(OperationContext* opCtx, const CollectionPtr& coll, std::int64_t expireAfterSeconds) const { - if (coll->ns().isTimeseriesBucketsCollection()) { - auto timeseriesNs = coll->ns().bucketsNamespaceToTimeseries(); - auto viewCatalog = DatabaseHolder::get(opCtx)->getViewCatalog(opCtx, timeseriesNs.db()); - invariant(viewCatalog); - auto viewDef = viewCatalog->lookup(opCtx, timeseriesNs.ns()); - uassert(ErrorCodes::NamespaceNotFound, - fmt::format("Could not find view definition for namespace: {}", - timeseriesNs.toString()), - viewDef); - - const auto bucketMaxSpan = Seconds(viewDef->timeseries()->getBucketMaxSpanSeconds()); + if (auto timeseries = coll->getTimeseriesOptions()) { + const auto bucketMaxSpan = Seconds(timeseries->getBucketMaxSpanSeconds()); // Don't delete data unless it is safely out of range of the bucket maximum time // range. On time-series collections, the _id (and thus RecordId) is the minimum diff --git a/src/mongo/db/views/SConscript b/src/mongo/db/views/SConscript index 00298b97541..5ca16d561ed 100644 --- a/src/mongo/db/views/SConscript +++ b/src/mongo/db/views/SConscript @@ -30,7 +30,6 @@ env.Library( '$BUILD_DIR/mongo/db/pipeline/aggregation', '$BUILD_DIR/mongo/db/query/collation/collator_factory_interface', '$BUILD_DIR/mongo/db/repl/repl_coordinator_interface', - '$BUILD_DIR/mongo/db/timeseries/timeseries_idl', 'resolved_view', ], LIBDEPS_PRIVATE=[ diff --git a/src/mongo/db/views/view.cpp b/src/mongo/db/views/view.cpp index 1f2f6d797bd..04987538658 100644 --- a/src/mongo/db/views/view.cpp +++ b/src/mongo/db/views/view.cpp @@ -41,12 +41,8 @@ ViewDefinition::ViewDefinition(StringData dbName, StringData viewName, StringData viewOnName, const BSONObj& pipeline, - std::unique_ptr<CollatorInterface> collator, - const boost::optional<TimeseriesOptions>& timeseries) - : _viewNss(dbName, viewName), - _viewOnNss(dbName, viewOnName), - _collator(std::move(collator)), - _timeseries(timeseries) { + std::unique_ptr<CollatorInterface> collator) + : _viewNss(dbName, viewName), _viewOnNss(dbName, viewOnName), _collator(std::move(collator)) { for (BSONElement e : pipeline) { _pipeline.push_back(e.Obj().getOwned()); } @@ -56,15 +52,13 @@ ViewDefinition::ViewDefinition(const ViewDefinition& other) : _viewNss(other._viewNss), _viewOnNss(other._viewOnNss), _collator(CollatorInterface::cloneCollator(other._collator.get())), - _pipeline(other._pipeline), - _timeseries(other._timeseries) {} + _pipeline(other._pipeline) {} ViewDefinition& ViewDefinition::operator=(const ViewDefinition& other) { _viewNss = other._viewNss; _viewOnNss = other._viewOnNss; _collator = CollatorInterface::cloneCollator(other._collator.get()); _pipeline = other._pipeline; - _timeseries = other._timeseries; return *this; } diff --git a/src/mongo/db/views/view.h b/src/mongo/db/views/view.h index 07edd41fddc..713e65e3f92 100644 --- a/src/mongo/db/views/view.h +++ b/src/mongo/db/views/view.h @@ -36,7 +36,6 @@ #include "mongo/bson/bsonobj.h" #include "mongo/db/namespace_string.h" #include "mongo/db/query/collation/collator_interface.h" -#include "mongo/db/timeseries/timeseries_gen.h" namespace mongo { @@ -53,8 +52,7 @@ public: StringData viewName, StringData viewOnName, const BSONObj& pipeline, - std::unique_ptr<CollatorInterface> collation, - const boost::optional<TimeseriesOptions>& timeseries); + std::unique_ptr<CollatorInterface> collation); /** * Copying a view 'other' clones its collator and does a simple copy of all other fields. @@ -93,10 +91,11 @@ public: } /** - * Returns the time-series options for the view, or boost::none if not a time-series view. + * Returns 'true' if this view is a time-series collection. That is, it is backed by a + * time-series buckets collection. */ - const boost::optional<TimeseriesOptions>& timeseries() const { - return _timeseries; + bool timeseries() const { + return _viewOnNss.isTimeseriesBucketsCollection(); } void setViewOn(const NamespaceString& viewOnNss); @@ -111,6 +110,5 @@ private: NamespaceString _viewOnNss; std::unique_ptr<CollatorInterface> _collator; std::vector<BSONObj> _pipeline; - boost::optional<TimeseriesOptions> _timeseries; }; } // namespace mongo diff --git a/src/mongo/db/views/view_catalog.cpp b/src/mongo/db/views/view_catalog.cpp index 0c49ff64041..6f3706c6aab 100644 --- a/src/mongo/db/views/view_catalog.cpp +++ b/src/mongo/db/views/view_catalog.cpp @@ -205,22 +205,11 @@ Status ViewCatalog::_reload(OperationContext* opCtx, ViewCatalogLookupBehavior l } } - boost::optional<TimeseriesOptions> timeseries; - if (view.hasField("timeseries")) { - try { - timeseries = - TimeseriesOptions::parse({"ViewCatalog::_reload"}, view["timeseries"].Obj()); - } catch (const DBException& ex) { - return ex.toStatus(); - } - } - _viewMap[viewName.ns()] = std::make_shared<ViewDefinition>(viewName.db(), viewName.coll(), view["viewOn"].str(), pipeline, - std::move(collator.getValue()), - timeseries); + std::move(collator.getValue())); return Status::OK(); }; @@ -290,8 +279,7 @@ Status ViewCatalog::_createOrUpdateView(OperationContext* opCtx, const NamespaceString& viewName, const NamespaceString& viewOn, const BSONArray& pipeline, - std::unique_ptr<CollatorInterface> collator, - const boost::optional<TimeseriesOptions>& timeseries) { + std::unique_ptr<CollatorInterface> collator) { invariant(opCtx->lockState()->isDbLockedForMode(viewName.db(), MODE_IX)); invariant(opCtx->lockState()->isCollectionLockedForMode(viewName, MODE_IX)); invariant(opCtx->lockState()->isCollectionLockedForMode( @@ -308,17 +296,10 @@ Status ViewCatalog::_createOrUpdateView(OperationContext* opCtx, if (collator) { viewDefBuilder.append("collation", collator->getSpec().toBSON()); } - if (timeseries) { - viewDefBuilder.append("timeseries", timeseries->toBSON()); - } BSONObj ownedPipeline = pipeline.getOwned(); - auto view = std::make_shared<ViewDefinition>(viewName.db(), - viewName.coll(), - viewOn.coll(), - ownedPipeline, - std::move(collator), - timeseries); + auto view = std::make_shared<ViewDefinition>( + viewName.db(), viewName.coll(), viewOn.coll(), ownedPipeline, std::move(collator)); // Check that the resulting dependency graph is acyclic and within the maximum depth. Status graphStatus = _upsertIntoGraph(opCtx, *(view.get())); @@ -511,8 +492,7 @@ Status ViewCatalog::createView(OperationContext* opCtx, const NamespaceString& viewName, const NamespaceString& viewOn, const BSONArray& pipeline, - const BSONObj& collation, - const boost::optional<TimeseriesOptions>& timeseries) { + const BSONObj& collation) { invariant(opCtx->lockState()->isDbLockedForMode(viewName.db(), MODE_IX)); invariant(opCtx->lockState()->isCollectionLockedForMode(viewName, MODE_IX)); invariant(opCtx->lockState()->isCollectionLockedForMode( @@ -543,7 +523,7 @@ Status ViewCatalog::createView(OperationContext* opCtx, catalogStorage.setIgnoreExternalChange(true); result = catalog.writable()->_createOrUpdateView( - opCtx, viewName, viewOn, pipeline, std::move(collator.getValue()), timeseries); + opCtx, viewName, viewOn, pipeline, std::move(collator.getValue())); } if (result.isOK()) { catalog.commit(); diff --git a/src/mongo/db/views/view_catalog.h b/src/mongo/db/views/view_catalog.h index a7752303bc0..9743f789a76 100644 --- a/src/mongo/db/views/view_catalog.h +++ b/src/mongo/db/views/view_catalog.h @@ -96,8 +96,7 @@ public: const NamespaceString& viewName, const NamespaceString& viewOn, const BSONArray& pipeline, - const BSONObj& collation, - const boost::optional<TimeseriesOptions>& timeseries); + const BSONObj& collation); /** * Drop the view named 'viewName'. @@ -176,8 +175,7 @@ private: const NamespaceString& viewName, const NamespaceString& viewOn, const BSONArray& pipeline, - std::unique_ptr<CollatorInterface> collator, - const boost::optional<TimeseriesOptions>& timeseries = boost::none); + std::unique_ptr<CollatorInterface> collator); /** * Parses the view definition pipeline, attempts to upsert into the view graph, and refreshes * the graph if necessary. Returns an error status if the resulting graph would be invalid. diff --git a/src/mongo/db/views/view_catalog_test.cpp b/src/mongo/db/views/view_catalog_test.cpp index 06c859cbcea..d7cc6c52e1b 100644 --- a/src/mongo/db/views/view_catalog_test.cpp +++ b/src/mongo/db/views/view_catalog_test.cpp @@ -121,8 +121,7 @@ public: MODE_X); WriteUnitOfWork wuow(opCtx); - Status s = - ViewCatalog::createView(opCtx, _db, viewName, viewOn, pipeline, collation, boost::none); + Status s = ViewCatalog::createView(opCtx, _db, viewName, viewOn, pipeline, collation); wuow.commit(); return s; @@ -531,13 +530,8 @@ TEST_F(ViewCatalogFixture, LookupRIDExistingViewRollback) { MODE_X); WriteUnitOfWork wunit(operationContext()); - ASSERT_OK(ViewCatalog::createView(operationContext(), - db(), - viewName, - viewOn, - emptyPipeline, - emptyCollation, - boost::none)); + ASSERT_OK(ViewCatalog::createView( + operationContext(), db(), viewName, viewOn, emptyPipeline, emptyCollation)); } auto resourceID = ResourceId(RESOURCE_COLLECTION, "db.view"_sd); auto collectionCatalog = CollectionCatalog::get(operationContext()); diff --git a/src/mongo/db/views/view_definition_test.cpp b/src/mongo/db/views/view_definition_test.cpp index 7ea401caee4..3971e39693d 100644 --- a/src/mongo/db/views/view_definition_test.cpp +++ b/src/mongo/db/views/view_definition_test.cpp @@ -46,12 +46,12 @@ namespace { const NamespaceString viewNss("testdb.testview"); const NamespaceString backingNss("testdb.testcoll"); +const NamespaceString bucketsColl("testdb.system.buckets.testcoll"); const BSONObj samplePipeline = BSON_ARRAY(BSON("limit" << 9)); -const TimeseriesOptions timeseries("time"); TEST(ViewDefinitionTest, ViewDefinitionCreationCorrectlyBuildsNamespaceStrings) { ViewDefinition viewDef( - viewNss.db(), viewNss.coll(), backingNss.coll(), samplePipeline, nullptr, boost::none); + viewNss.db(), viewNss.coll(), backingNss.coll(), samplePipeline, nullptr); ASSERT_EQ(viewDef.name(), viewNss); ASSERT_EQ(viewDef.viewOn(), backingNss); } @@ -59,12 +59,8 @@ TEST(ViewDefinitionTest, ViewDefinitionCreationCorrectlyBuildsNamespaceStrings) TEST(ViewDefinitionTest, CopyConstructorProperlyClonesAllFields) { auto collator = std::make_unique<CollatorInterfaceMock>(CollatorInterfaceMock::MockType::kReverseString); - ViewDefinition originalView(viewNss.db(), - viewNss.coll(), - backingNss.coll(), - samplePipeline, - std::move(collator), - timeseries); + ViewDefinition originalView( + viewNss.db(), viewNss.coll(), backingNss.coll(), samplePipeline, std::move(collator)); ViewDefinition copiedView(originalView); ASSERT_EQ(originalView.name(), copiedView.name()); @@ -75,18 +71,14 @@ TEST(ViewDefinitionTest, CopyConstructorProperlyClonesAllFields) { SimpleBSONObjComparator::kInstance.makeEqualTo())); ASSERT(CollatorInterface::collatorsMatch(originalView.defaultCollator(), copiedView.defaultCollator())); - ASSERT(originalView.timeseries()->toBSON().binaryEqual(copiedView.timeseries()->toBSON())); + ASSERT_EQ(originalView.timeseries(), copiedView.timeseries()); } TEST(ViewDefinitionTest, CopyAssignmentOperatorProperlyClonesAllFields) { auto collator = std::make_unique<CollatorInterfaceMock>(CollatorInterfaceMock::MockType::kReverseString); - ViewDefinition originalView(viewNss.db(), - viewNss.coll(), - backingNss.coll(), - samplePipeline, - std::move(collator), - timeseries); + ViewDefinition originalView( + viewNss.db(), viewNss.coll(), backingNss.coll(), samplePipeline, std::move(collator)); ViewDefinition copiedView = originalView; ASSERT_EQ(originalView.name(), copiedView.name()); @@ -97,21 +89,20 @@ TEST(ViewDefinitionTest, CopyAssignmentOperatorProperlyClonesAllFields) { SimpleBSONObjComparator::kInstance.makeEqualTo())); ASSERT(CollatorInterface::collatorsMatch(originalView.defaultCollator(), copiedView.defaultCollator())); - ASSERT(originalView.timeseries()->toBSON().binaryEqual(copiedView.timeseries()->toBSON())); } DEATH_TEST_REGEX(ViewDefinitionTest, SetViewOnFailsIfNewViewOnNotInSameDatabaseAsView, R"#(Invariant failure.*_viewNss.db\(\) == viewOnNss.db\(\))#") { ViewDefinition viewDef( - viewNss.db(), viewNss.coll(), backingNss.coll(), samplePipeline, nullptr, boost::none); + viewNss.db(), viewNss.coll(), backingNss.coll(), samplePipeline, nullptr); NamespaceString badViewOn("someOtherDb.someOtherCollection"); viewDef.setViewOn(badViewOn); } TEST(ViewDefinitionTest, SetViewOnSucceedsIfNewViewOnIsInSameDatabaseAsView) { ViewDefinition viewDef( - viewNss.db(), viewNss.coll(), backingNss.coll(), samplePipeline, nullptr, boost::none); + viewNss.db(), viewNss.coll(), backingNss.coll(), samplePipeline, nullptr); ASSERT_EQ(viewDef.viewOn(), backingNss); NamespaceString newViewOn("testdb.othercollection"); @@ -123,7 +114,7 @@ DEATH_TEST_REGEX(ViewDefinitionTest, SetPiplineFailsIfPipelineTypeIsNotArray, R"#(Invariant failure.*pipeline.type\(\) == Array)#") { ViewDefinition viewDef( - viewNss.db(), viewNss.coll(), backingNss.coll(), samplePipeline, nullptr, boost::none); + viewNss.db(), viewNss.coll(), backingNss.coll(), samplePipeline, nullptr); // We'll pass in a BSONElement that could be a valid array, but is BSONType::Object rather than // BSONType::Array. @@ -138,8 +129,7 @@ DEATH_TEST_REGEX(ViewDefinitionTest, } TEST(ViewDefinitionTest, SetPipelineSucceedsOnValidArrayBSONElement) { - ViewDefinition viewDef( - viewNss.db(), viewNss.coll(), backingNss.coll(), BSONObj(), nullptr, boost::none); + ViewDefinition viewDef(viewNss.db(), viewNss.coll(), backingNss.coll(), BSONObj(), nullptr); ASSERT(viewDef.pipeline().empty()); BSONObj matchStage = BSON("match" << BSON("x" << 9)); @@ -157,9 +147,8 @@ TEST(ViewDefinitionTest, SetPipelineSucceedsOnValidArrayBSONElement) { TEST(ViewDefinitionTest, ViewDefinitionCreationCorrectlySetsTimeseries) { ViewDefinition viewDef( - viewNss.db(), viewNss.coll(), backingNss.coll(), samplePipeline, nullptr, timeseries); + viewNss.db(), viewNss.coll(), bucketsColl.coll(), samplePipeline, nullptr); ASSERT(viewDef.timeseries()); - ASSERT_EQ(viewDef.timeseries()->getTimeField(), "time"); } } // namespace } // namespace mongo diff --git a/src/mongo/db/views/view_graph_test.cpp b/src/mongo/db/views/view_graph_test.cpp index 0dbdab653d4..5ace80ef15a 100644 --- a/src/mongo/db/views/view_graph_test.cpp +++ b/src/mongo/db/views/view_graph_test.cpp @@ -83,7 +83,7 @@ public: collator = std::move(factoryCollator.getValue()); } - return {db, view, viewOn, pipeline, std::move(collator), boost::none}; + return {db, view, viewOn, pipeline, std::move(collator)}; } private: |