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/01/25 15:58:11 UTC

[geode-native] branch develop updated: GEODE-8830: Fix exception handling in transactions (#720)

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 ec5e33f  GEODE-8830: Fix exception handling in transactions (#720)
ec5e33f is described below

commit ec5e33f886d4308c84266bcfe406e416da108fb8
Author: Mario Salazar de Torres <ma...@est.tech>
AuthorDate: Mon Jan 25 16:58:02 2021 +0100

    GEODE-8830: Fix exception handling in transactions (#720)
    
    - Exceptions were not correctly handled during transaction operations,
       leading to the derived type of the exception to be lost on the
       internal source calls.
     - Problem was that in the process of capturing an re-throwing the
       exceptions, a copy of its more generic type, `Exception` was created
       and passed throught, instead of the original exception. This has been
       sorted out.
     - Also, several integration test were added, in order to ensure that
       the correct type of exceptions are thrown when an error occurs while
       executing transsactions operations.
---
 cppcache/integration/test/TransactionsTest.cpp     | 119 ++++++++++++++++++---
 .../src/InternalCacheTransactionManager2PCImpl.cpp |   4 +-
 2 files changed, 107 insertions(+), 16 deletions(-)

diff --git a/cppcache/integration/test/TransactionsTest.cpp b/cppcache/integration/test/TransactionsTest.cpp
index eacaa04..edf81df 100644
--- a/cppcache/integration/test/TransactionsTest.cpp
+++ b/cppcache/integration/test/TransactionsTest.cpp
@@ -32,27 +32,38 @@ using apache::geode::client::Cache;
 using apache::geode::client::CacheableString;
 using apache::geode::client::CacheFactory;
 using apache::geode::client::CacheTransactionManager;
+using apache::geode::client::CommitConflictException;
+using apache::geode::client::Exception;
+using apache::geode::client::IllegalStateException;
 using apache::geode::client::Pool;
 using apache::geode::client::Region;
 using apache::geode::client::RegionShortcut;
 
-std::shared_ptr<Cache> createCache() {
-  auto cache = CacheFactory().set("log-level", "none").create();
-  return std::make_shared<Cache>(std::move(cache));
+const std::string regionName = "region";
+
+Cache createCache() {
+  return CacheFactory()
+      .set("statistic-sampling-enabled", "false")
+      .set("log-level", "none")
+      .create();
 }
 
-std::shared_ptr<Pool> createPool(Cluster& cluster,
-                                 std::shared_ptr<Cache> cache) {
-  auto poolFactory = cache->getPoolManager().createFactory();
+std::shared_ptr<Pool> createPool(Cluster& cluster, Cache& cache) {
+  auto poolFactory = cache.getPoolManager().createFactory();
   cluster.applyLocators(poolFactory);
   poolFactory.setPRSingleHopEnabled(true);
   return poolFactory.create("default");
 }
 
-void runClientOperations(std::shared_ptr<Cache> cache,
-                         std::shared_ptr<Region> region, int minEntryKey,
-                         int maxEntryKey, int numTx) {
-  auto transactionManager = cache->getCacheTransactionManager();
+std::shared_ptr<Region> setupRegion(Cache& cache) {
+  return cache.createRegionFactory(RegionShortcut::PROXY)
+      .setPoolName("default")
+      .create(regionName);
+}
+
+void runClientOperations(Cache& cache, std::shared_ptr<Region> region,
+                         int minEntryKey, int maxEntryKey, int numTx) {
+  auto transactionManager = cache.getCacheTransactionManager();
 
   for (int i = 0; i < numTx; i++) {
     auto theKey = (rand() % (maxEntryKey - minEntryKey)) + minEntryKey;
@@ -86,15 +97,13 @@ TEST(TransactionsTest, ExceptionWhenRollingBackTx) {
 
   auto cache = createCache();
   auto pool = createPool(cluster, cache);
-  auto region = cache->createRegionFactory(RegionShortcut::PROXY)
-                    .setPoolName("default")
-                    .create("region");
+  auto region = setupRegion(cache);
 
   std::vector<std::thread> clientThreads;
   for (int i = 0; i < NUM_THREADS; i++) {
     auto minKey = (i * keyRangeSize);
     auto maxKey = minKey + keyRangeSize - 1;
-    std::thread th(runClientOperations, cache, region, minKey, maxKey,
+    std::thread th(runClientOperations, std::ref(cache), region, minKey, maxKey,
                    TX_PER_CLIENT);
     clientThreads.push_back(std::move(th));
   }
@@ -109,4 +118,86 @@ TEST(TransactionsTest, ExceptionWhenRollingBackTx) {
 
 }  // TEST
 
+TEST(TransactionsTest, IlegalStateExceptionNoTx) {
+  Cluster cluster{LocatorCount{1}, ServerCount{1}};
+  cluster.start();
+
+  // Create regions
+  cluster.getGfsh()
+      .create()
+      .region()
+      .withName(regionName)
+      .withType("PARTITION")
+      .execute();
+
+  auto cache = createCache();
+  auto txm = cache.getCacheTransactionManager();
+  auto pool = createPool(cluster, cache);
+  auto region = setupRegion(cache);
+
+  EXPECT_THROW(txm->prepare(), IllegalStateException);
+  EXPECT_THROW(txm->commit(), IllegalStateException);
+  EXPECT_THROW(txm->rollback(), IllegalStateException);
+}  // TEST
+
+TEST(TransactionsTest, ExceptionConflictOnPrepare) {
+  Cluster cluster{LocatorCount{1}, ServerCount{1}};
+  cluster.start();
+
+  // Create regions
+  cluster.getGfsh()
+      .create()
+      .region()
+      .withName(regionName)
+      .withType("PARTITION")
+      .execute();
+
+  auto cache = createCache();
+  auto txm = cache.getCacheTransactionManager();
+  auto pool = createPool(cluster, cache);
+  auto region = setupRegion(cache);
+
+  txm->begin();
+  region->put("key", "A");
+  auto& tx_first_id = txm->suspend();
+  txm->begin();
+  region->put("key", "B");
+  txm->prepare();
+  auto& tx_second_id = txm->suspend();
+  txm->resume(tx_first_id);
+
+  EXPECT_THROW(txm->prepare(), CommitConflictException);
+  txm->resume(tx_second_id);
+  txm->rollback();
+
+}  // TEST
+
+TEST(TransactionsTest, ExceptionConflictOnCommit) {
+  Cluster cluster{LocatorCount{1}, ServerCount{1}};
+  cluster.start();
+
+  // Create regions
+  cluster.getGfsh()
+      .create()
+      .region()
+      .withName(regionName)
+      .withType("PARTITION")
+      .execute();
+
+  auto cache = createCache();
+  auto txm = cache.getCacheTransactionManager();
+  auto pool = createPool(cluster, cache);
+  auto region = setupRegion(cache);
+
+  txm->begin();
+  region->put("key", "A");
+  auto& tx_id = txm->suspend();
+  txm->begin();
+  region->put("key", "B");
+  txm->commit();
+  txm->resume(tx_id);
+
+  EXPECT_THROW(txm->commit(), CommitConflictException);
+}  // TEST
+
 }  // namespace
diff --git a/cppcache/src/InternalCacheTransactionManager2PCImpl.cpp b/cppcache/src/InternalCacheTransactionManager2PCImpl.cpp
index 0df0512..2ac0d9c 100644
--- a/cppcache/src/InternalCacheTransactionManager2PCImpl.cpp
+++ b/cppcache/src/InternalCacheTransactionManager2PCImpl.cpp
@@ -101,7 +101,7 @@ void InternalCacheTransactionManager2PCImpl::prepare() {
     }
   } catch (const Exception& ex) {
     LOGERROR("Unexpected exception during commit in prepare %s", ex.what());
-    throw ex;
+    throw;
   }
 }
 
@@ -200,7 +200,7 @@ void InternalCacheTransactionManager2PCImpl::afterCompletion(int32_t status) {
   } catch (const Exception& ex) {
     LOGERROR("Unexpected exception during completing transaction %s",
              ex.what());
-    throw ex;
+    throw;
   }
 }
 }  // namespace client