diff options
-rw-r--r-- | jstests/sharding/secondary_cache_reload_no_hang.js | 54 | ||||
-rw-r--r-- | src/mongo/db/db_raii.cpp | 21 | ||||
-rw-r--r-- | src/mongo/db/db_raii.h | 15 | ||||
-rw-r--r-- | src/mongo/db/s/shard_server_catalog_cache_loader.cpp | 7 |
4 files changed, 97 insertions, 0 deletions
diff --git a/jstests/sharding/secondary_cache_reload_no_hang.js b/jstests/sharding/secondary_cache_reload_no_hang.js new file mode 100644 index 00000000000..e6b54e04ba9 --- /dev/null +++ b/jstests/sharding/secondary_cache_reload_no_hang.js @@ -0,0 +1,54 @@ +/** + * TODO: SERVER-44105 maybe remove this test. + * This test is a simplified version that tries to simulate the condition described in + * SERVER-42737. It is very hard to replicate the exact condition because of: + * + * https://github.com/mongodb/mongo/blob/r4.3.0/src/mongo/db/s/shard_server_catalog_cache_loader.cpp#L293 + * + * This means that secondary should be replicating a newer refresh after that line + * above and hit the condition described in the ticket to hit the bug. + */ +(function() { +let rsOptions = {nodes: 2}; +let st = new ShardingTest({shards: {rs0: rsOptions, rs1: rsOptions}}); + +assert.commandWorked(st.s.adminCommand({enableSharding: 'test'})); +st.ensurePrimaryShard('test', st.shard0.shardName); +assert.commandWorked(st.s.adminCommand({shardCollection: 'test.user', key: {x: 1}})); +assert.commandWorked(st.s.adminCommand({split: 'test.user', middle: {x: 0}})); + +let coll = st.s.getDB('test').user; + +assert.commandWorked(coll.insert({x: -1})); +assert.commandWorked(coll.insert({x: 1})); + +assert.commandWorked( + st.s.adminCommand({moveChunk: 'test.user', find: {x: 0}, to: st.shard1.shardName})); + +// Manually set refreshing flag to true so secondary cache refresh will block. + +st.rs0.getPrimary().getDB('config').cache.collections.update( + {_id: 'test.user', fake: {'$exists': false}}, {$set: {refreshing: true}}); + +// Add a delay (sleep) to make sure that secondary will have the {refreshing: true} +// in the lastApplied snapshot and is blocked waiting for refreshing to become false +// before sending update to primary. + +let joinUpdate = startParallelShell( + 'sleep(1000);' + + 'db.getSiblingDB("config").cache.collections.update(' + + '{_id: "test.user", fake: {"$exists": false}},' + + '{$set: {refreshing: false, lastRefreshedCollectionVersion: Timestamp(5, 0)}});', + st.rs0.getPrimary().port); + +// This secondary read should not cause a hang. +st.s.setReadPref('secondary'); +let res = assert.commandWorked(coll.getDB('test').runReadCommand( + {find: 'user', filter: {dummy: {'$exists': false}}, readConcern: {level: 'local'}})); + +assert.eq(2, res.cursor.firstBatch.length, tojson(res)); + +joinUpdate(); + +st.stop(); +})(); diff --git a/src/mongo/db/db_raii.cpp b/src/mongo/db/db_raii.cpp index ede268a31f3..4e2fc2c8e45 100644 --- a/src/mongo/db/db_raii.cpp +++ b/src/mongo/db/db_raii.cpp @@ -46,6 +46,12 @@ namespace { const boost::optional<int> kDoNotChangeProfilingLevel = boost::none; +// TODO: SERVER-44105 remove +// If set to false, secondary reads should wait behind the PBW lock. +// Does nothing if gAllowSecondaryReadsDuringBatchApplication setting is false. +const auto allowSecondaryReadsDuringBatchApplication_DONT_USE = + OperationContext::declareDecoration<boost::optional<bool>>(); + } // namespace AutoStatsTracker::AutoStatsTracker(OperationContext* opCtx, @@ -92,6 +98,7 @@ AutoGetCollectionForRead::AutoGetCollectionForRead(OperationContext* opCtx, // Don't take the ParallelBatchWriterMode lock when the server parameter is set and our // storage engine supports snapshot reads. if (gAllowSecondaryReadsDuringBatchApplication.load() && + allowSecondaryReadsDuringBatchApplication_DONT_USE(opCtx).value_or(true) && opCtx->getServiceContext()->getStorageEngine()->supportsReadConcernSnapshot()) { _shouldNotConflictWithSecondaryBatchApplicationBlock.emplace(opCtx->lockState()); } @@ -388,4 +395,18 @@ LockMode getLockModeForQuery(OperationContext* opCtx, const boost::optional<Name return MODE_IS; } +BlockSecondaryReadsDuringBatchApplication_DONT_USE:: + BlockSecondaryReadsDuringBatchApplication_DONT_USE(OperationContext* opCtx) + : _opCtx(opCtx) { + auto allowSecondaryReads = &allowSecondaryReadsDuringBatchApplication_DONT_USE(opCtx); + allowSecondaryReads->swap(_originalSettings); + *allowSecondaryReads = false; +} + +BlockSecondaryReadsDuringBatchApplication_DONT_USE:: + ~BlockSecondaryReadsDuringBatchApplication_DONT_USE() { + auto allowSecondaryReads = &allowSecondaryReadsDuringBatchApplication_DONT_USE(_opCtx); + allowSecondaryReads->swap(_originalSettings); +} + } // namespace mongo diff --git a/src/mongo/db/db_raii.h b/src/mongo/db/db_raii.h index f5497c6f3a1..940e13c1f6f 100644 --- a/src/mongo/db/db_raii.h +++ b/src/mongo/db/db_raii.h @@ -259,4 +259,19 @@ private: PrepareConflictBehavior _originalValue; }; +/** + * TODO: SERVER-44105 remove + * RAII type for letting secondary reads to block behind the PBW lock. + * Note: Do not add additional usage. This is only temporary for ease of backport. + */ +struct BlockSecondaryReadsDuringBatchApplication_DONT_USE { +public: + BlockSecondaryReadsDuringBatchApplication_DONT_USE(OperationContext* opCtx); + ~BlockSecondaryReadsDuringBatchApplication_DONT_USE(); + +private: + OperationContext* _opCtx{nullptr}; + boost::optional<bool> _originalSettings; +}; + } // namespace mongo diff --git a/src/mongo/db/s/shard_server_catalog_cache_loader.cpp b/src/mongo/db/s/shard_server_catalog_cache_loader.cpp index ee2230b6333..37a59006a68 100644 --- a/src/mongo/db/s/shard_server_catalog_cache_loader.cpp +++ b/src/mongo/db/s/shard_server_catalog_cache_loader.cpp @@ -36,6 +36,7 @@ #include <memory> #include "mongo/db/client.h" +#include "mongo/db/db_raii.h" #include "mongo/db/operation_context.h" #include "mongo/db/operation_context_group.h" #include "mongo/db/read_concern.h" @@ -619,6 +620,12 @@ void ShardServerCatalogCacheLoader::_runSecondaryGetChunksSince( forcePrimaryCollectionRefreshAndWaitForReplication(opCtx, nss); // Read the local metadata. + + // Disallow reading on an older snapshot because this relies on being able to read the + // side effects of writes during secondary replication after being signalled from the + // CollectionVersionLogOpHandler. + BlockSecondaryReadsDuringBatchApplication_DONT_USE secondaryReadsBlockBehindReplication(opCtx); + auto swCollAndChunks = _getCompletePersistedMetadataForSecondarySinceVersion(opCtx, nss, catalogCacheSinceVersion); callbackFn(opCtx, std::move(swCollAndChunks)); |