Revert "SERVER-15192 Make AuthzManager logOp listener rollback-safe"

This reverts commit 295cb7943e21a22f9f3a95006de21b07b254afd2.

7 files changed, 25 insertions, 105 deletions

diff --git a/src/mongo/db/auth/authorization_manager.cpp b/src/mongo/db/auth/authorization_manager.cpp
index f8b8ca2b3b2..9e546d7da3d 100644
--- a/src/mongo/db/auth/authorization_manager.cpp
+++ b/src/mongo/db/auth/authorization_manager.cpp
@@ -993,14 +993,13 @@ namespace {
     }
 
     void AuthorizationManager::logOp(
-            OperationContext* txn,
             const char* op,
             const char* ns,
             const BSONObj& o,
             BSONObj* o2,
             bool* b) {
 
-        _externalState->logOp(txn, op, ns, o, o2, b);
+        _externalState->logOp(op, ns, o, o2, b);
         if (appliesToAuthzData(op, ns, o)) {
             _invalidateRelevantCacheData(op, ns, o, o2);
         }
diff --git a/src/mongo/db/auth/authorization_manager.h b/src/mongo/db/auth/authorization_manager.h
index c3371bb11c7..1e2d193b65f 100644
--- a/src/mongo/db/auth/authorization_manager.h
+++ b/src/mongo/db/auth/authorization_manager.h
@@ -427,8 +427,7 @@ namespace mongo {
          * Hook called by replication code to let the AuthorizationManager observe changes
          * to relevant collections.
          */
-        void logOp(OperationContext* txn,
-                   const char* opstr,
+        void logOp(const char* opstr,
                    const char* ns,
                    const BSONObj& obj,
                    BSONObj* patt,
diff --git a/src/mongo/db/auth/authz_manager_external_state.h b/src/mongo/db/auth/authz_manager_external_state.h
index 4c54cabc1aa..107724f9c59 100644
--- a/src/mongo/db/auth/authz_manager_external_state.h
+++ b/src/mongo/db/auth/authz_manager_external_state.h
@@ -229,7 +229,6 @@ namespace mongo {
         virtual void releaseAuthzUpdateLock() = 0;
 
         virtual void logOp(
-                OperationContext* txn,
                 const char* op,
                 const char* ns,
                 const BSONObj& o,
diff --git a/src/mongo/db/auth/authz_manager_external_state_local.cpp b/src/mongo/db/auth/authz_manager_external_state_local.cpp
index 989573e316a..507db983a53 100644
--- a/src/mongo/db/auth/authz_manager_external_state_local.cpp
+++ b/src/mongo/db/auth/authz_manager_external_state_local.cpp
@@ -36,14 +36,12 @@
 #include "mongo/bson/util/bson_extract.h"
 #include "mongo/db/auth/authorization_manager.h"
 #include "mongo/db/auth/user_document_parser.h"
-#include "mongo/db/operation_context.h"
 #include "mongo/util/log.h"
 #include "mongo/util/mongoutils/str.h"
 
 namespace mongo {
 
     using std::vector;
-    using boost::optional;
 
     AuthzManagerExternalStateLocal::AuthzManagerExternalStateLocal() :
         _roleGraphState(roleGraphStateInitial) {}
@@ -367,97 +365,47 @@ namespace {
         return status;
     }
 
-    class AuthzManagerExternalStateLocal::AuthzManagerLogOpHandler : public RecoveryUnit::Change {
-    public:
-        AuthzManagerLogOpHandler(AuthzManagerExternalStateLocal* externalState,
-                                 const std::string& op,
-                                 const std::string& ns,
-                                 const BSONObj& o,
-                                 const optional<BSONObj>& o2,
-                                 const optional<bool>& b):
-            _externalState(externalState),
-            _op(op),
-            _ns(ns),
-            _o(o.getOwned()),
-            _o2(o2 ? o2->getOwned() : o2), // since o2 is an optional<BSONObj>
-            _b(b) {
+    void AuthzManagerExternalStateLocal::logOp(
+            const char* op,
+            const char* ns,
+            const BSONObj& o,
+            BSONObj* o2,
+            bool* b) {
 
-        }
+        if (ns == AuthorizationManager::rolesCollectionNamespace.ns() ||
+            ns == AuthorizationManager::adminCommandNamespace.ns()) {
 
-        virtual void commit() {
-            boost::lock_guard<boost::mutex> lk(_externalState->_roleGraphMutex);
-            Status status = _externalState->_roleGraph.handleLogOp(_op.c_str(),
-                                                                   NamespaceString(_ns.c_str()),
-                                                                   _o,
-                                                                   _o2 ? _o2.get_ptr() : NULL);
+            boost::lock_guard<boost::mutex> lk(_roleGraphMutex);
+            Status status = _roleGraph.handleLogOp(op, NamespaceString(ns), o, o2);
 
             if (status == ErrorCodes::OplogOperationUnsupported) {
-                _externalState->_roleGraph = RoleGraph();
-                _externalState->_roleGraphState = _externalState->roleGraphStateInitial;
+                _roleGraph = RoleGraph();
+                _roleGraphState = roleGraphStateInitial;
                 BSONObjBuilder oplogEntryBuilder;
-                oplogEntryBuilder << "op" << _op << "ns" << _ns << "o" << _o;
-                if (_o2)
-                    oplogEntryBuilder << "o2" << _o2;
-                if (_b)
-                    oplogEntryBuilder << "b" << _b;
+                oplogEntryBuilder << "op" << op << "ns" << ns << "o" << o;
+                if (o2)
+                    oplogEntryBuilder << "o2" << *o2;
+                if (b)
+                    oplogEntryBuilder << "b" << *b;
                 error() << "Unsupported modification to roles collection in oplog; "
                     "restart this process to reenable user-defined roles; " << status.reason() <<
                     "; Oplog entry: " << oplogEntryBuilder.done();
             }
             else if (!status.isOK()) {
                 warning() << "Skipping bad update to roles collection in oplog. " << status <<
-                    " Oplog entry: " << _op;
+                    " Oplog entry: " << op;
             }
-            status = _externalState->_roleGraph.recomputePrivilegeData();
+            status = _roleGraph.recomputePrivilegeData();
             if (status == ErrorCodes::GraphContainsCycle) {
-                _externalState->_roleGraphState = _externalState->roleGraphStateHasCycle;
+                _roleGraphState = roleGraphStateHasCycle;
                 error() << "Inconsistent role graph during authorization manager initialization.  "
                     "Only direct privileges available. " << status.reason() <<
-                    " after applying oplog entry " << _op;
+                    " after applying oplog entry " << op;
             }
             else {
                 fassert(17183, status);
-                _externalState->_roleGraphState = _externalState->roleGraphStateConsistent;
+                _roleGraphState = roleGraphStateConsistent;
             }
-
-        }
-
-        virtual void rollback() { }
-
-    private:
-        AuthzManagerExternalStateLocal* _externalState;
-        const std::string _op;
-        const std::string _ns;
-        const BSONObj _o;
-        const optional<BSONObj> _o2;
-        const optional<bool> _b;
-    };
-
-    void AuthzManagerExternalStateLocal::logOp(
-            OperationContext* txn,
-            const char* op,
-            const char* ns,
-            const BSONObj& o,
-            BSONObj* o2,
-            bool* b) {
-
-        if (ns == AuthorizationManager::rolesCollectionNamespace.ns() ||
-            ns == AuthorizationManager::adminCommandNamespace.ns()) {
-
-            std::string opStr(op);
-            std::string nsStr(ns);
-            optional<BSONObj> o2Copy;
-            if (o2)
-                o2Copy = *o2;
-            optional<bool> bCopy;
-            if (b)
-                bCopy = *b;
-            txn->recoveryUnit()->registerChange(new AuthzManagerLogOpHandler(this,
-                                                                             opStr,
-                                                                             nsStr,
-                                                                             o,
-                                                                             o2Copy,
-                                                                             bCopy));
         }
     }
 
diff --git a/src/mongo/db/auth/authz_manager_external_state_local.h b/src/mongo/db/auth/authz_manager_external_state_local.h
index 87648b911c0..1fd95a6cadc 100644
--- a/src/mongo/db/auth/authz_manager_external_state_local.h
+++ b/src/mongo/db/auth/authz_manager_external_state_local.h
@@ -65,7 +65,6 @@ namespace mongo {
                                                 std::vector<BSONObj>* result);
 
         virtual void logOp(
-                OperationContext* txn,
                 const char* op,
                 const char* ns,
                 const BSONObj& o,
@@ -83,11 +82,6 @@ namespace mongo {
         };
 
         /**
-         * RecoveryUnit::Change subclass used to commit work for AuthzManager logOp listener.
-         */
-        class AuthzManagerLogOpHandler;
-
-        /**
          * Initializes the role graph from the contents of the admin.system.roles collection.
          */
         Status _initializeRoleGraph(OperationContext* txn);
diff --git a/src/mongo/db/auth/authz_manager_external_state_mock.cpp b/src/mongo/db/auth/authz_manager_external_state_mock.cpp
index c031cbc52cc..bcf2c7847c8 100644
--- a/src/mongo/db/auth/authz_manager_external_state_mock.cpp
+++ b/src/mongo/db/auth/authz_manager_external_state_mock.cpp
@@ -152,17 +152,14 @@ namespace {
             toInsert = document.copy();
         }
         _documents[collectionName].push_back(toInsert);
-
         if (_authzManager) {
             _authzManager->logOp(
-                    txn,
                     "i",
                     collectionName.ns().c_str(),
                     toInsert,
                     NULL,
                     NULL);
         }
-
         return Status::OK();
     }
 
@@ -193,17 +190,14 @@ namespace {
             BSONObj newObj = document.getObject().copy();
             *iter = newObj;
             BSONObj idQuery = driver.makeOplogEntryQuery(newObj, false);
-
             if (_authzManager) {
                 _authzManager->logOp(
-                        txn,
                         "u",
                         collectionName.ns().c_str(),
                         logObj,
                         &idQuery,
                         NULL);
             }
-
             return Status::OK();
         }
         else if (status == ErrorCodes::NoMatchingDocument && upsert) {
@@ -249,17 +243,14 @@ namespace {
             BSONObj idQuery = (*iter)["_id"].wrap();
             _documents[collectionName].erase(iter);
             ++n;
-
             if (_authzManager) {
                 _authzManager->logOp(
-                        txn,
                         "d",
                         collectionName.ns().c_str(),
                         idQuery,
                         NULL,
                         NULL);
             }
-
         }
         *numRemoved = n;
         return Status::OK();
diff --git a/src/mongo/db/repl/oplog.cpp b/src/mongo/db/repl/oplog.cpp
index f44be889852..e4d64cdb1f9 100644
--- a/src/mongo/db/repl/oplog.cpp
+++ b/src/mongo/db/repl/oplog.cpp
@@ -379,10 +379,6 @@ namespace {
         if ( getGlobalReplicationCoordinator()->isReplEnabled() ) {
             _logOp(txn, opstr, ns, 0, obj, patt, b, fromMigrate);
         }
-        //
-        // rollback-safe logOp listeners
-        //
-        getGlobalAuthorizationManager()->logOp(txn, opstr, ns, obj, patt, b);
 
         try {
             // TODO SERVER-15192 remove this once all listeners are rollback-safe.
@@ -396,6 +392,7 @@ namespace {
             txn->recoveryUnit()->registerChange(new RollbackPreventer());
             logOpForSharding(txn, opstr, ns, obj, patt, fromMigrate);
             logOpForDbHash(ns);
+            getGlobalAuthorizationManager()->logOp(opstr, ns, obj, patt, b);
 
             if ( strstr( ns, ".system.js" ) ) {
                 Scope::storedFuncMod(); // this is terrible
@@ -786,19 +783,12 @@ namespace {
         else {
             throw MsgAssertionException( 14825 , ErrorMsg("error in applyOperation : unknown opType ", *opType) );
         }
-
-        // AuthorizationManager's logOp method registers a RecoveryUnit::Change
-        // and to do so we need to have begun a UnitOfWork
-        WriteUnitOfWork wuow(txn);
         getGlobalAuthorizationManager()->logOp(
-                txn,
                 opType,
                 ns,
                 o,
                 fieldO2.isABSONObj() ? &o2 : NULL,
                 !fieldB.eoo() ? &valueB : NULL );
-        wuow.commit();
-
         return failedUpdate;
     }