You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "alex-plekhanov (via GitHub)" <gi...@apache.org> on 2023/04/17 16:11:19 UTC
[GitHub] [ignite] alex-plekhanov commented on a diff in pull request #10647: IGNITE-19286 NPE in case of simultaneous cache destroy and transaction rollback
alex-plekhanov commented on code in PR #10647:
URL: https://github.com/apache/ignite/pull/10647#discussion_r1168960568
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/transactions/StartImplicitlyTxOnStopCacheTest.java:
##########
@@ -100,4 +126,69 @@ public void test() throws Exception {
assertNull(client.cache(DEFAULT_CACHE_NAME));
}
+
+ /** @throws Exception If failed. */
+ @Test
+ public void testTxStartAfterGatewayBlockedOnCacheDestroy() throws Exception {
+ IgniteEx crd = (IgniteEx)startGridsMultiThreaded(2);
+ GridCacheSharedContext<Object, Object> cctx = crd.context().cache().context();
+
+ // Cache group with multiple caches are important here, partition topology will not be stopped on cache destroy.
+ crd.createCache(new CacheConfiguration<>(DEFAULT_CACHE_NAME + "_1")
+ .setGroupName(GROUP)
+ .setAtomicityMode(CacheAtomicityMode.TRANSACTIONAL));
+
+ CountDownLatch txStarted = new CountDownLatch(1);
+ CountDownLatch gatewayStopped = new CountDownLatch(1);
+
+ IgniteTxManager tm = Mockito.spy(cctx.tm());
+ setFieldValue(crd.context().cache().context(), "txMgr", tm);
Review Comment:
`setTxManager`
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/transactions/StartImplicitlyTxOnStopCacheTest.java:
##########
@@ -100,4 +126,69 @@ public void test() throws Exception {
assertNull(client.cache(DEFAULT_CACHE_NAME));
}
+
+ /** @throws Exception If failed. */
+ @Test
+ public void testTxStartAfterGatewayBlockedOnCacheDestroy() throws Exception {
+ IgniteEx crd = (IgniteEx)startGridsMultiThreaded(2);
+ GridCacheSharedContext<Object, Object> cctx = crd.context().cache().context();
+
+ // Cache group with multiple caches are important here, partition topology will not be stopped on cache destroy.
+ crd.createCache(new CacheConfiguration<>(DEFAULT_CACHE_NAME + "_1")
+ .setGroupName(GROUP)
+ .setAtomicityMode(CacheAtomicityMode.TRANSACTIONAL));
+
+ CountDownLatch txStarted = new CountDownLatch(1);
+ CountDownLatch gatewayStopped = new CountDownLatch(1);
+
+ IgniteTxManager tm = Mockito.spy(cctx.tm());
+ setFieldValue(crd.context().cache().context(), "txMgr", tm);
+
+ Mockito.doAnswer(m -> {
+ // Create tx after gateway was stopped (but not blocked).
+ txStarted.countDown();
+ gatewayStopped.await();
+
+ return m.callRealMethod();
+ }).when(tm).onCreated(Mockito.any(), Mockito.any());
+
+ GridCacheGateway gate = Mockito.spy(cctx.cache().cache(DEFAULT_CACHE_NAME).context().gate());
+ setFieldValue(crd.context().cache().context().cache().cache(DEFAULT_CACHE_NAME).context(), "gate", gate);
+
+ Mockito.doAnswer(m -> {
+ // Await tx gateway enter and mark gateway to stop.
+ txStarted.await();
+
+ return m.callRealMethod();
+ }).when(gate).stopped();
Review Comment:
Looks redundant (`destroyCache` is called only after `txStarted.await()`)
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/transactions/StartImplicitlyTxOnStopCacheTest.java:
##########
@@ -100,4 +126,69 @@ public void test() throws Exception {
assertNull(client.cache(DEFAULT_CACHE_NAME));
}
+
+ /** @throws Exception If failed. */
+ @Test
+ public void testTxStartAfterGatewayBlockedOnCacheDestroy() throws Exception {
+ IgniteEx crd = (IgniteEx)startGridsMultiThreaded(2);
+ GridCacheSharedContext<Object, Object> cctx = crd.context().cache().context();
+
+ // Cache group with multiple caches are important here, partition topology will not be stopped on cache destroy.
+ crd.createCache(new CacheConfiguration<>(DEFAULT_CACHE_NAME + "_1")
+ .setGroupName(GROUP)
+ .setAtomicityMode(CacheAtomicityMode.TRANSACTIONAL));
+
+ CountDownLatch txStarted = new CountDownLatch(1);
+ CountDownLatch gatewayStopped = new CountDownLatch(1);
+
+ IgniteTxManager tm = Mockito.spy(cctx.tm());
+ setFieldValue(crd.context().cache().context(), "txMgr", tm);
+
+ Mockito.doAnswer(m -> {
+ // Create tx after gateway was stopped (but not blocked).
+ txStarted.countDown();
+ gatewayStopped.await();
+
+ return m.callRealMethod();
+ }).when(tm).onCreated(Mockito.any(), Mockito.any());
+
+ GridCacheGateway gate = Mockito.spy(cctx.cache().cache(DEFAULT_CACHE_NAME).context().gate());
+ setFieldValue(crd.context().cache().context().cache().cache(DEFAULT_CACHE_NAME).context(), "gate", gate);
Review Comment:
Too many `context().cache()`, one can be ommited. Or even `cctx.cacheContext(CU.cacheId(DEFAULT_CACHE_NAME))` can be used.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org