Fix review comments

Signed-off-by: Aleksandr Ivanov <aivanov71@bloomberg.net>
bloomberg · Sep 6, 2024 · 4ee0138 · 4ee0138
1 parent 215a651
commit 4ee0138
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 28 deletions.
diff --git a/src/groups/mqb/mqba/mqba_clientsession.cpp b/src/groups/mqb/mqba/mqba_clientsession.cpp
@@ -747,8 +747,6 @@ void ClientSession::onHandleConfiguredDispatched(
     // PRECONDITIONS
     BSLS_ASSERT_SAFE(dispatcher()->inDispatcherThread(this));
 
-    bdlma::LocalSequentialAllocator<2048> localAlloc(d_state.d_allocator_p);
-
     const unsigned int qId =
         streamParamsCtrlMsg.choice().isConfigureQueueStreamValue()
             ? streamParamsCtrlMsg.choice().configureQueueStream().qId()
@@ -757,22 +755,23 @@ void ClientSession::onHandleConfiguredDispatched(
     ClientSessionState::QueueStateMap::iterator queueStateIter =
         d_queueSessionManager.queues().find(qId);
     if (queueStateIter != d_queueSessionManager.queues().end()) {
-        d_logOpStream << "Configure queue '"
-                      << queueStateIter->second.d_handle_p->queue()->uri()
-                      << "'";
+        d_currentOpDescription
+            << "Configure queue '"
+            << queueStateIter->second.d_handle_p->queue()->uri() << "'";
     }
     else {
-        d_logOpStream << "Configure queue [qId=" << qId << "]";
+        d_currentOpDescription << "Configure queue [qId=" << qId << "]";
     }
 
     if (isDisconnected()) {
         // The client is disconnected or the channel is down
-        logOperationTime();
+        logOperationTime(d_currentOpDescription);
         return;  // RETURN
     }
 
     // Send success/error response to client
-    bmqp_ctrlmsg::ControlMessage response(&localAlloc);
+    bdlma::LocalSequentialAllocator<2048> localAlloc(d_state.d_allocator_p);
+    bmqp_ctrlmsg::ControlMessage          response(&localAlloc);
 
     response.rId() = streamParamsCtrlMsg.rId().value();
 
@@ -831,7 +830,7 @@ void ClientSession::onHandleConfiguredDispatched(
                       << ", request: " << streamParamsCtrlMsg
                       << "]: " << response;
 
-        logOperationTime();
+        logOperationTime(d_currentOpDescription);
         return;  // RETURN
     }
 
@@ -842,7 +841,7 @@ void ClientSession::onHandleConfiguredDispatched(
     // Send the response
     sendPacket(d_state.d_schemaEventBuilder.blob(), true);
 
-    logOperationTime();
+    logOperationTime(d_currentOpDescription);
 }
 
 void ClientSession::initiateShutdownDispatched(
@@ -1118,15 +1117,15 @@ void ClientSession::closeChannel()
     channel()->close(status);
 }
 
-void ClientSession::logOperationTime()
+void ClientSession::logOperationTime(mwcu::MemOutStream& opDescription)
 {
     if (d_beginTimestamp) {
         BALL_LOG_INFO_BLOCK
         {
             const bsls::Types::Int64 elapsed =
                 mwcsys::Time::highResolutionTimer() - d_beginTimestamp;
             BALL_LOG_OUTPUT_STREAM
-                << description() << ": " << d_logOpStream.str()
+                << description() << ": " << opDescription.str()
                 << " took: " << mwcu::PrintUtil::prettyTimeInterval(elapsed)
                 << " (" << elapsed << " nanoseconds)";
         }
@@ -1135,10 +1134,10 @@ void ClientSession::logOperationTime()
     else {
         BSLS_PERFORMANCEHINT_UNLIKELY_HINT;
         BALL_LOG_WARN << "d_beginTimestamp was not initialized for operation: "
-                      << d_logOpStream.str();
+                      << opDescription.str();
     }
 
-    d_logOpStream.reset();
+    opDescription.reset();
 }
 
 void ClientSession::processDisconnectAllQueues(
@@ -1359,8 +1358,8 @@ void ClientSession::openQueueCb(
 
     const bsl::string& queueUri =
         handleParamsCtrlMsg.choice().openQueue().handleParameters().uri();
-    d_logOpStream << "Open queue '" << queueUri << "'";
-    logOperationTime();
+    d_currentOpDescription << "Open queue '" << queueUri << "'";
+    logOperationTime(d_currentOpDescription);
 }
 
 void ClientSession::processCloseQueue(
@@ -1442,8 +1441,8 @@ void ClientSession::closeQueueCb(
 
     const bsl::string& queueUri =
         handleParamsCtrlMsg.choice().closeQueue().handleParameters().uri();
-    d_logOpStream << "Close queue '" << queueUri << "'";
-    logOperationTime();
+    d_currentOpDescription << "Close queue '" << queueUri << "'";
+    logOperationTime(d_currentOpDescription);
 }
 
 void ClientSession::processConfigureStream(
@@ -2605,7 +2604,7 @@ ClientSession::ClientSession(
 , d_shutdownChain(allocator)
 , d_shutdownCallback()
 , d_beginTimestamp(0)
-, d_logOpStream(256, allocator)
+, d_currentOpDescription(256, allocator)
 {
     // Register this client to the dispatcher
     mqbi::Dispatcher::ProcessorHandle processor = dispatcher->registerClient(
@@ -2647,8 +2646,8 @@ ClientSession::~ClientSession()
     // Unregister from the dispatcher
     dispatcher()->unregisterClient(this);
 
-    d_logOpStream << "Close session";
-    logOperationTime();
+    d_currentOpDescription << "Close session";
+    logOperationTime(d_currentOpDescription);
 }
 
 // MANIPULATORS

diff --git a/src/groups/mqb/mqba/mqba_clientsession.h b/src/groups/mqb/mqba/mqba_clientsession.h
@@ -394,9 +394,8 @@ class ClientSession : public mqbnet::Session,
     bsls::Types::Int64 d_beginTimestamp;
     // HiRes timer value of the begin session/queue operation
 
-    mwcu::MemOutStream d_logOpStream;
-    // Stream for constructing session/queue operation log,
-    // used by logOperationTime().
+    mwcu::MemOutStream d_currentOpDescription;
+    // Stream for constructing current session/queue operation description.
 
   private:
     // NOT IMPLEMENTED
@@ -607,10 +606,10 @@ class ClientSession : public mqbnet::Session,
 
     void closeChannel();
 
-    /// Log session/queue operation time for the operation stored in
-    /// `d_logOpStream` using the stored operation begin timestamp. Reset the
-    /// begin timestamp to 0 and `d_logOpStream`.
-    void logOperationTime();
+    /// Log session/queue operation time for the specified `opDescription`
+    /// using the stored operation begin timestamp. After logging reset
+    /// `opDescription` and set begin timestamp to 0.
+    void logOperationTime(mwcu::MemOutStream& opDescription);
 
     // PRIVATE ACCESSORS
 

diff --git a/src/groups/mqb/mqbnet/mqbnet_tcpsessionfactory.cpp b/src/groups/mqb/mqbnet/mqbnet_tcpsessionfactory.cpp
@@ -690,6 +690,9 @@ void TCPSessionFactory::channelStateCallback(
         }
         else {
             {  // Save begin session timestamp
+               // TODO: it's possible to store this timestamp directly in one
+               // of the mwcio::Channel implementations, so we don't need a
+               // mutex synchronization for them at all.
                 bslmt::LockGuard<bslmt::Mutex> guard(&d_mutex);  // LOCK
                 d_timestampMap[channel.get()] =
                     mwcsys::Time::highResolutionTimer();