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.
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::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 @@
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
+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 @@
}
} catch (const Exception& ex) {
LOGERROR("Unexpected exception during commit in prepare %s", ex.what());
- throw ex;
+ throw;
}
}
@@ -200,7 +200,7 @@
} catch (const Exception& ex) {
LOGERROR("Unexpected exception during completing transaction %s",
ex.what());
- throw ex;
+ throw;
}
}
} // namespace client
