-
Notifications
You must be signed in to change notification settings - Fork 548
CXX-1998 'CommitQuorum' option support for 'createIndexes’ command on MongoDB 4.4 #691
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
cfbff4d
f4780f8
43005fd
a4630ce
cd7f158
bb8c358
63e5a70
64f220a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,22 +14,21 @@ | |
|
|
||
| #pragma once | ||
|
|
||
| #include <vector> | ||
|
|
||
| #include <bsoncxx/builder/basic/array.hpp> | ||
| #include <bsoncxx/builder/basic/document.hpp> | ||
| #include <bsoncxx/document/view_or_value.hpp> | ||
| #include <bsoncxx/string/to_string.hpp> | ||
| #include <bsoncxx/types/bson_value/view.hpp> | ||
| #include <mongocxx/config/private/prelude.hh> | ||
| #include <mongocxx/exception/error_code.hpp> | ||
| #include <mongocxx/exception/logic_error.hpp> | ||
| #include <mongocxx/exception/operation_exception.hpp> | ||
| #include <mongocxx/exception/write_exception.hpp> | ||
| #include <mongocxx/options/index_view.hpp> | ||
| #include <mongocxx/private/client_session.hh> | ||
| #include <mongocxx/private/libbson.hh> | ||
| #include <mongocxx/private/libmongoc.hh> | ||
|
|
||
| #include <mongocxx/config/private/prelude.hh> | ||
| #include <vector> | ||
|
|
||
| namespace mongocxx { | ||
| MONGOCXX_INLINE_NAMESPACE_BEGIN | ||
|
|
@@ -39,7 +38,8 @@ using bsoncxx::builder::basic::kvp; | |
|
|
||
| class index_view::impl { | ||
| public: | ||
| impl(mongoc_collection_t* collection) : _coll{collection} {} | ||
| impl(mongoc_collection_t* collection, mongoc_client_t* client) | ||
| : _coll{collection}, _client{client} {} | ||
|
|
||
| impl(const impl& i) = default; | ||
|
|
||
|
|
@@ -134,6 +134,26 @@ class index_view::impl { | |
| opts_doc.append(bsoncxx::builder::concatenate_doc{session->_get_impl().to_document()}); | ||
| } | ||
|
|
||
| if (options.commit_quorum()) { | ||
jmikola marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| auto server_description = scoped_server_description(libmongoc::client_select_server( | ||
| _client, true /* for_writes */, nullptr /* read_prefs */, &error)); | ||
| if (!server_description.sd) | ||
| throw_exception<write_exception>(error); | ||
|
|
||
| auto is_master = libmongoc::server_description_ismaster(server_description.sd); | ||
|
|
||
| bson_iter_t iter; | ||
| if (!bson_iter_init_find(&iter, is_master, "maxWireVersion") || | ||
| bson_iter_int32(&iter) < 9) { | ||
| throw write_exception{ | ||
| error_code::k_invalid_parameter, | ||
| "option 'commitQuorum' not available on the current server version"}; | ||
| } | ||
|
|
||
| command = | ||
| make_document(concatenate(command), concatenate(options.commit_quorum()->view())); | ||
| } | ||
|
|
||
| libbson::scoped_bson_t command_bson{command}; | ||
| libbson::scoped_bson_t opts_bson{opts_doc.view()}; | ||
|
|
||
|
|
@@ -219,6 +239,16 @@ class index_view::impl { | |
| } | ||
|
|
||
| mongoc_collection_t* _coll; | ||
| mongoc_client_t* _client; | ||
|
|
||
| class scoped_server_description { | ||
| public: | ||
| explicit scoped_server_description(mongoc_server_description_t* sd) : sd(sd) {} | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh nice, I forgot about marking that explicit. |
||
| ~scoped_server_description() { | ||
| mongoc_server_description_destroy(sd); | ||
| } | ||
| mongoc_server_description_t* sd; | ||
| }; | ||
| }; | ||
| MONGOCXX_INLINE_NAMESPACE_END | ||
| } // namespace mongocxx | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -172,6 +172,46 @@ TEST_CASE("create_one", "[index_view]") { | |
| REQUIRE_NOTHROW(indexes.create_one(keys1.view(), options.view())); | ||
| REQUIRE_THROWS_AS(indexes.create_one(keys2.view(), options.view()), operation_exception); | ||
| } | ||
|
|
||
| SECTION("commitQuorum option") { | ||
| if (test_util::get_topology(mongodb_client) == "single") { | ||
| WARN("skip: commitQuorum option requires a replica set"); | ||
| return; | ||
| } | ||
|
|
||
| collection coll = db["index_view_create_one_commit_quorum"]; | ||
| coll.drop(); | ||
| coll.insert_one({}); // Ensure that the collection exists. | ||
| index_view indexes = coll.indexes(); | ||
|
|
||
| auto key = make_document(kvp("a", 1)); | ||
| index_model model(key.view()); | ||
| options::index_view options; | ||
|
|
||
| auto commit_quorum_regex = | ||
| Catch::Matches("(.*)commit( )?quorum(.*)", Catch::CaseSensitive::No); | ||
bazile-clyde marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| bool is_supported = test_util::get_max_wire_version(mongodb_client) >= 9; | ||
| CAPTURE(is_supported); | ||
|
|
||
| SECTION("works with int") { | ||
| options.commit_quorum(1); | ||
| if (is_supported) { | ||
| REQUIRE_NOTHROW(indexes.create_one(model, options)); | ||
| } else { | ||
| REQUIRE_THROWS_WITH(indexes.create_one(model, options), commit_quorum_regex); | ||
| } | ||
| } | ||
|
|
||
| SECTION("works with string") { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This section, and the section below, appear to be the ones failing. As the capturing of IIUC an exception is expected because the test runs against a single topology, but "majority" should only be supported on a replica set. And the section below expects a server to always reject the value "bad_str" I still believe this is due to the intermediate 4.3.0 server version this is running against on zSeries. The server may have implemented validation after 4.3.0, so the reported wire version is still 9, but invalid commitQuorum values are still accepted. https://jira.mongodb.org/browse/SERVER-41846 in particular (which has fixVersion 4.3.1) seems related. We could investigate further and check with the server team. But IMO, it is less important that we test server side validation of these values, and more important that we test that I'd argue for removing the "fails with invalid string" test below, and only testing the "is_supported" case here.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Per our discussion, I skipped single typologies and got rid of the |
||
| options.commit_quorum("majority"); | ||
| if (is_supported) { | ||
| REQUIRE_NOTHROW(indexes.create_one(model, options)); | ||
| } else { | ||
| REQUIRE_THROWS_WITH(indexes.create_one(model, options), commit_quorum_regex); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| TEST_CASE("create_many", "[index_view]") { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.