SERVER-35024 atClusterTime incorrectly uses round_to_oldest option when opening transactions. Refactor WiredTigerBeginTxnBlock to use enum parameters with safe defaults.

10 files changed, 189 insertions, 90 deletions

diff --git a/src/mongo/db/storage/wiredtiger/SConscript b/src/mongo/db/storage/wiredtiger/SConscript
index 7d4705126b8..aee0fb5d8a5 100644
--- a/src/mongo/db/storage/wiredtiger/SConscript
+++ b/src/mongo/db/storage/wiredtiger/SConscript
@@ -36,6 +36,7 @@ if wiredtiger:
     wtEnv.Library(
         target='storage_wiredtiger_core',
         source= [
+            'wiredtiger_begin_transaction_block.cpp',
             'wiredtiger_global_options.cpp',
             'wiredtiger_index.cpp',
             'wiredtiger_kv_engine.cpp',
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_begin_transaction_block.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_begin_transaction_block.cpp
new file mode 100644
index 00000000000..1a016421510
--- /dev/null
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_begin_transaction_block.cpp
@@ -0,0 +1,89 @@
+/**
+ *    Copyright (C) 2018 MongoDB Inc.
+ *
+ *    This program is free software: you can redistribute it and/or  modify
+ *    it under the terms of the GNU Affero General Public License, version 3,
+ *    as published by the Free Software Foundation.
+ *
+ *
+ *    This program is distributed in the hope that it will be useful,
+ *    but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *    GNU Affero General Public License for more details.
+ *
+ *    You should have received a copy of the GNU Affero General Public License
+ *    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ *    As a special exception, the copyright holders give permission to link the
+ *    code of portions of this program with the OpenSSL library under certain
+ *    conditions as described in each individual source file and distribute
+ *    linked combinations including the program with the OpenSSL library. You
+ *    must comply with the GNU Affero General Public License in all respects for
+ *    all of the code used other than as permitted herein. If you modify file(s)
+ *    with this exception, you may extend this exception to your version of the
+ *    file(s), but you are not obligated to do so. If you do not wish to do so,
+ *    delete this exception statement from your version. If you delete this
+ *    exception statement from all source files in the program, then also delete
+ *    it in the license file.
+ */
+
+#define MONGO_LOG_DEFAULT_COMPONENT ::mongo::logger::LogComponent::kStorage
+
+#include "mongo/platform/basic.h"
+
+#include <cstdio>
+
+#include "mongo/db/storage/wiredtiger/wiredtiger_begin_transaction_block.h"
+#include "mongo/db/storage/wiredtiger/wiredtiger_util.h"
+#include "mongo/util/errno_util.h"
+#include "mongo/util/log.h"
+
+namespace mongo {
+
+WiredTigerBeginTxnBlock::WiredTigerBeginTxnBlock(WT_SESSION* session, IgnorePrepared ignorePrepare)
+    : _session(session) {
+    invariant(!_rollback);
+    invariantWTOK(_session->begin_transaction(
+        _session, (ignorePrepare == IgnorePrepared::kIgnore) ? "ignore_prepare=true" : nullptr));
+    _rollback = true;
+}
+
+WiredTigerBeginTxnBlock::WiredTigerBeginTxnBlock(WT_SESSION* session, const char* config)
+    : _session(session) {
+    invariant(!_rollback);
+    invariantWTOK(_session->begin_transaction(_session, config));
+    _rollback = true;
+}
+
+WiredTigerBeginTxnBlock::~WiredTigerBeginTxnBlock() {
+    if (_rollback) {
+        invariant(_session->rollback_transaction(_session, nullptr) == 0);
+    }
+}
+
+Status WiredTigerBeginTxnBlock::setTimestamp(Timestamp readTimestamp, RoundToOldest roundToOldest) {
+    invariant(_rollback);
+    char readTSConfigString[15 /* read_timestamp= */ + 16 /* 16 hexadecimal digits */ +
+                            17 /* ,round_to_oldest= */ + 5 /* false */ + 1 /* trailing null */];
+    auto size = std::snprintf(readTSConfigString,
+                              sizeof(readTSConfigString),
+                              "read_timestamp=%llx,round_to_oldest=%s",
+                              readTimestamp.asULL(),
+                              (roundToOldest == RoundToOldest::kRound) ? "true" : "false");
+    if (size < 0) {
+        int e = errno;
+        error() << "error snprintf " << errnoWithDescription(e);
+        fassertFailedNoTrace(40664);
+    }
+    invariant(static_cast<std::size_t>(size) < sizeof(readTSConfigString));
+
+    auto status = wtRCToStatus(_session->timestamp_transaction(_session, readTSConfigString));
+    return status;
+}
+
+void WiredTigerBeginTxnBlock::done() {
+    invariant(_rollback);
+    _rollback = false;
+}
+
+}  // namespace mongo
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_begin_transaction_block.h b/src/mongo/db/storage/wiredtiger/wiredtiger_begin_transaction_block.h
new file mode 100644
index 00000000000..c7010cf1e4b
--- /dev/null
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_begin_transaction_block.h
@@ -0,0 +1,80 @@
+/**
+ *    Copyright (C) 2018 MongoDB Inc.
+ *
+ *    This program is free software: you can redistribute it and/or  modify
+ *    it under the terms of the GNU Affero General Public License, version 3,
+ *    as published by the Free Software Foundation.
+ *
+ *
+ *    This program is distributed in the hope that it will be useful,
+ *    but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *    GNU Affero General Public License for more details.
+ *
+ *    You should have received a copy of the GNU Affero General Public License
+ *    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ *    As a special exception, the copyright holders give permission to link the
+ *    code of portions of this program with the OpenSSL library under certain
+ *    conditions as described in each individual source file and distribute
+ *    linked combinations including the program with the OpenSSL library. You
+ *    must comply with the GNU Affero General Public License in all respects for
+ *    all of the code used other than as permitted herein. If you modify file(s)
+ *    with this exception, you may extend this exception to your version of the
+ *    file(s), but you are not obligated to do so. If you do not wish to do so,
+ *    delete this exception statement from your version. If you delete this
+ *    exception statement from all source files in the program, then also delete
+ *    it in the license file.
+ */
+
+#pragma once
+
+#include <string>
+
+#include <wiredtiger.h>
+
+#include "mongo/base/status.h"
+#include "mongo/bson/timestamp.h"
+
+namespace mongo {
+
+/**
+ * When constructed, this object begins a WiredTiger transaction on the provided session. The
+ * transaction will be rolled back if dismissRollback is not called before the object is destructed.
+ */
+class WiredTigerBeginTxnBlock {
+public:
+    // Whether or not to ignore prepared transactions.
+    enum class IgnorePrepared {
+        kNoIgnore,  // Do not ignore prepared transactions and return prepare conflicts.
+        kIgnore     // Ignore prepared transactions and show prepared, but uncommitted data.
+    };
+
+    // Whether or not to round up to the oldest timestamp when the read timestamp is behind it.
+    enum class RoundToOldest {
+        kNoRound,  // Do not round to the oldest timestamp. BadValue error may be returned.
+        kRound     // Round the read timestamp up to the oldest timestamp when it is behind.
+    };
+
+    WiredTigerBeginTxnBlock(WT_SESSION* session,
+                            IgnorePrepared ignorePrepared = IgnorePrepared::kNoIgnore);
+    WiredTigerBeginTxnBlock(WT_SESSION* session, const char* config);
+    ~WiredTigerBeginTxnBlock();
+
+    /**
+     * Sets the read timestamp on the opened transaction. Cannot be called after a call to done().
+     */
+    Status setTimestamp(Timestamp, RoundToOldest roundToOldest = RoundToOldest::kNoRound);
+
+    /**
+     * End the begin transaction block. Must be called to ensure the opened transaction
+     * is not be rolled back.
+     */
+    void done();
+
+private:
+    WT_SESSION* _session;
+    bool _rollback = false;
+};
+
+}  // namespace mongo
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.cpp
index df20c04eb92..1fc3ec265dc 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.cpp
@@ -36,6 +36,7 @@
 
 #include "mongo/bson/bsonobjbuilder.h"
 #include "mongo/db/server_options.h"
+#include "mongo/db/storage/wiredtiger/wiredtiger_begin_transaction_block.h"
 #include "mongo/db/storage/wiredtiger/wiredtiger_kv_engine.h"
 #include "mongo/db/storage/wiredtiger/wiredtiger_prepare_conflict.h"
 #include "mongo/db/storage/wiredtiger/wiredtiger_session_cache.h"
@@ -347,8 +348,9 @@ void WiredTigerRecoveryUnit::_txnOpen() {
             WiredTigerBeginTxnBlock txnOpen(session, _ignorePrepared);
 
             if (_isOplogReader) {
-                auto status = txnOpen.setTimestamp(
-                    Timestamp(_oplogManager->getOplogReadTimestamp()), true /* roundToOldest */);
+                auto status =
+                    txnOpen.setTimestamp(Timestamp(_oplogManager->getOplogReadTimestamp()),
+                                         WiredTigerBeginTxnBlock::RoundToOldest::kRound);
                 fassert(50771, status);
             }
             txnOpen.done();
@@ -382,7 +384,7 @@ void WiredTigerRecoveryUnit::_txnOpen() {
         }
         case ReadSource::kProvided: {
             WiredTigerBeginTxnBlock txnOpen(session, _ignorePrepared);
-            auto status = txnOpen.setTimestamp(_readAtTimestamp, session);
+            auto status = txnOpen.setTimestamp(_readAtTimestamp);
 
             if (!status.isOK() && status.code() == ErrorCodes::BadValue) {
                 uasserted(ErrorCodes::SnapshotTooOld,
@@ -463,7 +465,8 @@ void WiredTigerRecoveryUnit::setPrepareTimestamp(Timestamp timestamp) {
 }
 
 void WiredTigerRecoveryUnit::setIgnorePrepared(bool value) {
-    _ignorePrepared = value;
+    _ignorePrepared = (value) ? WiredTigerBeginTxnBlock::IgnorePrepared::kIgnore
+                              : WiredTigerBeginTxnBlock::IgnorePrepared::kNoIgnore;
 }
 
 void WiredTigerRecoveryUnit::setTimestampReadSource(ReadSource readSource,
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.h b/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.h
index dba89b45470..b5aecb63077 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.h
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.h
@@ -41,6 +41,7 @@
 #include "mongo/db/record_id.h"
 #include "mongo/db/repl/read_concern_level.h"
 #include "mongo/db/storage/recovery_unit.h"
+#include "mongo/db/storage/wiredtiger/wiredtiger_begin_transaction_block.h"
 #include "mongo/db/storage/wiredtiger/wiredtiger_session_cache.h"
 #include "mongo/util/timer.h"
 
@@ -166,7 +167,8 @@ private:
 
     // Ignoring prepared transactions will not return prepare conflicts and allow seeing prepared,
     // but uncommitted data.
-    bool _ignorePrepared = false;
+    WiredTigerBeginTxnBlock::IgnorePrepared _ignorePrepared{
+        WiredTigerBeginTxnBlock::IgnorePrepared::kNoIgnore};
     Timestamp _commitTimestamp;
     Timestamp _prepareTimestamp;
     boost::optional<Timestamp> _lastTimestampSet;
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_session_cache.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_session_cache.cpp
index b75e77fb1b5..c77307b9efe 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_session_cache.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_session_cache.cpp
@@ -126,52 +126,6 @@ WT_CURSOR* WiredTigerSession::getCursor(const std::string& uri, uint64_t id, boo
     return c;
 }
 
-WiredTigerBeginTxnBlock::WiredTigerBeginTxnBlock(WT_SESSION* session, bool ignorePrepare)
-    : _session(session) {
-    invariant(!_rollback);
-    invariantWTOK(
-        _session->begin_transaction(_session, ignorePrepare ? "ignore_prepare=true" : nullptr));
-    _rollback = true;
-}
-
-WiredTigerBeginTxnBlock::WiredTigerBeginTxnBlock(WT_SESSION* session, std::string config)
-    : _session(session) {
-    invariant(!_rollback);
-    invariantWTOK(_session->begin_transaction(_session, config.c_str()));
-    _rollback = true;
-}
-
-WiredTigerBeginTxnBlock::~WiredTigerBeginTxnBlock() {
-    if (_rollback) {
-        invariant(_session->rollback_transaction(_session, nullptr) == 0);
-    }
-}
-
-Status WiredTigerBeginTxnBlock::setTimestamp(Timestamp readTimestamp, bool roundToOldest) {
-    invariant(_rollback);
-    char readTSConfigString[15 /* read_timestamp= */ + 16 /* 16 hexadecimal digits */ +
-                            17 /* ,round_to_oldest= */ + 5 /* false */ + 1 /* trailing null */];
-    auto size = std::snprintf(readTSConfigString,
-                              sizeof(readTSConfigString),
-                              "read_timestamp=%llx,round_to_oldest=%s",
-                              readTimestamp.asULL(),
-                              (roundToOldest) ? "true" : "false");
-    if (size < 0) {
-        int e = errno;
-        error() << "error snprintf " << errnoWithDescription(e);
-        fassertFailedNoTrace(40664);
-    }
-    invariant(static_cast<std::size_t>(size) < sizeof(readTSConfigString));
-
-    auto status = wtRCToStatus(_session->timestamp_transaction(_session, readTSConfigString));
-    return status;
-}
-
-void WiredTigerBeginTxnBlock::done() {
-    invariant(_rollback);
-    _rollback = false;
-}
-
 void WiredTigerSession::releaseCursor(uint64_t id, WT_CURSOR* cursor) {
     invariant(_session);
     invariant(cursor);
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_session_cache.h b/src/mongo/db/storage/wiredtiger/wiredtiger_session_cache.h
index c199ff23866..93646469f11 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_session_cache.h
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_session_cache.h
@@ -150,32 +150,6 @@ private:
 };
 
 /**
- * When constructed, this object begins a WiredTiger transaction on the provided session. The
- * transaction will be rolled back if dismissRollback is not called before the object is destructed.
- */
-class WiredTigerBeginTxnBlock {
-public:
-    WiredTigerBeginTxnBlock(WT_SESSION* session, bool ignorePrepare = false);
-    WiredTigerBeginTxnBlock(WT_SESSION* session, std::string config);
-    ~WiredTigerBeginTxnBlock();
-
-    /**
-     * Sets the read timestamp on the opened transaction. Cannot be called after a call to done().
-     */
-    Status setTimestamp(Timestamp, bool roundToOldest = false);
-
-    /**
-     * End the begin transaction block. Must be called to ensure the opened transaction
-     * is not be rolled back.
-     */
-    void done();
-
-private:
-    WT_SESSION* _session;
-    bool _rollback = false;
-};
-
-/**
  *  This cache implements a shared pool of WiredTiger sessions with the goal to amortize the
  *  cost of session creation and destruction over multiple uses.
  */
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_size_storer.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_size_storer.cpp
index a331d3abd6a..47a60a7439d 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_size_storer.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_size_storer.cpp
@@ -38,6 +38,7 @@
 #include "mongo/bson/bsonobj.h"
 #include "mongo/bson/bsonobjbuilder.h"
 #include "mongo/db/service_context.h"
+#include "mongo/db/storage/wiredtiger/wiredtiger_begin_transaction_block.h"
 #include "mongo/db/storage/wiredtiger/wiredtiger_customization_hooks.h"
 #include "mongo/db/storage/wiredtiger/wiredtiger_record_store.h"
 #include "mongo/db/storage/wiredtiger/wiredtiger_session_cache.h"
@@ -203,7 +204,7 @@ void WiredTigerSizeStorer::syncCache(bool syncToDisk) {
         return;  // Nothing to do.
 
     WT_SESSION* session = _session.getSession();
-    WiredTigerBeginTxnBlock txnOpen(session, syncToDisk ? "sync=true" : "");
+    WiredTigerBeginTxnBlock txnOpen(session, syncToDisk ? "sync=true" : nullptr);
 
     for (Map::iterator it = myMap.begin(); it != myMap.end(); ++it) {
         string uriKey = it->first;
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_snapshot_manager.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_snapshot_manager.cpp
index 38efb37ff6c..77b6d875a59 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_snapshot_manager.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_snapshot_manager.cpp
@@ -33,17 +33,10 @@
 
 #include "mongo/db/storage/wiredtiger/wiredtiger_snapshot_manager.h"
 
-#include <algorithm>
-#include <cstdio>
-
-#include "mongo/db/concurrency/write_conflict_exception.h"
+#include "mongo/db/storage/wiredtiger/wiredtiger_begin_transaction_block.h"
 #include "mongo/db/storage/wiredtiger/wiredtiger_oplog_manager.h"
-#include "mongo/db/storage/wiredtiger/wiredtiger_record_store.h"
-#include "mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.h"
-#include "mongo/db/storage/wiredtiger/wiredtiger_session_cache.h"
 #include "mongo/db/storage/wiredtiger/wiredtiger_util.h"
 #include "mongo/util/log.h"
-#include "mongo/util/mongoutils/str.h"
 
 namespace mongo {
 
@@ -90,9 +83,9 @@ Timestamp WiredTigerSnapshotManager::beginTransactionOnCommittedSnapshot(
     return *_committedSnapshot;
 }
 
-Timestamp WiredTigerSnapshotManager::beginTransactionOnLocalSnapshot(WT_SESSION* session,
-                                                                     bool ignorePrepare) const {
-    WiredTigerBeginTxnBlock txnOpen(session, ignorePrepare);
+Timestamp WiredTigerSnapshotManager::beginTransactionOnLocalSnapshot(
+    WT_SESSION* session, WiredTigerBeginTxnBlock::IgnorePrepared ignorePrepared) const {
+    WiredTigerBeginTxnBlock txnOpen(session, ignorePrepared);
 
     stdx::lock_guard<stdx::mutex> lock(_localSnapshotMutex);
     invariant(_localSnapshot);
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_snapshot_manager.h b/src/mongo/db/storage/wiredtiger/wiredtiger_snapshot_manager.h
index 8503b2e57cc..493e8dfb1ee 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_snapshot_manager.h
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_snapshot_manager.h
@@ -35,6 +35,7 @@
 #include "mongo/base/disallow_copying.h"
 #include "mongo/bson/timestamp.h"
 #include "mongo/db/storage/snapshot_manager.h"
+#include "mongo/db/storage/wiredtiger/wiredtiger_begin_transaction_block.h"
 #include "mongo/stdx/mutex.h"
 
 namespace mongo {
@@ -68,7 +69,8 @@ public:
      *
      * Throws if no local snapshot has been set.
      */
-    Timestamp beginTransactionOnLocalSnapshot(WT_SESSION* session, bool ignorePrepare) const;
+    Timestamp beginTransactionOnLocalSnapshot(
+        WT_SESSION* session, WiredTigerBeginTxnBlock::IgnorePrepared ignorePrepared) const;
 
     /**
      * Returns lowest SnapshotName that could possibly be used by a future call to