Revert "SERVER-36956 SnapshotTooOld errors will always increase the snapshot history window size"

This reverts commit 8899b34.

Author: Dianna Hohensee

jstests/noPassthrough/snapshotWindow_serverParameters.js

@@ -17,6 +17,17 @@
                                false /*hasUpperBound*/,
                                "unused" /*upperOutOfBounds*/);
 
+    // Valid parameter values are in the range [0, 100].
+    testNumericServerParameter("cachePressureThreshold",
+                               true /*isStartupParameter*/,
+                               true /*isRuntimeParameter*/,
+                               50 /*defaultValue*/,
+                               70 /*nonDefaultValidValue*/,
+                               true /*hasLowerBound*/,
+                               -1 /*lowerOutOfBounds*/,
+                               true /*hasUpperBound*/,
+                               101 /*upperOutOfBounds*/);
+
     // Valid parameter values are in the range (0, 1).
     testNumericServerParameter("snapshotWindowMultiplicativeDecrease",
                                true /*isStartupParameter*/,
@@ -32,18 +43,18 @@
     testNumericServerParameter("snapshotWindowAdditiveIncreaseSeconds",
                                true /*isStartupParameter*/,
                                true /*isRuntimeParameter*/,
-                               1 /*defaultValue*/,
+                               2 /*defaultValue*/,
                                10 /*nonDefaultValidValue*/,
                                true /*hasLowerBound*/,
                                0 /*lowerOutOfBounds*/,
                                false /*hasUpperBound*/,
                                "unused" /*upperOutOfBounds*/);
 
     // Valid parameter values are in the range [1, infinity).
-    testNumericServerParameter("decreaseHistoryIfNotNeededPeriodSeconds",
+    testNumericServerParameter("checkCachePressurePeriodSeconds",
                                true /*isStartupParameter*/,
                                true /*isRuntimeParameter*/,
-                               15 /*defaultValue*/,
+                               5 /*defaultValue*/,
                                8 /*nonDefaultValidValue*/,
                                true /*hasLowerBound*/,
                                0 /*lowerOutOfBounds*/,
@@ -60,4 +71,15 @@
                                0 /*lowerOutOfBounds*/,
                                false /*hasUpperBound*/,
                                "unused" /*upperOutOfBounds*/);
+
+    // Valid parameter values are in the range [1, infinity).
+    testNumericServerParameter("minMillisBetweenSnapshotWindowDec",
+                               true /*isStartupParameter*/,
+                               true /*isRuntimeParameter*/,
+                               500 /*defaultValue*/,
+                               2 * 1000 /*nonDefaultValidValue*/,
+                               true /*hasLowerBound*/,
+                               0 /*lowerOutOfBounds*/,
+                               false /*hasUpperBound*/,
+                               "unused" /*upperOutOfBounds*/);
 })();

src/mongo/db/db.cpp

