Skip to content

Conversation

@jmikola
Copy link
Member

@jmikola jmikola commented Oct 20, 2021

https://jira.mongodb.org/browse/DRIVERS-1958

aggregate-write-readPreference tests use client-level readConcern and writeConcern options to assert that they are still inherited and passed in the aggregate command; however, readConcern is generally not supported for write stages in server versions prior to 4.2.

Note: I realized this was an issue after merging the PHPLIB work for #1062 (DRIVERS-823), as we had test failures on MongoDB 3.6 and 4.0 (libmongoc correctly does not inherit a client-level read concern when assembling a command destined for a pre-4.2 server). Tagging @jyemin and @benjirewis are reviewers as Java and Go appear to be the only other drivers that have implemented that ticket thus far.

FWIW, I also verified that MongoDB 4.0 does allow a "local" RC to be specified. If drivers weren't experiencing test failures here, it's likely that they're just passing that inherited value along to the server and it's being silently ignored ("local" is the default, so there is no significance):

rs0:PRIMARY> db.foo.runCommand({aggregate:"foo",pipeline:[{$match:{x:1}},{$out:"bar"}],readConcern:{level:"local"},cursor:{}})
{
	"cursor" : {
		"firstBatch" : [ ],
		"id" : NumberLong(0),
		"ns" : "test.foo"
	},
	"ok" : 1,
	"operationTime" : Timestamp(1634741761, 3),
	"$clusterTime" : {
		"clusterTime" : Timestamp(1634741761, 3),
		"signature" : {
			"hash" : BinData(0,"AAAAAAAAAAAAAAAAAAAAAAAAAAA="),
			"keyId" : NumberLong(0)
		}
	}
}
rs0:PRIMARY> db.foo.runCommand({aggregate:"foo",pipeline:[{$match:{x:1}},{$out:"bar"}],readConcern:{level:"majority"},cursor:{}})
{
	"operationTime" : Timestamp(1634741761, 3),
	"ok" : 0,
	"errmsg" : "$out cannot be used with a 'majority' read concern level",
	"code" : 72,
	"codeName" : "InvalidOptions",
	"$clusterTime" : {
		"clusterTime" : Timestamp(1634741761, 3),
		"signature" : {
			"hash" : BinData(0,"AAAAAAAAAAAAAAAAAAAAAAAAAAA="),
			"keyId" : NumberLong(0)
		}
	}
}

@jmikola jmikola requested review from benjirewis and jyemin October 20, 2021 14:46
@jmikola jmikola requested a review from a team as a code owner October 20, 2021 14:46
Copy link
Contributor

@jyemin jyemin left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@benjirewis benjirewis left a comment

Choose a reason for hiding this comment

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

Don't forget to modify the JSON, too.

aggregate-write-readPreference tests use client-level readConcern and writeConcern options to assert that they are still inherited and passed in the aggregate command; however, readConcern is generally not supported for write stages in server versions prior to 4.2.
@jmikola jmikola merged commit dce2c48 into mongodb:master Oct 20, 2021
@jmikola jmikola deleted the drivers-1958 branch October 20, 2021 17:45
p-mongo pushed a commit to p-mongo/specifications that referenced this pull request Oct 25, 2021
* upstream/master: (47 commits)
  DRIVERS-1959: Expect either valid:false or warning:true, but not both (mongodb#1083)
  DRIVERS-1958: Do not expect readConcern for pre-4.2 servers (mongodb#1081)
  DRIVERS-1918 Update rules for $readPreference passing to include OP_MSG (mongodb#1076)
  DRIVERS-1956: Skip tests utilizing $out on serverless (mongodb#1080)
  DRIVERS-1519: srvMaxHosts option for limiting SRV seeding (mongodb#1069)
  Revert "Skip "listIndexes pins the cursor to a connection" on serverl… (mongodb#1079)
  Fix formatting for load-balanced topology
  Skip "listIndexes pins the cursor to a connection" on serverless (mongodb#1077)
  DRIVERS-823: Support $out and $merge on secondaries (mongodb#1062)
  DRIVERS-1141 Add missing new lines on serverConnectionId spec tests (mongodb#1074)
  DRIVERS-1631 Clarify minPoolSize requirement (mongodb#1072)
  DRIVERS-521 Use test22 for custom service name endpoint (mongodb#1073)
  DRIVERS-1929 Add missing "hello" to load balanacer spec test (mongodb#1071)
  DRIVERS-1912: Fix change log and last modified date (mongodb#1070)
  DRIVERS-521 Allow custom SRV service names with srvServiceName URI option (mongodb#1067)
  Fix link to load balancer spec (mongodb#1068)
  DRIVERS-1912 Clarify maxWireVersion check for maxStalenessSeconds (mongodb#1064)
  DRIVERS-1621: Clarify error expectation rules for parseErrors (mongodb#1066)
  Add link to ServerType in SDAM spec test description (mongodb#683)
  DRIVERS-1914 Max staleness test updates
  ...
kevinAlbs pushed a commit to kevinAlbs/specifications that referenced this pull request Nov 9, 2021
…1081)

aggregate-write-readPreference tests use client-level readConcern and writeConcern options to assert that they are still inherited and passed in the aggregate command; however, readConcern is generally not supported for write stages in server versions prior to 4.2.

As noted in the test comments, pre-4.2 servers may still allow a "local" read concern anyway, but some drivers may avoid inheriting a client-level read concern for pre-4.2. Skipping these tests for pre-4.2 servers avoids any ambiguity.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants