[CDRIVER-2744] _mongoc_cursor_new_with_opts() rejects primary read preference within transaction Created: 12/Jul/18  Updated: 28/Oct/23  Resolved: 17/Jul/18

Status: Closed
Project: C Driver
Component/s: libmongoc
Affects Version/s: 1.11.0
Fix Version/s: 1.12.0

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 PHPC-1236 Upgrade libmongoc to 1.11.1 Closed
Related
related to PHPC-1251 Upgrade libmongoc to 1.12.0 Closed
related to PHPLIB-375 Consider omitting readPreference opti... Backlog

 Description   

In mongodb/mongo-php-library#562, a user reported an issue executing a Collection::find() operation in PHPLIB. Because the library explicitly assigns the collection's read preference on the query, it triggered the following code path in _mongoc_cursor_new_with_opts():

   if (_mongoc_client_session_in_txn (cursor->client_session)) {
      if (!IS_PREF_PRIMARY (user_prefs)) {
         bson_set_error (&cursor->error,
                         MONGOC_ERROR_CURSOR,
                         MONGOC_ERROR_CURSOR_INVALID_CURSOR,
                         "Read preference in a transaction must be primary");
         GOTO (finish);
      }
 
      if (user_prefs) {
         bson_set_error (
            &cursor->error,
            MONGOC_ERROR_CURSOR,
            MONGOC_ERROR_CURSOR_INVALID_CURSOR,
            "Cannot set read preferences after starting transaction");
         GOTO (finish);
      }

The first check for a non-primary read preference is consistent with _mongoc_client_command_with_opts() and mongoc_cmd_parts_assemble(), but the second conditional only exists in this function. I don't believe it serves a purpose, as a primary read preference should be permitted at this point.

Note: I realize that it's a bit redundant for PHPLIB to add a read preference to a query if we're not communicating with mongos, since we would have already done server selection by this point. That is likely be something we can refactor independently of this issue.



 Comments   
Comment by Githook User [ 05/Nov/18 ]

Author:

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

Message: Regression tests for find() and aggregate() using primary RP within transaction

This bumps the PHPC requirement to 1.5.2+, since that includes libmongoc 1.12 and CDRIVER-2744.
Branch: master
https://github.com/mongodb/mongo-php-library/commit/ba0b611acba37aeb43266a6ef01b836c56754a98

Comment by Githook User [ 17/Jul/18 ]

Author:

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

Message: CDRIVER-2744 more txn read pref tests

Supplement C tests with more YAML tests that read functions accept an
explicit read pref of "primary" in a transaction.
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/b8be5158e55fdb4a074b9e98f48eb8d33568df46

Comment by Githook User [ 13/Jul/18 ]

Author:

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

Message: CDRIVER-2744: Allow primary RP in _mongoc_cursor_new_with_opts()
Branch: r1.11
https://github.com/mongodb/mongo-c-driver/commit/5eedeb9105159c9962e238d0ff7d4498c85d65bb

Comment by Githook User [ 13/Jul/18 ]

Author:

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

Message: CDRIVER-2744: Allow primary RP in _mongoc_cursor_new_with_opts()
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/4da2a1191b533158544155d2ed02f633cd124f60

Comment by Jeremy Mikola [ 12/Jul/18 ]

https://evergreen.mongodb.com/version/5b47ba080305b97579e94afd (filtered to just a few replica set variants)

Comment by Jeremy Mikola [ 12/Jul/18 ]

https://github.com/mongodb/mongo-c-driver/pull/517

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