[CDRIVER-580] Write concern with w=0 and j=true is a GLE op Created: 13/Mar/15  Updated: 08/Jan/24  Resolved: 06/Apr/15

Status: Closed
Project: C Driver
Component/s: None
Affects Version/s: 1.1.2
Fix Version/s: 1.1.5

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

Issue Links:
Depends
is depended on by PHPC-179 WriteConcern debug handler should den... Closed
is depended on by CDRIVER-581 URI parsing should not assume write c... Closed
is depended on by PHPC-178 WriteConcern should allow unset options Closed
Related
related to PHPC-280 WriteConcern should always set journa... Closed
related to CDRIVER-587 URI parsing logs warning on w < -1 bu... Closed
related to CDRIVER-590 Support readPreference connection str... Closed

 Description   

Currently, _mongoc_write_concern_needs_gle() returns true only if w is neither 0 or -1. This function is used by both the legacy and write command code paths. If the user were to specify w:0 and j:true, the driver skips sending a GLE. Although an edge case, this is unfortunate in that the server would otherwise have upgraded that w:0 to w:1 in order to honor the j:true option. For write commands, if the user specifies any values for a write concern, they should be included in the command(s).

I believe there are also implications for replSetGetConfig.settings.getLastErrorDefaults, as that option kicks in only when the driver does not specify a write concern.

A fix will start with having the driver differentiate between false and unset values for journal and fsync_ on the write concern struct. From there, the logic for _mongoc_write_concern_needs_gle() and BSON serialization can be changed.



 Comments   
Comment by Githook User [ 07/Oct/15 ]

Author:

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

Message: Merge remote-tracking branch 'upstream/master' into 1.2.0-dev

  1. By A. Jesse Jiryu Davis (7) and others
  • upstream/master:
    CDRIVER-606 Call mongoc_init () in test-secondary
    Begin next release's changelog
    Spelling.
    Include header for suppress_one_message() in test-mongoc-uri.c.
    CDRIVER-605 fix function name in collection_find_indexes page.
    Fix version no. in guide to building from release tarball.
    post release bump
    Release 1.1.4
    CDRIVER-580: fsync or j in write concern imply GLE
    Use write concern macros instead of magic numbers
    CDRIVER-560: Include private header in mongoc-client-pool.c.

Conflicts:
src/libbson
src/mongoc/mongoc-client-pool.c
src/mongoc/mongoc-uri.c
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/673d4a7aab3d8b46a676ea97b4cd46c5956f976c

Comment by Githook User [ 07/Oct/15 ]

Author:

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

Message: CDRIVER-590: Support readPreference connection string option

This adds support for parsing the readPreference URI option. It also implements validation of the read preference and returns a NULL URI pointer on failure, similar to the write concern validation added by CDRIVER-580.

Unfortunately, mongoc_uri_get_read_prefs() was already taken for the API function that returns read preference tags from the URI. This commit adds a new function, mongoc_uri_get_read_prefs_t(), which returns the actual mongoc_read_prefs_t struct (now used by the client constructor).
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/c425c5f020c1050cded2ecd4f997ce1461f6c68c

Comment by Githook User [ 07/Apr/15 ]

Author:

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

Message: Merge remote-tracking branch 'upstream/master' into 1.2.0-dev

  1. By A. Jesse Jiryu Davis (7) and others
  • upstream/master:
    CDRIVER-606 Call mongoc_init () in test-secondary
    Begin next release's changelog
    Spelling.
    Include header for suppress_one_message() in test-mongoc-uri.c.
    CDRIVER-605 fix function name in collection_find_indexes page.
    Fix version no. in guide to building from release tarball.
    post release bump
    Release 1.1.4
    CDRIVER-580: fsync or j in write concern imply GLE
    Use write concern macros instead of magic numbers
    CDRIVER-560: Include private header in mongoc-client-pool.c.

Conflicts:
src/libbson
src/mongoc/mongoc-client-pool.c
src/mongoc/mongoc-uri.c
Branch: 1.2.0-dev
https://github.com/mongodb/mongo-c-driver/commit/673d4a7aab3d8b46a676ea97b4cd46c5956f976c

Comment by Githook User [ 07/Apr/15 ]

Author:

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

Message: Merge branch 'pr-194'

Comment by Githook User [ 07/Apr/15 ]

Author:

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

Message: CDRIVER-580: fsync or j in write concern imply GLE

The fsync and journal options should imply a GLE; however, they logically conflict with w=0 and w=-1, so the write concern should be considered invalid with errors raised during URI parsing or write command execution.

If fsync or journal have been specified (true or false), they should be included in the write concern BSON.
Branch: 1.2.0-dev
https://github.com/mongodb/mongo-c-driver/commit/4c2af639c80d7e38fc8f8354a9b3a4bf02d39df0

Comment by Githook User [ 07/Apr/15 ]

Author:

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

Message: CDRIVER-590: Support readPreference connection string option

This adds support for parsing the readPreference URI option. It also implements validation of the read preference and returns a NULL URI pointer on failure, similar to the write concern validation added by CDRIVER-580.

Unfortunately, mongoc_uri_get_read_prefs() was already taken for the API function that returns read preference tags from the URI. This commit adds a new function, mongoc_uri_get_read_prefs_t(), which returns the actual mongoc_read_prefs_t struct (now used by the client constructor).
Branch: 1.2.0-dev
https://github.com/mongodb/mongo-c-driver/commit/c425c5f020c1050cded2ecd4f997ce1461f6c68c

Comment by Githook User [ 06/Apr/15 ]

Author:

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

Message: Merge branch 'pr-194'

Comment by Githook User [ 06/Apr/15 ]

Author:

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