@@ -611,7 +611,7 @@ ExitCode _initAndListen(int listenPort) {
     // release periodically in order to avoid storage cache pressure build up.
     if (storageEngine->supportsReadConcernSnapshot()) {
         PeriodicThreadToAbortExpiredTransactions::get(serviceContext)->start();
-        PeriodicThreadToDecreaseSnapshotHistoryIfNotNeeded::get(serviceContext)->start();
+        PeriodicThreadToDecreaseSnapshotHistoryCachePressure::get(serviceContext)->start();
     }
 
     // Set up the logical session cache
@@ -929,7 +929,7 @@ void shutdownTask(const ShutdownTaskArgs& shutdownArgs) {
     if (auto storageEngine = serviceContext->getStorageEngine()) {
         if (storageEngine->supportsReadConcernSnapshot()) {
             PeriodicThreadToAbortExpiredTransactions::get(serviceContext)->stop();
-            PeriodicThreadToDecreaseSnapshotHistoryIfNotNeeded::get(serviceContext)->stop();
+            PeriodicThreadToDecreaseSnapshotHistoryCachePressure::get(serviceContext)->stop();
         }
 
         ServiceContext::UniqueOperationContext uniqueOpCtx;

src/mongo/db/periodic_runner_job_decrease_snapshot_cache_pressure.cpp

@@ -43,26 +43,26 @@
 
 namespace mongo {
 
-auto PeriodicThreadToDecreaseSnapshotHistoryIfNotNeeded::get(ServiceContext* serviceContext)
-    -> PeriodicThreadToDecreaseSnapshotHistoryIfNotNeeded& {
+auto PeriodicThreadToDecreaseSnapshotHistoryCachePressure::get(ServiceContext* serviceContext)
+    -> PeriodicThreadToDecreaseSnapshotHistoryCachePressure& {
     auto& jobContainer = _serviceDecoration(serviceContext);
     jobContainer._init(serviceContext);
     return jobContainer;
 }
 
-auto PeriodicThreadToDecreaseSnapshotHistoryIfNotNeeded::operator-> () const noexcept
+auto PeriodicThreadToDecreaseSnapshotHistoryCachePressure::operator-> () const noexcept
     -> PeriodicJobAnchor* {
     stdx::lock_guard lk(_mutex);
     return _anchor.get();
 }
 
-auto PeriodicThreadToDecreaseSnapshotHistoryIfNotNeeded::operator*() const noexcept
+auto PeriodicThreadToDecreaseSnapshotHistoryCachePressure::operator*() const noexcept
     -> PeriodicJobAnchor& {
     stdx::lock_guard lk(_mutex);
     return *_anchor;
 }
 
-void PeriodicThreadToDecreaseSnapshotHistoryIfNotNeeded::_init(ServiceContext* serviceContext) {
+void PeriodicThreadToDecreaseSnapshotHistoryCachePressure::_init(ServiceContext* serviceContext) {
     stdx::lock_guard lk(_mutex);
     if (_anchor) {
         return;
@@ -72,7 +72,7 @@ void PeriodicThreadToDecreaseSnapshotHistoryIfNotNeeded::_init(ServiceContext* s
     invariant(periodicRunner);
 
     PeriodicRunner::PeriodicJob job(
-        "startPeriodicThreadToDecreaseSnapshotHistoryIfNotNeeded",
+        "startPeriodicThreadToDecreaseSnapshotHistoryCachePressure",
         [](Client* client) {
             try {
                 // The opCtx destructor handles unsetting itself from the Client.
@@ -88,18 +88,17 @@ void PeriodicThreadToDecreaseSnapshotHistoryIfNotNeeded::_init(ServiceContext* s
                 }
             }
         },
-        Seconds(snapshotWindowParams.decreaseHistoryIfNotNeededPeriodSeconds.load()));
+        Seconds(snapshotWindowParams.checkCachePressurePeriodSeconds.load()));
 
     _anchor = std::make_shared<PeriodicJobAnchor>(periodicRunner->makeJob(std::move(job)));
 
-    SnapshotWindowParams::observeDecreaseHistoryIfNotNeededPeriodSeconds.addObserver([anchor =
-                                                                                          _anchor](
+    SnapshotWindowParams::observeCheckCachePressurePeriodSeconds.addObserver([anchor = _anchor](
         const auto& secs) {
         try {
             anchor->setPeriod(Seconds(secs));
         } catch (const DBException& ex) {
-            log() << "Failed to update the period of the thread which decreases data history "
-                     "target window size if there have been no new SnapshotTooOld errors."
+            log() << "Failed to update the period of the thread which decreases data history cache "
+                     "target size if there is cache pressure."
                   << ex.toStatus();
         }
     });

src/mongo/db/periodic_runner_job_decrease_snapshot_cache_pressure.h

@@ -38,18 +38,17 @@
 namespace mongo {
 
 /**
- * Periodically checks whether there has been any storage engine cache pressure and SnapshotTooOld
- * errors to determine whether the maintained snapshot history window target setting should be
- * decreased. If there has been cache pressure and no new SnapshotTooOld errors in the last period,
- * then the target window size will be decrease. Maintaining too much snapshot and write history can
- * slow down the system. Runs once every decreaseHistoryIfNotNeededPeriodSeconds.
+ * Periodically checks for storage engine cache pressure to determine whether the maintained
+ * snapshot history window target setting should be decreased. Maintaining too much snapshot and
+ * write history can slow down the system. Runs once every checkCachePressurePeriodSeconds.
  *
  * This function should only ever be called once, during mongod server startup (db.cpp).
  * The PeriodicRunner will handle shutting down the job on shutdown, no extra handling necessary.
  */
-class PeriodicThreadToDecreaseSnapshotHistoryIfNotNeeded {
+class PeriodicThreadToDecreaseSnapshotHistoryCachePressure {
 public:
-    static PeriodicThreadToDecreaseSnapshotHistoryIfNotNeeded& get(ServiceContext* serviceContext);
+    static PeriodicThreadToDecreaseSnapshotHistoryCachePressure& get(
+        ServiceContext* serviceContext);
 
     PeriodicJobAnchor* operator->() const noexcept;
     PeriodicJobAnchor& operator*() const noexcept;
@@ -58,7 +57,7 @@ class PeriodicThreadToDecreaseSnapshotHistoryIfNotNeeded {
     void _init(ServiceContext* serviceContext);
 
     inline static const auto _serviceDecoration =
-        ServiceContext::declareDecoration<PeriodicThreadToDecreaseSnapshotHistoryIfNotNeeded>();
+        ServiceContext::declareDecoration<PeriodicThreadToDecreaseSnapshotHistoryCachePressure>();
 
     mutable stdx::mutex _mutex;
     std::shared_ptr<PeriodicJobAnchor> _anchor;

src/mongo/db/service_entry_point_common.cpp

@@ -860,7 +860,6 @@ void execCommandDatabase(OperationContext* opCtx,
             // of successful future PIT atClusterTime requests.
             auto engine = opCtx->getServiceContext()->getStorageEngine();
             if (engine && engine->supportsReadConcernSnapshot()) {
-                SnapshotWindowUtil::incrementSnapshotTooOldErrorCount();
                 SnapshotWindowUtil::increaseTargetSnapshotWindowSize(opCtx);
             }
         } else {

src/mongo/db/snapshot_window_options.h

@@ -64,42 +64,49 @@ struct SnapshotWindowParams {
     AtomicWord<int> targetSnapshotHistoryWindowInSeconds{
         maxTargetSnapshotHistoryWindowInSeconds.load()};
 
+    // cachePressureThreshold (startup & runtime server paramter, range [0, 100]).
+    //
+    // Dictates what percentage of cache in use is considered too high. This setting helps preempt
+    // storage cache pressure immobilizing the system. Attempts to increase
+    // targetSnapshotHistoryWindowInSeconds will be ignored when the cache pressure reaches this
+    // threshold. Additionally, a periodic task will decrease targetSnapshotHistoryWindowInSeconds
+    // when cache pressure exceeds the threshold.
+    AtomicWord<int> cachePressureThreshold{50};
+
     // snapshotWindowMultiplicativeDecrease (startup & runtime server paramter, range (0,1)).
     //
-    // Controls by what multiplier the target snapshot history window size setting is decreased
-    // when there is cache pressure.
+    // Controls by what multiplier the target snapshot history window setting is decreased when
+    // cache pressure becomes too high, per the cachePressureThreshold setting.
     AtomicDouble snapshotWindowMultiplicativeDecrease{0.75};
 
     // snapshotWindowAdditiveIncreaseSeconds (startup & runtime server paramter, range 1+).
     //
-    // Controls by how much the target snapshot history window size setting is increased when we
-    // need to service older snapshots for global point-in-time reads.
-    AtomicWord<int> snapshotWindowAdditiveIncreaseSeconds{1};
+    // Controls by how much the target snapshot history window setting is increased when cache
+    // pressure is OK, per cachePressureThreshold, and we need to service older snapshots for global
+    // point-in-time reads.
+    AtomicWord<int> snapshotWindowAdditiveIncreaseSeconds{2};
 
     // minMillisBetweenSnapshotWindowInc (startup & runtime server paramter, range 0+).
+    // minMillisBetweenSnapshotWindowDec (startup & runtime server paramter, range 0+).
     //
-    // Controls how often attempting to increase the target snapshot window will have an effect.
-    // Multiple callers within minMillisBetweenSnapshotWindowInc will have the same effect as one.
-    // This protects the system because it takes time for the target snapshot window to affect the
-    // actual storage engine snapshot window. The stable timestamp must move forward for the window
-    // between it and oldest timestamp to grow or shrink.
+    // Controls how often attempting to increase/decrease the target snapshot window will have an
+    // effect. Multiple callers within minMillisBetweenSnapshotWindowInc will have the same effect
+    // as one. This protects the system because it takes time for the target snapshot window to
+    // affect the actual storage engine snapshot window. The stable timestamp must move forward for
+    // the window between it and oldest timestamp to grow or shrink.
     AtomicWord<int> minMillisBetweenSnapshotWindowInc{500};
+    AtomicWord<int> minMillisBetweenSnapshotWindowDec{500};
 
-    // decreaseHistoryIfNotNeededPeriodSeconds (startup & runtime server paramter, range 1+)
+    // checkCachePressurePeriodSeconds (startup & runtime server paramter, range 1+)
     //
     // Controls the period of the task that checks for cache pressure and decreases
-    // targetSnapshotHistoryWindowInSeconds if there has been pressure and no new SnapshotTooOld
-    // errors.
-    //
-    // This should not run very frequently. It is preferable to increase the window size, and cache
-    // pressure, rather than failing PIT reads.
-    AtomicWord<int> decreaseHistoryIfNotNeededPeriodSeconds{15};
-
-    static inline MutableObserverRegistry<decltype(
-        decreaseHistoryIfNotNeededPeriodSeconds)::WordType>
-        observeDecreaseHistoryIfNotNeededPeriodSeconds;
+    // targetSnapshotHistoryWindowInSeconds if the pressure is above cachePressureThreshold. The
+    // target window size setting must not be decreased too fast because time must be allowed for
+    // the storage engine to attempt to act on the new setting.
+    AtomicWord<int> checkCachePressurePeriodSeconds{5};
 
-    AtomicWord<long long> snapshotTooOldErrorCount{0};
+    static inline MutableObserverRegistry<decltype(checkCachePressurePeriodSeconds)::WordType>
+        observeCheckCachePressurePeriodSeconds;
 };
 
 extern SnapshotWindowParams snapshotWindowParams;

src/mongo/db/snapshot_window_options.idl

@@ -41,6 +41,14 @@ server_parameters:
     cpp_varname: "snapshotWindowParams.maxTargetSnapshotHistoryWindowInSeconds"
     validator: { gte: 0 }
 
+  cachePressureThreshold:
+    description: "Cache pressure threshold"
+    set_at: [ startup, runtime ]
+    cpp_varname: "snapshotWindowParams.cachePressureThreshold"
+    validator:
+      gte: 0
+      lte: 100
+
   snapshotWindowMultiplicativeDecrease:
     description: "Snapshot window multiplicative decrease"
     set_at: [ startup, runtime ]
@@ -61,9 +69,15 @@ server_parameters:
     cpp_varname: "snapshotWindowParams.minMillisBetweenSnapshotWindowInc"
     validator: { gte: 1 }
 
-  decreaseHistoryIfNotNeededPeriodSeconds:
+  minMillisBetweenSnapshotWindowDec:
+    description: "Minimum duration between snapshot window decrement, in milliseconds"
+    set_at: [ startup, runtime ]
+    cpp_varname: "snapshotWindowParams.minMillisBetweenSnapshotWindowDec"
+    validator: { gte: 1 }
+
+  checkCachePressurePeriodSeconds:
     description: "Check cache pressure period, in seconds"
     set_at: [ startup, runtime ]
-    cpp_varname: "snapshotWindowParams.decreaseHistoryIfNotNeededPeriodSeconds"
+    cpp_varname: "snapshotWindowParams.checkCachePressurePeriodSeconds"
     validator: { gte: 1 }
-    on_update: std::ref(SnapshotWindowParams::observeDecreaseHistoryIfNotNeededPeriodSeconds)
+    on_update: std::ref(SnapshotWindowParams::observeCheckCachePressurePeriodSeconds)

src/mongo/db/snapshot_window_util.cpp

@@ -52,12 +52,25 @@ namespace SnapshotWindowUtil {
 // Adds concurrency control to increaseTargetSnapshotWindowSize() and
 // decreaseTargetSnapshotWindowSize(). They should not run concurrently with themselves or one
 // another, since they act on and modify the same storage parameters. Further guards the static
-// variable "_snapshotWindowLastIncreasedAt" used in increaseTargetSnapshotWindowSize().
+// variables "_snapshotWindowLastDecreasedAt" and "_snapshotWindowLastIncreasedAt" used in
+// increaseTargetSnapshotWindowSize() and decreaseSnapshowWindow().
 stdx::mutex snapshotWindowMutex;
 
 namespace {
 
-void _decreaseTargetSnapshotWindowSize(WithLock, OperationContext* opCtx) {
+void _decreaseTargetSnapshotWindowSize(WithLock lock, OperationContext* opCtx) {
+    // Tracks the last time that the snapshot window was decreased so that it does not go down so
+    // fast that the system does not have time to react and reduce snapshot availability.
+    static Date_t _snapshotWindowLastDecreasedAt{Date_t::min()};
+
+    if (_snapshotWindowLastDecreasedAt >
+        (Date_t::now() -
+         Milliseconds(snapshotWindowParams.minMillisBetweenSnapshotWindowDec.load()))) {
+        // We have already decreased the window size in the last minMillisBetweenSnapshotWindowDec
+        // milliseconds.
+        return;
+    }
+
     snapshotWindowParams.targetSnapshotHistoryWindowInSeconds.store(
         snapshotWindowParams.targetSnapshotHistoryWindowInSeconds.load() *
         snapshotWindowParams.snapshotWindowMultiplicativeDecrease.load());
@@ -67,6 +80,8 @@ void _decreaseTargetSnapshotWindowSize(WithLock, OperationContext* opCtx) {
     StorageEngine* engine = opCtx->getServiceContext()->getStorageEngine();
     invariant(engine);
     engine->setOldestTimestampFromStable();
+
+    _snapshotWindowLastDecreasedAt = Date_t::now();
 }
 
 }  // namespace
@@ -90,6 +105,23 @@ void increaseTargetSnapshotWindowSize(OperationContext* opCtx) {
         return;
     }
 
+    // If the cache pressure is already too high, we will not put more pressure on it by increasing
+    // the window size.
+    StorageEngine* engine = opCtx->getServiceContext()->getStorageEngine();
+    if (engine && engine->isCacheUnderPressure(opCtx)) {
+        warning() << "Attempted to increase the time window of available snapshots for "
+                     "point-in-time operations (readConcern level 'snapshot' or transactions), but "
+                     "the storage engine cache pressure, per the cachePressureThreshold setting of "
+                     "'"
+                  << snapshotWindowParams.cachePressureThreshold.load()
+                  << "', is too high to allow it to increase. If this happens frequently, consider "
+                     "either increasing the cache pressure threshold or increasing the memory "
+                     "available to the storage engine cache, in order to improve the success rate "
+                     "or speed of point-in-time requests.";
+        _decreaseTargetSnapshotWindowSize(lock, opCtx);
+        return;
+    }
+
     if (snapshotWindowParams.targetSnapshotHistoryWindowInSeconds.load() ==
         snapshotWindowParams.maxTargetSnapshotHistoryWindowInSeconds.load()) {
         warning() << "Attempted to increase the time window of available snapshots for "
@@ -119,28 +151,10 @@ void decreaseTargetSnapshotWindowSize(OperationContext* opCtx) {
     stdx::unique_lock<stdx::mutex> lock(snapshotWindowMutex);
 
     StorageEngine* engine = opCtx->getServiceContext()->getStorageEngine();
-    if (engine) {
-        static auto lastInsertsCount = 0;
-        static auto lastSnapshotErrorCount = 0;
-
-        auto currentInsertsCount = engine->getCacheOverflowTableInsertCount(opCtx);
-        auto currentSnapshotErrorCount = snapshotWindowParams.snapshotTooOldErrorCount.load();
-
-        // Only decrease the snapshot window size if there were writes to the cache overflow table
-        // and there has been no new SnapshotTooOld errors in the same time period.
-        if (currentInsertsCount > lastInsertsCount &&
-            currentSnapshotErrorCount == lastSnapshotErrorCount) {
-            _decreaseTargetSnapshotWindowSize(lock, opCtx);
-        }
-
-        lastInsertsCount = currentInsertsCount;
-        lastSnapshotErrorCount = currentSnapshotErrorCount;
+    if (engine && engine->isCacheUnderPressure(opCtx)) {
+        _decreaseTargetSnapshotWindowSize(lock, opCtx);
     }
 }
 
-void incrementSnapshotTooOldErrorCount() {
-    snapshotWindowParams.snapshotTooOldErrorCount.addAndFetch(1);
-}
-
 }  // namespace SnapshotWindowUtil
 }  // namespace mongo