Skip to content

CDRIVER-4113: Thread-safe pool for server session objects #847

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

Merged
merged 23 commits into from
Sep 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
77b0eaf
CXX-4113: Thread-safe pool for server session objects
vector-of-bool Aug 11, 2021
da8b2ae
Remove unused node pointers on server sessions
vector-of-bool Aug 16, 2021
93abae4
Track pool size, and don't loop while holding the lock to clear
vector-of-bool Aug 16, 2021
a96145d
Missing dist listing for ts-pool files
vector-of-bool Aug 16, 2021
783ff1e
Cleanup and some redesign of pool API
vector-of-bool Aug 19, 2021
cf0528e
Declare special pool for server sessions
vector-of-bool Aug 19, 2021
fd0af0d
Fix pruning pool not leaking the 10,001st session
vector-of-bool Aug 20, 2021
95ba5e5
Use typed pool fns
vector-of-bool Aug 20, 2021
bc6e22e
Visiting pool items, and checking that items are returned to the pool
vector-of-bool Aug 21, 2021
a967c74
Merge branch 'master' into CDRIVER-4113
vector-of-bool Aug 23, 2021
e9af59e
Fix unacknowledged sessions tests
vector-of-bool Aug 23, 2021
978529f
Merge remote-tracking branch 'upstream/master' into CDRIVER-4113
vector-of-bool Aug 24, 2021
750aa74
Merge branch 'master' into CDRIVER-4113
vector-of-bool Sep 1, 2021
ea7a98d
Fix missing 'inline's
vector-of-bool Sep 1, 2021
c8efbae
Mention the LIFO nature of the thread-pool, and the non-conformance o…
vector-of-bool Sep 1, 2021
653f966
auditing the pool is compile-time optional
vector-of-bool Sep 1, 2021
7cebe51
Missed some PR comments
vector-of-bool Sep 3, 2021
9441b1c
Remove -Waggregate-return
vector-of-bool Sep 7, 2021
8f113c9
No being clever with pool auditing
vector-of-bool Sep 7, 2021
fde797e
Merge remote-tracking branch 'upstream/master' into CDRIVER-4113
vector-of-bool Sep 10, 2021
eb35bb2
Fix unused variables and function warnings
vector-of-bool Sep 10, 2021
4802f10
Fix leak of mongoc_server_session_t::lsid in case of uuid failure
vector-of-bool Sep 10, 2021
16752df
Address pr comments
vector-of-bool Sep 13, 2021
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
1 change: 0 additions & 1 deletion build/maintainer-flags.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
-Wall
-Waggregate-return
-Wdeclaration-after-statement
-Wempty-body
-Wformat
Expand Down
10 changes: 10 additions & 0 deletions src/libbson/src/bson/bson-macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -301,5 +301,15 @@
#define BSON_GNUC_DEPRECATED_FOR(f) BSON_GNUC_DEPRECATED
#endif

/**
* @brief Mark the attached declared entity as "possibly-unused."
*
* Does nothing on MSVC.
*/
#if defined(__GNUC__) || defined(__clang__)
#define BSON_MAYBE_UNUSED __attribute__ ((unused))
#else
#define BSON_MAYBE_UNUSED /* Nothing for other compilers */
#endif

