Skip to content

Commit 188f5b4

Browse files
Andrew WittenkevinAlbs
authored andcommitted
CDRIVER-3322 only push used sessions
* CDRIVER-3322 only pushes used server sessions to pool, runs test for destroying unused session in pooled mode, updates tests to mark sessions as used.
1 parent 743dff2 commit 188f5b4

File tree

5 files changed

+106
-11
lines changed

5 files changed

+106
-11
lines changed

src/libmongoc/src/mongoc/mongoc-client-session-private.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#define UNKNOWN_COMMIT_RESULT "UnknownTransactionCommitResult"
2828
#define MAX_TIME_MS_EXPIRED "MaxTimeMSExpired"
2929
#define DEFAULT_MAX_COMMIT_TIME_MS 0
30+
#define SESSION_NEVER_USED (-1)
3031

3132
#define MONGOC_DEFAULT_WTIMEOUT_FOR_COMMIT_RETRY 10000
3233

src/libmongoc/src/mongoc/mongoc-client-session.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@
2424
#include "mongoc-read-prefs-private.h"
2525
#include "mongoc-error-private.h"
2626

27-
#define SESSION_NEVER_USED (-1)
28-
2927
#define WITH_TXN_TIMEOUT_MS (120 * 1000)
3028

3129
static void

src/libmongoc/src/mongoc/mongoc-topology.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1466,7 +1466,14 @@ _mongoc_topology_push_server_session (mongoc_topology_t *topology,
14661466
/* silences clang scan-build */
14671467
BSON_ASSERT (!topology->session_pool || (topology->session_pool->next &&
14681468
topology->session_pool->prev));
1469-
CDL_PREPEND (topology->session_pool, server_session);
1469+
1470+
/* add server session (lsid) to session pool to be reused only if the
1471+
* server session has been used (the server is aware of the session) */
1472+
if (server_session->last_used_usec == SESSION_NEVER_USED) {
1473+
_mongoc_server_session_destroy (server_session);
1474+
} else {
1475+
CDL_PREPEND (topology->session_pool, server_session);
1476+
}
14701477
}
14711478

14721479
bson_mutex_unlock (&topology->mutex);

src/libmongoc/tests/test-mongoc-client-pool.c

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -361,15 +361,58 @@ test_client_pool_destroy_without_pushing (void)
361361
bson_destroy (cmd);
362362
}
363363

364+
static void
365+
command_started_cb (const mongoc_apm_command_started_t *event)
366+
{
367+
int *count;
368+
369+
if (strcmp (mongoc_apm_command_started_get_command_name (event),
370+
"endSessions") != 0) {
371+
return;
372+
}
373+
374+
count = (int *) mongoc_apm_command_started_get_context (event);
375+
count++;
376+
}
377+
378+
/* tests that creating and destroying an unused session
379+
* in pooled mode does not result in an error log */
380+
static void
381+
test_client_pool_create_unused_session (void *context)
382+
{
383+
mongoc_client_t *client;
384+
mongoc_client_pool_t *pool;
385+
mongoc_client_session_t *session;
386+
mongoc_apm_callbacks_t *callbacks;
387+
bson_error_t error;
388+
int count = 0;
389+
390+
capture_logs (true);
391+
392+
callbacks = mongoc_apm_callbacks_new ();
393+
pool = test_framework_client_pool_new ();
394+
client = mongoc_client_pool_pop (pool);
395+
session = mongoc_client_start_session (client, NULL, &error);
396+
397+
mongoc_apm_set_command_started_cb (callbacks, command_started_cb);
398+
mongoc_client_pool_set_apm_callbacks (pool, callbacks, &count);
399+
400+
mongoc_client_session_destroy (session);
401+
mongoc_client_pool_push (pool, client);
402+
mongoc_client_pool_destroy (pool);
403+
mongoc_apm_callbacks_destroy (callbacks);
404+
ASSERT_CMPINT (count, ==, 0);
405+
ASSERT_NO_CAPTURED_LOGS ("mongoc_client_pool_destroy");
406+
}
407+
364408
void
365409
test_client_pool_install (TestSuite *suite)
366410
{
367411
TestSuite_Add (suite, "/ClientPool/basic", test_mongoc_client_pool_basic);
368412
TestSuite_Add (
369413
suite, "/ClientPool/try_pop", test_mongoc_client_pool_try_pop);
370-
TestSuite_Add (suite,
371-
"/ClientPool/pop_timeout",
372-
test_mongoc_client_pool_pop_timeout);
414+
TestSuite_Add (
415+
suite, "/ClientPool/pop_timeout", test_mongoc_client_pool_pop_timeout);
373416
TestSuite_Add (suite,
374417
"/ClientPool/min_size_zero",
375418
test_mongoc_client_pool_min_size_zero);
@@ -384,9 +427,17 @@ test_client_pool_install (TestSuite *suite)
384427
TestSuite_Add (
385428
suite, "/ClientPool/handshake", test_mongoc_client_pool_handshake);
386429

430+
TestSuite_AddFull (suite,
431+
"/ClientPool/create_client_pool_unused_session",
432+
test_client_pool_create_unused_session,
433+
NULL /* dtor */,
434+
NULL /* ctx */,
435+
test_framework_skip_if_no_sessions);
387436
#ifndef MONGOC_ENABLE_SSL
388437
TestSuite_Add (
389438
suite, "/ClientPool/ssl_disabled", test_mongoc_client_pool_ssl_disabled);
390439
#endif
391-
TestSuite_AddLive (suite, "/ClientPool/destroy_without_push", test_client_pool_destroy_without_pushing);
440+
TestSuite_AddLive (suite,
441+
"/ClientPool/destroy_without_push",
442+
test_client_pool_destroy_without_pushing);
392443
}

