From a8fa9007d3c8176daaa7a791f67c3353dc627143 Mon Sep 17 00:00:00 2001 From: Kaitlin Mahar Date: Tue, 26 May 2020 15:44:17 -0400 Subject: [PATCH 1/3] Add patch files and update vendoring script --- etc/CDRIVER-3532-directConnection.diff | 153 ++++++++++++++++++ ...3623-fix-setting-apm-in-pooled-client.diff | 17 ++ etc/vendor-libmongoc.sh | 8 + 3 files changed, 178 insertions(+) create mode 100644 etc/CDRIVER-3532-directConnection.diff create mode 100644 etc/CDRIVER-3623-fix-setting-apm-in-pooled-client.diff diff --git a/etc/CDRIVER-3532-directConnection.diff b/etc/CDRIVER-3532-directConnection.diff new file mode 100644 index 000000000..697cedc73 --- /dev/null +++ b/etc/CDRIVER-3532-directConnection.diff @@ -0,0 +1,153 @@ +diff --git a/Sources/CLibMongoC/mongoc/mongoc-topology.c b/Sources/CLibMongoC/mongoc/mongoc-topology.c +index 987f98e4e..7b9136bfc 100644 +--- a/Sources/CLibMongoC/mongoc/mongoc-topology.c ++++ a/Sources/CLibMongoC/mongoc/mongoc-topology.c +@@ -210,6 +210,8 @@ mongoc_topology_new (const mongoc_uri_t *uri, bool single_threaded) + uint32_t id; + const mongoc_host_list_t *hl; + mongoc_rr_data_t rr_data; ++ bool has_directconnection; ++ bool directconnection; + + BSON_ASSERT (uri); + +@@ -328,12 +330,35 @@ mongoc_topology_new (const mongoc_uri_t *uri, bool single_threaded) + + /* + * Set topology type from URI: +- * - if we've got a replicaSet name, initialize to RS_NO_PRIMARY +- * - otherwise, if the seed list has a single host, initialize to SINGLE ++ * + if directConnection=true ++ * - whether or not we have a replicaSet name, initialize to SINGLE ++ * (directConnect with SRV or multiple hosts triggers a URI parse error) ++ * + if directConnection=false ++ * - if we've got a replicaSet name, initialize to RS_NO_PRIMARY ++ * - otherwise, initialize to UNKNOWN ++ * + if directConnection was not specified in the URI (old behavior) ++ * - if we've got a replicaSet name, initialize to RS_NO_PRIMARY ++ * - otherwise, if the seed list has a single host, initialize to SINGLE + * - everything else gets initialized to UNKNOWN + */ ++ has_directconnection = mongoc_uri_has_option ( ++ uri, MONGOC_URI_DIRECTCONNECTION); ++ directconnection = has_directconnection && ++ mongoc_uri_get_option_as_bool (uri, MONGOC_URI_DIRECTCONNECTION, false); + hl = mongoc_uri_get_hosts (topology->uri); +- if (mongoc_uri_get_replica_set (topology->uri)) { ++ if (service && !has_directconnection) { ++ init_type = MONGOC_TOPOLOGY_UNKNOWN; ++ } else if (has_directconnection) { ++ if (directconnection) { ++ init_type = MONGOC_TOPOLOGY_SINGLE; ++ } else { ++ if (mongoc_uri_get_replica_set (topology->uri)) { ++ init_type = MONGOC_TOPOLOGY_RS_NO_PRIMARY; ++ } else { ++ init_type = MONGOC_TOPOLOGY_UNKNOWN; ++ } ++ } ++ } else if (mongoc_uri_get_replica_set (topology->uri)) { + init_type = MONGOC_TOPOLOGY_RS_NO_PRIMARY; + } else { + if (hl && hl->next) { +diff --git a/Sources/CLibMongoC/mongoc/mongoc-uri.c b/Sources/CLibMongoC/mongoc/mongoc-uri.c +index 8e6fa149e..31ac05272 100644 +--- a/Sources/CLibMongoC/mongoc/mongoc-uri.c ++++ b/Sources/CLibMongoC/mongoc/mongoc-uri.c +@@ -702,6 +702,14 @@ mongoc_uri_bson_append_or_replace_key (bson_t *options, + } + + ++bool ++mongoc_uri_has_option (const mongoc_uri_t *uri, const char *key) ++{ ++ bson_iter_t iter; ++ ++ return bson_iter_init_find_case (&iter, &uri->options, key); ++} ++ + bool + mongoc_uri_option_is_int32 (const char *key) + { +@@ -731,6 +739,7 @@ bool + mongoc_uri_option_is_bool (const char *key) + { + return !strcasecmp (key, MONGOC_URI_CANONICALIZEHOSTNAME) || ++ !strcasecmp (key, MONGOC_URI_DIRECTCONNECTION) || + !strcasecmp (key, MONGOC_URI_JOURNAL) || + !strcasecmp (key, MONGOC_URI_RETRYREADS) || + !strcasecmp (key, MONGOC_URI_RETRYWRITES) || +@@ -1390,6 +1399,41 @@ mongoc_uri_finalize_auth (mongoc_uri_t *uri, + return true; + } + ++static bool ++mongoc_uri_finalize_directconnection (mongoc_uri_t *uri, bson_error_t *error) ++{ ++ bool directconnection = false; ++ ++ directconnection = ++ mongoc_uri_get_option_as_bool (uri, MONGOC_URI_DIRECTCONNECTION, false); ++ if (!directconnection) { ++ return true; ++ } ++ ++ /* URI options spec: "The driver MUST report an error if the ++ * directConnection=true URI option is specified with an SRV URI, because ++ * the URI may resolve to multiple hosts. The driver MUST allow specifying ++ * directConnection=false URI option with an SRV URI." */ ++ if (uri->is_srv) { ++ MONGOC_URI_ERROR ( ++ error, "%s", "SRV URI not allowed with directConnection option"); ++ return false; ++ } ++ ++ /* URI options spec: "The driver MUST report an error if the ++ * directConnection=true URI option is specified with multiple seeds." */ ++ if (uri->hosts && uri->hosts->next) { ++ MONGOC_URI_ERROR ( ++ error, ++ "%s", ++ "Multiple seeds not allowed with directConnection option"); ++ return false; ++ } ++ ++ return true; ++ ++} ++ + static bool + mongoc_uri_parse_before_slash (mongoc_uri_t *uri, + const char *before_slash, +@@ -1502,6 +1546,10 @@ mongoc_uri_parse (mongoc_uri_t *uri, const char *str, bson_error_t *error) + goto error; + } + ++ if (!mongoc_uri_finalize_directconnection (uri, error)) { ++ goto error; ++ } ++ + bson_free (before_slash); + return true; + +diff --git a/Sources/CLibMongoC/include/CLibMongoC_mongoc-uri.h b/Sources/CLibMongoC/include/CLibMongoC_mongoc-uri.h +index a190b1f71..e08b7d43f 100644 +--- a/Sources/CLibMongoC/include/CLibMongoC_mongoc-uri.h ++++ b/Sources/CLibMongoC/include/CLibMongoC_mongoc-uri.h +@@ -40,6 +40,7 @@ + #define MONGOC_URI_CANONICALIZEHOSTNAME "canonicalizehostname" + #define MONGOC_URI_CONNECTTIMEOUTMS "connecttimeoutms" + #define MONGOC_URI_COMPRESSORS "compressors" ++#define MONGOC_URI_DIRECTCONNECTION "directconnection" + #define MONGOC_URI_GSSAPISERVICENAME "gssapiservicename" + #define MONGOC_URI_HEARTBEATFREQUENCYMS "heartbeatfrequencyms" + #define MONGOC_URI_JOURNAL "journal" +@@ -120,6 +121,8 @@ mongoc_uri_get_password (const mongoc_uri_t *uri); + MONGOC_EXPORT (bool) + mongoc_uri_set_password (mongoc_uri_t *uri, const char *password); + MONGOC_EXPORT (bool) ++mongoc_uri_has_option (const mongoc_uri_t *uri, const char *key); ++MONGOC_EXPORT (bool) + mongoc_uri_option_is_int32 (const char *key); + MONGOC_EXPORT (bool) + mongoc_uri_option_is_int64 (const char *key); diff --git a/etc/CDRIVER-3623-fix-setting-apm-in-pooled-client.diff b/etc/CDRIVER-3623-fix-setting-apm-in-pooled-client.diff new file mode 100644 index 000000000..f86d27aa7 --- /dev/null +++ b/etc/CDRIVER-3623-fix-setting-apm-in-pooled-client.diff @@ -0,0 +1,17 @@ +diff --git a/Sources/CLibMongoC/mongoc/mongoc-client.c b/Sources/CLibMongoC/mongoc/mongoc-client.c +index ea17e356c..77abca8e0 100644 +--- a/Sources/CLibMongoC/mongoc/mongoc-client.c ++++ b/Sources/CLibMongoC/mongoc/mongoc-client.c +@@ -2689,7 +2689,11 @@ _mongoc_client_set_apm_callbacks_private (mongoc_client_t *client, + } + + client->apm_context = context; +- mongoc_topology_set_apm_callbacks (client->topology, callbacks, context); ++ ++ /* A client pool sets APM callbacks for the entire pool. */ ++ if (client->topology->single_threaded) { ++ mongoc_topology_set_apm_callbacks (client->topology, callbacks, context); ++ } + + return true; + } diff --git a/etc/vendor-libmongoc.sh b/etc/vendor-libmongoc.sh index ffd7fecd8..565e88ffb 100755 --- a/etc/vendor-libmongoc.sh +++ b/etc/vendor-libmongoc.sh @@ -131,6 +131,14 @@ echo "RENAMING header files" echo "PATCHING libmongoc" git apply "${ETC_DIR}/inttypes-non-modular-header-workaround.diff" +# These patches are temporary workarounds to give us early access to the directConnection URI option and a bug fix. +# This should be removed from the script when we update our vendored libmongoc to 1.17 via SWIFT-766. +git apply "${ETC_DIR}/CDRIVER-3532-directConnection.diff" +git apply "${ETC_DIR}/CDRIVER-3623-fix-setting-apm-in-pooled-client.diff" + +# This is a temporary workaround to bring in support for the directConnection URI option. +# This and the patch file should be removed as part of SWIFT-766. + # Clang modules are build by a conventional structure with an `include` folder for public # includes, and an umbrella header used as the primary entry point. As part of the vendoring # process, we copy in our own handwritten umbrella file. Currently, there is no generated From 2912ea11afefd5697072621dd20de53f2207545b Mon Sep 17 00:00:00 2001 From: Kaitlin Mahar Date: Tue, 26 May 2020 15:46:06 -0400 Subject: [PATCH 2/3] Rerun vendoring script --- .../include/CLibMongoC_mongoc-uri.h | 3 ++ Sources/CLibMongoC/mongoc/mongoc-client.c | 6 ++- Sources/CLibMongoC/mongoc/mongoc-topology.c | 31 ++++++++++-- Sources/CLibMongoC/mongoc/mongoc-uri.c | 48 +++++++++++++++++++ 4 files changed, 84 insertions(+), 4 deletions(-) diff --git a/Sources/CLibMongoC/include/CLibMongoC_mongoc-uri.h b/Sources/CLibMongoC/include/CLibMongoC_mongoc-uri.h index 5973d6074..81507c0f7 100644 --- a/Sources/CLibMongoC/include/CLibMongoC_mongoc-uri.h +++ b/Sources/CLibMongoC/include/CLibMongoC_mongoc-uri.h @@ -40,6 +40,7 @@ #define MONGOC_URI_CANONICALIZEHOSTNAME "canonicalizehostname" #define MONGOC_URI_CONNECTTIMEOUTMS "connecttimeoutms" #define MONGOC_URI_COMPRESSORS "compressors" +#define MONGOC_URI_DIRECTCONNECTION "directconnection" #define MONGOC_URI_GSSAPISERVICENAME "gssapiservicename" #define MONGOC_URI_HEARTBEATFREQUENCYMS "heartbeatfrequencyms" #define MONGOC_URI_JOURNAL "journal" @@ -117,6 +118,8 @@ mongoc_uri_get_password (const mongoc_uri_t *uri); MONGOC_EXPORT (bool) mongoc_uri_set_password (mongoc_uri_t *uri, const char *password); MONGOC_EXPORT (bool) +mongoc_uri_has_option (const mongoc_uri_t *uri, const char *key); +MONGOC_EXPORT (bool) mongoc_uri_option_is_int32 (const char *key); MONGOC_EXPORT (bool) mongoc_uri_option_is_int64 (const char *key); diff --git a/Sources/CLibMongoC/mongoc/mongoc-client.c b/Sources/CLibMongoC/mongoc/mongoc-client.c index 735310fbf..94a80346b 100644 --- a/Sources/CLibMongoC/mongoc/mongoc-client.c +++ b/Sources/CLibMongoC/mongoc/mongoc-client.c @@ -2670,7 +2670,11 @@ _mongoc_client_set_apm_callbacks_private (mongoc_client_t *client, } client->apm_context = context; - mongoc_topology_set_apm_callbacks (client->topology, callbacks, context); + + /* A client pool sets APM callbacks for the entire pool. */ + if (client->topology->single_threaded) { + mongoc_topology_set_apm_callbacks (client->topology, callbacks, context); + } return true; } diff --git a/Sources/CLibMongoC/mongoc/mongoc-topology.c b/Sources/CLibMongoC/mongoc/mongoc-topology.c index 03e6c090c..d27e3e82f 100644 --- a/Sources/CLibMongoC/mongoc/mongoc-topology.c +++ b/Sources/CLibMongoC/mongoc/mongoc-topology.c @@ -210,6 +210,8 @@ mongoc_topology_new (const mongoc_uri_t *uri, bool single_threaded) uint32_t id; const mongoc_host_list_t *hl; mongoc_rr_data_t rr_data; + bool has_directconnection; + bool directconnection; BSON_ASSERT (uri); @@ -328,12 +330,35 @@ mongoc_topology_new (const mongoc_uri_t *uri, bool single_threaded) /* * Set topology type from URI: - * - if we've got a replicaSet name, initialize to RS_NO_PRIMARY - * - otherwise, if the seed list has a single host, initialize to SINGLE + * + if directConnection=true + * - whether or not we have a replicaSet name, initialize to SINGLE + * (directConnect with SRV or multiple hosts triggers a URI parse error) + * + if directConnection=false + * - if we've got a replicaSet name, initialize to RS_NO_PRIMARY + * - otherwise, initialize to UNKNOWN + * + if directConnection was not specified in the URI (old behavior) + * - if we've got a replicaSet name, initialize to RS_NO_PRIMARY + * - otherwise, if the seed list has a single host, initialize to SINGLE * - everything else gets initialized to UNKNOWN */ + has_directconnection = mongoc_uri_has_option ( + uri, MONGOC_URI_DIRECTCONNECTION); + directconnection = has_directconnection && + mongoc_uri_get_option_as_bool (uri, MONGOC_URI_DIRECTCONNECTION, false); hl = mongoc_uri_get_hosts (topology->uri); - if (mongoc_uri_get_replica_set (topology->uri)) { + if (service && !has_directconnection) { + init_type = MONGOC_TOPOLOGY_UNKNOWN; + } else if (has_directconnection) { + if (directconnection) { + init_type = MONGOC_TOPOLOGY_SINGLE; + } else { + if (mongoc_uri_get_replica_set (topology->uri)) { + init_type = MONGOC_TOPOLOGY_RS_NO_PRIMARY; + } else { + init_type = MONGOC_TOPOLOGY_UNKNOWN; + } + } + } else if (mongoc_uri_get_replica_set (topology->uri)) { init_type = MONGOC_TOPOLOGY_RS_NO_PRIMARY; } else { if (hl && hl->next) { diff --git a/Sources/CLibMongoC/mongoc/mongoc-uri.c b/Sources/CLibMongoC/mongoc/mongoc-uri.c index cad80d65d..a12ccbbc7 100644 --- a/Sources/CLibMongoC/mongoc/mongoc-uri.c +++ b/Sources/CLibMongoC/mongoc/mongoc-uri.c @@ -702,6 +702,14 @@ mongoc_uri_bson_append_or_replace_key (bson_t *options, } +bool +mongoc_uri_has_option (const mongoc_uri_t *uri, const char *key) +{ + bson_iter_t iter; + + return bson_iter_init_find_case (&iter, &uri->options, key); +} + bool mongoc_uri_option_is_int32 (const char *key) { @@ -731,6 +739,7 @@ bool mongoc_uri_option_is_bool (const char *key) { return !strcasecmp (key, MONGOC_URI_CANONICALIZEHOSTNAME) || + !strcasecmp (key, MONGOC_URI_DIRECTCONNECTION) || !strcasecmp (key, MONGOC_URI_JOURNAL) || !strcasecmp (key, MONGOC_URI_RETRYREADS) || !strcasecmp (key, MONGOC_URI_RETRYWRITES) || @@ -1335,6 +1344,41 @@ mongoc_uri_finalize_auth (mongoc_uri_t *uri, return true; } +static bool +mongoc_uri_finalize_directconnection (mongoc_uri_t *uri, bson_error_t *error) +{ + bool directconnection = false; + + directconnection = + mongoc_uri_get_option_as_bool (uri, MONGOC_URI_DIRECTCONNECTION, false); + if (!directconnection) { + return true; + } + + /* URI options spec: "The driver MUST report an error if the + * directConnection=true URI option is specified with an SRV URI, because + * the URI may resolve to multiple hosts. The driver MUST allow specifying + * directConnection=false URI option with an SRV URI." */ + if (uri->is_srv) { + MONGOC_URI_ERROR ( + error, "%s", "SRV URI not allowed with directConnection option"); + return false; + } + + /* URI options spec: "The driver MUST report an error if the + * directConnection=true URI option is specified with multiple seeds." */ + if (uri->hosts && uri->hosts->next) { + MONGOC_URI_ERROR ( + error, + "%s", + "Multiple seeds not allowed with directConnection option"); + return false; + } + + return true; + +} + static bool mongoc_uri_parse_before_slash (mongoc_uri_t *uri, const char *before_slash, @@ -1447,6 +1491,10 @@ mongoc_uri_parse (mongoc_uri_t *uri, const char *str, bson_error_t *error) goto error; } + if (!mongoc_uri_finalize_directconnection (uri, error)) { + goto error; + } + bson_free (before_slash); return true; From e4206d3eb3925b70a5d07f8c4c0854c39301d67f Mon Sep 17 00:00:00 2001 From: Kaitlin Mahar Date: Wed, 27 May 2020 13:13:34 -0400 Subject: [PATCH 3/3] remove extra comment in script --- etc/vendor-libmongoc.sh | 3 --- 1 file changed, 3 deletions(-) diff --git a/etc/vendor-libmongoc.sh b/etc/vendor-libmongoc.sh index 565e88ffb..e4a096388 100755 --- a/etc/vendor-libmongoc.sh +++ b/etc/vendor-libmongoc.sh @@ -136,9 +136,6 @@ git apply "${ETC_DIR}/inttypes-non-modular-header-workaround.diff" git apply "${ETC_DIR}/CDRIVER-3532-directConnection.diff" git apply "${ETC_DIR}/CDRIVER-3623-fix-setting-apm-in-pooled-client.diff" -# This is a temporary workaround to bring in support for the directConnection URI option. -# This and the patch file should be removed as part of SWIFT-766. - # Clang modules are build by a conventional structure with an `include` folder for public # includes, and an umbrella header used as the primary entry point. As part of the vendoring # process, we copy in our own handwritten umbrella file. Currently, there is no generated