SERVER-20360: break out logicalOp vs networkOp

11 files changed, 92 insertions, 73 deletions

diff --git a/src/mongo/db/clientlistplugin.cpp b/src/mongo/db/clientlistplugin.cpp
index 6428381dc35..1c9171e1f0e 100644
--- a/src/mongo/db/clientlistplugin.cpp
+++ b/src/mongo/db/clientlistplugin.cpp
@@ -114,7 +114,7 @@ private:
 
             tablecell(ss, curOp->elapsedSeconds());
 
-            tablecell(ss, curOp->getOp());
+            tablecell(ss, curOp->getNetworkOp());
             tablecell(ss, html::escape(curOp->getNS()));
 
             if (curOp->haveQuery()) {
diff --git a/src/mongo/db/commands.cpp b/src/mongo/db/commands.cpp
index 256e3fd2ead..dc5147e346e 100644
--- a/src/mongo/db/commands.cpp
+++ b/src/mongo/db/commands.cpp
@@ -490,6 +490,12 @@ void runCommands(OperationContext* txn,
         LOG(2) << "run command " << request.getDatabase() << ".$cmd" << ' '
                << c->getRedactedCopyForLogging(request.getCommandArgs());
 
+        {
+            // Try to set this as early as possible, as soon as we have figured out the command.
+            stdx::lock_guard<Client> lk(*txn->getClient());
+            CurOp::get(txn)->setLogicalOp_inlock(c->getLogicalOp());
+        }
+
         Command::execCommand(txn, c, request, replyBuilder);
     }
 
diff --git a/src/mongo/db/commands.h b/src/mongo/db/commands.h
index 8993f9e657a..7b6108bff8e 100644
--- a/src/mongo/db/commands.h
+++ b/src/mongo/db/commands.h
@@ -241,6 +241,10 @@ public:
         return false;
     }
 
+    virtual OperationType getLogicalOp() const {
+        return dbCommand;
+    }
+
     /** @param webUI expose the command in the web ui as localhost:28017/<name>
         @param oldName an optional old, deprecated name for the command
     */
diff --git a/src/mongo/db/commands/find_cmd.cpp b/src/mongo/db/commands/find_cmd.cpp
index 088ecbcdb08..b951bd551ec 100644
--- a/src/mongo/db/commands/find_cmd.cpp
+++ b/src/mongo/db/commands/find_cmd.cpp
@@ -101,6 +101,10 @@ public:
         help << "query for documents";
     }
 
+    OperationType getLogicalOp() const override {
+        return dbQuery;
+    }
+
     /**
      * A find command does not increment the command counter, but rather increments the
      * query counter.
diff --git a/src/mongo/db/commands/getmore_cmd.cpp b/src/mongo/db/commands/getmore_cmd.cpp
index aa593cf86d1..842985afcda 100644
--- a/src/mongo/db/commands/getmore_cmd.cpp
+++ b/src/mongo/db/commands/getmore_cmd.cpp
@@ -103,6 +103,10 @@ public:
         help << "retrieve more results from an existing cursor";
     }
 
+    OperationType getLogicalOp() const override {
+        return dbGetMore;
+    }
+
     /**
      * A getMore command increments the getMore counter, not the command counter.
      */
