Skip to content

Commit 25e7f29

Browse files
authored
Fix $size error in statistics computation. (#139)
Commit b4467f7 broke statistics computation when a recheck generation includes a collection-metadata task, e.g., if an index or collection attribute mismatches. This fixes that by refining the aggregation logic. Tests are added to shore up this functionality.
1 parent 0686b70 commit 25e7f29

File tree

2 files changed

+121
-16
lines changed

2 files changed

+121
-16
lines changed

internal/verifier/migration_verifier_test.go

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,113 @@ func (suite *IntegrationTestSuite) TestVerifierFetchDocuments() {
567567
)
568568
}
569569

570+
func (suite *IntegrationTestSuite) TestGetPersistedNamespaceStatistics_Metadata() {
571+
ctx := suite.Context()
572+
verifier := suite.BuildVerifier()
573+
verifier.SetVerifyAll(true)
574+
575+
dbName := suite.DBNameForTest()
576+
577+
err := verifier.srcClient.Database(dbName).CreateCollection(
578+
ctx,
579+
"foo",
580+
)
581+
suite.Require().NoError(err)
582+
583+
runner := RunVerifierCheck(ctx, suite.T(), verifier)
584+
suite.Require().NoError(runner.AwaitGenerationEnd())
585+
586+
stats, err := verifier.GetPersistedNamespaceStatistics(ctx)
587+
suite.Require().NoError(err)
588+
589+
suite.Assert().Equal(
590+
mslices.Of(NamespaceStats{
591+
Namespace: dbName + ".foo",
592+
}),
593+
stats,
594+
"stats should be as expected",
595+
)
596+
597+
suite.Require().NoError(runner.StartNextGeneration())
598+
suite.Require().NoError(runner.AwaitGenerationEnd())
599+
600+
stats, err = verifier.GetPersistedNamespaceStatistics(ctx)
601+
suite.Require().NoError(err)
602+
603+
suite.Assert().Equal(
604+
mslices.Of(NamespaceStats{
605+
Namespace: dbName + ".foo",
606+
}),
607+
stats,
608+
"stats should be as expected",
609+
)
610+
}
611+
612+
func (suite *IntegrationTestSuite) TestGetPersistedNamespaceStatistics_OneDoc() {
613+
ctx := suite.Context()
614+
verifier := suite.BuildVerifier()
615+
verifier.SetVerifyAll(true)
616+
617+
bsonDoc := lo.Must(bson.Marshal(bson.D{{"_id", "foo"}}))
618+
619+
dbName := suite.DBNameForTest()
620+
_, err := verifier.srcClient.Database(dbName).Collection("foo").
621+
InsertOne(ctx, bsonDoc)
622+
suite.Require().NoError(err)
623+
624+
err = verifier.dstClient.Database(dbName).CreateCollection(
625+
ctx,
626+
"foo",
627+
)
628+
suite.Require().NoError(err)
629+
630+
runner := RunVerifierCheck(ctx, suite.T(), verifier)
631+
suite.Require().NoError(runner.AwaitGenerationEnd())
632+
633+
stats, err := verifier.GetPersistedNamespaceStatistics(ctx)
634+
suite.Require().NoError(err)
635+
636+
suite.Require().NotEmpty(stats)
637+
suite.Assert().NotZero(stats[0].BytesCompared, "bytes compared should be set")
638+
639+
suite.Assert().Equal(
640+
mslices.Of(NamespaceStats{
641+
Namespace: dbName + ".foo",
642+
DocsCompared: 1,
643+
TotalDocs: 1,
644+
BytesCompared: stats[0].BytesCompared,
645+
TotalBytes: types.ByteCount(len(bsonDoc)),
646+
PartitionsDone: 1,
647+
}),
648+
stats,
649+
"stats should be as expected",
650+
)
651+
652+
suite.Require().NoError(runner.StartNextGeneration())
653+
suite.Require().NoError(runner.AwaitGenerationEnd())
654+
655+
stats, err = verifier.GetPersistedNamespaceStatistics(ctx)
656+
suite.Require().NoError(err)
657+
658+
suite.Require().NotEmpty(stats)
659+
suite.Assert().NotZero(stats[0].BytesCompared, "bytes compared should be set")
660+
661+
suite.Assert().Equal(
662+
mslices.Of(NamespaceStats{
663+
Namespace: dbName + ".foo",
664+
DocsCompared: 1,
665+
TotalDocs: 1,
666+
BytesCompared: stats[0].BytesCompared,
667+
PartitionsDone: 1,
668+
669+
// NB: TotalBytes is 0 because we can’t compute that from the
670+
// change stream.
671+
}),
672+
stats,
673+
"stats should be as expected",
674+
)
675+
}
676+
570677
func (suite *IntegrationTestSuite) TestGetPersistedNamespaceStatistics_Recheck() {
571678
ctx := suite.Context()
572679
verifier := suite.BuildVerifier()

internal/verifier/statistics.go

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -114,28 +114,26 @@ const perNsStatsPipelineTemplate = `[
114114
In generation 0 we can get the total docs from the
115115
verify-collection tasks.
116116
117-
In later generations we don’t have verify-collection tasks,
117+
In later generations we may not have verify-collection tasks,
118118
so we add up the individual recheck batch tasks. Note that,
119119
in these tasks source_documents_count refers to the actual
120120
number of docs found on the source, not all the documents
121121
checked.
122122
*/}}
123123
"totalDocs": {
124-
"$cond": [
125-
{ "$or": [
126-
{ "$eq": [ "$type", "{{.VerifyCollType}}" ] },
127-
{ "$and": [
128-
{ "$eq": [ "$type", "{{.VerifyDocsType}}" ] },
129-
{ "$ne": [ "$generation", 0 ] }
130-
] }
131-
] },
132-
{ "$cond": [
133-
{"$eq": [ "$generation", 0 ]},
134-
"$source_documents_count",
135-
{ "$size": "$_ids" }
136-
] },
137-
0
138-
]
124+
"$cond": {
125+
"if": {"$eq": [ "$generation", 0 ]},
126+
"then": { "$cond": {
127+
"if": { "$eq": [ "$type", "{{.VerifyCollType}}" ] },
128+
"then": "$source_documents_count",
129+
"else": 0
130+
} },
131+
"else": { "$cond": {
132+
"if": { "$eq": [ "$type", "{{.VerifyDocsType}}" ] },
133+
"then": { "$size": "$_ids" },
134+
"else": 0
135+
} }
136+
}
139137
},
140138
141139
{{/*

0 commit comments

Comments
 (0)