[CDRIVER-2615] Do not allow a session to be used with an unacknowledged write concern Created: 17/Apr/18  Updated: 28/Oct/23  Resolved: 23/May/18

Status: Closed
Project: C Driver
Component/s: None
Affects Version/s: 1.9.0
Fix Version/s: 1.10.0, 1.10.1

Type: Bug Priority: Major - P3
Reporter: Jeremy Mikola Assignee: A. Jesse Jiryu Davis
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
is depended on by CXX-1518 Prohibit using unacknowledged writes ... Closed
is depended on by DRIVERS-456 Prohibit using unacknowledged writes ... Closed
is depended on by PHPC-1126 Prohibit using unacknowledged writes ... Closed
Related
is related to CDRIVER-2432 Unacknowledged writes should not incl... Closed
is related to PHPC-1163 Do not allow a session to be used wit... Closed
is related to DRIVERS-456 Prohibit using unacknowledged writes ... Closed

 Description   

Per the Driver Session spec:

Therefore drivers MUST NOT send a session ID with unacknowledged writes under any circumstances:

  • For unacknowledged writes with an explicit session, drivers SHOULD raise an error. If a driver allows users to provide an explicit session with an unacknowledged write (e.g. for backwards compatibility), the driver MUST NOT send the session ID.
  • For unacknowledged writes without an explicit session, drivers SHOULD NOT use an implicit session. If a driver creates an implicit session for unacknowledged writes without an explicit session, the driver MUST NOT send the session ID.

Drivers MUST document the behavior of unacknowledged writes for both explicit and implicit sessions.

While researching this in PHPC-1163, I realized that libmongoc also does not enforce this. Using APM, we should test that lsid fields are omitted from command documents for the following cases:

  • Executing a write operation or command with an explicit unacknowledged write concern
  • Executing a write operation or command with an inherited unacknowledged write concern
  • Executing a write operation or command with an explicit unacknowledged write concern and an explicit session
  • Executing a write operation or command with an inherited unacknowledged write concern and an explicit session

The spec states that drivers "SHOULD raise an error" when explicit sessions are mixed with unacknowledged writes, but I noticed that _mongoc_write_opmsg() silently omits the session in that case. That function is used for insert/update/delete and bulk writes, so I would propose we do the same for commands and findAndModify.

There is already a test_unacknowledged() case in the test suite, but it looks like the APM callback doesn't assert anything for unacknowledged write concerns.

Note: users could technically add a "writeConcern" field to their command document without using the "writeConcern" option for command_with_opts(), but I don't think we want to both inspecting that.



 Comments   
Comment by Githook User [ 29/May/18 ]

Author:

{'username': 'ajdavis', 'name': 'A. Jesse Jiryu Davis', 'email': 'jesse@mongodb.com'}

Message: CDRIVER-2615 prohibit session and w=0 bulk write
Branch: r1.10
https://github.com/mongodb/mongo-c-driver/commit/577018f9f66f2149e644528f42cbbb8c8ec96feb

Comment by Githook User [ 23/May/18 ]

Author:

{'username': 'ajdavis', 'name': 'A. Jesse Jiryu Davis', 'email': 'jesse@mongodb.com'}

Message: CDRIVER-2615 prohibit session and w=0 bulk write
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/21a66cb0dd202c9eabefc3ddea3a4aa51f573f1a

Comment by A. Jesse Jiryu Davis [ 22/May/18 ]

We got the collection write functions like mongoc_collection_insert_one, but not mongoc_bulk_operation_execute.

Comment by Githook User [ 07/May/18 ]

Author:

{'email': 'jesse@mongodb.com', 'name': 'A. Jesse Jiryu Davis', 'username': 'ajdavis'}

Message: CDRIVER-2615 prohibit explicit session with w=0
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/93c7ec74450adc04924cfbeb9d7efaa848d30556

Comment by A. Jesse Jiryu Davis [ 05/May/18 ]

Got it, thanks.

Comment by Jeremy Mikola [ 05/May/18 ]

jesse: I believe the test in 887451f should already test that unacknowledged writes never include an lsid (implicit or explicit), but you will definitely need to add an error if users combine explicit sessions with unacknowledged writes.

Comment by A. Jesse Jiryu Davis [ 05/May/18 ]

Must still throw an error if an explicit session is used with an unacknowledged write, and verify that no implicit session is created with an unacknowledged write.

Comment by Githook User [ 24/Apr/18 ]

Author:

{'email': 'jmikola@gmail.com', 'username': 'jmikola', 'name': 'Jeremy Mikola'}

Message: PHPC-1163: Prohibit session with unacknowledged write concern

This adds checks to ensure that explicit sessions and unacknowledged write concerns (explicit or inherited) will not be mixed when executing bulk writes or commands. Additionally, we ensure that command execution does not create its own implicit session (per PHPC-1152) if the effective write concern is unacknowledged.

Note: libmongoc still needs to ensure that it does not create implicit sessions for unacknowledged commands (CDRIVER-2615).
Branch: master
https://github.com/mongodb/mongo-php-driver/commit/cceeda84c128d08abc297e5fd9c9d8f5d7b07faa

Comment by Githook User [ 24/Apr/18 ]

Author:

{'email': 'jmikola@gmail.com', 'username': 'jmikola', 'name': 'Jeremy Mikola'}

Message: PHPC-1163: Prohibit session with unacknowledged write concern

This adds checks to ensure that explicit sessions and unacknowledged write concerns (explicit or inherited) will not be mixed when executing bulk writes or commands. Additionally, we ensure that command execution does not create its own implicit session (per PHPC-1152) if the effective write concern is unacknowledged.

