You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by bb...@apache.org on 2020/10/09 18:16:30 UTC

[geode-native] branch develop updated: GEODE-8565: Remove bucketServerLocation if timeout error (#662)

This is an automated email from the ASF dual-hosted git repository.

bbender pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode-native.git


The following commit(s) were added to refs/heads/develop by this push:
     new 5f06ae2  GEODE-8565: Remove bucketServerLocation if timeout error (#662)
5f06ae2 is described below

commit 5f06ae2044004397492249fcf13bcd90e75b955b
Author: Alberto Bustamante Reyes <al...@users.noreply.github.com>
AuthorDate: Fri Oct 9 20:16:23 2020 +0200

    GEODE-8565: Remove bucketServerLocation if timeout error (#662)
    
    * GEODE-8565: Remove bucketServerLocation if timeout error
    * Verify client metadata was removed at first error
    * Remove unnecessary header and fix log file name
    * Changes to reduce indentation
---
 .../integration/test/PartitionRegionOpsTest.cpp    | 63 ++++++++++++++++++++++
 cppcache/src/ThinClientPoolDM.cpp                  | 32 +++++------
 2 files changed, 79 insertions(+), 16 deletions(-)

diff --git a/cppcache/integration/test/PartitionRegionOpsTest.cpp b/cppcache/integration/test/PartitionRegionOpsTest.cpp
index c50b539..27fea22 100644
--- a/cppcache/integration/test/PartitionRegionOpsTest.cpp
+++ b/cppcache/integration/test/PartitionRegionOpsTest.cpp
@@ -16,6 +16,7 @@
  */
 
 #include <chrono>
+#include <fstream>
 #include <future>
 #include <iostream>
 #include <random>
@@ -46,11 +47,22 @@ using apache::geode::client::RegionShortcut;
 
 using std::chrono::minutes;
 
+std::string getClientLogName() {
+  std::string testSuiteName(::testing::UnitTest::GetInstance()
+                                ->current_test_info()
+                                ->test_case_name());
+  std::string testCaseName(
+      ::testing::UnitTest::GetInstance()->current_test_info()->name());
+  std::string logFileName(testSuiteName + "/" + testCaseName + "/client.log");
+  return logFileName;
+}
+
 Cache createCache() {
   using apache::geode::client::CacheFactory;
 
   auto cache = CacheFactory()
                    .set("log-level", "debug")
+                   .set("log-file", getClientLogName())
                    .set("statistic-sampling-enabled", "false")
                    .create();
 
@@ -90,6 +102,53 @@ void getEntries(std::shared_ptr<Region> region, int numEntries) {
   }
 }
 
+void removeLogFromPreviousExecution() {
+  std::string logFileName(getClientLogName());
+  std::ifstream previousTestLog(logFileName.c_str());
+  if (previousTestLog.good()) {
+    std::cout << "Removing log from previous execution: " << logFileName
+              << std::endl;
+    remove(logFileName.c_str());
+  }
+}
+
+void verifyMetadataWasRemovedAtFirstError() {
+  std::ifstream testLog(getClientLogName().c_str());
+  std::string fileLine;
+  bool ioErrors = false;
+  bool timeoutErrors = false;
+  bool metadataRemovedDueToIoErr = false;
+  bool metadataRemovedDueToTimeout = false;
+  std::regex timeoutRegex(
+      "sendRequestConnWithRetry: Giving up for endpoint(.*)reason: timed out "
+      "waiting for endpoint.");
+  std::regex ioErrRegex(
+      "sendRequestConnWithRetry: Giving up for endpoint(.*)reason: IO error "
+      "for endpoint.");
+  std::regex removingMetadataDueToIoErrRegex(
+      "Removing bucketServerLocation(.*)due to GF_IOERR");
+  std::regex removingMetadataDueToTimeoutRegex(
+      "Removing bucketServerLocation(.*)due to GF_TIMEOUT");
+
+  if (testLog.is_open()) {
+    while (std::getline(testLog, fileLine)) {
+      if (std::regex_search(fileLine, timeoutRegex)) {
+        timeoutErrors = true;
+      } else if (std::regex_search(fileLine, ioErrRegex)) {
+        ioErrors = true;
+      } else if (std::regex_search(fileLine, removingMetadataDueToIoErrRegex)) {
+        metadataRemovedDueToIoErr = true;
+      } else if (std::regex_search(fileLine,
+                                   removingMetadataDueToTimeoutRegex)) {
+        metadataRemovedDueToTimeout = true;
+      }
+    }
+  }
+  ASSERT_TRUE((timeoutErrors == metadataRemovedDueToTimeout) &&
+              (ioErrors == metadataRemovedDueToIoErr) &&
+              (metadataRemovedDueToTimeout != metadataRemovedDueToIoErr));
+}
+
 void putPartitionedRegionWithRedundancyServerGoesDown(bool singleHop) {
   Cluster cluster{LocatorCount{1}, ServerCount{2}};
   cluster.start();
@@ -158,7 +217,9 @@ void getPartitionedRegionWithRedundancyServerGoesDown(bool singleHop) {
  */
 TEST(PartitionRegionOpsTest,
      getPartitionedRegionWithRedundancyServerGoesDownSingleHop) {
+  removeLogFromPreviousExecution();
   getPartitionedRegionWithRedundancyServerGoesDown(true);
+  verifyMetadataWasRemovedAtFirstError();
 }
 
 /**
@@ -173,7 +234,9 @@ TEST(PartitionRegionOpsTest,
  */
 TEST(PartitionRegionOpsTest,
      putPartitionedRegionWithRedundancyServerGoesDownSingleHop) {
+  removeLogFromPreviousExecution();
   putPartitionedRegionWithRedundancyServerGoesDown(true);
+  verifyMetadataWasRemovedAtFirstError();
 }
 
 /**
diff --git a/cppcache/src/ThinClientPoolDM.cpp b/cppcache/src/ThinClientPoolDM.cpp
index 65634d4..f4ff158 100644
--- a/cppcache/src/ThinClientPoolDM.cpp
+++ b/cppcache/src/ThinClientPoolDM.cpp
@@ -1427,17 +1427,17 @@ GfErrType ThinClientPoolDM::sendSyncRequest(
             GF_SAFE_DELETE_CON(conn);
           }
           excludeServers.insert(ServerLocation(ep->name()));
-          if (error == GF_IOERR) {
-            if (m_clientMetadataService) {
-              auto sl = std::make_shared<BucketServerLocation>(ep->name());
-              LOGINFO("Removing bucketServerLocation %s due to GF_IOERR",
-                      sl->toString().c_str());
-              m_clientMetadataService->removeBucketServerLocation(sl);
-            }
+          if ((error == GF_IOERR || error == GF_TIMEOUT) &&
+              m_clientMetadataService) {
+            auto sl = std::make_shared<BucketServerLocation>(ep->name());
+            LOGINFO("Removing bucketServerLocation %s due to %s",
+                    sl->toString().c_str(),
+                    (error == GF_IOERR ? "GF_IOERR" : "GF_TIMEOUT"));
+            m_clientMetadataService->removeBucketServerLocation(sl);
           }
         }
       } else {
-        return error;  // server exception while sending credentail message to
+        return error;  // server exception while sending credential message to
       }
       // server...
     }
@@ -2332,7 +2332,7 @@ TcrConnection* ThinClientPoolDM::getConnectionFromQueueW(
   if (theEP != nullptr) {
     conn = getFromEP(theEP);
     if (!conn) {
-      LOGFINER("Creating connection to endpint as not found in pool ");
+      LOGFINER("Creating connection to endpoint as not found in pool ");
       *error = createPoolConnectionToAEndPoint(conn, theEP, maxConnLimit, true);
       if (*error == GF_CLIENT_WAIT_TIMEOUT ||
           *error == GF_CLIENT_WAIT_TIMEOUT_REFRESH_PRMETADATA) {
@@ -2351,13 +2351,13 @@ TcrConnection* ThinClientPoolDM::getConnectionFromQueueW(
                   version);
         }
         return nullptr;
-      } else if (*error == GF_IOERR) {
-        if (m_clientMetadataService) {
-          auto sl = std::make_shared<BucketServerLocation>(theEP->name());
-          LOGINFO("Removing bucketServerLocation %s due to GF_IOERR",
-                  sl->toString().c_str());
-          m_clientMetadataService->removeBucketServerLocation(sl);
-        }
+      } else if ((*error == GF_IOERR || *error == GF_TIMEOUT) &&
+                 m_clientMetadataService) {
+        auto sl = std::make_shared<BucketServerLocation>(theEP->name());
+        LOGINFO("Removing bucketServerLocation %s due to %s",
+                sl->toString().c_str(),
+                (*error == GF_IOERR ? "GF_IOERR" : "GF_TIMEOUT"));
+        m_clientMetadataService->removeBucketServerLocation(sl);
       }
     }
   }