diff --git a/src/mongo/db/commands/write_commands/batch_executor.cpp b/src/mongo/db/commands/write_commands/batch_executor.cpp
index bedc9c8f4bf..9e9f4a7c464 100644
--- a/src/mongo/db/commands/write_commands/batch_executor.cpp
+++ b/src/mongo/db/commands/write_commands/batch_executor.cpp
@@ -160,7 +160,7 @@ void noteInCriticalSection(WriteErrorDetail* staleError) {
  * Translates write item type to wire protocol op code. Helper for
  * WriteBatchExecutor::applyWriteItem().
  */
-int getOpCode(const BatchItemRef& currWrite) {
+OperationType getOpCode(const BatchItemRef& currWrite) {
     switch (currWrite.getRequest()->getBatchType()) {
         case BatchedCommandRequest::BatchType_Insert:
             return dbInsert;
@@ -482,12 +482,11 @@ static bool checkIndexConstraints(OperationContext* txn,
 static void beginCurrentOp(OperationContext* txn, const BatchItemRef& currWrite) {
     stdx::lock_guard<Client> lk(*txn->getClient());
     CurOp* const currentOp = CurOp::get(txn);
-    currentOp->setOp_inlock(getOpCode(currWrite));
+    currentOp->setNetworkOp_inlock(getOpCode(currWrite));
+    currentOp->setLogicalOp_inlock(getOpCode(currWrite));
     currentOp->ensureStarted();
     currentOp->setNS_inlock(currWrite.getRequest()->getNS().ns());
 
-    currentOp->debug().op = currentOp->getOp();
-
     if (currWrite.getOpType() == BatchedCommandRequest::BatchType_Insert) {
         currentOp->setQuery_inlock(currWrite.getDocument());
         currentOp->debug().query = currWrite.getDocument();
@@ -558,7 +557,7 @@ static void finishCurrentOp(OperationContext* txn, WriteErrorDetail* opError) {
     recordCurOpMetrics(txn);
     Top::get(txn->getClient()->getServiceContext())
         .record(currentOp->getNS(),
-                currentOp->getOp(),
+                currentOp->getNetworkOp(),
                 1,  // "write locked"
                 currentOp->totalTimeMicros(),
                 currentOp->isCommand());
@@ -567,8 +566,8 @@ static void finishCurrentOp(OperationContext* txn, WriteErrorDetail* opError) {
         currentOp->debug().exceptionInfo =
             ExceptionInfo(opError->getErrMessage(), opError->getErrCode());
 
-        LOG(3) << " Caught Assertion in " << opToString(currentOp->getOp()) << ", continuing "
-               << causedBy(opError->getErrMessage());
+        LOG(3) << " Caught Assertion in " << opToString(currentOp->getNetworkOp())
+               << ", continuing " << causedBy(opError->getErrMessage());
     }
 
     bool logAll = logger::globalLogDomain()->shouldLog(logger::LogComponent::kWrite,
@@ -583,7 +582,7 @@ static void finishCurrentOp(OperationContext* txn, WriteErrorDetail* opError) {
     }
 
     if (currentOp->shouldDBProfile(executionTime)) {
-        profile(txn, CurOp::get(txn)->getOp());
+        profile(txn, CurOp::get(txn)->getNetworkOp());
     }
 }
 
diff --git a/src/mongo/db/curop.cpp b/src/mongo/db/curop.cpp
index 83a60a21eee..b7f2105664e 100644
--- a/src/mongo/db/curop.cpp
+++ b/src/mongo/db/curop.cpp
@@ -246,14 +246,11 @@ CurOp::CurOp(OperationContext* opCtx, CurOpStack* stack) : _stack(stack) {
     _progressMeter.finished();
     _numYields = 0;
     _expectedLatencyMs = 0;
-    _op = 0;
+    _networkOp = opInvalid;
+    _logicalOp = opInvalid;
     _command = NULL;
 }
 
-void CurOp::setOp_inlock(int op) {
-    _op = op;
-}
-
 ProgressMeter& CurOp::setMessage_inlock(const char* msg,
                                         std::string name,
                                         unsigned long long progressMeterTotal,
@@ -309,25 +306,12 @@ void CurOp::reportState(BSONObjBuilder* builder) {
         builder->append("microsecs_running", static_cast<long long int>(elapsedMicros()));
     }
 
-    const char* opName;
-    if (_command && _command->name == "find") {
-        // If the operation is a find command, we report "op" as "query", for consistency with
-        // OP_QUERY legacy find operations.
-        opName = opToString(dbQuery);
-    } else if (_command && _command->name == "getMore") {
-        // If the operation is a getMore command, we report "op" as "getmore", for consistency
-        // with OP_GET_MORE legacy find operations.
-        opName = opToString(dbGetMore);
-    } else {
-        opName = opToString(_op);
-    }
-    builder->append("op", opName);
-
+    builder->append("op", opToString(_logicalOp));
     builder->append("ns", _ns);
 
-    if (_op == dbInsert) {
+    if (_networkOp == dbInsert) {
         _query.append(*builder, "insert");
-    } else if (!_command && _op == dbQuery) {
+    } else if (!_command && _networkOp == dbQuery) {
         // This is a legacy OP_QUERY. We upconvert the "query" field of the currentOp output to look
         // similar to a find command.
         //
@@ -470,7 +454,8 @@ uint64_t CurOp::MaxTimeTracker::getRemainingMicros() const {
 }
 
 void OpDebug::reset() {
-    op = 0;
+    networkOp = opInvalid;
+    logicalOp = opInvalid;
     iscommand = false;
     query = BSONObj();
     updateobj = BSONObj();
@@ -527,7 +512,7 @@ string OpDebug::report(const CurOp& curop, const SingleThreadedLockStats& lockSt
     if (iscommand)
         s << "command ";
     else
-        s << opToString(op) << ' ';
+        s << opToString(networkOp) << ' ';
 
     s << curop.getNS();
 
@@ -600,7 +585,7 @@ string OpDebug::report(const CurOp& curop, const SingleThreadedLockStats& lockSt
     }
 
     if (iscommand) {
-        s << " protocol:" << getProtoString(op);
+        s << " protocol:" << getProtoString(networkOp);
     }
 
     s << " " << executionTime << "ms";
@@ -641,26 +626,6 @@ void appendAsObjOrString(StringData name,
 }
 }  // namespace
 
-bool OpDebug::isFindCommand() const {
-    return iscommand && str::equals(query.firstElement().fieldName(), "find");
-}
-
-bool OpDebug::isGetMoreCommand() const {
-    return iscommand && str::equals(query.firstElement().fieldName(), "getMore");
-}
-
-int OpDebug::getLogicalOpType() const {
-    if (isFindCommand()) {
-        return dbQuery;
-    } else if (isGetMoreCommand()) {
-        return dbGetMore;
-    } else if (iscommand) {
-        return dbCommand;
-    } else {
-        return op;
-    }
-}
-
 #define OPDEBUG_APPEND_NUMBER(x) \
     if (x != -1)                 \
     b.appendNumber(#x, (x))
@@ -673,17 +638,16 @@ void OpDebug::append(const CurOp& curop,
                      BSONObjBuilder& b) const {
     const size_t maxElementSize = 50 * 1024;
 
-    int logicalOpType = getLogicalOpType();
-    b.append("op", opToString(logicalOpType));
+    b.append("op", opToString(logicalOp));
 
     NamespaceString nss = NamespaceString(curop.getNS());
     b.append("ns", nss.ns());
 
-    if (!iscommand && op == dbQuery) {
+    if (!iscommand && networkOp == dbQuery) {
         appendAsObjOrString(
             "query", upconvertQueryEntry(query, nss, ntoreturn, ntoskip), maxElementSize, &b);
     } else if (!query.isEmpty()) {
-        const char* fieldName = (logicalOpType == dbCommand) ? "command" : "query";
+        const char* fieldName = (logicalOp == dbCommand) ? "command" : "query";
         appendAsObjOrString(fieldName, query, maxElementSize, &b);
     } else if (!iscommand && curop.haveQuery()) {
         appendAsObjOrString("query", curop.query(), maxElementSize, &b);
@@ -728,7 +692,7 @@ void OpDebug::append(const CurOp& curop,
     OPDEBUG_APPEND_NUMBER(nreturned);
     OPDEBUG_APPEND_NUMBER(responseLength);
     if (iscommand) {
-        b.append("protocol", getProtoString(op));
+        b.append("protocol", getProtoString(networkOp));
     }
     b.append("millis", executionTime);
 
diff --git a/src/mongo/db/curop.h b/src/mongo/db/curop.h
index 7f28fa1167f..3dec4f7e30b 100644
--- a/src/mongo/db/curop.h
+++ b/src/mongo/db/curop.h
@@ -39,6 +39,7 @@
 #include "mongo/util/progress_meter.h"
 #include "mongo/util/thread_safe_string.h"
 #include "mongo/util/time_support.h"
+#include "mongo/util/net/message.h"
 
 namespace mongo {
 
@@ -149,7 +150,12 @@ public:
     // -------------------
 
     // basic options
-    int op;
+    // _networkOp represents the network-level op code: OP_QUERY, OP_GET_MORE, OP_COMMAND, etc.
+    OperationType networkOp;  // only set this through setNetworkOp_inlock() to keep synced
+    // _logicalOp is the logical operation type, ie 'dbQuery' regardless of whether this is an
+    // OP_QUERY find, a find command using OP_QUERY, or a find command using OP_COMMAND.
+    // Similarly, the return value will be dbGetMore for both OP_GET_MORE and getMore command.
+    OperationType logicalOp;  // only set this through setNetworkOp_inlock() to keep synced
     bool iscommand;
     BSONObj query;
     BSONObj updateobj;
@@ -262,9 +268,20 @@ public:
     void enter_inlock(const char* ns, int dbProfileLevel);
 
     /**
-     * Sets the type of the current operation to "op".
+     * Sets the type of the current network operation.
      */
-    void setOp_inlock(int op);
+    void setNetworkOp_inlock(OperationType op) {
+        _networkOp = op;
+        _debug.networkOp = op;
+    }
+
+    /**
+     * Sets the type of the current logical operation.
+     */
+    void setLogicalOp_inlock(OperationType op) {
+        _logicalOp = op;
+        _debug.logicalOp = op;
+    }
 
     /**
      * Marks the current operation as being a command.
@@ -304,10 +321,19 @@ public:
     void raiseDbProfileLevel(int dbProfileLevel);
 
     /**
-     * Gets the type of the current operation.
+     * Gets the network operation type. No lock is required if called by the thread executing
+     * the operation, but the lock must be held if called from another thread.
      */
-    int getOp() const {
-        return _op;
+    OperationType getNetworkOp() const {
+        return _networkOp;
+    }
+
+    /**
+     * Gets the logical operation type. No lock is required if called by the thread executing
+     * the operation, but the lock must be held if called from another thread.
+     */
+    OperationType getLogicalOp() const {
+        return _logicalOp;
     }
 
     /**
@@ -466,7 +492,14 @@ private:
     Command* _command;
     long long _start;
     long long _end;
-    int _op;
+
+    // _networkOp represents the network-level op code: OP_QUERY, OP_GET_MORE, OP_COMMAND, etc.
+    OperationType _networkOp;  // only set this through setNetworkOp_inlock() to keep synced
+    // _logicalOp is the logical operation type, ie 'dbQuery' regardless of whether this is an
+    // OP_QUERY find, a find command using OP_QUERY, or a find command using OP_COMMAND.
+    // Similarly, the return value will be dbGetMore for both OP_GET_MORE and getMore command.
+    OperationType _logicalOp;  // only set this through setNetworkOp_inlock() to keep synced
+
     bool _isCommand;
     int _dbprofile;  // 0=off, 1=slow, 2=all
     std::string _ns;
diff --git a/src/mongo/db/db_raii.cpp b/src/mongo/db/db_raii.cpp
index bc502683693..120a1eafda6 100644
--- a/src/mongo/db/db_raii.cpp
+++ b/src/mongo/db/db_raii.cpp
@@ -103,7 +103,7 @@ AutoGetCollectionForRead::~AutoGetCollectionForRead() {
     auto currentOp = CurOp::get(_txn);
     Top::get(_txn->getClient()->getServiceContext())
         .record(currentOp->getNS(),
-                currentOp->getOp(),
+                currentOp->getNetworkOp(),
                 -1,  // "read locked"
                 _timer.micros(),
                 currentOp->isCommand());
@@ -183,7 +183,7 @@ void OldClientContext::_finishInit() {
 }
 
 void OldClientContext::_checkNotStale() const {
-    switch (CurOp::get(_txn)->getOp()) {
+    switch (CurOp::get(_txn)->getNetworkOp()) {
         case dbGetMore:  // getMore is special and should be handled elsewhere.
         case dbUpdate:   // update & delete check shard version in instance.cpp, so don't check
         case dbDelete:   // here as well.
@@ -200,7 +200,7 @@ OldClientContext::~OldClientContext() {
     auto currentOp = CurOp::get(_txn);
     Top::get(_txn->getClient()->getServiceContext())
         .record(currentOp->getNS(),
-                currentOp->getOp(),
+                currentOp->getNetworkOp(),
                 _txn->lockState()->isWriteLocked() ? 1 : -1,
                 _timer.micros(),
                 currentOp->isCommand());
diff --git a/src/mongo/db/index_builder.cpp b/src/mongo/db/index_builder.cpp
index 687cd3f4985..b347c5de7d2 100644
--- a/src/mongo/db/index_builder.cpp
+++ b/src/mongo/db/index_builder.cpp
@@ -89,7 +89,7 @@ void IndexBuilder::run() {
 
     {
         stdx::lock_guard<Client> lk(*txn.getClient());
-        CurOp::get(txn)->setOp_inlock(dbInsert);
+        CurOp::get(txn)->setNetworkOp_inlock(dbInsert);
     }
     NamespaceString ns(_index["ns"].String());
 
diff --git a/src/mongo/db/instance.cpp b/src/mongo/db/instance.cpp
index 5c762d059c6..1f9c2c58c28 100644
--- a/src/mongo/db/instance.cpp
+++ b/src/mongo/db/instance.cpp
@@ -419,7 +419,7 @@ void assembleResponse(OperationContext* txn,
                       DbResponse& dbresponse,
                       const HostAndPort& remote) {
     // before we lock...
-    int op = m.operation();
+    OperationType op = m.operation();
     bool isCommand = false;
 
     DbMessage dbmsg(m);
@@ -492,16 +492,20 @@ void assembleResponse(OperationContext* txn,
         case dbDelete:
             globalOpCounters.gotDelete();
             break;
+        default:
+            break;
     }
 
     CurOp& currentOp = *CurOp::get(txn);
     {
         stdx::lock_guard<Client> lk(*txn->getClient());
-        currentOp.setOp_inlock(op);
+        // Commands handling code will reset this if the operation is a command
+        // which is logically a basic CRUD operation like query, insert, etc.
+        currentOp.setNetworkOp_inlock(op);
+        currentOp.setLogicalOp_inlock(op);
     }
 
     OpDebug& debug = currentOp.debug();
-    debug.op = op;
 
     long long logThreshold = serverGlobalParams.slowMS;
     LogComponent responseComponent(LogComponent::kQuery);
@@ -552,7 +556,8 @@ void assembleResponse(OperationContext* txn,
                 logThreshold = 10;
                 receivedKillCursors(txn, m);
             } else if (op != dbInsert && op != dbUpdate && op != dbDelete) {
-                log(LogComponent::kQuery) << "    operation isn't supported: " << op << endl;
+                log(LogComponent::kQuery)
+                    << "    operation isn't supported: " << static_cast<int>(op) << endl;
                 currentOp.done();
                 shouldLog = true;
             } else {