Skip to content

CDRIVER-3625 monitor with a thread-per-server #607

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .evergreen/compile-unix.sh
Original file line number Diff line number Diff line change
Expand Up @@ -262,10 +262,10 @@ mkfifo pipe || true
if [ -e pipe ]; then
set +o xtrace
tee error.log < pipe &
run_valgrind ./src/libmongoc/test-libmongoc -d -F test-results.json 2>pipe
run_valgrind ./src/libmongoc/test-libmongoc --no-fork -d -F test-results.json 2>pipe
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lets debug logs to print to stdout. There's likely a better way to
accomplish that. But the only downside of running with --no-fork I'm aware of is
that the first test failure fails the entire process. That did not seem like a
big loss to me. Though, I'm happy to change back and look for alternatives if
there's disagreement

rm pipe
else
run_valgrind ./src/libmongoc/test-libmongoc -d -F test-results.json
run_valgrind ./src/libmongoc/test-libmongoc --no-fork -d -F test-results.json
fi

# Check if the error.log exists, and is more than 0 byte
Expand Down
6 changes: 3 additions & 3 deletions .evergreen/run-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ case "$OS" in
check_mongocryptd

chmod +x src/libmongoc/Debug/test-libmongoc.exe
./src/libmongoc/Debug/test-libmongoc.exe $TEST_ARGS
./src/libmongoc/Debug/test-libmongoc.exe $TEST_ARGS -d
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried putting --no-fork here, but then stumbled on an existing bug: https://jira.mongodb.org/browse/CDRIVER-3637

;;

*)
Expand All @@ -104,9 +104,9 @@ case "$OS" in

if [ "$VALGRIND" = "on" ]; then
. $DIR/valgrind.sh
run_valgrind ./src/libmongoc/test-libmongoc --no-fork $TEST_ARGS
run_valgrind ./src/libmongoc/test-libmongoc --no-fork $TEST_ARGS -d
else
./src/libmongoc/test-libmongoc --no-fork $TEST_ARGS
./src/libmongoc/test-libmongoc --no-fork $TEST_ARGS -d
fi

