pivotal-jbarrett commented on a change in pull request #706:
URL: https://github.com/apache/geode-native/pull/706#discussion_r539515389



##########
File path: cppcache/src/TcrMessage.cpp
##########
@@ -2090,17 +2090,16 @@ TcrMessageReply::TcrMessageReply(bool decodeAll,
 
 TcrMessagePing::TcrMessagePing(DataOutput* dataOutput, bool decodeAll) {
   m_msgType = TcrMessage::PING;
+  // -1 is NOTX constant server-side, PING should *never* be part of a
+  // transaction
+  m_txId = -1;

Review comment:
       Isn't this the default? Look at `TcrMessage` default constructor.

##########
File path: cppcache/src/TcrMessage.cpp
##########
@@ -2090,17 +2090,16 @@ TcrMessageReply::TcrMessageReply(bool decodeAll,
 
 TcrMessagePing::TcrMessagePing(DataOutput* dataOutput, bool decodeAll) {
   m_msgType = TcrMessage::PING;
+  // -1 is NOTX constant server-side, PING should *never* be part of a
+  // transaction
+  m_txId = -1;
   m_decodeAll = decodeAll;
   m_request.reset(dataOutput);
   m_request->writeInt(m_msgType);
   m_request->writeInt(static_cast<int32_t>(
       0));  // 17 is fixed message len ...  PING only has a header.
   m_request->writeInt(static_cast<int32_t>(0));  // Number of parts.
-  // int32_t txId = TcrMessage::m_transactionId++;
-  // Setting the txId to 0 for all ping message as it is not being used on the
-  // SERVER side or the
-  // client side.
-  m_request->writeInt(static_cast<int32_t>(0));
+  m_request->writeInt(m_txId);

Review comment:
       Look at `TcrMessage::writeHeader` and see that almost all the messages 
are using this method to do some of the message bits in here. Perhaps it makes 
sense to normalize this message to match.

##########
File path: cppcache/src/TcrMessage.cpp
##########
@@ -2109,13 +2108,16 @@ TcrMessagePing::TcrMessagePing(DataOutput* dataOutput, 
bool decodeAll) {
 TcrMessageCloseConnection::TcrMessageCloseConnection(DataOutput* dataOutput,
                                                      bool decodeAll) {
   m_msgType = TcrMessage::CLOSE_CONNECTION;
+  // -1 is NOTX constant server-side, CLOSE_CONNECTION should *never* be part
+  // of a transaction
+  m_txId = -1;
   m_decodeAll = decodeAll;
   m_request.reset(dataOutput);
   m_request->writeInt(m_msgType);
   m_request->writeInt(static_cast<int32_t>(6));
   m_request->writeInt(static_cast<int32_t>(1));  // Number of parts.
   // int32_t txId = TcrMessage::m_transactionId++;
-  m_request->writeInt(static_cast<int32_t>(0));
+  m_request->writeInt(m_txId);

Review comment:
       See comment about this in ping message.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to