Skip to content

Commit bfc46eb

Browse files
authored
REP-5318 Hide updateDescription in update events. (#57)
This prevents one means of triggering a BSONObjectTooLarge error. Such errors could still happen from over-large `_id` fields, but users are unlikely to have them. Found while running migration-verifier’s change stream against mongosync’s nondeterministic tests.
1 parent 5fc7a04 commit bfc46eb

File tree

3 files changed

+105
-20
lines changed

3 files changed

+105
-20
lines changed

internal/verifier/change_stream.go

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -116,17 +116,38 @@ func (verifier *Verifier) HandleChangeStreamEvents(ctx context.Context, batch []
116116
// and omit fullDocument, but $bsonSize was new in MongoDB 4.4, and we still
117117
// want to verify migrations from 4.2. fullDocument is unlikely to be a
118118
// bottleneck anyway.
119-
func (verifier *Verifier) GetChangeStreamFilter() []bson.D {
119+
func (verifier *Verifier) GetChangeStreamFilter() (pipeline mongo.Pipeline) {
120+
120121
if len(verifier.srcNamespaces) == 0 {
121-
return []bson.D{{bson.E{"$match", bson.D{{"ns.db", bson.D{{"$ne", verifier.metaDBName}}}}}}}
122-
}
123-
filter := bson.A{}
124-
for _, ns := range verifier.srcNamespaces {
125-
db, coll := SplitNamespace(ns)
126-
filter = append(filter, bson.D{{"ns", bson.D{{"db", db}, {"coll", coll}}}})
122+
pipeline = mongo.Pipeline{
123+
{{"$match", bson.D{
124+
{"ns.db", bson.D{{"$ne", verifier.metaDBName}}},
125+
}}},
126+
}
127+
} else {
128+
filter := []bson.D{}
129+
for _, ns := range verifier.srcNamespaces {
130+
db, coll := SplitNamespace(ns)
131+
filter = append(filter, bson.D{
132+
{"ns", bson.D{
133+
{"db", db},
134+
{"coll", coll},
135+
}},
136+
})
137+
}
138+
pipeline = mongo.Pipeline{
139+
{{"$match", bson.D{{"$or", filter}}}},
140+
}
127141
}
128-
stage := bson.D{{"$match", bson.D{{"$or", filter}}}}
129-
return []bson.D{stage}
142+
143+
return append(
144+
pipeline,
145+
bson.D{
146+
{"$unset", []string{
147+
"updateDescription",
148+
}},
149+
},
150+
)
130151
}
131152

132153
// This function reads a single `getMore` response into a slice.

internal/verifier/change_stream_test.go

Lines changed: 74 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,16 @@ package verifier
22

33
import (
44
"context"
5+
"strings"
56
"testing"
67
"time"
78

9+
"github.com/10gen/migration-verifier/internal/testutil"
810
"github.com/10gen/migration-verifier/internal/util"
911
"github.com/10gen/migration-verifier/mslices"
1012
"github.com/pkg/errors"
1113
"github.com/samber/lo"
14+
"github.com/stretchr/testify/assert"
1215
"github.com/stretchr/testify/require"
1316
"go.mongodb.org/mongo-driver/bson"
1417
"go.mongodb.org/mongo-driver/bson/primitive"
@@ -20,19 +23,24 @@ import (
2023
func TestChangeStreamFilter(t *testing.T) {
2124
verifier := Verifier{}
2225
verifier.SetMetaDBName("metadb")
23-
require.Equal(t, []bson.D{{{"$match", bson.D{{"ns.db", bson.D{{"$ne", "metadb"}}}}}}},
24-
verifier.GetChangeStreamFilter())
26+
assert.Contains(t,
27+
verifier.GetChangeStreamFilter(),
28+
bson.D{
29+
{"$match", bson.D{{"ns.db", bson.D{{"$ne", "metadb"}}}}},
30+
},
31+
)
2532
verifier.srcNamespaces = []string{"foo.bar", "foo.baz", "test.car", "test.chaz"}
26-
require.Equal(t, []bson.D{
27-
{{"$match", bson.D{
28-
{"$or", bson.A{
29-
bson.D{{"ns", bson.D{{"db", "foo"}, {"coll", "bar"}}}},
30-
bson.D{{"ns", bson.D{{"db", "foo"}, {"coll", "baz"}}}},
31-
bson.D{{"ns", bson.D{{"db", "test"}, {"coll", "car"}}}},
32-
bson.D{{"ns", bson.D{{"db", "test"}, {"coll", "chaz"}}}},
33+
assert.Contains(t,
34+
verifier.GetChangeStreamFilter(),
35+
bson.D{{"$match", bson.D{
36+
{"$or", []bson.D{
37+
{{"ns", bson.D{{"db", "foo"}, {"coll", "bar"}}}},
38+
{{"ns", bson.D{{"db", "foo"}, {"coll", "baz"}}}},
39+
{{"ns", bson.D{{"db", "test"}, {"coll", "car"}}}},
40+
{{"ns", bson.D{{"db", "test"}, {"coll", "chaz"}}}},
3341
}},
3442
}}},
35-
}, verifier.GetChangeStreamFilter())
43+
)
3644
}
3745

3846
// TestChangeStreamResumability creates a verifier, starts its change stream,
@@ -445,3 +453,59 @@ func (suite *IntegrationTestSuite) TestCreateForbidden() {
445453
suite.Require().ErrorAs(err, &eventErr)
446454
suite.Assert().Equal("create", eventErr.Event.OpType)
447455
}
456+
457+
func (suite *IntegrationTestSuite) TestLargeEvents() {
458+
ctx := suite.Context()
459+
460+
docID := 123
461+
462+
makeDoc := func(char string, len int) bson.D {
463+
return bson.D{{"_id", docID}, {"str", strings.Repeat(char, len)}}
464+
}
465+
466+
smallDoc := testutil.MustMarshal(makeDoc("a", 1))
467+
suite.T().Logf("small size: %v", len(smallDoc))
468+
maxBSONSize := 16 * 1024 * 1024
469+
470+
maxStringLen := maxBSONSize - len(smallDoc) - 1
471+
472+
db := suite.srcMongoClient.Database(suite.DBNameForTest())
473+
suite.Require().NoError(db.CreateCollection(ctx, "mystuff"))
474+
475+
verifier := suite.BuildVerifier()
476+
verifierRunner := RunVerifierCheck(suite.Context(), suite.T(), verifier)
477+
suite.Require().NoError(verifierRunner.AwaitGenerationEnd())
478+
479+
coll := db.Collection("mystuff")
480+
_, err := coll.InsertOne(
481+
ctx,
482+
makeDoc("a", maxStringLen),
483+
)
484+
suite.Require().NoError(err, "should insert")
485+
486+
updated, err := coll.UpdateByID(
487+
ctx,
488+
docID,
489+
bson.D{
490+
{"$set", bson.D{
491+
// smallDoc happens to be the minimum length to subtract
492+
// in order to satisfy the server’s requirements on
493+
// document sizes in updates.
494+
{"str", strings.Repeat("b", maxStringLen-len(smallDoc))},
495+
}},
496+
},
497+
)
498+
suite.Require().NoError(err, "should update")
499+
suite.Require().EqualValues(1, updated.ModifiedCount)
500+
501+
replaced, err := coll.ReplaceOne(
502+
ctx,
503+
bson.D{{"_id", docID}},
504+
makeDoc("c", maxStringLen-len(smallDoc)),
505+
)
506+
suite.Require().NoError(err, "should replace")
507+
suite.Require().EqualValues(1, replaced.ModifiedCount)
508+
509+
suite.Require().NoError(verifier.WritesOff(ctx))
510+
suite.Require().NoError(verifierRunner.Await())
511+
}

internal/verifier/migration_verifier_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1536,7 +1536,7 @@ func (suite *IntegrationTestSuite) TestBackgroundInIndexSpec() {
15361536
verifier.SetNamespaceMap()
15371537

15381538
runner := RunVerifierCheck(ctx, suite.T(), verifier)
1539-
runner.AwaitGenerationEnd()
1539+
suite.Require().NoError(runner.AwaitGenerationEnd())
15401540

15411541
status, err := verifier.GetVerificationStatus()
15421542
suite.Require().NoError(err)

0 commit comments

Comments
 (0)