From 7648b1dd2ff5d90d48e2df144b851be7cb388b3d Mon Sep 17 00:00:00 2001 From: Cheahuychou Mao Date: Sun, 17 May 2020 17:58:09 -0400 Subject: SERVER-40441 Make arbiters not try to setup the sessions collection or check if it exists in LogicalSessionCache refresh/reap thread (cherry picked from commit 35c86a14e2ad2ef8d14fe7ea3ea02951f30e646a) --- .../replsets/sessions_collection_auto_healing.js | 36 ++++++++++- jstests/replsets/sessions_collection_reaping.js | 75 ++++++++++++++++++++++ src/mongo/db/SConscript | 1 + src/mongo/db/logical_session_cache_impl.cpp | 53 +++++++++------ src/mongo/shell/check_log.js | 14 +++- 5 files changed, 154 insertions(+), 25 deletions(-) create mode 100644 jstests/replsets/sessions_collection_reaping.js diff --git a/jstests/replsets/sessions_collection_auto_healing.js b/jstests/replsets/sessions_collection_auto_healing.js index b75ed876d25..9ebf6107862 100644 --- a/jstests/replsets/sessions_collection_auto_healing.js +++ b/jstests/replsets/sessions_collection_auto_healing.js @@ -9,7 +9,11 @@ load('jstests/libs/sessions_collection.js'); var replTest = new ReplSetTest({ name: 'refresh', - nodes: [{rsConfig: {votes: 1, priority: 1}}, {rsConfig: {votes: 0, priority: 0}}] + nodes: [ + {/* primary */}, + {/* secondary */ rsConfig: {priority: 0}}, + {/* arbiter */ rsConfig: {arbiterOnly: true}} + ] }); var nodes = replTest.startSet(); @@ -21,6 +25,11 @@ load('jstests/libs/sessions_collection.js'); var secondary = replTest.getSecondary(); var secondaryAdmin = secondary.getDB("admin"); + let arbiter = replTest.getArbiter(); + + const refreshErrorMsgRegex = + new RegExp("Failed to refresh session cache, will try again at the next refresh interval"); + // Get the current value of the TTL index so that we can verify it's being properly applied. let res = assert.commandWorked( primary.adminCommand({getParameter: 1, localLogicalSessionTimeoutMinutes: 1})); @@ -64,6 +73,22 @@ load('jstests/libs/sessions_collection.js'); replTest.awaitReplication(); validateSessionsCollection(secondary, false, false, timeoutMinutes); } + + // Test that a refresh on an arbiter does not create the sessions collection. + { + validateSessionsCollection(primary, false, false, timeoutMinutes); + + assert.commandWorked(arbiter.adminCommand({clearLog: 'global'})); + assert.commandWorked(arbiter.adminCommand({refreshLogicalSessionCacheNow: 1})); + + validateSessionsCollection(primary, false, false, timeoutMinutes); + + if (!jsTest.options().useRandomBinVersionsWithinReplicaSet) { + // Verify that the arbiter did not try to set up the session collection or refresh. + assert.eq(false, checkLog.checkContainsOnce(arbiter, refreshErrorMsgRegex)); + } + } + // Test that a refresh on the primary creates the sessions collection. { validateSessionsCollection(primary, false, false, timeoutMinutes); @@ -87,6 +112,15 @@ load('jstests/libs/sessions_collection.js'); validateSessionsCollection(primary, true, false, timeoutMinutes); } + // Test that a refresh on an arbiter will not create the TTL index on the sessions collection. + { + validateSessionsCollection(primary, true, false, timeoutMinutes); + + assert.commandWorked(arbiter.adminCommand({refreshLogicalSessionCacheNow: 1})); + + validateSessionsCollection(primary, true, false, timeoutMinutes); + } + // Test that a refresh on the primary will create the TTL index on the sessions collection. { validateSessionsCollection(primary, true, false, timeoutMinutes); diff --git a/jstests/replsets/sessions_collection_reaping.js b/jstests/replsets/sessions_collection_reaping.js new file mode 100644 index 00000000000..a93e756dd63 --- /dev/null +++ b/jstests/replsets/sessions_collection_reaping.js @@ -0,0 +1,75 @@ +(function() { + "use strict"; + + // This test makes assertions about the number of sessions, which are not compatible with + // implicit sessions. + TestData.disableImplicitSessions = true; + + let replTest = new ReplSetTest({ + name: 'reaping', + nodes: [ + {/* primary */}, + {/* secondary */ rsConfig: {priority: 0}}, + {/* arbiter */ rsConfig: {arbiterOnly: true}} + ], + nodeOptions: {setParameter: {TransactionRecordMinimumLifetimeMinutes: 0}} + }); + let nodes = replTest.startSet(); + + replTest.initiate(); + let primary = replTest.getPrimary(); + let sessionsCollOnPrimary = primary.getDB("config").system.sessions; + let transactionsCollOnPrimary = primary.getDB("config").transactions; + + replTest.awaitSecondaryNodes(); + let secondary = replTest.getSecondary(); + let arbiter = replTest.getArbiter(); + + const dbName = jsTestName(); + const collName = "test"; + const reapErrorMsgRegex = + new RegExp("Sessions collection is not set up.*waiting until next sessions reap interval"); + + // Set up a session with a retryable write. + let session = primary.startSession({retryWrites: 1}); + assert.commandWorked(session.getDatabase(dbName)[collName].save({x: 1})); + assert.commandWorked(primary.adminCommand({refreshLogicalSessionCacheNow: 1})); + assert.eq(1, sessionsCollOnPrimary.count()); + assert.eq(1, transactionsCollOnPrimary.count()); + + // Remove the session doc so the session gets reaped when reapLogicalSessionCacheNow is run. + assert.commandWorked(sessionsCollOnPrimary.remove({})); + + // Test that a reap on a secondary does not lead to the on-disk state reaping of the session + // since the session does not exist in the secondary's session catalog. + { + assert.commandWorked(secondary.adminCommand({clearLog: 'global'})); + assert.commandWorked(secondary.adminCommand({reapLogicalSessionCacheNow: 1})); + + assert.eq(1, transactionsCollOnPrimary.count()); + assert.eq(false, checkLog.checkContainsOnce(secondary, reapErrorMsgRegex)); + } + + // Test that a reap on an arbiter does not lead to reaping of the session. + { + assert.commandWorked(arbiter.adminCommand({clearLog: 'global'})); + assert.commandWorked(arbiter.adminCommand({reapLogicalSessionCacheNow: 1})); + + assert.eq(1, transactionsCollOnPrimary.count()); + + if (!jsTest.options().useRandomBinVersionsWithinReplicaSet) { + // Verify that the arbiter did not try to reap the session. + assert.eq(false, checkLog.checkContainsOnce(arbiter, reapErrorMsgRegex)); + } + } + + // Test that a reap on the primary works as expected. + { + assert.commandWorked(primary.adminCommand({clearLog: 'global'})); + assert.commandWorked(primary.adminCommand({reapLogicalSessionCacheNow: 1})); + + assert.eq(0, transactionsCollOnPrimary.count()); + } + + replTest.stopSet(); +})(); diff --git a/src/mongo/db/SConscript b/src/mongo/db/SConscript index 9e5be51f019..d984190c76d 100644 --- a/src/mongo/db/SConscript +++ b/src/mongo/db/SConscript @@ -1373,6 +1373,7 @@ env.Library( ], LIBDEPS_PRIVATE=[ 's/sharding_api_d', + '$BUILD_DIR/mongo/db/repl/repl_coordinator_interface' ] ) diff --git a/src/mongo/db/logical_session_cache_impl.cpp b/src/mongo/db/logical_session_cache_impl.cpp index 81bd2f95a93..967bf2a8851 100644 --- a/src/mongo/db/logical_session_cache_impl.cpp +++ b/src/mongo/db/logical_session_cache_impl.cpp @@ -37,6 +37,7 @@ #include "mongo/db/logical_session_id.h" #include "mongo/db/logical_session_id_helpers.h" #include "mongo/db/operation_context.h" +#include "mongo/db/repl/replication_coordinator.h" #include "mongo/db/s/operation_sharding_state.h" #include "mongo/db/server_parameters.h" #include "mongo/db/service_context.h" @@ -211,6 +212,21 @@ void LogicalSessionCacheImpl::_periodicReap(Client* client) { } Status LogicalSessionCacheImpl::_reap(Client* client) { + boost::optional uniqueCtx; + auto* const opCtx = [&] { + if (client->getOperationContext()) { + return client->getOperationContext(); + } + + uniqueCtx.emplace(client->makeOperationContext()); + return uniqueCtx->get(); + }(); + + const auto replCoord = repl::ReplicationCoordinator::get(opCtx); + if (replCoord && replCoord->getMemberState().arbiter()) { + return Status::OK(); + } + // Take the lock to update some stats. { stdx::lock_guard lk(_mutex); @@ -224,16 +240,6 @@ Status LogicalSessionCacheImpl::_reap(Client* client) { _stats.setTransactionReaperJobCount(_stats.getTransactionReaperJobCount() + 1); } - boost::optional uniqueCtx; - auto* const opCtx = [&] { - if (client->getOperationContext()) { - return client->getOperationContext(); - } - - uniqueCtx.emplace(client->makeOperationContext()); - return uniqueCtx->get(); - }(); - int numReaped = 0; try { @@ -280,6 +286,22 @@ Status LogicalSessionCacheImpl::_reap(Client* client) { } void LogicalSessionCacheImpl::_refresh(Client* client) { + // get or make an opCtx + boost::optional uniqueCtx; + auto* const opCtx = [&client, &uniqueCtx] { + if (client->getOperationContext()) { + return client->getOperationContext(); + } + + uniqueCtx.emplace(client->makeOperationContext()); + return uniqueCtx->get(); + }(); + + const auto replCoord = repl::ReplicationCoordinator::get(opCtx); + if (replCoord && replCoord->getMemberState().arbiter()) { + return; + } + // Stats for serverStatus: { stdx::lock_guard lk(_mutex); @@ -302,17 +324,6 @@ void LogicalSessionCacheImpl::_refresh(Client* client) { _stats.setLastSessionsCollectionJobDurationMillis(millis.count()); }); - // get or make an opCtx - boost::optional uniqueCtx; - auto* const opCtx = [&client, &uniqueCtx] { - if (client->getOperationContext()) { - return client->getOperationContext(); - } - - uniqueCtx.emplace(client->makeOperationContext()); - return uniqueCtx->get(); - }(); - ON_BLOCK_EXIT([&opCtx] { clearShardingOperationFailedStatus(opCtx); }); auto setupStatus = _sessionsColl->setupSessionsCollection(opCtx); diff --git a/src/mongo/shell/check_log.js b/src/mongo/shell/check_log.js index d49955f5786..3898e42d165 100644 --- a/src/mongo/shell/check_log.js +++ b/src/mongo/shell/check_log.js @@ -34,9 +34,17 @@ var checkLog; if (logMessages === null) { return false; } - for (let logMsg of logMessages) { - if (logMsg.includes(msg)) { - return true; + if (msg instanceof RegExp) { + for (let logMsg of logMessages) { + if (logMsg.search(msg) != -1) { + return true; + } + } + } else { + for (let logMsg of logMessages) { + if (logMsg.includes(msg)) { + return true; + } } } return false; -- cgit v1.2.1