diff --git a/Changelog.md b/Changelog.md index ded8ffd..1091598 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,8 @@ ## Unreleased +- Make exact-ID explain assertions accept MongoDB IDHACK as well as named index plans, while verifying the built-in index, bounded examined work and absence of collection scans. Add plan-layout regression coverage and a forced-scan negative control. Test-only change; no API, schema or MongoDB minimum-version change and no migration required. [#94](https://github.com/NEventStore/NEventStore.Persistence.MongoDB/issues/94) + - Atomically advance snapshot metadata without losing concurrent or delayed commit increments. Older snapshots cannot regress the recorded snapshot revision. Preserve configured write concerns; legacy heads without an unsnapshotted counter use the current head-minus-snapshot value as a fallback. Existing incorrect counters are not repaired. [#90](https://github.com/NEventStore/NEventStore.Persistence.MongoDB/issues/90) - Keep stream-head revisions monotonic when synchronous or asynchronous commit metadata writes complete out of order, while retaining every write's unsnapshotted event increment and existing snapshot revision. No API/schema breaking changes or migration are required; existing regressed metadata is not backfilled. Snapshot-update races (#90) and deletion/recreation races (#96) remain separate. See [stream-head invariants](docs/Stream-Heads.md). [#89](https://github.com/NEventStore/NEventStore.Persistence.MongoDB/issues/89) diff --git a/src/NEventStore.Persistence.MongoDB.Tests/AcceptanceTests/Issues/Issue73ExplainPlans.cs b/src/NEventStore.Persistence.MongoDB.Tests/AcceptanceTests/Issues/Issue73ExplainPlans.cs index f3624fe..dcf84cb 100644 --- a/src/NEventStore.Persistence.MongoDB.Tests/AcceptanceTests/Issues/Issue73ExplainPlans.cs +++ b/src/NEventStore.Persistence.MongoDB.Tests/AcceptanceTests/Issues/Issue73ExplainPlans.cs @@ -19,6 +19,57 @@ internal sealed class ExplainPlanResult public int TotalKeysExamined { get; set; } public int TotalDocsExamined { get; set; } + + public int Returned { get; set; } + + // MongoDB 4.2-7 can use the IDHACK fast path without an indexName. + // Named index access also handles classic, SBE and newer EXPRESS plans. + public bool UsesBuiltinIdIndex => Indexes.Contains("_id_") || Stages.Contains("IDHACK"); + + public static ExplainPlanResult From(BsonDocument explain) + { + var result = new ExplainPlanResult(); + WalkPlan(explain["queryPlanner"]["winningPlan"], result); + var executionStats = explain["executionStats"].AsBsonDocument; + result.TotalKeysExamined = executionStats["totalKeysExamined"].ToInt32(); + result.TotalDocsExamined = executionStats["totalDocsExamined"].ToInt32(); + result.Returned = executionStats["nReturned"].ToInt32(); + return result; + } + + private static void WalkPlan(BsonValue value, ExplainPlanResult result) + { + if (!value.IsBsonDocument) + { + return; + } + + var document = value.AsBsonDocument; + if (document.TryGetValue("stage", out BsonValue? stageValue)) + { + result.Stages.Add(stageValue.AsString); + } + + if (document.TryGetValue("indexName", out BsonValue? indexNameValue)) + { + result.Indexes.Add(indexNameValue.AsString); + } + + foreach (BsonElement element in document.Elements) + { + if (element.Value.IsBsonDocument) + { + WalkPlan(element.Value, result); + } + else if (element.Value.IsBsonArray) + { + foreach (BsonValue arrayItem in element.Value.AsBsonArray) + { + WalkPlan(arrayItem, result); + } + } + } + } } #if MSTEST @@ -111,13 +162,9 @@ protected override void Because() [MongoSnapshotFields.FullQualifiedBucketId] = "default", [MongoSnapshotFields.FullQualifiedStreamId] = "stream-1" }), - ["deleteStreamHeadById"] = ExplainFind( - _streamHeads!, - new BsonDocument(MongoStreamHeadFields.Id, new BsonDocument - { - [MongoStreamHeadFields.BucketId] = "default", - [MongoStreamHeadFields.StreamId] = "stream-1" - })), + ["deleteStreamHeadById"] = ExplainFind(_streamHeads!, StreamHeadIdFilter()), + ["deleteStreamHeadByIdWithoutIndex"] = ExplainFind( + _streamHeads!, StreamHeadIdFilter(), hint: new BsonDocument("$natural", 1)), ["purgeBucketSnapshotsFilter"] = ExplainFind( _snapshots!, new BsonDocument(MongoSnapshotFields.FullQualifiedBucketId, "default")), @@ -179,7 +226,29 @@ public void Snapshot_delete_filter_should_use_bucket_stream_revision_index() [Fact] public void Stream_head_delete_by_id_should_use_the_builtin_id_index() { - _plans!["deleteStreamHeadById"].Indexes.Should().Contain("_id_"); + using var indexes = _streamHeads!.Indexes.List(); + indexes.ToList().Should().Contain(index => index["name"] == "_id_" && + index["key"].AsBsonDocument.Equals(new BsonDocument(MongoStreamHeadFields.Id, 1))); + + var plan = _plans!["deleteStreamHeadById"]; + plan.UsesBuiltinIdIndex.Should().BeTrue("an exact embedded-_id lookup must use the built-in index"); + plan.Stages.Should().NotContain("COLLSCAN"); + plan.Returned.Should().Be(1); + plan.TotalKeysExamined.Should().Be(1); + plan.TotalDocsExamined.Should().Be(1); + } + + [Fact] + public void Forced_collection_scan_must_not_qualify_as_id_index_access() + { + // The same query and data with a natural hint prove the check detects + // lost index use, even while the built-in index still exists. + var plan = _plans!["deleteStreamHeadByIdWithoutIndex"]; + plan.UsesBuiltinIdIndex.Should().BeFalse(); + plan.Stages.Should().Contain("COLLSCAN"); + plan.Returned.Should().Be(1); + plan.TotalKeysExamined.Should().Be(0); + plan.TotalDocsExamined.Should().Be(3); } [Fact] @@ -273,11 +342,18 @@ private static BsonDocument CreateSnapshotDocument(string bucketId, string strea }; } + private static BsonDocument StreamHeadIdFilter() => new(MongoStreamHeadFields.Id, new BsonDocument + { + [MongoStreamHeadFields.BucketId] = "default", + [MongoStreamHeadFields.StreamId] = "stream-1" + }); + private ExplainPlanResult ExplainFind( IMongoCollection collection, BsonDocument filter, BsonDocument? sort = null, - int? limit = null) + int? limit = null, + BsonDocument? hint = null) { var explainCommand = new BsonDocument { @@ -299,48 +375,13 @@ private ExplainPlanResult ExplainFind( explainCommand["explain"].AsBsonDocument["limit"] = limit.Value; } - var explain = collection.Database.RunCommand(explainCommand); - var result = new ExplainPlanResult(); - WalkPlan(explain["queryPlanner"]["winningPlan"], result); - - var executionStats = explain["executionStats"].AsBsonDocument; - result.TotalKeysExamined = executionStats["totalKeysExamined"].ToInt32(); - result.TotalDocsExamined = executionStats["totalDocsExamined"].ToInt32(); - return result; - } - - private static void WalkPlan(BsonValue value, ExplainPlanResult result) - { - if (!value.IsBsonDocument) + if (hint != null) { - return; + explainCommand["explain"].AsBsonDocument["hint"] = hint; } - var document = value.AsBsonDocument; - if (document.TryGetValue("stage", out BsonValue? stageValue)) - { - result.Stages.Add(stageValue.AsString); - } - - if (document.TryGetValue("indexName", out BsonValue? indexNameValue)) - { - result.Indexes.Add(indexNameValue.AsString); - } - - foreach (BsonElement element in document.Elements) - { - if (element.Value.IsBsonDocument) - { - WalkPlan(element.Value, result); - } - else if (element.Value.IsBsonArray) - { - foreach (BsonValue arrayItem in element.Value.AsBsonArray) - { - WalkPlan(arrayItem, result); - } - } - } + var explain = collection.Database.RunCommand(explainCommand); + return ExplainPlanResult.From(explain); } } } \ No newline at end of file diff --git a/src/NEventStore.Persistence.MongoDB.Tests/AcceptanceTests/Issues/Issue94ExplainPlans.cs b/src/NEventStore.Persistence.MongoDB.Tests/AcceptanceTests/Issues/Issue94ExplainPlans.cs new file mode 100644 index 0000000..2a19ad6 --- /dev/null +++ b/src/NEventStore.Persistence.MongoDB.Tests/AcceptanceTests/Issues/Issue94ExplainPlans.cs @@ -0,0 +1,87 @@ +using FluentAssertions; +using global::MongoDB.Bson; +using NEventStore.Persistence.AcceptanceTests.BDD; + +#if MSTEST +using Microsoft.VisualStudio.TestTools.UnitTesting; +#endif + +namespace NEventStore.Persistence.MongoDB.Tests.AcceptanceTests.Issues +{ +#if MSTEST + [TestClass] +#endif + public class Issue_94_exact_id_explain_forms : SpecificationBase + { + private readonly Dictionary _plans = new(); + + protected override void Because() + { + // IDHACK omits indexName; classic, SBE and EXPRESS plans can expose it + // at different depths. These fixtures complement the live-server audit. + _plans["idhack"] = Parse("{ stage: 'IDHACK' }"); + _plans["classic"] = Parse("{ stage: 'FETCH', inputStage: { stage: 'IXSCAN', indexName: '_id_' } }"); + _plans["sbe"] = Parse("{ queryPlan: { stage: 'FETCH', inputStage: { stage: 'IXSCAN', indexName: '_id_' } } }"); + _plans["express"] = Parse("{ stage: 'EXPRESS_IXSCAN', indexName: '_id_' }"); + _plans["array"] = Parse("{ stage: 'SHARD_MERGE', shards: [{ winningPlan: { stage: 'IDHACK' } }] }"); + _plans["scan"] = Parse("{ stage: 'COLLSCAN' }"); + _plans["otherIndex"] = Parse("{ stage: 'IXSCAN', indexName: 'other_index' }"); + + var rejected = Explain("{ stage: 'COLLSCAN' }"); + rejected["queryPlanner"]["rejectedPlans"] = new BsonArray + { + BsonDocument.Parse("{ stage: 'IXSCAN', indexName: '_id_' }"), + BsonDocument.Parse("{ stage: 'IDHACK' }") + }; + _plans["rejected"] = ExplainPlanResult.From(rejected); + } + + [Fact] + public void Idhack_without_an_index_name_still_uses_the_id_index() + { + _plans["idhack"].Indexes.Should().BeEmpty(); + _plans["idhack"].UsesBuiltinIdIndex.Should().BeTrue(); + } + + [Fact] + public void Named_id_indexes_are_recognized_in_classic_sbe_and_express_plans() + { + foreach (var name in new[] { "classic", "sbe", "express" }) + _plans[name].UsesBuiltinIdIndex.Should().BeTrue(name); + } + + [Fact] + public void Plans_nested_in_arrays_are_recognized() + { + _plans["array"].UsesBuiltinIdIndex.Should().BeTrue(); + } + + [Fact] + public void Collection_scans_other_indexes_and_rejected_id_plans_are_not_id_access() + { + foreach (var name in new[] { "scan", "otherIndex", "rejected" }) + _plans[name].UsesBuiltinIdIndex.Should().BeFalse(name); + } + + [Fact] + public void Execution_work_and_result_count_are_preserved() + { + _plans["idhack"].TotalKeysExamined.Should().Be(1); + _plans["idhack"].TotalDocsExamined.Should().Be(1); + _plans["idhack"].Returned.Should().Be(1); + } + + private static ExplainPlanResult Parse(string winningPlan) => ExplainPlanResult.From(Explain(winningPlan)); + + private static BsonDocument Explain(string winningPlan) => new() + { + ["queryPlanner"] = new BsonDocument("winningPlan", BsonDocument.Parse(winningPlan)), + ["executionStats"] = new BsonDocument + { + ["totalKeysExamined"] = 1, + ["totalDocsExamined"] = 1, + ["nReturned"] = 1, + }, + }; + } +}