[CDRIVER-2432] Unacknowledged writes should not include transaction ID or lsid for retryable writes Created: 18/Dec/17 Updated: 28/Oct/23 Resolved: 12/Jan/18 |
|
| Status: | Closed |
| Project: | C Driver |
| Component/s: | libmongoc |
| Affects Version/s: | 1.9.0 |
| Fix Version/s: | 1.10.0 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Jeremy Mikola | Assignee: | Xiangyu Yao (Inactive) |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||||||||||||||
| Description |
|
Per the Retryable Writes spec, unacknowledged writes should never be retried. Also, the Sessions Spec says "a driver MUST NOT send a session ID with unacknowledged writes. This is true for both implicit and explicit sessions." While testing PHPC with libmongoc 1.9.0-rc1, I discovered that libmongoc is still including transaction IDs in unacknowledged write commands. This is likely the result of _allow_txn_number() in mongoc-cmd.c failing to return to return false. We should be able to trust that the "writeConcern" field, if any, is already merged into the write command before _allow_txn_number() is called, so fixing this should only require _allow_txn_number() checking for w:0. Additionally, this fix should catch the case where mongoc_cmd_parts_append_opts() merged in a "writeConcern" option (from a with_opts() function, or the user explicitly provided "writeConcern" in the original command document (as PHPLIB does through PHPC). Finally, the driver MUST NOT include "lsid" (the logical session ID) in unacknowledged writes. |
| Comments |
| Comment by Githook User [ 12/Jan/18 ] |
|
Author: {'email': 'xiangyu.yao24@gmail.com', 'name': 'Xiangyu Yao', 'username': 'xy24'}Message: |
| Comment by A. Jesse Jiryu Davis [ 09/Jan/18 ] |
|
Thanks Jeremy. findAndModify doesn't allow unacknowledged writes (the driver and server ignore you if you include writeConcern: {w: 0}). But this is certainly an issue for generic command runners and basic write operations. |
| Comment by Jeremy Mikola [ 08/Jan/18 ] |
|
mongoc_cmd_parts_assemble() is called from several contexts, so _allow_txn_number() would still be called from other command execution paths (e.g. generic command runners, findAndModify). That said, it's possible that the line you referenced in mongoc-write-command.c prematurely assigns MONGOC_CMD_PARTS_ALLOW_TXN_NUMBER_YES. An unacknowledged write concern should prohibit a transaction number just as multi writes do. Side note: I believe I reported this ticket with basic write operations (i.e. insert, update, delete) in mind, but I think we should also consider unacknowledged write concerns on other commands (e.g. findAndModify) and ensure those are not retried. |
| Comment by Xiangyu Yao (Inactive) [ 08/Jan/18 ] |
|
It seems this line already determines whether the transaction ID should be included or not before mongoc_cmd_parts_assemble which eventually uses _allow_txn_number(). So I guessed _allow_txn_number() was actually never used probably? |
| Comment by A. Jesse Jiryu Davis [ 03/Jan/18 ] |
|
Xiangyu, you should test this with a mock_server_t. |
| Comment by Jeremy Mikola [ 18/Dec/17 ] |
|
Until I'll defer to jesse is this issue should be addressed in 1.9.x or wait until 1.10. Note that the server doesn't care if we retry unacknowledged writes. This requirement is just something we decided to enforce on the client side for consistency across all drivers (see: Why are unacknowledged write concerns unsupported?). |