Message: CDRIVER-580: fsync or j in write concern imply GLE

The fsync and journal options should imply a GLE; however, they logically conflict with w=0 and w=-1, so the write concern should be considered invalid with errors raised during URI parsing or write command execution.

If fsync or journal have been specified (true or false), they should be included in the write concern BSON.
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/4c2af639c80d7e38fc8f8354a9b3a4bf02d39df0

Comment by Jeremy Mikola [ 30/Mar/15 ]

I believe the linked PR is ready to go. The last todo item (warning message suppression) was implemented last week.

Comment by Jeremy Mikola [ 20/Mar/15 ]

PR is updated and includes test cases for reporting errors on an invalid write concern during both URI parsing and write command execution. There's an outstanding question on whether the warning emission is OK and how we might want to suppress or assert it during the test.

While looking into the URI parsing, I noticed that invalid w values (i.e. anything less than -1) trigger a warning but are otherwise ignored. I opened CDRIVER-587 to track that, since it's outside the scope of this PR.

Comment by Jeremy Mikola [ 19/Mar/15 ]

I spoke to mira.carey@mongodb.com about this earlier today. Asserts were a bad idea because it could unpleasantly abort the program and this is really more of a runtime error, since write concerns could be generated from dynamic input.

Jason pointed out that _mongoc_write_concern_is_valid() is the common choke point for runtime usage of write concerns. I'm going to implement a _mongoc_write_concern_is_valid() function in the private API (akin to mongoc_read_prefs_is_valid()), which will check that we have a logically correct data structure. If not, we can report an error in the result structure, which is consistent with existing runtime error reporting. I'll add tests for the validation function, as well as a functional test for raising the error when attempting a write.

URI parsing will get a consistent error reporting and I'll add the failing case for "mongodb://host/?w=0&j=true".

I'll follow up once the PR is updated.

Comment by Jeremy Mikola [ 19/Mar/15 ]

On second thought, BSON_ASSERT() was a premature suggestion. I'll follow up in a bit with a more robust solution.

Comment by A. Jesse Jiryu Davis [ 19/Mar/15 ]

Sounds great Jeremy thanks! Let's include a test of the URI too? "mongodb://host/?w=0&j=true" shouldn't parse.

Comment by Jeremy Mikola [ 19/Mar/15 ]

I concur that w=0 combined with either j=true or fsync=true is a logical error; however, the C driver's write concern component doesn't have a convention for reporting errors if the user decides to set the struct to such a state. I'm happy to add some error reporting to the PR to address that, if we can agree on how to go about it.

At a glance, I expect BSON_ASSERT() calls on the setter methods in mongoc-write-concern.c would suffice (instead of catching all of the places that use a write concern and call _mongoc_write_concern_needs_gle(). I believe that would also implicitly handle concerns with expressing an invalid write concern during URI parsing.

Comment by A. Jesse Jiryu Davis [ 19/Mar/15 ]

bjori I completely agree with you that only providing j=true implies "acknowledged". In this case no "w" value is sent to the server, we accept the server's default "w" value.

But it looks like we should treat w=0, j=true as an error in the C Driver, not as an acknowledged write.

Comment by David Golden [ 19/Mar/15 ]

FWIW, the next gen perl driver also considers

{w:0, j:1}

to be an error: https://github.com/mongodb/mongo-perl-driver/blob/master/lib/MongoDB/WriteConcern.pm#L82-L84

Comment by Bernie Hackett [ 19/Mar/15 ]

Passing j=true with legacy op codes requires we send a getLastError command to the server, and hence get an acknowledgement. There is no way around it. Passing w=0 means "don't ask for acknowledgement". I think passing w=0&j=true should be an error. There is no way to do what the user is asking for.

Also, I wasn't aware that passing

{w: 0, j: true}

to a write command forced the server to upgrade the w value to 1. Though that's interesting by itself, it doesn't change the fact that the user is requesting something that is not possible. Not to mention the fact that drivers are supposed to use legacy op codes with w=0, not write commands, since write commands always return a response.

Comment by Hannes Magnusson [ 19/Mar/15 ]

Using the older Mongo objects (as apposed to MongoClient) and only providing j=true in the connection uri will result in the reported behaviour.
The user is explicitly requesting the write to be journaled. That may or may not imply "acknoweldge" as is currently defined as "written to memory", which can be thought of as being implementation specific, and maybe even storageEngine specific.

If setting w=0&j=true on MongoClient is deemed an error, the ticket is still valid as there is no such error issued and the write concern is silently downgraded.

Comment by Bernie Hackett [ 19/Mar/15 ]

In PyMongo 3.0 we've made this really clear by raising an error:

https://github.com/mongodb/mongo-python-driver/blob/master/pymongo/write_concern.py#L72-L74

Comment by Bernie Hackett [ 19/Mar/15 ]

If the user were to specify w:0 and j:true, the driver skips sending a GLE. Although an edge case, this is unfortunate in that the server would otherwise have upgraded that w:0 to w:1 in order to honor the j:true option.

This seems amazingly counter-intuitive. Setting w=0 means "don't acknowledge my writes". The Java and python drivers do the opposite of what you propose, and always have.

Comment by A. Jesse Jiryu Davis [ 19/Mar/15 ]

I believe the desired behavior, making "w: 0, j: true" an acknowledged op, diverges from other drivers. PyMongo considers "w: 0" to be the user's express intention to do unacknowledged legacy writes. There's no spec of course. behackett, jeff.yemin, care to comment?

Of course if the user simply specifies "j: true", the driver should call getlasterror with "j: true" and no "w" at all (the default, acknowledged write concern). Same for the "writeConcern" document sent with write commands.

Comment by Jeremy Mikola [ 13/Mar/15 ]

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

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