src/libmongoc/tests/test-mongoc-client-session.c

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
#undef MONGOC_LOG_DOMAIN
1515
#define MONGOC_LOG_DOMAIN "session-test"
1616

17-
1817
/*
1918
* Prevent failing on pedantic GCC/clang warning: "ISO C forbids conversion of
2019
* function pointer to object pointer type."
@@ -757,6 +756,32 @@ test_end_sessions_pooled (void *ctx)
757756
_test_end_sessions (true);
758757
}
759758

759+
/* Sends ping to server via client_session. useful for marking
760+
* server_sessions as used so that they are pushed back to the session pool */
761+
static void
762+
send_ping (mongoc_client_t *client, mongoc_client_session_t *client_session)
763+
{
764+
bson_t ping_cmd = BSON_INITIALIZER;
765+
bson_t opts = BSON_INITIALIZER;
766+
bson_error_t error;
767+
bool ret;
768+
769+
BCON_APPEND (&ping_cmd, "ping", BCON_INT32 (1));
770+
771+
ret = mongoc_client_session_append (client_session, &opts, &error);
772+
ASSERT_OR_PRINT(ret, error);
773+
774+
ret = mongoc_client_command_with_opts (client,
775+
"admin",
776+
&ping_cmd,
777+
NULL,
778+
&opts,
779+
NULL,
780+
&error);
781+
ASSERT_OR_PRINT (ret, error);
782+
bson_destroy(&opts);
783+
bson_destroy (&ping_cmd);
784+
}
760785

761786
static void
762787
_test_end_sessions_many (bool pooled)
@@ -782,6 +807,7 @@ _test_end_sessions_many (bool pooled)
782807
for (i = 0; i < sizeof sessions / sizeof (mongoc_client_session_t *); i++) {
783808
sessions[i] = mongoc_client_start_session (client, NULL, &error);
784809
ASSERT_OR_PRINT (sessions[i], error);
810+
send_ping (client, sessions[i]);
785811
}
786812

787813
for (i = 0; i < sizeof sessions / sizeof (mongoc_client_session_t *); i++) {
@@ -1213,7 +1239,10 @@ check_session_returned (session_test_t *test, const bson_t *lsid)
12131239
}
12141240
}
12151241

1216-
if (!found) {
1242+
/* Server session will only be returned to the pool if it has
1243+
* been used. It is expected behavior for found to be false if
1244+
* ss->last_used_usec == SESSION_NEVER_USED */
1245+
if (!found && ss && ss->last_used_usec != SESSION_NEVER_USED) {
12171246
fprintf (stderr,
12181247
"server session %s not returned to pool\n",
12191248
bson_as_json (lsid, NULL));
@@ -2222,7 +2251,11 @@ test_cursor_implicit_session (void *ctx)
22222251
ASSERT_POOL_SIZE (topology, 0);
22232252
ASSERT_SESSIONS_MATCH (&test->sent_lsid, &find_lsid);
22242253

2225-
/* push a new server session into the pool */
2254+
/* push a new server session into the pool. server session is only pushed
2255+
* if it is used. therefore mark session as used prior to
2256+
* destroying session by sending a ping */
2257+
bson_reinit (&test->sent_lsid);
2258+
send_ping (test->client, cs);
22262259
mongoc_client_session_destroy (cs);
22272260
ASSERT_POOL_SIZE (topology, 1);
22282261
ASSERT_SESSIONS_DIFFER (&find_lsid, &topology->session_pool->lsid);
@@ -2267,7 +2300,12 @@ test_change_stream_implicit_session (void *ctx)
22672300
ASSERT_POOL_SIZE (topology, 0);
22682301
BSON_ASSERT (change_stream->implicit_session);
22692302

2270-
/* push a new server session into the pool */
2303+
2304+
/* push a new server session into the pool. server session is only pushed
2305+
* if it is used. therefore mark session as used prior to
2306+
* destroying session by sending a ping */
2307+
bson_reinit (&test->sent_lsid);
2308+
send_ping (test->client, cs);
22712309
mongoc_client_session_destroy (cs);
22722310
ASSERT_POOL_SIZE (topology, 1);
22732311
ASSERT_SESSIONS_DIFFER (&aggregate_lsid, &topology->session_pool->lsid);

0 commit comments

Comments
 (0)