[
https://issues.apache.org/jira/browse/GEODE-8442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17250040#comment-17250040
]
ASF GitHub Bot commented on GEODE-8442:
---------------------------------------
moleske commented on a change in pull request #713:
URL: https://github.com/apache/geode-native/pull/713#discussion_r543801818
##########
File path: cppcache/integration/test/ContainsKeyOnServerExceptionTest.cpp
##########
@@ -0,0 +1,111 @@
+/*
+ * 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/Region.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 disconnectingStr("Disconnecting from the endpoint");
+
+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;
+}
+
+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));
+}
+
+void executeTestCaseHandleException() {
+ Cluster cluster{
+ LocatorCount{1}, ServerCount{3},
+ 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 (...) {
+ try {
+ region->containsKeyOnServer(CacheableKey::create(7));
+ } catch (
+ apache::geode::client::TransactionDataNodeHasDepartedException& ex) {
+ std::cerr << "Caught exception: " << ex.what() << std::endl;
+ }
+ }
+
+ EXPECT_FALSE(region->containsKey(7));
+
+} // executeTestCaseHandleException
+
+TEST(ContainsKeyOnServerExceptionTest, handleException) {
+ executeTestCaseHandleException();
Review comment:
Why is this a function that is called only in the test? It doesn't seem
like you are reusing this, so would it be simpler to just inline?
##########
File path: cppcache/src/ThinClientRegion.cpp
##########
@@ -846,7 +847,8 @@ bool ThinClientRegion::containsValueForKey_remote(
auto rptr = CacheableBoolean::create(ret);
- rptr = std::dynamic_pointer_cast<CacheableBoolean>(handleReplay(err, rptr));
+ // rptr = std::dynamic_pointer_cast<CacheableBoolean>(handleReplay(err,
Review comment:
If this is truly not needed, just deleted it. Same for commented code
above on line 802
##########
File path: cppcache/integration/test/ContainsKeyOnServerExceptionTest.cpp
##########
@@ -0,0 +1,111 @@
+/*
+ * 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/Region.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 disconnectingStr("Disconnecting from the endpoint");
+
+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;
+}
+
+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));
+}
+
+void executeTestCaseHandleException() {
+ Cluster cluster{
+ LocatorCount{1}, ServerCount{3},
+ 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 {
Review comment:
try/catches in a test always make me a little concerned. This probably
comes down to one's testing philosophy, but I generally try to avoid them and
let the errors explode. It seems in this set of try/catches, you are only
printing that an exception was caught, no changes in assertions or test
expectations.
##########
File path: cppcache/integration/framework/Cluster.cpp
##########
@@ -393,6 +395,11 @@ void
Cluster::applyLocators(apache::geode::client::PoolFactory &poolFactory) {
}
}
+void Cluster::applyServer(apache::geode::client::PoolFactory &poolFactory,
Review comment:
is `apply` the right verb for this function? This function is actually
adding a server (based on `poolFactory.addServer` call)
----------------------------------------------------------------
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:
us...@infra.apache.org
> Exception in server not identified correctly in client
> ------------------------------------------------------
>
> Key: GEODE-8442
> URL: https://issues.apache.org/jira/browse/GEODE-8442
> Project: Geode
> Issue Type: Bug
> Components: native client
> Reporter: Alberto Bustamante Reyes
> Assignee: Mario Kevo
> Priority: Major
> Labels: needs-review, pull-request-available
>
> Native client is not identifying correctly an exception returned by the
> server.
> This is a log from an exception properly identified (I have introduced "????"
> where I think I have to hide sensitive information):
> {code}
> [error 2020/07/22 09:17:10.831337 UTC ????]
> CacheTransactionManager::failover: An exception
> (org.apache.geode.cache.TransactionDataNodeHasDepartedException: Could not
> find transaction host for TXId:
> ????(default_GeodeDS:1:loner):3:Native_gdbdedfebe1:default_GeodeDS:6022
> at
> org.apache.geode.internal.cache.tier.sockets.command.TXFailoverCommand.cmdExecute(TXFailoverCommand.java:126)
> at
> org.apache.geode.internal.cache.tier.sockets.BaseCommand.execute(BaseCommand.java:177)
> at
> org.apache.geode.internal.cache.tier.sockets.ServerConnection.doNormalMessage(ServerConnection.java:848)
> at
> org.apache.geode.internal.cache.tier.sockets.OriginalServerConnection.doOneMessage(OriginalServerConnection.java:72)
> at
> org.apache.geode.internal.cache.tier.sockets.ServerConnection.run(ServerConnection.java:1212)
> at
> java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
> at
> java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
> at
> org.apache.geode.internal.cache.tier.sockets.AcceptorImpl.lambda$initializeServerConnectionThreadPool$3(AcceptorImpl.java:676)
> at
> org.apache.geode.logging.internal.executors.LoggingThreadFactory.lambda$newThread$0(LoggingThreadFactory.java:119)
> at java.base/java.lang.Thread.run(Thread.java:834)
> ) happened at remote server.
> data-access: write_element:[TransactionDataNodeHasDepartedException]
> ????.get(????)::apache::geode::client::TransactionDataNodeHasDepartedException:org.apache.geode.cache.TransactionDataNodeHasDepartedException:
> Could not find transaction host for TXId:
> ????(default_GeodeDS:1:loner):3:Native_gdbdedfebe1:default_GeodeDS:6022
> {code}
> But in this case, although the exception received is also a
> TransactionDataNodeHasDepartedException, it is "translated" to an Unknown
> exception:
> {code}
> [error 2020/07/22 09:17:10.979506 UTC ????] Region::containsKeyOnServer:: An
> exception (org.apache.geode.cache.TransactionDataNodeHasDepartedException:
> Transaction data node 192.168.240.13(dms-server-1:1)<v1>:41000 has departed.
> To proceed, rollback this transaction and begin a new one., caused by
> org.apache.geode.internal.cache.ForceReattemptException: PartitionResponse
> got remote CacheClosedException
> at
> org.apache.geode.internal.cache.tx.PartitionedTXRegionStub.containsKey(PartitionedTXRegionStub.java:247)
> at
> org.apache.geode.internal.cache.TXStateStub.containsKey(TXStateStub.java:431)
> at
> org.apache.geode.internal.cache.TXStateProxyImpl.containsKey(TXStateProxyImpl.java:530)
> at
> org.apache.geode.internal.cache.PartitionedRegion.containsKey(PartitionedRegion.java:6402)
> at
> org.apache.geode.internal.cache.tier.sockets.command.ContainsKey66.cmdExecute(ContainsKey66.java:138)
> at
> org.apache.geode.internal.cache.tier.sockets.BaseCommand.execute(BaseCommand.java:177)
> at
> org.apache.geode.internal.cache.tier.sockets.ServerConnection.doNormalMessage(ServerConnection.java:848)
> at
> org.apache.geode.internal.cache.tier.sockets.OriginalServerConnection.doOneMessage(OriginalServerConnection.java:72)
> at
> org.apache.geode.internal.cache.tier.sockets.ServerConnection.run(ServerConnection.java:1212)
> at
> java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
> at
> java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
> at
> org.apache.geode.internal.cache.tier.sockets.AcceptorImpl.lambda$initializeServerConnectionThreadPool$3(AcceptorImpl.java:676)
> at
> org.apache.geode.logging.internal.executors.LoggingThreadFactory.lambda$newThread$0(LoggingThreadFactory.java:119)
> at java.base/java.lang.Thread.run(Thread.java:834)
> Caused by: org.apache.geode.internal.cache.ForceReattemptException:
> PartitionResponse got remote CacheClosedException
> at
> org.apache.geode.internal.cache.partitioned.PartitionMessage$PartitionResponse.waitForCacheException(PartitionMessage.java:837)
> at
> org.apache.geode.internal.cache.partitioned.ContainsKeyValueMessage$ContainsKeyValueResponse.waitForContainsResult(ContainsKeyValueMessage.java:295)
> at
> org.apache.geode.internal.cache.PartitionedRegion.containsKeyRemotely(PartitionedRegion.java:6558)
> at
> org.apache.geode.internal.cache.tx.PartitionedTXRegionStub.containsKey(PartitionedTXRegionStub.java:231)
> ... 13 more
> Caused by: org.apache.geode.cache.CacheClosedException: Remote cache is
> closed: 192.168.240.13(dms-server-1:1)<v1>:41000
> at
> org.apache.geode.internal.cache.GemFireCacheImpl.getCacheClosedException(GemFireCacheImpl.java:1630)
> at
> org.apache.geode.internal.cache.GemFireCacheImpl.getCacheClosedException(GemFireCacheImpl.java:1619)
> at
> org.apache.geode.internal.cache.partitioned.PartitionMessage.process(PartitionMessage.java:307)
> at
> org.apache.geode.distributed.internal.DistributionMessage.scheduleAction(DistributionMessage.java:376)
> at
> org.apache.geode.distributed.internal.DistributionMessage$1.run(DistributionMessage.java:440)
> at
> java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
> at
> java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
> at
> org.apache.geode.distributed.internal.ClusterOperationExecutors.runUntilShutdown(ClusterOperationExecutors.java:442)
> at
> org.apache.geode.distributed.internal.ClusterOperationExecutors.doPartitionRegionThread(ClusterOperationExecutors.java:422)
> ... 2 more
> ) happened at remote server.
> [info 2020/07/22 09:17:10.979514 UTC ????] error code: 12
> [fine 2020/07/22 09:17:10.979869 UTC ????] sendSyncRequest:: retry = 0
> [fine 2020/07/22 09:17:10.979874 UTC ????] sendRequestConnWithRetry::
> maxSendRetries = 1
> data-access: write_element:[Exception]
> SmSessions.containsKeyOnServer(????):apache::geode::client::UnknownException:Replay
> is unsupported:
> org.apache.geode.cache.TransactionDataNodeHasDepartedException: Transaction
> data node 192.168.240.13(????)<v1>:41000 has departed. To proceed, rollback
> this transaction and begin a new one, caused by
> org.apache.geode.internal.cache.ForceReattemptException: PartitionResponse
> got remote CacheClosedException
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
