SERVER-32843 Allow multiple times in OpObservers

The OpObserverRegistry was introduced as an abstraction to allow
decoupling making data modifications from the side effects, which
need to happen as a result of these modifications, such as op log
writes, retryable writes, etc.

Some of the OpObserver's methods currently return the OpTime which
resulted from logging the operation to the replication oplog. In
addition, in certain cases, the OpTime resulting from an earlier
OpObserver might be needed by a later one, which is the case with
retryable writes.

In order to support these requirements, the OpObserver(s) chain
should have access to some common per-operation structure, where
runtime information could be persisted.

1 files changed, 30 insertions, 16 deletions

diff --git a/src/mongo/db/op_observer_registry_test.cpp b/src/mongo/db/op_observer_registry_test.cpp
index 6447962e323..5589860e1e7 100644
--- a/src/mongo/db/op_observer_registry_test.cpp
+++ b/src/mongo/db/op_observer_registry_test.cpp
@@ -26,8 +26,12 @@
  *    it in the license file.
  */
 
+#include "mongo/platform/basic.h"
+
 #include "mongo/db/op_observer_registry.h"
+
 #include "mongo/db/op_observer_noop.h"
+#include "mongo/db/operation_context_noop.h"
 #include "mongo/db/repl/optime.h"
 #include "mongo/unittest/death_test.h"
 #include "mongo/unittest/unittest.h"
@@ -52,7 +56,8 @@ struct TestObserver : public OpObserverNoop {
                                   const NamespaceString& collectionName,
                                   OptionalCollectionUUID uuid) {
         drops++;
-        return opTime;
+        OpObserver::Times::get(opCtx).reservedOpTimes.push_back(opTime);
+        return {};
     }
     repl::OpTime onRenameCollection(OperationContext* opCtx,
                                     const NamespaceString& fromCollection,
@@ -61,7 +66,8 @@ struct TestObserver : public OpObserverNoop {
                                     bool dropTarget,
                                     OptionalCollectionUUID dropTargetUUID,
                                     bool stayTemp) {
-        return opTime;
+        OpObserver::Times::get(opCtx).reservedOpTimes.push_back(opTime);
+        return {};
     }
 };
 
