Skip to content

GODRIVER-2277 Remove causal consistency prose test #10 #879

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Mar 22, 2022
Merged

GODRIVER-2277 Remove causal consistency prose test #10 #879

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ebd0c1a
GODRIVER-2277 Remove causal consistency prose test #10
prestonvasquez Mar 18, 2022
d9768f7
GODRIVER-2277 Remove causal consistency prose test no. 10
prestonvasquez Mar 18, 2022
87acfec
add unack writes subtest to TestSessions
prestonvasquez Mar 21, 2022
bf0def2
Merge branch 'GODRIVER-2277.removeCausalConsistencyProseTest10' of gi…
prestonvasquez Mar 21, 2022
f2cb620
couple assertion and msg
prestonvasquez Mar 21, 2022
a02f062
use non-verbose error handling
prestonvasquez Mar 21, 2022
a3b8196
allow error reassignment
prestonvasquez Mar 21, 2022
c6090ba
make assert msg more verbose
prestonvasquez Mar 21, 2022
7ba09da
clean up assert msg
prestonvasquez Mar 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 0 additions & 16 deletions mongo/integration/causal_consistency_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"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"
)

// set of operations that support read concerns taken from read/write concern spec.
Expand Down Expand Up @@ -193,21 +192,6 @@ func TestCausalConsistency_Supported(t *testing.T) {
assert.NotNil(mt, sentOptime, "expected operation time on command, got nil")
assert.True(mt, currOptime.Equal(*sentOptime), "expected operation time %v, got %v", currOptime, sentOptime)
})
unackWcOpts := options.Collection().SetWriteConcern(writeconcern.New(writeconcern.W(0)))
mt.RunOpts("unacknowledged write", mtest.NewOptions().CollectionOptions(unackWcOpts), func(mt *mtest.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, in what situations does this test fail? It does not fail locally for me against a 5.0 replica set.

do we want to tweak the test to assert that doing an unacknowledged write within a session results in an error?

If this is client-side error (thrown within the driver), then that could be a valuable test. If the server just doesn't support unacknowledged writes with explicit sessions, then I don't think we need to test that behavior on our side.

Copy link
Member Author

@prestonvasquez prestonvasquez Mar 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test should fail if the 0-number write concern being sent over the wire is acknowledged, which should request[s] no acknowledgment of the write operation.

The error itself is thrown by the *mongo.Collection.InsertOne method, specifically whenever the wire message flag is moreToCome. I can see how we determine this message from the code base, but have no idea why the relationship between unacknowledged writes and moreToCome is 1-1. The server does not explicitly create an error for this and it is technically recommended behavior.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed offline, the moreToCome bit is set on every unacknowledged write when we are not actively batching. As you mentioned, we throw ErrUnacknowledgedWrite here according to the recommended behavior in the spec.

// unacknowledged write should not update the operationTime of a session

sess, err := mt.Client.StartSession()
assert.Nil(mt, err, "StartSession error: %v", err)
defer sess.EndSession(context.Background())

_ = mongo.WithSession(context.Background(), sess, func(sc mongo.SessionContext) error {
_, _ = mt.Coll.InsertOne(sc, bson.D{{"x", 1}})
return nil
})
optime := sess.OperationTime()
assert.Nil(mt, optime, "expected operation time nil, got %v", optime)
})
mt.Run("clusterTime included", func(mt *mtest.T) {
// $clusterTime should be included in commands if the deployment supports cluster times

Expand Down
17 changes: 17 additions & 0 deletions mongo/integration/sessions_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"go.mongodb.org/mongo-driver/mongo/integration/mtest"
"go.mongodb.org/mongo-driver/mongo/options"
"go.mongodb.org/mongo-driver/mongo/readpref"
"go.mongodb.org/mongo-driver/mongo/writeconcern"
"go.mongodb.org/mongo-driver/x/mongo/driver/session"
)

Expand Down Expand Up @@ -341,6 +342,22 @@ func TestSessions(t *testing.T) {
assertCollectionCount(mt, int64(numDocs))
})
})

unackWcOpts := options.Collection().SetWriteConcern(writeconcern.New(writeconcern.W(0)))
mt.RunOpts("unacknowledged write", mtest.NewOptions().CollectionOptions(unackWcOpts), func(mt *mtest.T) {
// unacknowledged write during a session should result in an error
sess, err := mt.Client.StartSession()
assert.Nil(mt, err, "StartSession error: %v", err)
defer sess.EndSession(context.Background())

err = mongo.WithSession(context.Background(), sess, func(sc mongo.SessionContext) error {
_, err := mt.Coll.InsertOne(sc, bson.D{{"x", 1}})
return err
})

assert.Equal(mt, err, mongo.ErrUnacknowledgedWrite,
"expected ErrUnacknowledgedWrite on unacknowledged write in session, got %v", err)
})
}

func assertCollectionCount(mt *mtest.T, expectedCount int64) {
Expand Down