Note: libmongoc still needs to ensure that it does not create implicit sessions for unacknowledged commands (CDRIVER-2615).
Branch: v1.4
https://github.com/mongodb/mongo-php-driver/commit/cceeda84c128d08abc297e5fd9c9d8f5d7b07faa

Comment by Githook User [ 23/Apr/18 ]

Author:

{'email': 'jmikola@gmail.com', 'username': 'jmikola', 'name': 'Jeremy Mikola'}

Message: CDRIVER-2615 test that unacknowledged commands never include lsid
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/887451f8a4dd08c2b308272dba7e4ed8b1788ced

Comment by Githook User [ 23/Apr/18 ]

Author:

{'email': 'jmikola@gmail.com', 'username': 'jmikola', 'name': 'Jeremy Mikola'}

Message: CDRIVER-2615 omit explicit client session when WC is unacknowledged

We omit the client session instead of raising an error for BC and
consistency with _mongoc_write_opmsg() for CRUD ops.
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/92476ac1572c103fddbbc3566a5b21589685a3ea

Comment by Githook User [ 23/Apr/18 ]

Author:

{'email': 'jmikola@gmail.com', 'username': 'jmikola', 'name': 'Jeremy Mikola'}

Message: CDRIVER-2615 ensure parts.assembled.is_acknowledged is accurate
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/bae87f51cde4a128dd841055d92522f2c819a6b4

Comment by Jeremy Mikola [ 20/Apr/18 ]

https://mongodbcr.appspot.com/198420001/

Comment by Jeremy Mikola [ 20/Apr/18 ]

Some observations while implementing tests:

mongoc_cmd_parts_assemble

Command execution generally passes through mongoc_cmd_parts_assemble(). While this function currently avoids creating implicit sessions for unacknowledged write concerns, it has no such protection for explicit unacknowledged write concerns specified by the user (or inherited, as we'll see below). We can likely address this by adding some additional checks to this condition.

_mongoc_client_command_with_opts

_mongoc_client_command_with_opts() is the root method for all of the command_with_opts variants in the client and database classes. This function depends on mongoc_cmd_parts_append_opts() to process user-specified options. Internally, mongoc_cmd_parts_append_opts() updates parts->assembled.is_acknowledged whenever it encounters a write concern that would be applied to the command.

When inheriting a write concern from the client or database object, _mongoc_client_command_with_opts() manually appends to parts.extra. This currently neglects to update parts->assembled.is_acknowledged, so we need to fix that.

CRUD and bulk writes

Best I can tell, all CRUD and bulk write methods route through _mongoc_write_command_execute(). For anything involving sessions, control will need to pass through _mongoc_write_opmsg(), which already has logic to ignore explicit sessions for unacknowledged write concerns.

mongoc_collection_find_and_modify_with_opts

mongoc_collection_find_and_modify_with_opts() has its own logic for inheriting the write concern, which dates back to f7a07b8 in CDRIVER-824. Historically, mongoc_collection_find_and_modify_with_opts() has only ever inherited an acknowledged write concern (it ignores an unacknowledged write concern on the collection).

By my own testing, the server also seems to ignore an unacknowledged write concern if explicitly passed in the command. findAndModify always returns a response. Consider:

> db.runCommand({findAndModify:"foo",update:{$inc:{x:1}},new:true,writeConcern:{w:0}});
{
	"lastErrorObject" : {
		"n" : 1,
		"updatedExisting" : true
	},
	"value" : {
		"_id" : ObjectId("5ada498db560305eb848924f"),
		"x" : 3
	},
	"ok" : 1
}

In any event, if we want to be strict and still prohibit an lsid with an explicit unacknowledged write concern provided by the user, we're covered by mongoc_collection_find_and_modify_with_opts() depending on mongoc_cmd_parts_append_opts(), which in turn sets parts->assembled.is_acknowledged.

_mongoc_cursor_run_command

This may be an edge case, but _mongoc_cursor_run_command() also has logic to append a write concern to parts.extra. Since this bypasses mongoc_cmd_parts_append_opts(), it's possible that parts->assembled.is_acknowledged may get out of sync. We should fix this to set parts->assembled.is_acknowledged according to the write concern being applied.

Best I can tell, this code path is only relevant to mongoc_collection_command(), mongoc_database_command(), and mongoc_client_command(). While these are old APIs (they return a cursor), none of these methods appear to be deprecated in the documentation.

Comment by Githook User [ 19/Apr/18 ]

Author:

{'email': 'jmikola@gmail.com', 'username': 'jmikola', 'name': 'Jeremy Mikola'}

Message: PHPC-1163: Prohibit session with unacknowledged write concern

This adds checks to ensure that explicit sessions and unacknowledged write concerns (explicit or inherited) will not be mixed when executing bulk writes or commands. Additionally, we ensure that command execution does not create its own implicit session (per PHPC-1152) if the effective write concern is unacknowledged.

Note: libmongoc still needs to ensure that it does not create implicit sessions for unacknowledged commands (CDRIVER-2615).
Branch: 1.4-phpc-1163
https://github.com/mongodb/mongo-php-driver/commit/393537d80fc3138977a42b4b703b8c859619c34d

Comment by A. Jesse Jiryu Davis [ 18/Apr/18 ]

Thanks for the report. Right, if people call a function with "command" in the name and put the write concern in the command document, rather than in "opts" or some other sanctioned way of configuring it, we just pass the write concern to the server without any special treatment.

Generated at Wed Feb 07 21:15:47 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.