@@ -81,22 +87,22 @@ struct OpObserverRegistryTest : public unittest::Test {
     OpObserverRegistry registry;
     /**
      * The 'op' function calls an observer method on the registry that returns an OpTime.
-     * The method checks that the registry correctly merges the results of the registered observers.
+     * The method checks that the registry correctly returns only the first observer's `OpTime`.
      */
     void checkConsistentOpTime(stdx::function<repl::OpTime()> op) {
         const repl::OpTime myTime(Timestamp(1, 1), 1);
         ASSERT(op() == repl::OpTime());
         observer1->opTime = myTime;
         ASSERT(op() == myTime);
-        observer2->opTime = myTime;
+        observer2->opTime = repl::OpTime(Timestamp(1, 1), 2);
         ASSERT(op() == myTime);
         observer1->opTime = {};
-        ASSERT(op() == myTime);
+        ASSERT(op() == repl::OpTime{});
     }
 
     /**
      * The 'op' function calls an observer method on the registry that returns an OpTime.
-     * The method checks that the registry invariants if the observers return conflicting times.
+     * The method checks that the registry invariants if the observers return multiple times.
      */
     void checkInconsistentOpTime(stdx::function<repl::OpTime()> op) {
         observer1->opTime = repl::OpTime(Timestamp(1, 1), 1);
@@ -106,67 +112,75 @@ struct OpObserverRegistryTest : public unittest::Test {
 };
 
 TEST_F(OpObserverRegistryTest, NoObservers) {
+    OperationContextNoop opCtx;
     // Check that it's OK to call observer methods with no observers registered.
-    registry.onDropDatabase(nullptr, "test");
+    registry.onDropDatabase(&opCtx, "test");
 }
 
 TEST_F(OpObserverRegistryTest, TwoObservers) {
+    OperationContextNoop opCtx;
     ASSERT_EQUALS(testObservers, 2);
     registry.addObserver(std::move(unique1));
     registry.addObserver(std::move(unique2));
-    registry.onDropDatabase(nullptr, "test");
+    registry.onDropDatabase(&opCtx, "test");
     ASSERT_EQUALS(observer1->drops, 1);
     ASSERT_EQUALS(observer2->drops, 1);
 }
 
 TEST_F(OpObserverRegistryTest, ThrowingObserver1) {
+    OperationContextNoop opCtx;
     unique1 = stdx::make_unique<ThrowingObserver>();
     observer1 = unique1.get();
     registry.addObserver(std::move(unique1));
     registry.addObserver(std::move(unique2));
-    ASSERT_THROWS(registry.onDropDatabase(nullptr, "test"), AssertionException);
+    ASSERT_THROWS(registry.onDropDatabase(&opCtx, "test"), AssertionException);
     ASSERT_EQUALS(observer1->drops, 1);
     ASSERT_EQUALS(observer2->drops, 0);
 }
 
 TEST_F(OpObserverRegistryTest, ThrowingObserver2) {
+    OperationContextNoop opCtx;
     unique2 = stdx::make_unique<ThrowingObserver>();
     observer2 = unique1.get();
     registry.addObserver(std::move(unique1));
     registry.addObserver(std::move(unique2));
-    ASSERT_THROWS(registry.onDropDatabase(nullptr, "test"), AssertionException);
+    ASSERT_THROWS(registry.onDropDatabase(&opCtx, "test"), AssertionException);
     ASSERT_EQUALS(observer1->drops, 1);
     ASSERT_EQUALS(observer2->drops, 1);
 }
 
 TEST_F(OpObserverRegistryTest, OnDropCollectionObserverResultReturnsRightTime) {
+    OperationContextNoop opCtx;
     registry.addObserver(std::move(unique1));
-    registry.addObserver(std::move(unique2));
-    auto op = [&]() -> repl::OpTime { return registry.onDropCollection(nullptr, testNss, {}); };
+    registry.addObserver(std::make_unique<OpObserverNoop>());
+    auto op = [&]() -> repl::OpTime { return registry.onDropCollection(&opCtx, testNss, {}); };
     checkConsistentOpTime(op);
 }
 
 TEST_F(OpObserverRegistryTest, OnRenameCollectionObserverResultReturnsRightTime) {
+    OperationContextNoop opCtx;
     registry.addObserver(std::move(unique1));
-    registry.addObserver(std::move(unique2));
+    registry.addObserver(std::make_unique<OpObserverNoop>());
     auto op = [&]() -> repl::OpTime {
-        return registry.onRenameCollection(nullptr, testNss, testNss, {}, false, {}, false);
+        return registry.onRenameCollection(&opCtx, testNss, testNss, {}, false, {}, false);
     };
     checkConsistentOpTime(op);
 }
 
 DEATH_TEST_F(OpObserverRegistryTest, OnDropCollectionReturnsInconsistentTime, "invariant") {
+    OperationContextNoop opCtx;
     registry.addObserver(std::move(unique1));
     registry.addObserver(std::move(unique2));
-    auto op = [&]() -> repl::OpTime { return registry.onDropCollection(nullptr, testNss, {}); };
+    auto op = [&]() -> repl::OpTime { return registry.onDropCollection(&opCtx, testNss, {}); };
     checkInconsistentOpTime(op);
 }
 
 DEATH_TEST_F(OpObserverRegistryTest, OnRenameCollectionReturnsInconsistentTime, "invariant") {
+    OperationContextNoop opCtx;
     registry.addObserver(std::move(unique1));
     registry.addObserver(std::move(unique2));
     auto op = [&]() -> repl::OpTime {
-        return registry.onRenameCollection(nullptr, testNss, testNss, {}, false, {}, false);
+        return registry.onRenameCollection(&opCtx, testNss, testNss, {}, false, {}, false);
     };
     checkInconsistentOpTime(op);
 }