#endif /* BSON_MACROS_H */
3 changes: 3 additions & 0 deletions src/libmongoc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,7 @@ set (SOURCES ${SOURCES}
${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
${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-ts-pool.c
${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-uri.c
${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-util.c
${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-version-functions.c
Expand Down Expand Up @@ -615,6 +616,7 @@ set (HEADERS
${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-stream-gridfs.h
${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-stream-socket.h
${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-topology-description.h
${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-ts-pool-private.h
${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-uri.h
${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-version-functions.h
${PROJECT_SOURCE_DIR}/src/mongoc/mongoc-write-concern.h
Expand Down Expand Up @@ -998,6 +1000,7 @@ set (test-libmongoc-sources
${PROJECT_SOURCE_DIR}/tests/test-mongoc-topology-scanner.c
${PROJECT_SOURCE_DIR}/tests/test-mongoc-topology.c
${PROJECT_SOURCE_DIR}/tests/test-mongoc-transactions.c
${PROJECT_SOURCE_DIR}/tests/test-mongoc-ts-pool.c
${PROJECT_SOURCE_DIR}/tests/test-mongoc-uri.c
${PROJECT_SOURCE_DIR}/tests/test-mongoc-usleep.c
${PROJECT_SOURCE_DIR}/tests/test-mongoc-util.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 @@ -168,6 +168,7 @@ set (src_libmongoc_src_mongoc_DIST_noinst_hs
mongoc-topology-background-monitoring-private.h
mongoc-topology-scanner-private.h
mongoc-trace-private.h
mongoc-ts-pool-private.h
mongoc-uri-private.h
mongoc-util-private.h
mongoc-write-command-private.h
Expand Down Expand Up @@ -253,6 +254,7 @@ set (src_libmongoc_src_mongoc_DIST_cs
mongoc-topology-description.c
mongoc-topology-description-apm.c
mongoc-topology-scanner.c
mongoc-ts-pool.c
mongoc-uri.c
mongoc-util.c
mongoc-version-functions.c
Expand Down
2 changes: 1 addition & 1 deletion src/libmongoc/src/mongoc/mongoc-client-pool.c
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ mongoc_client_pool_destroy (mongoc_client_pool_t *pool)
EXIT;
}

if (pool->topology->session_pool) {
if (!mongoc_server_session_pool_is_empty (pool->topology->session_pool)) {
client = mongoc_client_pool_pop (pool);
_mongoc_client_end_sessions (client);
mongoc_client_pool_push (pool, client);
Expand Down
12 changes: 6 additions & 6 deletions src/libmongoc/src/mongoc/mongoc-client-session-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ struct _mongoc_session_opt_t {
};

typedef struct _mongoc_server_session_t {
struct _mongoc_server_session_t *prev, *next;
int64_t last_used_usec;
bson_t lsid; /* logical session id */
int64_t txn_number; /* transaction number */
Expand Down Expand Up @@ -102,16 +101,17 @@ _mongoc_client_session_handle_reply (mongoc_client_session_t *session,
const char *cmd_name,
const bson_t *reply);

mongoc_server_session_t *
_mongoc_server_session_new (bson_error_t *error);
bool
_mongoc_server_session_init (mongoc_server_session_t *session,
bson_error_t *error);

void
_mongoc_server_session_destroy (mongoc_server_session_t *session);

bool
_mongoc_server_session_timed_out (const mongoc_server_session_t *server_session,
int64_t session_timeout_minutes);

void
_mongoc_server_session_destroy (mongoc_server_session_t *server_session);

mongoc_client_session_t *
_mongoc_client_session_new (mongoc_client_t *client,
mongoc_server_session_t *server_session,
Expand Down
47 changes: 17 additions & 30 deletions src/libmongoc/src/mongoc/mongoc-client-session.c
Original file line number Diff line number Diff line change
Expand Up @@ -709,35 +709,26 @@ _mongoc_client_session_handle_reply (mongoc_client_session_t *session,
}
}


mongoc_server_session_t *
_mongoc_server_session_new (bson_error_t *error)
bool
_mongoc_server_session_init (mongoc_server_session_t *self, bson_error_t *error)
{
uint8_t uuid_data[16];
mongoc_server_session_t *s;

ENTRY;

BSON_ASSERT (self);
if (!_mongoc_server_session_uuid (uuid_data, error)) {
RETURN (NULL);
RETURN (false);
}

s = bson_malloc0 (sizeof (mongoc_server_session_t));
s->last_used_usec = SESSION_NEVER_USED;
s->prev = NULL;
s->next = NULL;
bson_init (&s->lsid);
bson_append_binary (
&s->lsid, "id", 2, BSON_SUBTYPE_UUID, uuid_data, sizeof uuid_data);

/* transaction number is a positive integer and will be incremented before
* each use, so ensure it is initialized to zero. */
s->txn_number = 0;
self->txn_number = 0;
self->last_used_usec = SESSION_NEVER_USED;
bson_init (&self->lsid);
BSON_APPEND_BINARY (
&self->lsid, "id", BSON_SUBTYPE_UUID, uuid_data, sizeof uuid_data);

RETURN (s);
RETURN (true);
}


bool
_mongoc_server_session_timed_out (const mongoc_server_session_t *server_session,
int64_t session_timeout_minutes)
Expand Down Expand Up @@ -766,14 +757,9 @@ _mongoc_server_session_timed_out (const mongoc_server_session_t *server_session,


void
_mongoc_server_session_destroy (mongoc_server_session_t *server_session)
_mongoc_server_session_destroy (mongoc_server_session_t *self)
{
ENTRY;

bson_destroy (&server_session->lsid);
bson_free (server_session);

EXIT;
bson_destroy (&self->lsid);
}


Expand Down Expand Up @@ -806,7 +792,8 @@ _mongoc_client_session_new (mongoc_client_t *client,
DEFAULT_MAX_COMMIT_TIME_MS);

if (opts) {
mongoc_optional_copy (&opts->causal_consistency, &session->opts.causal_consistency);
mongoc_optional_copy (&opts->causal_consistency,
&session->opts.causal_consistency);
mongoc_optional_copy (&opts->snapshot, &session->opts.snapshot);
txn_opts_set (&session->opts.default_txn_opts,
opts->default_txn_opts.read_concern,
Expand Down Expand Up @@ -1684,9 +1671,9 @@ mongoc_client_session_destroy (mongoc_client_session_t *session)
_mongoc_client_push_server_session (session->client,
session->server_session);
} else {
/* If the client has been reset, destroy the server session instead of
pushing it back into the topology's pool. */
_mongoc_server_session_destroy (session->server_session);
/** If the client has been reset, destroy the server session instead of
* pushing it back into the topology's pool. */
mongoc_server_session_pool_drop (session->server_session);
}

txn_opts_cleanup (&session->opts.default_txn_opts);
Expand Down
4 changes: 2 additions & 2 deletions src/libmongoc/src/mongoc/mongoc-client.c
Original file line number Diff line number Diff line change
Expand Up @@ -3013,7 +3013,7 @@ _mongoc_client_end_sessions (mongoc_client_t *client)
mongoc_cluster_t *cluster = &client->cluster;
bool r;

if (t->session_pool) {
while (!mongoc_server_session_pool_is_empty (t->session_pool)) {
prefs = mongoc_read_prefs_new (MONGOC_READ_PRIMARY_PREFERRED);
server_id =
mongoc_topology_select_server_id (t, MONGOC_SS_READ, prefs, &error);
Expand Down Expand Up @@ -3087,7 +3087,7 @@ mongoc_client_reset (mongoc_client_t *client)
client->client_sessions = mongoc_set_new (8, NULL, NULL);

/* Server sessions are owned by us, so we clear the pool on reset. */
_mongoc_topology_clear_session_pool (client->topology);
mongoc_server_session_pool_clear (client->topology->session_pool);
}

mongoc_change_stream_t *
Expand Down
19 changes: 14 additions & 5 deletions src/libmongoc/src/mongoc/mongoc-topology-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "mongoc-uri.h"
#include "mongoc-client-session-private.h"
#include "mongoc-crypt-private.h"
#include "mongoc-ts-pool-private.h"

#define MONGOC_TOPOLOGY_MIN_HEARTBEAT_FREQUENCY_MS 500
#define MONGOC_TOPOLOGY_SOCKET_CHECK_INTERVAL_MS 5000
Expand Down Expand Up @@ -62,6 +63,17 @@ typedef struct _mongoc_rr_data_t {
char *txt_record_opts;
} mongoc_rr_data_t;

struct _mongoc_topology_t;

MONGOC_DECL_SPECIAL_TS_POOL (
mongoc_server_session_t,
mongoc_server_session_pool,
struct _mongoc_topology_t,
/* ctor/dtor/prune are defined in the new_with_params call */
NULL,
NULL,
NULL)

typedef bool (*_mongoc_rr_resolver_fn) (const char *service,
mongoc_rr_type_t rr_type,
mongoc_rr_data_t *rr_data,
Expand Down Expand Up @@ -102,7 +114,7 @@ typedef struct _mongoc_topology_t {
bool single_threaded;
bool stale;

mongoc_server_session_t *session_pool;
mongoc_server_session_pool session_pool;

/* Is client side encryption enabled? */
bool cse_enabled;
Expand Down Expand Up @@ -215,9 +227,6 @@ _mongoc_topology_push_server_session (mongoc_topology_t *topology,
bool
_mongoc_topology_end_sessions_cmd (mongoc_topology_t *topology, bson_t *cmd);

void
_mongoc_topology_clear_session_pool (mongoc_topology_t *topology);

void
_mongoc_topology_do_blocking_scan (mongoc_topology_t *topology,
bson_error_t *error);
Expand Down Expand Up @@ -255,7 +264,7 @@ _mongoc_topology_handle_app_error (mongoc_topology_t *topology,
void
_mongoc_topology_clear_connection_pool (mongoc_topology_t *topology,
uint32_t server_id,
const bson_oid_t* service_id);
const bson_oid_t *service_id);

void
mongoc_topology_rescan_srv (mongoc_topology_t *topology);
Expand Down
Loading