[GitHub] [geode-native] jvarenina commented on a change in pull request #705: GEODE-8543: Fix for uneccessary connections closure

[GitBox <gi...@apache.org>]

jvarenina commented on a change in pull request #705:
URL: https://github.com/apache/geode-native/pull/705#discussion_r569326130



##########
File path: cppcache/src/TcrEndpoint.cpp
##########
@@ -1155,6 +1157,10 @@ void TcrEndpoint::setConnectionStatus(bool status) {
       m_isActiveEndpoint = false;
       LOGFINE("Disconnected from endpoint %s", m_name.c_str());
       triggerRedundancyThread();
+      // Test hook
+      if (TcrEndpoint::TEST_DISCONNECTIONS) {
+        TcrEndpoint::listOfDisconnectedServers.push_back(m_name.c_str());

Review comment:
       Thanks for comments!
   
   Would it be acceptable to use existing counter and method `numberOfTimesFailed()` from `TcrEndpoint` to check for disconnections?
   
   ```
   int32_t numberOfTimesFailed() { return m_numberOfTimesFailed; }
    ...
   void TcrEndpoint::setConnectionStatus(bool status) {
     std::lock_guard<decltype(m_connectionLock)> guard(m_connectionLock);
       if (m_connected != status) {
         bool connected = m_connected;
         m_connected = status;
         if (connected) {
           m_numberOfTimesFailed += 1;
           m_isAuthenticated = false;
           // disconnected
           LOGFINE("Disconnecting from endpoint %s", m_name.c_str());
           closeConnections();
           ...
   ```
   I think it would be possible to use it, because `TcrEndpoint` object is not destroyed when endpoint disconnects from client.
   
   It would be used in test case something like this:
   ```
   auto cacheImpl = CacheRegionHelper::getCacheImpl(cache.get());
   
   // shut down one endpoint (server)
   
   ASSERT_EQ(cacheImpl->getNumberOfTimeEndpointDisconnected(poolName, epRunningHostname), 0);
   ASSERT_GT(cacheImpl->getNumberOfTimeEndpointDisconnected(poolName, epDisconnectedHostname), 0);
   
   ```
   `ThinClientPoolDM` is actually owner of `TcrEndpoint` collection, so `getNumberOfTimeEndpointDisconnected(endpoint)` would basically find given endpoint and return it's `numberOfTimesFailed()`.
   
   
   




----------------------------------------------------------------
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:
users@infra.apache.org