From c9dd01fbc431f9d9a6266dcedade20d14d43c57e Mon Sep 17 00:00:00 2001 From: Qingyang Hu Date: Fri, 16 Feb 2024 14:15:09 -0500 Subject: [PATCH 1/2] GODRIVER-3043 Use default write/read concerns in the index search commands. --- Makefile | 2 +- mongo/collection.go | 5 +- mongo/integration/search_index_prose_test.go | 115 ++++++++++++++---- mongo/search_index_view.go | 40 ++---- .../searchIndexIgnoresReadWriteConcern.json | 2 +- .../searchIndexIgnoresReadWriteConcern.yml | 4 +- .../driver/operation/create_search_indexes.go | 42 +++---- x/mongo/driver/operation/drop_search_index.go | 37 ++---- .../driver/operation/update_search_index.go | 39 ++---- 9 files changed, 147 insertions(+), 139 deletions(-) diff --git a/Makefile b/Makefile index 2c21ef0fa9..7543ff6053 100644 --- a/Makefile +++ b/Makefile @@ -158,7 +158,7 @@ evg-test-load-balancers: .PHONY: evg-test-search-index evg-test-search-index: - go test ./mongo/integration -run TestSearchIndexProse -v -timeout $(TEST_TIMEOUT)s >> test.suite + go test ./mongo/integration -run TestSearchIndexProse -v -timeout 3600s >> test.suite .PHONY: evg-test-ocsp evg-test-ocsp: diff --git a/mongo/collection.go b/mongo/collection.go index c7b2a8a113..58692aa983 100644 --- a/mongo/collection.go +++ b/mongo/collection.go @@ -1775,8 +1775,11 @@ func (coll *Collection) Indexes() IndexView { // SearchIndexes returns a SearchIndexView instance that can be used to perform operations on the search indexes for the collection. func (coll *Collection) SearchIndexes() SearchIndexView { + c, _ := coll.Clone() // Clone() always return a nil error. + c.readConcern = nil + c.writeConcern = nil return SearchIndexView{ - coll: coll, + coll: c, } } diff --git a/mongo/integration/search_index_prose_test.go b/mongo/integration/search_index_prose_test.go index 002150c36e..5f13746f58 100644 --- a/mongo/integration/search_index_prose_test.go +++ b/mongo/integration/search_index_prose_test.go @@ -7,6 +7,7 @@ package integration import ( + "bytes" "context" "os" "sync" @@ -20,6 +21,8 @@ import ( "go.mongodb.org/mongo-driver/mongo" "go.mongodb.org/mongo-driver/mongo/integration/mtest" "go.mongodb.org/mongo-driver/mongo/options" + "go.mongodb.org/mongo-driver/mongo/readconcern" + "go.mongodb.org/mongo-driver/mongo/writeconcern" ) func TestSearchIndexProse(t *testing.T) { @@ -61,7 +64,9 @@ func TestSearchIndexProse(t *testing.T) { if !cursor.Next(ctx) { break } - if cursor.Current.Lookup("queryable").Boolean() { + name := cursor.Current.Lookup("name").StringValue() + queryable := cursor.Current.Lookup("queryable").Boolean() + if name == searchName && queryable { doc = cursor.Current } else { t.Logf("cursor: %s, sleep 5 seconds...", cursor.Current.String()) @@ -69,7 +74,6 @@ func TestSearchIndexProse(t *testing.T) { } } require.NotNil(mt, doc, "got empty document") - assert.Equal(mt, searchName, doc.Lookup("name").StringValue(), "unmatched name") expected, err := bson.Marshal(definition) require.NoError(mt, err, "failed to marshal definition") actual := doc.Lookup("latestDefinition").Value @@ -110,7 +114,9 @@ func TestSearchIndexProse(t *testing.T) { if !cursor.Next(ctx) { return nil } - if cursor.Current.Lookup("queryable").Boolean() { + name := cursor.Current.Lookup("name").StringValue() + queryable := cursor.Current.Lookup("queryable").Boolean() + if name == *opts.Name && queryable { return cursor.Current } t.Logf("cursor: %s, sleep 5 seconds...", cursor.Current.String()) @@ -126,7 +132,6 @@ func TestSearchIndexProse(t *testing.T) { doc := getDocument(opts) require.NotNil(mt, doc, "got empty document") - assert.Equal(mt, *opts.Name, doc.Lookup("name").StringValue(), "unmatched name") expected, err := bson.Marshal(definition) require.NoError(mt, err, "failed to marshal definition") actual := doc.Lookup("latestDefinition").Value @@ -162,7 +167,9 @@ func TestSearchIndexProse(t *testing.T) { if !cursor.Next(ctx) { break } - if cursor.Current.Lookup("queryable").Boolean() { + name := cursor.Current.Lookup("name").StringValue() + queryable := cursor.Current.Lookup("queryable").Boolean() + if name == searchName && queryable { doc = cursor.Current } else { t.Logf("cursor: %s, sleep 5 seconds...", cursor.Current.String()) @@ -170,7 +177,6 @@ func TestSearchIndexProse(t *testing.T) { } } require.NotNil(mt, doc, "got empty document") - require.Equal(mt, searchName, doc.Lookup("name").StringValue(), "unmatched name") err = view.DropOne(ctx, searchName) require.NoError(mt, err, "failed to drop index") @@ -204,37 +210,49 @@ func TestSearchIndexProse(t *testing.T) { require.NoError(mt, err, "failed to create index") require.Equal(mt, searchName, index, "unmatched name") - getDocument := func() bson.Raw { - for { - cursor, err := view.List(ctx, opts) - require.NoError(mt, err, "failed to list") + var doc bson.Raw + for doc == nil { + cursor, err := view.List(ctx, opts) + require.NoError(mt, err, "failed to list") - if !cursor.Next(ctx) { - return nil - } - if cursor.Current.Lookup("queryable").Boolean() { - return cursor.Current - } + if !cursor.Next(ctx) { + break + } + name := cursor.Current.Lookup("name").StringValue() + queryable := cursor.Current.Lookup("queryable").Boolean() + if name == searchName && queryable { + doc = cursor.Current + } else { t.Logf("cursor: %s, sleep 5 seconds...", cursor.Current.String()) time.Sleep(5 * time.Second) } } - - doc := getDocument() require.NotNil(mt, doc, "got empty document") - require.Equal(mt, searchName, doc.Lookup("name").StringValue(), "unmatched name") definition = bson.D{{"mappings", bson.D{{"dynamic", true}}}} - err = view.UpdateOne(ctx, searchName, definition) - require.NoError(mt, err, "failed to drop index") - doc = getDocument() - require.NotNil(mt, doc, "got empty document") - assert.Equal(mt, searchName, doc.Lookup("name").StringValue(), "unmatched name") - assert.Equal(mt, "READY", doc.Lookup("status").StringValue(), "unexpected status") expected, err := bson.Marshal(definition) require.NoError(mt, err, "failed to marshal definition") - actual := doc.Lookup("latestDefinition").Value - assert.Equal(mt, expected, actual, "unmatched definition") + err = view.UpdateOne(ctx, searchName, definition) + require.NoError(mt, err, "failed to update index") + for doc == nil { + cursor, err := view.List(ctx, opts) + require.NoError(mt, err, "failed to list") + + if !cursor.Next(ctx) { + break + } + name := cursor.Current.Lookup("name").StringValue() + queryable := cursor.Current.Lookup("queryable").Boolean() + status := cursor.Current.Lookup("status").StringValue() + latestDefinition := doc.Lookup("latestDefinition").Value + if name == searchName && queryable && status == "READY" && bytes.Equal(latestDefinition, expected) { + doc = cursor.Current + } else { + t.Logf("cursor: %s, sleep 5 seconds...", cursor.Current.String()) + time.Sleep(5 * time.Second) + } + } + require.NotNil(mt, doc, "got empty document") }) mt.Run("case 5: dropSearchIndex suppresses namespace not found errors", func(mt *mtest.T) { @@ -250,4 +268,47 @@ func TestSearchIndexProse(t *testing.T) { err = collection.SearchIndexes().DropOne(ctx, "foo") require.NoError(mt, err) }) + + mt.RunOpts("case 6: Driver can successfully create and list search indexes with non-default readConcern and writeConcern", + mtest.NewOptions().CollectionOptions(options.Collection().SetWriteConcern(writeconcern.New(writeconcern.W(1))).SetReadConcern(readconcern.Majority())), + func(mt *mtest.T) { + ctx := context.Background() + + _, err := mt.Coll.InsertOne(ctx, bson.D{}) + require.NoError(mt, err, "failed to insert") + + view := mt.Coll.SearchIndexes() + + definition := bson.D{{"mappings", bson.D{{"dynamic", false}}}} + searchName := "test-search-index6" + opts := options.SearchIndexes().SetName(searchName) + index, err := view.CreateOne(ctx, mongo.SearchIndexModel{ + Definition: definition, + Options: opts, + }) + require.NoError(mt, err, "failed to create index") + require.Equal(mt, searchName, index, "unmatched name") + var doc bson.Raw + for doc == nil { + cursor, err := view.List(ctx, opts) + require.NoError(mt, err, "failed to list") + + if !cursor.Next(ctx) { + break + } + name := cursor.Current.Lookup("name").StringValue() + queryable := cursor.Current.Lookup("queryable").Boolean() + if name == searchName && queryable { + doc = cursor.Current + } else { + t.Logf("cursor: %s, sleep 5 seconds...", cursor.Current.String()) + time.Sleep(5 * time.Second) + } + } + require.NotNil(mt, doc, "got empty document") + expected, err := bson.Marshal(definition) + require.NoError(mt, err, "failed to marshal definition") + actual := doc.Lookup("latestDefinition").Value + assert.Equal(mt, expected, actual, "unmatched definition") + }) } diff --git a/mongo/search_index_view.go b/mongo/search_index_view.go index 6a7871531e..695a396425 100644 --- a/mongo/search_index_view.go +++ b/mongo/search_index_view.go @@ -13,7 +13,6 @@ import ( "go.mongodb.org/mongo-driver/bson" "go.mongodb.org/mongo-driver/mongo/options" - "go.mongodb.org/mongo-driver/mongo/writeconcern" "go.mongodb.org/mongo-driver/x/bsonx/bsoncore" "go.mongodb.org/mongo-driver/x/mongo/driver" "go.mongodb.org/mongo-driver/x/mongo/driver/operation" @@ -134,20 +133,13 @@ func (siv SearchIndexView) CreateMany( return nil, err } - wc := siv.coll.writeConcern - if sess.TransactionRunning() { - wc = nil - } - if !writeconcern.AckWrite(wc) { - sess = nil - } - selector := makePinnedSelector(sess, siv.coll.writeSelector) op := operation.NewCreateSearchIndexes(indexes). - Session(sess).WriteConcern(wc).ClusterClock(siv.coll.client.clock). - Database(siv.coll.db.name).Collection(siv.coll.name).CommandMonitor(siv.coll.client.monitor). - Deployment(siv.coll.client.deployment).ServerSelector(selector).ServerAPI(siv.coll.client.serverAPI). + Session(sess).CommandMonitor(siv.coll.client.monitor). + ServerSelector(selector).ClusterClock(siv.coll.client.clock). + Collection(siv.coll.name).Database(siv.coll.db.name). + Deployment(siv.coll.client.deployment).ServerAPI(siv.coll.client.serverAPI). Timeout(siv.coll.client.timeout) err = op.Execute(ctx) @@ -196,20 +188,12 @@ func (siv SearchIndexView) DropOne( return err } - wc := siv.coll.writeConcern - if sess.TransactionRunning() { - wc = nil - } - if !writeconcern.AckWrite(wc) { - sess = nil - } - selector := makePinnedSelector(sess, siv.coll.writeSelector) op := operation.NewDropSearchIndex(name). - Session(sess).WriteConcern(wc).CommandMonitor(siv.coll.client.monitor). + Session(sess).CommandMonitor(siv.coll.client.monitor). ServerSelector(selector).ClusterClock(siv.coll.client.clock). - Database(siv.coll.db.name).Collection(siv.coll.name). + Collection(siv.coll.name).Database(siv.coll.db.name). Deployment(siv.coll.client.deployment).ServerAPI(siv.coll.client.serverAPI). Timeout(siv.coll.client.timeout) @@ -258,20 +242,12 @@ func (siv SearchIndexView) UpdateOne( return err } - wc := siv.coll.writeConcern - if sess.TransactionRunning() { - wc = nil - } - if !writeconcern.AckWrite(wc) { - sess = nil - } - selector := makePinnedSelector(sess, siv.coll.writeSelector) op := operation.NewUpdateSearchIndex(name, indexDefinition). - Session(sess).WriteConcern(wc).CommandMonitor(siv.coll.client.monitor). + Session(sess).CommandMonitor(siv.coll.client.monitor). ServerSelector(selector).ClusterClock(siv.coll.client.clock). - Database(siv.coll.db.name).Collection(siv.coll.name). + Collection(siv.coll.name).Database(siv.coll.db.name). Deployment(siv.coll.client.deployment).ServerAPI(siv.coll.client.serverAPI). Timeout(siv.coll.client.timeout) diff --git a/testdata/index-management/searchIndexIgnoresReadWriteConcern.json b/testdata/index-management/searchIndexIgnoresReadWriteConcern.json index 47b4ccfa79..edf71b7b7e 100644 --- a/testdata/index-management/searchIndexIgnoresReadWriteConcern.json +++ b/testdata/index-management/searchIndexIgnoresReadWriteConcern.json @@ -95,7 +95,7 @@ ] }, { - "description": "createSearchIndex ignores read and write concern", + "description": "createSearchIndexes ignores read and write concern", "operations": [ { "name": "createSearchIndexes", diff --git a/testdata/index-management/searchIndexIgnoresReadWriteConcern.yml b/testdata/index-management/searchIndexIgnoresReadWriteConcern.yml index 64d06d7924..73fef27de5 100644 --- a/testdata/index-management/searchIndexIgnoresReadWriteConcern.yml +++ b/testdata/index-management/searchIndexIgnoresReadWriteConcern.yml @@ -49,11 +49,11 @@ tests: writeConcern: { $$exists: false } readConcern: { $$exists: false } - - description: "createSearchIndex ignores read and write concern" + - description: "createSearchIndexes ignores read and write concern" operations: - name: createSearchIndexes object: *collection0 - arguments: + arguments: models: [] expectError: # This test always errors in a non-Atlas environment. The test functions as a unit test by asserting diff --git a/x/mongo/driver/operation/create_search_indexes.go b/x/mongo/driver/operation/create_search_indexes.go index a16f9d716b..cb0d807952 100644 --- a/x/mongo/driver/operation/create_search_indexes.go +++ b/x/mongo/driver/operation/create_search_indexes.go @@ -15,7 +15,6 @@ import ( "go.mongodb.org/mongo-driver/bson" "go.mongodb.org/mongo-driver/event" "go.mongodb.org/mongo-driver/mongo/description" - "go.mongodb.org/mongo-driver/mongo/writeconcern" "go.mongodb.org/mongo-driver/x/bsonx/bsoncore" "go.mongodb.org/mongo-driver/x/mongo/driver" "go.mongodb.org/mongo-driver/x/mongo/driver/session" @@ -23,19 +22,18 @@ import ( // CreateSearchIndexes performs a createSearchIndexes operation. type CreateSearchIndexes struct { - indexes bsoncore.Document - session *session.Client - clock *session.ClusterClock - collection string - monitor *event.CommandMonitor - crypt driver.Crypt - database string - deployment driver.Deployment - selector description.ServerSelector - writeConcern *writeconcern.WriteConcern - result CreateSearchIndexesResult - serverAPI *driver.ServerAPIOptions - timeout *time.Duration + indexes bsoncore.Document + session *session.Client + clock *session.ClusterClock + collection string + monitor *event.CommandMonitor + crypt driver.Crypt + database string + deployment driver.Deployment + selector description.ServerSelector + result CreateSearchIndexesResult + serverAPI *driver.ServerAPIOptions + timeout *time.Duration } // CreateSearchIndexResult represents a single search index result in CreateSearchIndexesResult. @@ -109,9 +107,15 @@ func (csi *CreateSearchIndexes) Execute(ctx context.Context) error { return driver.Operation{ CommandFn: csi.command, ProcessResponseFn: csi.processResponse, + Client: csi.session, + Clock: csi.clock, CommandMonitor: csi.monitor, + Crypt: csi.crypt, Database: csi.database, Deployment: csi.deployment, + Selector: csi.selector, + ServerAPI: csi.serverAPI, + Timeout: csi.timeout, }.Execute(ctx) } @@ -214,16 +218,6 @@ func (csi *CreateSearchIndexes) ServerSelector(selector description.ServerSelect return csi } -// WriteConcern sets the write concern for this operation. -func (csi *CreateSearchIndexes) WriteConcern(writeConcern *writeconcern.WriteConcern) *CreateSearchIndexes { - if csi == nil { - csi = new(CreateSearchIndexes) - } - - csi.writeConcern = writeConcern - return csi -} - // ServerAPI sets the server API version for this operation. func (csi *CreateSearchIndexes) ServerAPI(serverAPI *driver.ServerAPIOptions) *CreateSearchIndexes { if csi == nil { diff --git a/x/mongo/driver/operation/drop_search_index.go b/x/mongo/driver/operation/drop_search_index.go index 25cde8154b..3992c83165 100644 --- a/x/mongo/driver/operation/drop_search_index.go +++ b/x/mongo/driver/operation/drop_search_index.go @@ -14,7 +14,6 @@ import ( "go.mongodb.org/mongo-driver/event" "go.mongodb.org/mongo-driver/mongo/description" - "go.mongodb.org/mongo-driver/mongo/writeconcern" "go.mongodb.org/mongo-driver/x/bsonx/bsoncore" "go.mongodb.org/mongo-driver/x/mongo/driver" "go.mongodb.org/mongo-driver/x/mongo/driver/session" @@ -22,19 +21,18 @@ import ( // DropSearchIndex performs an dropSearchIndex operation. type DropSearchIndex struct { - index string - session *session.Client - clock *session.ClusterClock - collection string - monitor *event.CommandMonitor - crypt driver.Crypt - database string - deployment driver.Deployment - selector description.ServerSelector - writeConcern *writeconcern.WriteConcern - result DropSearchIndexResult - serverAPI *driver.ServerAPIOptions - timeout *time.Duration + index string + session *session.Client + clock *session.ClusterClock + collection string + monitor *event.CommandMonitor + crypt driver.Crypt + database string + deployment driver.Deployment + selector description.ServerSelector + result DropSearchIndexResult + serverAPI *driver.ServerAPIOptions + timeout *time.Duration } // DropSearchIndexResult represents a dropSearchIndex result returned by the server. @@ -93,7 +91,6 @@ func (dsi *DropSearchIndex) Execute(ctx context.Context) error { Database: dsi.database, Deployment: dsi.deployment, Selector: dsi.selector, - WriteConcern: dsi.writeConcern, ServerAPI: dsi.serverAPI, Timeout: dsi.timeout, }.Execute(ctx) @@ -196,16 +193,6 @@ func (dsi *DropSearchIndex) ServerSelector(selector description.ServerSelector) return dsi } -// WriteConcern sets the write concern for this operation. -func (dsi *DropSearchIndex) WriteConcern(writeConcern *writeconcern.WriteConcern) *DropSearchIndex { - if dsi == nil { - dsi = new(DropSearchIndex) - } - - dsi.writeConcern = writeConcern - return dsi -} - // ServerAPI sets the server API version for this operation. func (dsi *DropSearchIndex) ServerAPI(serverAPI *driver.ServerAPIOptions) *DropSearchIndex { if dsi == nil { diff --git a/x/mongo/driver/operation/update_search_index.go b/x/mongo/driver/operation/update_search_index.go index ba807986c9..64f2da7f6f 100644 --- a/x/mongo/driver/operation/update_search_index.go +++ b/x/mongo/driver/operation/update_search_index.go @@ -14,7 +14,6 @@ import ( "go.mongodb.org/mongo-driver/event" "go.mongodb.org/mongo-driver/mongo/description" - "go.mongodb.org/mongo-driver/mongo/writeconcern" "go.mongodb.org/mongo-driver/x/bsonx/bsoncore" "go.mongodb.org/mongo-driver/x/mongo/driver" "go.mongodb.org/mongo-driver/x/mongo/driver/session" @@ -22,20 +21,19 @@ import ( // UpdateSearchIndex performs a updateSearchIndex operation. type UpdateSearchIndex struct { - index string - definition bsoncore.Document - session *session.Client - clock *session.ClusterClock - collection string - monitor *event.CommandMonitor - crypt driver.Crypt - database string - deployment driver.Deployment - selector description.ServerSelector - writeConcern *writeconcern.WriteConcern - result UpdateSearchIndexResult - serverAPI *driver.ServerAPIOptions - timeout *time.Duration + index string + definition bsoncore.Document + session *session.Client + clock *session.ClusterClock + collection string + monitor *event.CommandMonitor + crypt driver.Crypt + database string + deployment driver.Deployment + selector description.ServerSelector + result UpdateSearchIndexResult + serverAPI *driver.ServerAPIOptions + timeout *time.Duration } // UpdateSearchIndexResult represents a single index in the updateSearchIndexResult result. @@ -95,7 +93,6 @@ func (usi *UpdateSearchIndex) Execute(ctx context.Context) error { Database: usi.database, Deployment: usi.deployment, Selector: usi.selector, - WriteConcern: usi.writeConcern, ServerAPI: usi.serverAPI, Timeout: usi.timeout, }.Execute(ctx) @@ -209,16 +206,6 @@ func (usi *UpdateSearchIndex) ServerSelector(selector description.ServerSelector return usi } -// WriteConcern sets the write concern for this operation. -func (usi *UpdateSearchIndex) WriteConcern(writeConcern *writeconcern.WriteConcern) *UpdateSearchIndex { - if usi == nil { - usi = new(UpdateSearchIndex) - } - - usi.writeConcern = writeConcern - return usi -} - // ServerAPI sets the server API version for this operation. func (usi *UpdateSearchIndex) ServerAPI(serverAPI *driver.ServerAPIOptions) *UpdateSearchIndex { if usi == nil { From 1cfc999895a77a6dae1e8574558ca7229c1ea353 Mon Sep 17 00:00:00 2001 From: Qingyang Hu Date: Thu, 21 Mar 2024 11:40:29 -0400 Subject: [PATCH 2/2] minor updates --- Makefile | 3 ++- mongo/integration/search_index_prose_test.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 7543ff6053..88bc756390 100644 --- a/Makefile +++ b/Makefile @@ -158,7 +158,8 @@ evg-test-load-balancers: .PHONY: evg-test-search-index evg-test-search-index: - go test ./mongo/integration -run TestSearchIndexProse -v -timeout 3600s >> test.suite + # Double the timeout to wait for the responses from the server. + go test ./mongo/integration -run TestSearchIndexProse -v -timeout $(shell echo "$$(( $(TEST_TIMEOUT) * 2))")s >> test.suite .PHONY: evg-test-ocsp evg-test-ocsp: diff --git a/mongo/integration/search_index_prose_test.go b/mongo/integration/search_index_prose_test.go index 5f13746f58..3d7e0ffb10 100644 --- a/mongo/integration/search_index_prose_test.go +++ b/mongo/integration/search_index_prose_test.go @@ -280,7 +280,7 @@ func TestSearchIndexProse(t *testing.T) { view := mt.Coll.SearchIndexes() definition := bson.D{{"mappings", bson.D{{"dynamic", false}}}} - searchName := "test-search-index6" + const searchName = "test-search-index-case6" opts := options.SearchIndexes().SetName(searchName) index, err := view.CreateOne(ctx, mongo.SearchIndexModel{ Definition: definition,