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 2021/04/26 15:49:26 UTC

[geode-native] branch develop updated: GEODE-8442: Exception in server not identified correctly in client (#713)

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 0f40b8d   GEODE-8442: Exception in server not identified correctly in client (#713)
0f40b8d is described below

commit 0f40b8d8b5d530c5f650593526c01c0185d5efba
Author: Mario Kevo <48...@users.noreply.github.com>
AuthorDate: Mon Apr 26 17:49:19 2021 +0200

     GEODE-8442: Exception in server not identified correctly in client (#713)
    
    * Exception in server not identified correctly in client
---
 .../resources/cacheserver1_fpr_transaction.xml     |  40 +++++++++
 .../resources/cacheserver2_fpr_transaction.xml     |  40 +++++++++
 .../resources/cacheserver3_fpr_transaction.xml     |  40 +++++++++
 cppcache/integration/framework/Cluster.cpp         |  22 +++++
 cppcache/integration/framework/Cluster.h           |  10 +++
 cppcache/integration/framework/Gfsh.cpp            |   8 ++
 cppcache/integration/framework/Gfsh.h              |   1 +
 cppcache/integration/test/CMakeLists.txt           |   1 +
 .../test/ContainsKeyOnServerExceptionTest.cpp      | 100 +++++++++++++++++++++
 cppcache/src/BucketServerLocation.hpp              |   2 +-
 cppcache/src/ThinClientRegion.cpp                  |   6 +-
 tests/javaobject/CacheWriterTransactionDelay.java  |  45 ++++++++++
 12 files changed, 310 insertions(+), 5 deletions(-)

diff --git a/cppcache/integration-test/resources/cacheserver1_fpr_transaction.xml b/cppcache/integration-test/resources/cacheserver1_fpr_transaction.xml
new file mode 100644
index 0000000..cdbd124
--- /dev/null
+++ b/cppcache/integration-test/resources/cacheserver1_fpr_transaction.xml
@@ -0,0 +1,40 @@
+<?xml version="1.0"?>
+
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+  
+       http://www.apache.org/licenses/LICENSE-2.0
+  
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+
+<cache xmlns="http://geode.apache.org/schema/cache"
+       xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xsi:schemaLocation="http://geode.apache.org/schema/cache http://geode.apache.org/schema/cache/cache-1.0.xsd"
+       version="1.0">
+	<cache-server port="40401"/>
+
+  <region name="region">
+    <region-attributes data-policy="partition">
+      <partition-attributes>
+        <partition-resolver>
+          <class-name>javaobject.CustomFixedPartitionResolver1</class-name>
+        </partition-resolver>
+		<fixed-partition-attributes partition-name="P1" is-primary="true" num-buckets="15"/>				
+		<fixed-partition-attributes partition-name="P6" is-primary="true" num-buckets="15"/>
+      </partition-attributes>
+      <cache-writer>
+        <class-name>javaobject.CacheWriterTransactionDelay</class-name>
+      </cache-writer>
+    </region-attributes>
+  </region>
+</cache>
diff --git a/cppcache/integration-test/resources/cacheserver2_fpr_transaction.xml b/cppcache/integration-test/resources/cacheserver2_fpr_transaction.xml
new file mode 100644
index 0000000..ff92015
--- /dev/null
+++ b/cppcache/integration-test/resources/cacheserver2_fpr_transaction.xml
@@ -0,0 +1,40 @@
+<?xml version="1.0"?>
+
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+  
+       http://www.apache.org/licenses/LICENSE-2.0
+  
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+
+<cache xmlns="http://geode.apache.org/schema/cache"
+       xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xsi:schemaLocation="http://geode.apache.org/schema/cache http://geode.apache.org/schema/cache/cache-1.0.xsd"
+       version="1.0">
+	<cache-server port="40402"/>
+
+  <region name="region">
+    <region-attributes data-policy="partition">
+      <partition-attributes>
+        <partition-resolver>
+          <class-name>javaobject.CustomFixedPartitionResolver1</class-name>
+        </partition-resolver>	
+		<fixed-partition-attributes partition-name="P2" is-primary="true" num-buckets="15"/>
+		<fixed-partition-attributes partition-name="P4" is-primary="true" num-buckets="15"/>
+      </partition-attributes>
+      <cache-writer>
+        <class-name>javaobject.CacheWriterTransactionDelay</class-name>
+      </cache-writer>
+    </region-attributes>
+  </region>
+</cache>
diff --git a/cppcache/integration-test/resources/cacheserver3_fpr_transaction.xml b/cppcache/integration-test/resources/cacheserver3_fpr_transaction.xml
new file mode 100644
index 0000000..13ec87d
--- /dev/null
+++ b/cppcache/integration-test/resources/cacheserver3_fpr_transaction.xml
@@ -0,0 +1,40 @@
+<?xml version="1.0"?>
+
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+  
+       http://www.apache.org/licenses/LICENSE-2.0
+  
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+
+<cache xmlns="http://geode.apache.org/schema/cache"
+       xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xsi:schemaLocation="http://geode.apache.org/schema/cache http://geode.apache.org/schema/cache/cache-1.0.xsd"
+       version="1.0">
+	<cache-server port="40403"/>
+
+  <region name="region">
+    <region-attributes data-policy="partition">
+      <partition-attributes>
+        <partition-resolver>
+          <class-name>javaobject.CustomFixedPartitionResolver1</class-name>
+        </partition-resolver>
+		<fixed-partition-attributes partition-name="P3" is-primary="true" num-buckets="15"/>		
+		<fixed-partition-attributes partition-name="P5" is-primary="true" num-buckets="15"/>
+      </partition-attributes>
+      <cache-writer>
+        <class-name>javaobject.CacheWriterTransactionDelay</class-name>
+      </cache-writer>
+    </region-attributes>
+  </region>
+</cache>
diff --git a/cppcache/integration/framework/Cluster.cpp b/cppcache/integration/framework/Cluster.cpp
index 5e930de..4db213e 100644
--- a/cppcache/integration/framework/Cluster.cpp
+++ b/cppcache/integration/framework/Cluster.cpp
@@ -205,6 +205,7 @@ void Server::start() {
           .withClasspath(cluster_.getClasspath())
           .withSecurityManager(cluster_.getSecurityManager())
           .withCacheXMLFile(getCacheXMLFile())
+          .withConserveSockets(cluster_.getConserveSockets())
           .withPreferIPv6(cluster_.getUseIPv6());
 
   if (!cluster_.getUser().empty()) {
@@ -301,6 +302,20 @@ Cluster::Cluster(LocatorCount initialLocators, ServerCount initialServers,
 }
 
 Cluster::Cluster(LocatorCount initialLocators, ServerCount initialServers,
+                 ConserveSockets conserveSockets, CacheXMLFiles cacheXMLFiles)
+    : name_(std::string(::testing::UnitTest::GetInstance()
+                            ->current_test_info()
+                            ->test_suite_name()) +
+            "/" +
+            ::testing::UnitTest::GetInstance()->current_test_info()->name()),
+      initialLocators_(initialLocators.get()),
+      initialServers_(initialServers.get()) {
+  jmxManagerPort_ = Framework::getAvailablePort();
+  cacheXMLFiles_ = cacheXMLFiles.get();
+  conserveSockets_ = conserveSockets.get();
+}
+
+Cluster::Cluster(LocatorCount initialLocators, ServerCount initialServers,
                  std::vector<uint16_t> &serverPorts)
     : name_(std::string(::testing::UnitTest::GetInstance()
                             ->current_test_info()
@@ -413,6 +428,11 @@ void Cluster::applyLocators(apache::geode::client::PoolFactory &poolFactory) {
   }
 }
 
+void Cluster::applyServer(apache::geode::client::PoolFactory &poolFactory,
+                          ServerAddress oneServer) {
+  poolFactory.addServer(oneServer.address, oneServer.port);
+}
+
 Gfsh &Cluster::getGfsh() { return gfsh_; }
 
 std::vector<Server> &Cluster::getServers() { return servers_; }
@@ -431,6 +451,8 @@ std::vector<std::string> &Cluster::getCacheXMLFiles() { return cacheXMLFiles_; }
 
 bool Cluster::getUseIPv6() { return useIPv6_; }
 
+bool Cluster::getConserveSockets() { return conserveSockets_; }
+
 void Cluster::start() { start(std::function<void()>()); }
 
 void Cluster::start(std::function<void()> extraGfshCommands) {
diff --git a/cppcache/integration/framework/Cluster.h b/cppcache/integration/framework/Cluster.h
index fee4886..baea45c 100644
--- a/cppcache/integration/framework/Cluster.h
+++ b/cppcache/integration/framework/Cluster.h
@@ -123,6 +123,7 @@ using Password = NamedType<std::string, struct PasswordParameter>;
 using CacheXMLFiles =
     NamedType<std::vector<std::string>, struct CacheXMLFilesParameter>;
 using UseIpv6 = NamedType<bool, struct UseIpv6Parameter>;
+using ConserveSockets = NamedType<bool, struct useConserveSocketsParameter>;
 
 class Cluster {
  public:
@@ -149,6 +150,9 @@ class Cluster {
   Cluster(LocatorCount initialLocators, ServerCount initialServers,
           CacheXMLFiles cacheXMLFiles);
 
+  Cluster(LocatorCount initialLocators, ServerCount initialServers,
+          ConserveSockets conserveSockets, CacheXMLFiles cacheXMLFiles);
+
   Cluster(Name name, LocatorCount initialLocators, ServerCount initialServers,
           UseIpv6 useIPv6);
 
@@ -192,6 +196,9 @@ class Cluster {
 
   void applyLocators(apache::geode::client::PoolFactory &poolFactory);
 
+  void applyServer(apache::geode::client::PoolFactory &poolFactory,
+                   ServerAddress server);
+
   void useSsl(const bool requireSslAuthentication, const std::string keystore,
               const std::string truststore, const std::string keystorePassword,
               const std::string truststorePassword);
@@ -229,6 +236,8 @@ class Cluster {
 
   bool getUseIPv6();
 
+  bool getConserveSockets();
+
  private:
   std::string name_;
   std::string classpath_;
@@ -265,6 +274,7 @@ class Cluster {
   std::string hostName_;
 
   bool useIPv6_ = false;
+  bool conserveSockets_ = false;
 
   uint16_t distributedSystemId_ = 0;
 
diff --git a/cppcache/integration/framework/Gfsh.cpp b/cppcache/integration/framework/Gfsh.cpp
index 343a762..6ca5a1c 100644
--- a/cppcache/integration/framework/Gfsh.cpp
+++ b/cppcache/integration/framework/Gfsh.cpp
@@ -353,6 +353,14 @@ Gfsh::Start::Server &Gfsh::Start::Server::withSystemProperty(
   return *this;
 }
 
+Gfsh::Start::Server &Gfsh::Start::Server::withConserveSockets(
+    bool conserveSockets) {
+  if (conserveSockets) {
+    command_ += " --J=-Dgemfire.conserve-sockets=true";
+  }
+  return *this;
+}
+
 Gfsh::Stop::Stop(Gfsh &gfsh) : gfsh_(gfsh) {}
 
 Gfsh::Stop::Server Gfsh::Stop::server() { return Server{gfsh_}; }
diff --git a/cppcache/integration/framework/Gfsh.h b/cppcache/integration/framework/Gfsh.h
index 816c62e..344a2f6 100644
--- a/cppcache/integration/framework/Gfsh.h
+++ b/cppcache/integration/framework/Gfsh.h
@@ -193,6 +193,7 @@ class Gfsh {
 
       Server &withSystemProperty(const std::string &key,
                                  const std::string &value);
+      Server &withConserveSockets(bool conserveSockets);
     };
 
    private:
diff --git a/cppcache/integration/test/CMakeLists.txt b/cppcache/integration/test/CMakeLists.txt
index 955f7ea..d3363e5 100644
--- a/cppcache/integration/test/CMakeLists.txt
+++ b/cppcache/integration/test/CMakeLists.txt
@@ -59,6 +59,7 @@ add_executable(cpp-integration-test
   TransactionsTest.cpp
   WanDeserializationTest.cpp
   DisconnectEndPointAtException.cpp
+  ContainsKeyOnServerExceptionTest.cpp
 )
 if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
   target_compile_options(cpp-integration-test
diff --git a/cppcache/integration/test/ContainsKeyOnServerExceptionTest.cpp b/cppcache/integration/test/ContainsKeyOnServerExceptionTest.cpp
new file mode 100644
index 0000000..2a2dbd5
--- /dev/null
+++ b/cppcache/integration/test/ContainsKeyOnServerExceptionTest.cpp
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include <framework/Cluster.h>
+#include <framework/TestConfig.h>
+
+#include <geode/Cache.hpp>
+#include <geode/CacheFactory.hpp>
+#include <geode/CacheTransactionManager.hpp>
+#include <geode/RegionFactory.hpp>
+#include <geode/RegionShortcut.hpp>
+
+namespace {
+using apache::geode::client::Cache;
+using apache::geode::client::CacheableKey;
+using apache::geode::client::CacheableString;
+using apache::geode::client::CacheFactory;
+using apache::geode::client::CacheTransactionManager;
+using apache::geode::client::Pool;
+using apache::geode::client::Region;
+using apache::geode::client::RegionShortcut;
+
+std::string getClientLogName() {
+  std::string testSuiteName(::testing::UnitTest::GetInstance()
+                                ->current_test_info()
+                                ->test_suite_name());
+  std::string testCaseName(
+      ::testing::UnitTest::GetInstance()->current_test_info()->name());
+  std::string logFileName(testSuiteName + "/" + testCaseName + "/client.log");
+  return logFileName;
+}
+
+std::shared_ptr<Cache> createCache() {
+  auto cache = CacheFactory()
+                   .set("log-level", "none")
+                   .set("log-file", getClientLogName())
+                   .create();
+  return std::make_shared<Cache>(std::move(cache));
+}
+
+TEST(ContainsKeyOnServerExceptionTest, handleException) {
+  Cluster cluster{
+      LocatorCount{1}, ServerCount{3}, ConserveSockets(true),
+      CacheXMLFiles({
+          std::string(getFrameworkString(FrameworkVariable::TestCacheXmlDir)) +
+              "/cacheserver1_fpr_transaction.xml",
+          std::string(getFrameworkString(FrameworkVariable::TestCacheXmlDir)) +
+              "/cacheserver2_fpr_transaction.xml",
+          std::string(getFrameworkString(FrameworkVariable::TestCacheXmlDir)) +
+              "/cacheserver3_fpr_transaction.xml",
+      })};
+  cluster.start([&]() {
+    cluster.getGfsh()
+        .deploy()
+        .jar(getFrameworkString(FrameworkVariable::JavaObjectJarPath))
+        .execute();
+  });
+
+  auto cache = createCache();
+  auto poolFactory = cache->getPoolManager().createFactory();
+  ServerAddress serverAddress = cluster.getServers()[0].getAddress();
+  serverAddress.port = 40401;
+  cluster.applyServer(poolFactory, serverAddress);
+  auto pool = poolFactory.create("default");
+  auto region = cache->createRegionFactory(RegionShortcut::PROXY)
+                    .setPoolName("default")
+                    .create("region");
+
+  auto transactionManager = cache->getCacheTransactionManager();
+
+  // this key will be always routed towards server[1]
+  int theKey = 7;
+  std::string theValue = "theValue";
+  try {
+    transactionManager->begin();
+    region->put(theKey, theValue);
+    transactionManager->commit();
+  } catch (...) {
+    EXPECT_THROW(
+        region->containsKeyOnServer(CacheableKey::create(7)),
+        apache::geode::client::TransactionDataNodeHasDepartedException);
+  }
+
+  EXPECT_FALSE(region->containsKey(7));
+}
+
+}  // namespace
diff --git a/cppcache/src/BucketServerLocation.hpp b/cppcache/src/BucketServerLocation.hpp
index fd4e92d..c5ca16e 100644
--- a/cppcache/src/BucketServerLocation.hpp
+++ b/cppcache/src/BucketServerLocation.hpp
@@ -151,7 +151,7 @@ class BucketServerLocation : public ServerLocation {
 
   BucketServerLocation(
       const BucketServerLocation&
-          rhs)  //:ServerLocation(rhs.getServerName(),rhs.getPort())
+          rhs)  //: ServerLocation(rhs.getServerName(),rhs.getPort())
   {
     this->m_serverName = rhs.m_serverName;
     this->m_port = rhs.m_port;
diff --git a/cppcache/src/ThinClientRegion.cpp b/cppcache/src/ThinClientRegion.cpp
index 4fa88ba..aba0f97 100644
--- a/cppcache/src/ThinClientRegion.cpp
+++ b/cppcache/src/ThinClientRegion.cpp
@@ -744,6 +744,7 @@ bool ThinClientRegion::containsKeyOnServer(
   TcrMessageReply reply(true, m_tcrdm.get());
   reply.setMessageTypeRequest(TcrMessage::CONTAINS_KEY);
   err = m_tcrdm->sendSyncRequest(request, reply);
+  if (err != GF_NOERR) return err;
 
   switch (reply.getMessageType()) {
     case TcrMessage::RESPONSE:
@@ -771,7 +772,6 @@ bool ThinClientRegion::containsKeyOnServer(
 
   auto rptr = CacheableBoolean::create(ret);
 
-  rptr = std::dynamic_pointer_cast<CacheableBoolean>(handleReplay(err, rptr));
   throwExceptionIfError("Region::containsKeyOnServer ", err);
   return rptr->value();
 }
@@ -790,7 +790,7 @@ bool ThinClientRegion::containsValueForKey_remote(
   TcrMessageReply reply(true, m_tcrdm.get());
   reply.setMessageTypeRequest(TcrMessage::CONTAINS_KEY);
   err = m_tcrdm->sendSyncRequest(request, reply);
-  // if ( err != GF_NOERR ) return ret;
+  if (err != GF_NOERR) return ret;
 
   switch (reply.getMessageType()) {
     case TcrMessage::RESPONSE:
@@ -818,8 +818,6 @@ bool ThinClientRegion::containsValueForKey_remote(
 
   auto rptr = CacheableBoolean::create(ret);
 
-  rptr = std::dynamic_pointer_cast<CacheableBoolean>(handleReplay(err, rptr));
-
   throwExceptionIfError("Region::containsValueForKey ", err);
   return rptr->value();
 }
diff --git a/tests/javaobject/CacheWriterTransactionDelay.java b/tests/javaobject/CacheWriterTransactionDelay.java
new file mode 100644
index 0000000..a8f376e
--- /dev/null
+++ b/tests/javaobject/CacheWriterTransactionDelay.java
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package javaobject;
+
+import java.util.*;
+import org.apache.geode.cache.CacheWriter;
+import org.apache.geode.cache.CacheWriterException;
+import org.apache.geode.cache.EntryEvent;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionEvent;
+import org.apache.geode.cache.RegionService;
+
+public class CacheWriterTransactionDelay implements CacheWriter<String, String> {
+
+  public CacheWriterTransactionDelay() {}
+
+  public void beforeCreate(EntryEvent<String, String> event) {
+    final RegionService cache = event.getRegion().getRegionService();
+    cache.close();
+  }
+
+  public void beforeUpdate(EntryEvent<String, String> event) {}
+
+  public void beforeDestroy(EntryEvent<String, String> event) {}
+
+  public void beforeRegionDestroy(RegionEvent<String, String> event) {}
+
+  public void beforeRegionClear(RegionEvent<String, String> event) {}
+
+  public void close() {}
+}