;;
Expand Down
2 changes: 2 additions & 0 deletions src/libmongoc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,7 @@ set (SOURCES ${SOURCES}
${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-stream-gridfs-upload.c
${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-stream-socket.c
${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-topology.c
${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-topology-background-monitoring.c
${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-topology-description.c
${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-topology-description-apm.c
${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-topology-scanner.c
Expand Down Expand Up @@ -866,6 +867,7 @@ set (test-libmongoc-sources
${PROJECT_SOURCE_DIR}/tests/test-mongoc-array.c
${PROJECT_SOURCE_DIR}/tests/test-mongoc-async.c
${PROJECT_SOURCE_DIR}/tests/test-mongoc-aws.c
${PROJECT_SOURCE_DIR}/tests/test-mongoc-background-monitoring.c
${PROJECT_SOURCE_DIR}/tests/test-mongoc-buffer.c
${PROJECT_SOURCE_DIR}/tests/test-mongoc-bulk.c
${PROJECT_SOURCE_DIR}/tests/test-mongoc-change-stream.c
Expand Down
2 changes: 2 additions & 0 deletions src/libmongoc/src/mongoc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ set (src_libmongoc_src_mongoc_DIST_noinst_hs
mongoc-topology-description-apm-private.h
mongoc-topology-description-private.h
mongoc-topology-private.h
mongoc-topology-background-monitoring-private.h
mongoc-topology-scanner-private.h
mongoc-trace-private.h
mongoc-uri-private.h
Expand All @@ -170,6 +171,7 @@ set (src_libmongoc_src_mongoc_DIST_cs
mongoc-array.c
mongoc-async.c
mongoc-async-cmd.c
mongoc-topology-background-monitoring.c
mongoc-buffer.c
mongoc-bulk-operation.c
mongoc-change-stream.c
Expand Down
8 changes: 5 additions & 3 deletions src/libmongoc/src/mongoc/mongoc-client-pool.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "mongoc-queue-private.h"
#include "mongoc-thread-private.h"
#include "mongoc-topology-private.h"
#include "mongoc-topology-background-monitoring-private.h"
#include "mongoc-trace-private.h"

#ifdef MONGOC_ENABLE_SSL
Expand Down Expand Up @@ -218,9 +219,10 @@ mongoc_client_pool_destroy (mongoc_client_pool_t *pool)
static void
_start_scanner_if_needed (mongoc_client_pool_t *pool)
{
if (!_mongoc_topology_start_background_scanner (pool->topology)) {
MONGOC_ERROR ("Background scanner did not start!");
abort ();
if (!pool->topology->single_threaded) {
bson_mutex_lock (&pool->topology->mutex);
_mongoc_topology_background_monitoring_start (pool->topology);
bson_mutex_unlock (&pool->topology->mutex);
}
}

Expand Down
8 changes: 8 additions & 0 deletions src/libmongoc/src/mongoc/mongoc-client-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,14 @@ mongoc_stream_t *
mongoc_client_connect_tcp (int32_t connecttimeoutms,
const mongoc_host_list_t *host,
bson_error_t *error);

mongoc_stream_t *
mongoc_client_connect (bool buffered,
bool use_ssl,
void *ssl_opts_void,
const mongoc_uri_t *uri,
const mongoc_host_list_t *host,
bson_error_t *error);
BSON_END_DECLS

#endif /* MONGOC_CLIENT_PRIVATE_H */
99 changes: 63 additions & 36 deletions src/libmongoc/src/mongoc/mongoc-client.c
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ _mongoc_get_rr_search (const char *service,
*
* Fetch an SRV or TXT resource record and update put results in
* @rr_data.
*
*
* See RFCs 1464 and 2782, MongoDB's "Initial DNS Seedlist Discovery"
* spec, and MongoDB's "Polling SRV Records for Mongos Discovery"
* spec.
Expand Down Expand Up @@ -644,10 +644,12 @@ mongoc_client_connect_tcp (int32_t connecttimeoutms,
hints.ai_flags = 0;
hints.ai_protocol = 0;

TRACE ("DNS lookup for %s", host->host);
s = getaddrinfo (host->host, portstr, &hints, &result);

if (s != 0) {
mongoc_counter_dns_failure_inc ();
TRACE ("Failed to resolve %s", host->host);
bson_set_error (error,
MONGOC_ERROR_STREAM,
MONGOC_ERROR_STREAM_NAME_RESOLUTION,
Expand Down Expand Up @@ -764,49 +766,26 @@ mongoc_client_connect_unix (const mongoc_host_list_t *host, bson_error_t *error)
#endif
}


/*
*--------------------------------------------------------------------------
*
* mongoc_client_default_stream_initiator --
*
* A mongoc_stream_initiator_t that will handle the various type
* of supported sockets by MongoDB including TCP and UNIX.
*
* Language binding authors may want to implement an alternate
* version of this method to use their native stream format.
*
* Returns:
* A mongoc_stream_t if successful; otherwise NULL and @error is set.
*
* Side effects:
* @error is set if return value is NULL.
*
*--------------------------------------------------------------------------
*/

mongoc_stream_t *
mongoc_client_default_stream_initiator (const mongoc_uri_t *uri,
const mongoc_host_list_t *host,
void *user_data,
bson_error_t *error)
mongoc_client_connect (bool buffered,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The server monitors do not have a mongoc_client_t, but need to create a stream to a server given just a host address.

mongoc_client_connect establishes connection with just the address and SSL options, and is used by mongoc_client_default_stream_initiator, which takes the SSL options from a mongoc_client_t.

bool use_ssl,
void *ssl_opts_void,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sidenote: I found it frustrating to work with mongoc_ssl_opt_t. Since it is conditionally defined, using it requires either hiding the type or using typedefs.

const mongoc_uri_t *uri,
const mongoc_host_list_t *host,
bson_error_t *error)
{
mongoc_stream_t *base_stream = NULL;
int32_t connecttimeoutms;
#ifdef MONGOC_ENABLE_SSL
mongoc_client_t *client = (mongoc_client_t *) user_data;
const char *mechanism;
#endif

BSON_ASSERT (uri);
BSON_ASSERT (host);

#ifndef MONGOC_ENABLE_SSL
if (mongoc_uri_get_tls (uri)) {
if (ssl_opts_void || mongoc_uri_get_tls (uri)) {
bson_set_error (error,
MONGOC_ERROR_CLIENT,
MONGOC_ERROR_CLIENT_NO_ACCEPTABLE_PEER,
"SSL is not enabled in this build of mongo-c-driver.");
"TLS is not enabled in this build of mongo-c-driver.");
return NULL;
}
#endif
Expand Down Expand Up @@ -836,14 +815,17 @@ mongoc_client_default_stream_initiator (const mongoc_uri_t *uri,

#ifdef MONGOC_ENABLE_SSL
if (base_stream) {
mongoc_ssl_opt_t *ssl_opts;
const char *mechanism;

ssl_opts = (mongoc_ssl_opt_t *) ssl_opts_void;
mechanism = mongoc_uri_get_auth_mechanism (uri);

if (client->use_ssl ||
(mechanism && (0 == strcmp (mechanism, "MONGODB-X509")))) {
if (use_ssl || (mechanism && (0 == strcmp (mechanism, "MONGODB-X509")))) {
mongoc_stream_t *original = base_stream;

base_stream = mongoc_stream_tls_new_with_hostname (
base_stream, host->host, &client->ssl_opts, true);
base_stream, host->host, ssl_opts, true);

if (!base_stream) {
mongoc_stream_destroy (original);
Expand All @@ -863,9 +845,54 @@ mongoc_client_default_stream_initiator (const mongoc_uri_t *uri,
}
#endif

return base_stream ? mongoc_stream_buffered_new (base_stream, 1024) : NULL;
if (!base_stream) {
return NULL;
}
if (buffered) {
return mongoc_stream_buffered_new (base_stream, 1024);
}
return base_stream;
}

/*
*--------------------------------------------------------------------------
*
* mongoc_client_default_stream_initiator --
*
* A mongoc_stream_initiator_t that will handle the various type
* of supported sockets by MongoDB including TCP and UNIX.
*
* Language binding authors may want to implement an alternate
* version of this method to use their native stream format.
*
* Returns:
* A mongoc_stream_t if successful; otherwise NULL and @error is set.
*
* Side effects:
* @error is set if return value is NULL.
*
*--------------------------------------------------------------------------
*/

mongoc_stream_t *
mongoc_client_default_stream_initiator (const mongoc_uri_t *uri,
const mongoc_host_list_t *host,
void *user_data,
bson_error_t *error)
{
void *ssl_opts_void = NULL;
bool use_ssl = false;
#ifdef MONGOC_ENABLE_SSL
mongoc_client_t *client = (mongoc_client_t *) user_data;

use_ssl = client->use_ssl;
ssl_opts_void = (void *) &client->ssl_opts;

#endif

return mongoc_client_connect (
true, use_ssl, ssl_opts_void, uri, host, error);
}

/*
*--------------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@

/*
* Copyright 2020-present MongoDB, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include "mongoc-prelude.h"

#ifndef MONGOC_TOPOLOGY_BACKGROUND_MONITORING_PRIVATE_H
#define MONGOC_TOPOLOGY_BACKGROUND_MONITORING_PRIVATE_H

/* Methods of mongoc_topology_t for managing background monitoring. */

struct _mongoc_topology_t;

void
_mongoc_topology_background_monitoring_start (
struct _mongoc_topology_t *topology);

void
_mongoc_topology_background_monitoring_reconcile (
struct _mongoc_topology_t *topology);

void
_mongoc_topology_background_monitoring_request_scan (
struct _mongoc_topology_t *topology);

void
_mongoc_topology_background_monitoring_stop (
struct _mongoc_topology_t *topology);

#endif /* MONGOC_TOPOLOGY_BACKGROUND_MONITORING_PRIVATE_H */
Loading