SERVER-5739 Fix races in RARELY/OCCASIONALLY.

Switch to using C++ instead of macros.
Fix SERVER-37247: these should fire on first hit.

13 files changed, 86 insertions, 84 deletions

diff --git a/src/mongo/db/cloner.cpp b/src/mongo/db/cloner.cpp
index 2768cb0b578..fc62f8f59a5 100644
--- a/src/mongo/db/cloner.cpp
+++ b/src/mongo/db/cloner.cpp
@@ -271,7 +271,8 @@ struct Cloner::Fun {
                 }
             });
 
-            RARELY if (time(0) - saveLast > 60) {
+            static Rarely sampler;
+            if (sampler.tick() && (time(0) - saveLast > 60)) {
                 log() << numSeen << " objects cloned so far from collection " << from_collection;
                 saveLast = time(0);
             }
diff --git a/src/mongo/db/repl/oplog_applier.cpp b/src/mongo/db/repl/oplog_applier.cpp
index 596a4ca2782..9d25e212065 100644
--- a/src/mongo/db/repl/oplog_applier.cpp
+++ b/src/mongo/db/repl/oplog_applier.cpp
@@ -148,7 +148,8 @@ void OplogApplier::enqueue(OperationContext* opCtx,
 void OplogApplier::enqueue(OperationContext* opCtx,
                            OplogBuffer::Batch::const_iterator begin,
                            OplogBuffer::Batch::const_iterator end) {
-    OCCASIONALLY {
+    static Occasionally sampler;
+    if (sampler.tick()) {
         LOG(2) << "oplog buffer has " << _oplogBuffer->getSize() << " bytes";
     }
     _oplogBuffer->pushAllNonBlocking(opCtx, begin, end);
diff --git a/src/mongo/db/repl/topology_coordinator.cpp b/src/mongo/db/repl/topology_coordinator.cpp
index d1564250b07..cd41e11e08e 100644
--- a/src/mongo/db/repl/topology_coordinator.cpp
+++ b/src/mongo/db/repl/topology_coordinator.cpp
@@ -252,8 +252,10 @@ HostAndPort TopologyCoordinator::chooseNewSyncSource(Date_t now,
     int needMorePings = (_memberData.size() - 1) * 2 - _getTotalPings();
 
     if (needMorePings > 0) {
-        OCCASIONALLY log() << "waiting for " << needMorePings
-                           << " pings from other members before syncing";
+        static Occasionally sampler;
+        if (sampler.tick()) {
+            log() << "waiting for " << needMorePings << " pings from other members before syncing";
+        }
         _syncSource = HostAndPort();
         return _syncSource;
     }
diff --git a/src/mongo/db/s/balancer/balancer.cpp b/src/mongo/db/s/balancer/balancer.cpp
index f5570a52507..86a80ee5032 100644
--- a/src/mongo/db/s/balancer/balancer.cpp
+++ b/src/mongo/db/s/balancer/balancer.cpp
@@ -360,8 +360,10 @@ void Balancer::_mainThread() {
                        << ", secondaryThrottle: "
                        << balancerConfig->getSecondaryThrottle().toBSON();
 
-                OCCASIONALLY warnOnMultiVersion(
-                    uassertStatusOK(_clusterStats->getStats(opCtx.get())));
+                static Occasionally sampler;
+                if (sampler.tick()) {
+                    warnOnMultiVersion(uassertStatusOK(_clusterStats->getStats(opCtx.get())));
+                }
 
                 Status status = _enforceTagRanges(opCtx.get());
                 if (!status.isOK()) {
diff --git a/src/mongo/db/s/migration_destination_manager.cpp b/src/mongo/db/s/migration_destination_manager.cpp
index 8f5ff485dad..8e19ad74a65 100644
--- a/src/mongo/db/s/migration_destination_manager.cpp
+++ b/src/mongo/db/s/migration_destination_manager.cpp
@@ -1102,9 +1102,11 @@ bool MigrationDestinationManager::_flushPendingWrites(OperationContext* opCtx,
                                                       const repl::OpTime& lastOpApplied) {
     if (!opReplicatedEnough(opCtx, lastOpApplied, _writeConcern)) {
         repl::OpTime op(lastOpApplied);
-        OCCASIONALLY log() << "migrate commit waiting for a majority of slaves for '" << _nss.ns()
-                           << "' " << redact(_min) << " -> " << redact(_max)
-                           << " waiting for: " << op;
+        static Occasionally sampler;
+        if (sampler.tick()) {
+            log() << "migrate commit waiting for a majority of slaves for '" << _nss.ns() << "' "
+                  << redact(_min) << " -> " << redact(_max) << " waiting for: " << op;
+        }
         return false;
     }
 
diff --git a/src/mongo/db/s/set_shard_version_command.cpp b/src/mongo/db/s/set_shard_version_command.cpp
index 12a8d0ef469..5b9ff6db26e 100644
--- a/src/mongo/db/s/set_shard_version_command.cpp
+++ b/src/mongo/db/s/set_shard_version_command.cpp
@@ -383,7 +383,10 @@ public:
                                        << ", requested version is " << requestedVersion.toString()
                                        << " but found version " << currVersion.toString();
 
-                OCCASIONALLY warning() << errmsg;
+                static Occasionally sampler;
+                if (sampler.tick()) {
+                    warning() << errmsg;
+                }
 
                 // WARNING: the exact fields below are important for compatibility with mongos
                 // version reload.
diff --git a/src/mongo/db/stats/counters.cpp b/src/mongo/db/stats/counters.cpp
index 81a77016a8b..0355fa8787b 100644
--- a/src/mongo/db/stats/counters.cpp
+++ b/src/mongo/db/stats/counters.cpp
@@ -36,50 +36,10 @@
 #include "mongo/db/stats/counters.h"
 
 #include "mongo/db/jsobj.h"
-#include "mongo/util/debug_util.h"
 #include "mongo/util/log.h"
 
 namespace mongo {
 
-using std::endl;
-
-OpCounters::OpCounters() {}
-
-void OpCounters::gotInserts(int n) {
-    RARELY _checkWrap();
-    _insert.fetchAndAdd(n);
-}
-
-void OpCounters::gotInsert() {
-    RARELY _checkWrap();
-    _insert.fetchAndAdd(1);
-}
-
-void OpCounters::gotQuery() {
-    RARELY _checkWrap();
-    _query.fetchAndAdd(1);
-}
-
-void OpCounters::gotUpdate() {
-    RARELY _checkWrap();
-    _update.fetchAndAdd(1);
-}
-
-void OpCounters::gotDelete() {
-    RARELY _checkWrap();
-    _delete.fetchAndAdd(1);
-}
-
-void OpCounters::gotGetMore() {
-    RARELY _checkWrap();
-    _getmore.fetchAndAdd(1);
-}
-
-void OpCounters::gotCommand() {
-    RARELY _checkWrap();
-    _command.fetchAndAdd(1);
-}
-
 void OpCounters::gotOp(int op, bool isCommand) {
     switch (op) {
         case dbInsert: /*gotInsert();*/
@@ -104,18 +64,14 @@ void OpCounters::gotOp(int op, bool isCommand) {
         case opReply:
             break;
         default:
-            log() << "OpCounters::gotOp unknown op: " << op << endl;
+            log() << "OpCounters::gotOp unknown op: " << op << std::endl;
     }
 }
 
-void OpCounters::_checkWrap() {
-    const int64_t MAX = 1ULL << 60;
-
-    bool wrap = _insert.loadRelaxed() > MAX || _query.loadRelaxed() > MAX ||
-        _update.loadRelaxed() > MAX || _delete.loadRelaxed() > MAX ||
-        _getmore.loadRelaxed() > MAX || _command.loadRelaxed() > MAX;
-
-    if (wrap) {
+void OpCounters::_checkWrap(CacheAligned<AtomicInt64> OpCounters::*counter, int n) {
+    static constexpr auto maxCount = AtomicInt64::WordType{1} << 60;
+    auto oldValue = (this->*counter).fetchAndAddRelaxed(n);
+    if (oldValue > maxCount) {
         _insert.store(0);
         _query.store(0);
         _update.store(0);
diff --git a/src/mongo/db/stats/counters.h b/src/mongo/db/stats/counters.h
index 07e38b251a9..4d6a057e427 100644
--- a/src/mongo/db/stats/counters.h
+++ b/src/mongo/db/stats/counters.h
@@ -47,14 +47,29 @@ namespace mongo {
  */
 class OpCounters {
 public:
-    OpCounters();
-    void gotInserts(int n);
-    void gotInsert();
-    void gotQuery();
-    void gotUpdate();
-    void gotDelete();
-    void gotGetMore();
-    void gotCommand();
+    OpCounters() = default;
+
+    void gotInserts(int n) {
+        _checkWrap(&OpCounters::_insert, n);
+    }
+    void gotInsert() {
+        _checkWrap(&OpCounters::_insert, 1);
+    }
+    void gotQuery() {
+        _checkWrap(&OpCounters::_query, 1);
+    }
+    void gotUpdate() {
+        _checkWrap(&OpCounters::_update, 1);
+    }
+    void gotDelete() {
+        _checkWrap(&OpCounters::_delete, 1);
+    }
+    void gotGetMore() {
+        _checkWrap(&OpCounters::_getmore, 1);
+    }
+    void gotCommand() {
+        _checkWrap(&OpCounters::_command, 1);
+    }
 
     void gotOp(int op, bool isCommand);
 
@@ -81,7 +96,8 @@ public:
     }
 
 private:
-    void _checkWrap();
+    // Increment member `counter` by `n`, resetting all counters if it was > 2^60.
+    void _checkWrap(CacheAligned<AtomicInt64> OpCounters::*counter, int n);
 
     CacheAligned<AtomicInt64> _insert;
     CacheAligned<AtomicInt64> _query;
diff --git a/src/mongo/dbtests/basictests.cpp b/src/mongo/dbtests/basictests.cpp
index ed130e0fca8..3fd33cffde6 100644
--- a/src/mongo/dbtests/basictests.cpp
+++ b/src/mongo/dbtests/basictests.cpp
@@ -57,7 +57,7 @@ using std::string;
 using std::stringstream;
 using std::vector;
 
-class Rarely {
+class RarelyTest {
 public:
     void run() {
         int first = 0;
@@ -72,10 +72,14 @@ public:
 
 private:
     void incRarely(int& c) {
-        RARELY++ c;
+        static mongo::Rarely s;
+        if (s.tick())
+            ++c;
     }
     void incRarely2(int& c) {
-        RARELY++ c;
+        static mongo::Rarely s;
+        if (s.tick())
+            ++c;
     }
 };
 
@@ -387,7 +391,7 @@ public:
     All() : Suite("basic") {}
 
     void setupTests() {
-        add<Rarely>();
+        add<RarelyTest>();
         add<Base64Tests>();
 
         add<stringbuildertests::simple1>();
diff --git a/src/mongo/platform/atomic_word.h b/src/mongo/platform/atomic_word.h
index 27076a4c3ac..60c13368737 100644
--- a/src/mongo/platform/atomic_word.h
+++ b/src/mongo/platform/atomic_word.h
@@ -125,6 +125,14 @@ public:
     }
 
     /**
+     * Like "fetchAndAdd", but with relaxed memory order. Appropriate where relative
+     * order of operations doesn't matter. A stat counter, for example.
+     */
+    WordType fetchAndAddRelaxed(WordType increment) {
+        return _value.fetch_add(increment, std::memory_order_relaxed);
+    }
+
+    /**
      * Get the current value of this, subtract "decrement" and store it, atomically.
      *
      * Returns the value of this before decrementing.
diff --git a/src/mongo/s/client/parallel.cpp b/src/mongo/s/client/parallel.cpp
index 40595a53415..83b7d8a0daf 100644
--- a/src/mongo/s/client/parallel.cpp
+++ b/src/mongo/s/client/parallel.cpp
@@ -375,7 +375,8 @@ void ParallelSortClusteredCursor::setupVersionAndHandleSlaveOk(
             // that the primary is now up on it's own and has to rely on other threads to refresh
             // node states.
 
-            OCCASIONALLY {
+            static Occasionally sampler;
+            if (sampler.tick()) {
                 const DBClientReplicaSet* repl = dynamic_cast<const DBClientReplicaSet*>(rawConn);
                 dassert(repl);
                 warning() << "Primary for " << repl->getServerAddress()
@@ -400,7 +401,8 @@ void ParallelSortClusteredCursor::setupVersionAndHandleSlaveOk(
             // It's okay if we don't set the version when talking to a secondary, we can
             // be stale in any case.
 
-            OCCASIONALLY {
+            static Occasionally sampler;
+            if (sampler.tick()) {
                 const DBClientReplicaSet* repl =
                     dynamic_cast<const DBClientReplicaSet*>(state->conn->getRawConn());
                 dassert(repl);
diff --git a/src/mongo/s/client/version_manager.cpp b/src/mongo/s/client/version_manager.cpp
index 2bc894d8463..2ee8ddf5c7c 100644
--- a/src/mongo/s/client/version_manager.cpp
+++ b/src/mongo/s/client/version_manager.cpp
@@ -218,7 +218,8 @@ bool initShardVersionEmptyNS(OperationContext* opCtx, DBClientBase* conn_in) {
         // checkShardVersion is required (which includes initShardVersion information) if these
         // connections are used.
 
-        OCCASIONALLY {
+        static Occasionally sampler;
+        if (sampler.tick()) {
             warning() << "failed to initialize new replica set connection version, "
                       << "will initialize on first use";
         }
diff --git a/src/mongo/util/debug_util.h b/src/mongo/util/debug_util.h
index 82c0dce414d..84068f0e1aa 100644
--- a/src/mongo/util/debug_util.h
+++ b/src/mongo/util/debug_util.h
@@ -33,6 +33,7 @@
 #pragma once
 
 #include "mongo/config.h"
+#include "mongo/platform/atomic_word.h"
 
 namespace mongo {
 
@@ -45,16 +46,19 @@ const bool kDebugBuild = false;
 #define MONGO_DEV if (kDebugBuild)
 #define DEV MONGO_DEV
 
-// The following declare one unique counter per enclosing function.
-// NOTE The implementation double-increments on a match, but we don't really care.
-#define MONGO_SOMETIMES(occasion, howOften) \
-    for (static unsigned occasion = 0; ++occasion % howOften == 0;)
-#define SOMETIMES MONGO_SOMETIMES
+template <unsigned period>
+class SampleEveryNth {
+public:
+    // Increment, returning true on first call and each subsequent 'period'.
+    bool tick() {
+        return _count.fetchAndAddRelaxed(1) % period == 0;
+    }
 
-#define MONGO_OCCASIONALLY SOMETIMES(occasionally, 16)
-#define OCCASIONALLY MONGO_OCCASIONALLY
+private:
+    AtomicInt64 _count{0};
+};
 
-#define MONGO_RARELY SOMETIMES(rarely, 128)
-#define RARELY MONGO_RARELY
+struct Occasionally : SampleEveryNth<16> {};
+struct Rarely : SampleEveryNth<128> {};
 
 }  // namespace mongo