Creation of dynamic cache

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Creation of dynamic cache

Denis Garus
Hello, Igniters!


If an error occurred during of creation dynamic cache, the
CacheAffinitySharedManager#processCacheStartRequests method will try to
rollback cache start routine.
```
try {
        if (startCache) {
                cctx.cache().prepareCacheStart(req.startCacheConfiguration(),
                        cacheDesc,
                        nearCfg,
                        evts.topologyVersion(),
                        req.disabledAfterStart());
                //some code
        }
}
catch (IgniteCheckedException e) {
        U.error(log, "Failed to initialize cache. Will try to rollback cache start
routine. " +
                "[cacheName=" + req.cacheName() + ']', e);

        cctx.cache().closeCaches(Collections.singleton(req.cacheName()), false);

        cctx.cache().completeCacheStartFuture(req, false, e);
}
```
Assume, that GridDhtPartitionsExchangeFuture will finish without any error
because of the exception is just logged.
Is this way right? What should return the Ignite#createCache method in this
case?

I can't check what could return in that case because it just doesn't work
this way now.
In the further, we're getting the critical error that stops ExchangeWorker
in a test environment
or stops node in a production environment.

Reproducer:
```
package org.apache.ignite.internal.processors.cache;

import org.apache.ignite.IgniteCheckedException;
import org.apache.ignite.configuration.CacheConfiguration;
import org.apache.ignite.internal.IgniteEx;
import org.apache.ignite.internal.util.typedef.internal.U;
import org.apache.ignite.testframework.GridTestUtils;
import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;

public class CreateCacheFreezeTest extends GridCommonAbstractTest {
    public void test() throws Exception {
        IgniteEx ignite = startGrid(0);

        U.registerMBean(ignite.context().config().getMBeanServer(),
ignite.name(), "FIRST_CACHE",
           
"org.apache.ignite.internal.processors.cache.CacheLocalMetricsMXBeanImpl",
            new DummyMBeanImpl(), DummyMBean.class);

        GridTestUtils.assertThrowsWithCause(() -> {
            ignite.createCache(new CacheConfiguration<>("FIRST_CACHE"));

            return 0;
        }, IgniteCheckedException.class);
        //The creation of SECOND_CACHE will hang because of ExchangeWorker
is stopped
        assertNotNull(ignite.createCache(new
CacheConfiguration<>("SECOND_CACHE")));
    }

    public interface DummyMBean {
        void noop();
    }
    static class DummyMBeanImpl implements DummyMBean {
        @Override public void noop() {
        }
    }
}
```



--
Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
Reply | Threaded
Open this post in threaded view
|

Re: Creation of dynamic cache

Mmuzaf
Denis,

At the first glance looks like the main problem here is about
rollbacking of cache creation due to exception. Incorrect rollback
leads us to critial problem with ExchangeWorker as expected. It's
normal behavior because since May 2018 we have `FailureHandler`
and we can handle it (`NoOpFailureHandler` used in tests by default).

AFAIK, each dymanic cache creadtion should lead us to minor topology
change. So, expected result here from my point:

 - PME happens, topVer [1, 1], cache NOT created
 - PME happens, topVer [1, 2], cache created


I think we should provide correct fix with handling cache creation.
Please, look at my PR [1] it's very crude and I'm sure it not cover
whole problem. But seems, these changes will fix your reproducer.

Hope, it helps you.

[1] https://github.com/apache/ignite/pull/4487/files


On Fri, 3 Aug 2018 at 14:54 Denis Garus <[hidden email]> wrote:

> Hello, Igniters!
>
>
> If an error occurred during of creation dynamic cache, the
> CacheAffinitySharedManager#processCacheStartRequests method will try to
> rollback cache start routine.
> ```
> try {
>         if (startCache) {
>
> cctx.cache().prepareCacheStart(req.startCacheConfiguration(),
>                         cacheDesc,
>                         nearCfg,
>                         evts.topologyVersion(),
>                         req.disabledAfterStart());
>                 //some code
>         }
> }
> catch (IgniteCheckedException e) {
>         U.error(log, "Failed to initialize cache. Will try to rollback
> cache start
> routine. " +
>                 "[cacheName=" + req.cacheName() + ']', e);
>
>         cctx.cache().closeCaches(Collections.singleton(req.cacheName()),
> false);
>
>         cctx.cache().completeCacheStartFuture(req, false, e);
> }
> ```
> Assume, that GridDhtPartitionsExchangeFuture will finish without any error
> because of the exception is just logged.
> Is this way right? What should return the Ignite#createCache method in this
> case?
>
> I can't check what could return in that case because it just doesn't work
> this way now.
> In the further, we're getting the critical error that stops ExchangeWorker
> in a test environment
> or stops node in a production environment.
>
> Reproducer:
> ```
> package org.apache.ignite.internal.processors.cache;
>
> import org.apache.ignite.IgniteCheckedException;
> import org.apache.ignite.configuration.CacheConfiguration;
> import org.apache.ignite.internal.IgniteEx;
> import org.apache.ignite.internal.util.typedef.internal.U;
> import org.apache.ignite.testframework.GridTestUtils;
> import
> org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
>
> public class CreateCacheFreezeTest extends GridCommonAbstractTest {
>     public void test() throws Exception {
>         IgniteEx ignite = startGrid(0);
>
>         U.registerMBean(ignite.context().config().getMBeanServer(),
> ignite.name(), "FIRST_CACHE",
>
> "org.apache.ignite.internal.processors.cache.CacheLocalMetricsMXBeanImpl",
>             new DummyMBeanImpl(), DummyMBean.class);
>
>         GridTestUtils.assertThrowsWithCause(() -> {
>             ignite.createCache(new CacheConfiguration<>("FIRST_CACHE"));
>
>             return 0;
>         }, IgniteCheckedException.class);
>         //The creation of SECOND_CACHE will hang because of ExchangeWorker
> is stopped
>         assertNotNull(ignite.createCache(new
> CacheConfiguration<>("SECOND_CACHE")));
>     }
>
>     public interface DummyMBean {
>         void noop();
>     }
>     static class DummyMBeanImpl implements DummyMBean {
>         @Override public void noop() {
>         }
>     }
> }
> ```
>
>
>
> --
> Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
>
--
--
Maxim Muzafarov
Reply | Threaded
Open this post in threaded view
|

Re: Creation of dynamic cache

Denis Garus
Thank you! This is what I need.

пн, 6 авг. 2018 г. в 16:09, Maxim Muzafarov <[hidden email]>:

> Denis,
>
> At the first glance looks like the main problem here is about
> rollbacking of cache creation due to exception. Incorrect rollback
> leads us to critial problem with ExchangeWorker as expected. It's
> normal behavior because since May 2018 we have `FailureHandler`
> and we can handle it (`NoOpFailureHandler` used in tests by default).
>
> AFAIK, each dymanic cache creadtion should lead us to minor topology
> change. So, expected result here from my point:
>
>  - PME happens, topVer [1, 1], cache NOT created
>  - PME happens, topVer [1, 2], cache created
>
>
> I think we should provide correct fix with handling cache creation.
> Please, look at my PR [1] it's very crude and I'm sure it not cover
> whole problem. But seems, these changes will fix your reproducer.
>
> Hope, it helps you.
>
> [1] https://github.com/apache/ignite/pull/4487/files
>
>
> On Fri, 3 Aug 2018 at 14:54 Denis Garus <[hidden email]> wrote:
>
> > Hello, Igniters!
> >
> >
> > If an error occurred during of creation dynamic cache, the
> > CacheAffinitySharedManager#processCacheStartRequests method will try to
> > rollback cache start routine.
> > ```
> > try {
> >         if (startCache) {
> >
> > cctx.cache().prepareCacheStart(req.startCacheConfiguration(),
> >                         cacheDesc,
> >                         nearCfg,
> >                         evts.topologyVersion(),
> >                         req.disabledAfterStart());
> >                 //some code
> >         }
> > }
> > catch (IgniteCheckedException e) {
> >         U.error(log, "Failed to initialize cache. Will try to rollback
> > cache start
> > routine. " +
> >                 "[cacheName=" + req.cacheName() + ']', e);
> >
> >         cctx.cache().closeCaches(Collections.singleton(req.cacheName()),
> > false);
> >
> >         cctx.cache().completeCacheStartFuture(req, false, e);
> > }
> > ```
> > Assume, that GridDhtPartitionsExchangeFuture will finish without any
> error
> > because of the exception is just logged.
> > Is this way right? What should return the Ignite#createCache method in
> this
> > case?
> >
> > I can't check what could return in that case because it just doesn't work
> > this way now.
> > In the further, we're getting the critical error that stops
> ExchangeWorker
> > in a test environment
> > or stops node in a production environment.
> >
> > Reproducer:
> > ```
> > package org.apache.ignite.internal.processors.cache;
> >
> > import org.apache.ignite.IgniteCheckedException;
> > import org.apache.ignite.configuration.CacheConfiguration;
> > import org.apache.ignite.internal.IgniteEx;
> > import org.apache.ignite.internal.util.typedef.internal.U;
> > import org.apache.ignite.testframework.GridTestUtils;
> > import
> > org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
> >
> > public class CreateCacheFreezeTest extends GridCommonAbstractTest {
> >     public void test() throws Exception {
> >         IgniteEx ignite = startGrid(0);
> >
> >         U.registerMBean(ignite.context().config().getMBeanServer(),
> > ignite.name(), "FIRST_CACHE",
> >
> >
> "org.apache.ignite.internal.processors.cache.CacheLocalMetricsMXBeanImpl",
> >             new DummyMBeanImpl(), DummyMBean.class);
> >
> >         GridTestUtils.assertThrowsWithCause(() -> {
> >             ignite.createCache(new CacheConfiguration<>("FIRST_CACHE"));
> >
> >             return 0;
> >         }, IgniteCheckedException.class);
> >         //The creation of SECOND_CACHE will hang because of
> ExchangeWorker
> > is stopped
> >         assertNotNull(ignite.createCache(new
> > CacheConfiguration<>("SECOND_CACHE")));
> >     }
> >
> >     public interface DummyMBean {
> >         void noop();
> >     }
> >     static class DummyMBeanImpl implements DummyMBean {
> >         @Override public void noop() {
> >         }
> >     }
> > }
> > ```
> >
> >
> >
> > --
> > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> >
> --
> --
> Maxim Muzafarov
>
Reply | Threaded
Open this post in threaded view
|

Re: Creation of dynamic cache

slava.koptilin
In reply to this post by Denis Garus
Hello Denis,

It seems we can just comment out the following catch block:
```
try {
        if (startCache) {

cctx.cache().prepareCacheStart(req.startCacheConfiguration(),
                        cacheDesc,
                        nearCfg,
                        evts.topologyVersion(),
                        req.disabledAfterStart());
                //some code
        }
}
//catch (IgniteCheckedException e) {
//        U.error(log, "Failed to initialize cache. Will try to rollback
cache start routine. [cacheName=" + req.cacheName() + ']', e);
//        cctx.cache().closeCaches(Collections.singleton(req.cacheName()),
false);
//        cctx.cache().completeCacheStartFuture(req, false, e);
//}
```
I think that this exception should be propery handled by
GridDhtPartitionsExchangeFuture#onCacheChangeRequest(boolean crd) method.
(please take a look at this JIRA ticket:
https://issues.apache.org/jira/browse/IGNITE-1094)

Best regards,
Slava.


пт, 3 авг. 2018 г. в 14:54, Denis Garus <[hidden email]>:

> Hello, Igniters!
>
>
> If an error occurred during of creation dynamic cache, the
> CacheAffinitySharedManager#processCacheStartRequests method will try to
> rollback cache start routine.
> ```
> try {
>         if (startCache) {
>
> cctx.cache().prepareCacheStart(req.startCacheConfiguration(),
>                         cacheDesc,
>                         nearCfg,
>                         evts.topologyVersion(),
>                         req.disabledAfterStart());
>                 //some code
>         }
> }
> catch (IgniteCheckedException e) {
>         U.error(log, "Failed to initialize cache. Will try to rollback
> cache start
> routine. " +
>                 "[cacheName=" + req.cacheName() + ']', e);
>
>         cctx.cache().closeCaches(Collections.singleton(req.cacheName()),
> false);
>
>         cctx.cache().completeCacheStartFuture(req, false, e);
> }
> ```
> Assume, that GridDhtPartitionsExchangeFuture will finish without any error
> because of the exception is just logged.
> Is this way right? What should return the Ignite#createCache method in this
> case?
>
> I can't check what could return in that case because it just doesn't work
> this way now.
> In the further, we're getting the critical error that stops ExchangeWorker
> in a test environment
> or stops node in a production environment.
>
> Reproducer:
> ```
> package org.apache.ignite.internal.processors.cache;
>
> import org.apache.ignite.IgniteCheckedException;
> import org.apache.ignite.configuration.CacheConfiguration;
> import org.apache.ignite.internal.IgniteEx;
> import org.apache.ignite.internal.util.typedef.internal.U;
> import org.apache.ignite.testframework.GridTestUtils;
> import
> org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
>
> public class CreateCacheFreezeTest extends GridCommonAbstractTest {
>     public void test() throws Exception {
>         IgniteEx ignite = startGrid(0);
>
>         U.registerMBean(ignite.context().config().getMBeanServer(),
> ignite.name(), "FIRST_CACHE",
>
> "org.apache.ignite.internal.processors.cache.CacheLocalMetricsMXBeanImpl",
>             new DummyMBeanImpl(), DummyMBean.class);
>
>         GridTestUtils.assertThrowsWithCause(() -> {
>             ignite.createCache(new CacheConfiguration<>("FIRST_CACHE"));
>
>             return 0;
>         }, IgniteCheckedException.class);
>         //The creation of SECOND_CACHE will hang because of ExchangeWorker
> is stopped
>         assertNotNull(ignite.createCache(new
> CacheConfiguration<>("SECOND_CACHE")));
>     }
>
>     public interface DummyMBean {
>         void noop();
>     }
>     static class DummyMBeanImpl implements DummyMBean {
>         @Override public void noop() {
>         }
>     }
> }
> ```
>
>
>
> --
> Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
>
Reply | Threaded
Open this post in threaded view
|

Re: Creation of dynamic cache

Denis Garus
Hello, Slava!

Yes, I agree with you. If we get rid of this catch block, everything works
well.
But, IgniteAbstractDynamicCacheStartFailTest isn't honest enough because it
can't expose this failure.
We should simulate not only RuntimeException but IgniteCheckedException as
well.
I want to add a simulation of IgniteCheckedException in during of cache
creation to IgniteAbstractDynamicCacheStartFailTest
and delete the catch block from
CacheAffinitySharedManager#processCacheStartRequests method.

What are your thoughts?

вт, 7 авг. 2018 г. в 15:31, Вячеслав Коптилин <[hidden email]>:

> Hello Denis,
>
> It seems we can just comment out the following catch block:
> ```
> try {
>         if (startCache) {
>
> cctx.cache().prepareCacheStart(req.startCacheConfiguration(),
>                         cacheDesc,
>                         nearCfg,
>                         evts.topologyVersion(),
>                         req.disabledAfterStart());
>                 //some code
>         }
> }
> //catch (IgniteCheckedException e) {
> //        U.error(log, "Failed to initialize cache. Will try to rollback
> cache start routine. [cacheName=" + req.cacheName() + ']', e);
> //        cctx.cache().closeCaches(Collections.singleton(req.cacheName()),
> false);
> //        cctx.cache().completeCacheStartFuture(req, false, e);
> //}
> ```
> I think that this exception should be propery handled by
> GridDhtPartitionsExchangeFuture#onCacheChangeRequest(boolean crd) method.
> (please take a look at this JIRA ticket:
> https://issues.apache.org/jira/browse/IGNITE-1094)
>
> Best regards,
> Slava.
>
>
> пт, 3 авг. 2018 г. в 14:54, Denis Garus <[hidden email]>:
>
> > Hello, Igniters!
> >
> >
> > If an error occurred during of creation dynamic cache, the
> > CacheAffinitySharedManager#processCacheStartRequests method will try to
> > rollback cache start routine.
> > ```
> > try {
> >         if (startCache) {
> >
> > cctx.cache().prepareCacheStart(req.startCacheConfiguration(),
> >                         cacheDesc,
> >                         nearCfg,
> >                         evts.topologyVersion(),
> >                         req.disabledAfterStart());
> >                 //some code
> >         }
> > }
> > catch (IgniteCheckedException e) {
> >         U.error(log, "Failed to initialize cache. Will try to rollback
> > cache start
> > routine. " +
> >                 "[cacheName=" + req.cacheName() + ']', e);
> >
> >         cctx.cache().closeCaches(Collections.singleton(req.cacheName()),
> > false);
> >
> >         cctx.cache().completeCacheStartFuture(req, false, e);
> > }
> > ```
> > Assume, that GridDhtPartitionsExchangeFuture will finish without any
> error
> > because of the exception is just logged.
> > Is this way right? What should return the Ignite#createCache method in
> this
> > case?
> >
> > I can't check what could return in that case because it just doesn't work
> > this way now.
> > In the further, we're getting the critical error that stops
> ExchangeWorker
> > in a test environment
> > or stops node in a production environment.
> >
> > Reproducer:
> > ```
> > package org.apache.ignite.internal.processors.cache;
> >
> > import org.apache.ignite.IgniteCheckedException;
> > import org.apache.ignite.configuration.CacheConfiguration;
> > import org.apache.ignite.internal.IgniteEx;
> > import org.apache.ignite.internal.util.typedef.internal.U;
> > import org.apache.ignite.testframework.GridTestUtils;
> > import
> > org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
> >
> > public class CreateCacheFreezeTest extends GridCommonAbstractTest {
> >     public void test() throws Exception {
> >         IgniteEx ignite = startGrid(0);
> >
> >         U.registerMBean(ignite.context().config().getMBeanServer(),
> > ignite.name(), "FIRST_CACHE",
> >
> >
> "org.apache.ignite.internal.processors.cache.CacheLocalMetricsMXBeanImpl",
> >             new DummyMBeanImpl(), DummyMBean.class);
> >
> >         GridTestUtils.assertThrowsWithCause(() -> {
> >             ignite.createCache(new CacheConfiguration<>("FIRST_CACHE"));
> >
> >             return 0;
> >         }, IgniteCheckedException.class);
> >         //The creation of SECOND_CACHE will hang because of
> ExchangeWorker
> > is stopped
> >         assertNotNull(ignite.createCache(new
> > CacheConfiguration<>("SECOND_CACHE")));
> >     }
> >
> >     public interface DummyMBean {
> >         void noop();
> >     }
> >     static class DummyMBeanImpl implements DummyMBean {
> >         @Override public void noop() {
> >         }
> >     }
> > }
> > ```
> >
> >
> >
> > --
> > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: Creation of dynamic cache

slava.koptilin
Hi,

> I want to add a simulation of IgniteCheckedException in during of cache
creation to IgniteAbstractDynamicCacheStartFailTest
> and delete the catch block from
CacheAffinitySharedManager#processCacheStartRequests method
Looks like a plan :)

Thanks,
S.

вт, 7 авг. 2018 г. в 16:42, Denis Garus <[hidden email]>:

> Hello, Slava!
>
> Yes, I agree with you. If we get rid of this catch block, everything works
> well.
> But, IgniteAbstractDynamicCacheStartFailTest isn't honest enough because it
> can't expose this failure.
> We should simulate not only RuntimeException but IgniteCheckedException as
> well.
> I want to add a simulation of IgniteCheckedException in during of cache
> creation to IgniteAbstractDynamicCacheStartFailTest
> and delete the catch block from
> CacheAffinitySharedManager#processCacheStartRequests method.
>
> What are your thoughts?
>
> вт, 7 авг. 2018 г. в 15:31, Вячеслав Коптилин <[hidden email]>:
>
> > Hello Denis,
> >
> > It seems we can just comment out the following catch block:
> > ```
> > try {
> >         if (startCache) {
> >
> > cctx.cache().prepareCacheStart(req.startCacheConfiguration(),
> >                         cacheDesc,
> >                         nearCfg,
> >                         evts.topologyVersion(),
> >                         req.disabledAfterStart());
> >                 //some code
> >         }
> > }
> > //catch (IgniteCheckedException e) {
> > //        U.error(log, "Failed to initialize cache. Will try to rollback
> > cache start routine. [cacheName=" + req.cacheName() + ']', e);
> > //
> cctx.cache().closeCaches(Collections.singleton(req.cacheName()),
> > false);
> > //        cctx.cache().completeCacheStartFuture(req, false, e);
> > //}
> > ```
> > I think that this exception should be propery handled by
> > GridDhtPartitionsExchangeFuture#onCacheChangeRequest(boolean crd) method.
> > (please take a look at this JIRA ticket:
> > https://issues.apache.org/jira/browse/IGNITE-1094)
> >
> > Best regards,
> > Slava.
> >
> >
> > пт, 3 авг. 2018 г. в 14:54, Denis Garus <[hidden email]>:
> >
> > > Hello, Igniters!
> > >
> > >
> > > If an error occurred during of creation dynamic cache, the
> > > CacheAffinitySharedManager#processCacheStartRequests method will try to
> > > rollback cache start routine.
> > > ```
> > > try {
> > >         if (startCache) {
> > >
> > > cctx.cache().prepareCacheStart(req.startCacheConfiguration(),
> > >                         cacheDesc,
> > >                         nearCfg,
> > >                         evts.topologyVersion(),
> > >                         req.disabledAfterStart());
> > >                 //some code
> > >         }
> > > }
> > > catch (IgniteCheckedException e) {
> > >         U.error(log, "Failed to initialize cache. Will try to rollback
> > > cache start
> > > routine. " +
> > >                 "[cacheName=" + req.cacheName() + ']', e);
> > >
> > >
>  cctx.cache().closeCaches(Collections.singleton(req.cacheName()),
> > > false);
> > >
> > >         cctx.cache().completeCacheStartFuture(req, false, e);
> > > }
> > > ```
> > > Assume, that GridDhtPartitionsExchangeFuture will finish without any
> > error
> > > because of the exception is just logged.
> > > Is this way right? What should return the Ignite#createCache method in
> > this
> > > case?
> > >
> > > I can't check what could return in that case because it just doesn't
> work
> > > this way now.
> > > In the further, we're getting the critical error that stops
> > ExchangeWorker
> > > in a test environment
> > > or stops node in a production environment.
> > >
> > > Reproducer:
> > > ```
> > > package org.apache.ignite.internal.processors.cache;
> > >
> > > import org.apache.ignite.IgniteCheckedException;
> > > import org.apache.ignite.configuration.CacheConfiguration;
> > > import org.apache.ignite.internal.IgniteEx;
> > > import org.apache.ignite.internal.util.typedef.internal.U;
> > > import org.apache.ignite.testframework.GridTestUtils;
> > > import
> > > org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
> > >
> > > public class CreateCacheFreezeTest extends GridCommonAbstractTest {
> > >     public void test() throws Exception {
> > >         IgniteEx ignite = startGrid(0);
> > >
> > >         U.registerMBean(ignite.context().config().getMBeanServer(),
> > > ignite.name(), "FIRST_CACHE",
> > >
> > >
> >
> "org.apache.ignite.internal.processors.cache.CacheLocalMetricsMXBeanImpl",
> > >             new DummyMBeanImpl(), DummyMBean.class);
> > >
> > >         GridTestUtils.assertThrowsWithCause(() -> {
> > >             ignite.createCache(new
> CacheConfiguration<>("FIRST_CACHE"));
> > >
> > >             return 0;
> > >         }, IgniteCheckedException.class);
> > >         //The creation of SECOND_CACHE will hang because of
> > ExchangeWorker
> > > is stopped
> > >         assertNotNull(ignite.createCache(new
> > > CacheConfiguration<>("SECOND_CACHE")));
> > >     }
> > >
> > >     public interface DummyMBean {
> > >         void noop();
> > >     }
> > >     static class DummyMBeanImpl implements DummyMBean {
> > >         @Override public void noop() {
> > >         }
> > >     }
> > > }
> > > ```
> > >
> > >
> > >
> > > --
> > > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: Creation of dynamic cache

Denis Garus
Hi, Slava!

I noticed that in the
IgniteAbstractDynamicCacheStartFailTest#testCacheStartAfterFailure method
the second started cache uses new configuration:
```
public void testCacheStartAfterFailure() throws Exception {
CacheConfiguration cfg = createCacheConfigsWithBrokenAffinityFun(
false, 1, 0, 1, false).get(0);

GridTestUtils.assertThrows(log, new Callable<Object>() {
@Override public Object call() throws Exception {
grid(0).getOrCreateCache(cfg);
return null;
}
}, CacheException.class, null);

// Correct the cache configuration. Default constructor creates a good
affinity function.
cfg.setAffinity(new BrokenAffinityFunction());

IgniteCache<Integer, Value> cache =
grid(0).getOrCreateCache(createCacheConfiguration(EXISTING_CACHE_NAME));
//<---- new configuration

checkCacheOperations(cache);
}
```
Is it a typo? May be it should be something like:

grid(0).getOrCreateCache(cfg); //<---- original configuration

Can I fix it in my PR?

вт, 7 авг. 2018 г. в 17:37, Вячеслав Коптилин <[hidden email]>:

> Hi,
>
> > I want to add a simulation of IgniteCheckedException in during of cache
> creation to IgniteAbstractDynamicCacheStartFailTest
> > and delete the catch block from
> CacheAffinitySharedManager#processCacheStartRequests method
> Looks like a plan :)
>
> Thanks,
> S.
>
> вт, 7 авг. 2018 г. в 16:42, Denis Garus <[hidden email]>:
>
> > Hello, Slava!
> >
> > Yes, I agree with you. If we get rid of this catch block, everything
> works
> > well.
> > But, IgniteAbstractDynamicCacheStartFailTest isn't honest enough because
> it
> > can't expose this failure.
> > We should simulate not only RuntimeException but IgniteCheckedException
> as
> > well.
> > I want to add a simulation of IgniteCheckedException in during of cache
> > creation to IgniteAbstractDynamicCacheStartFailTest
> > and delete the catch block from
> > CacheAffinitySharedManager#processCacheStartRequests method.
> >
> > What are your thoughts?
> >
> > вт, 7 авг. 2018 г. в 15:31, Вячеслав Коптилин <[hidden email]
> >:
> >
> > > Hello Denis,
> > >
> > > It seems we can just comment out the following catch block:
> > > ```
> > > try {
> > >         if (startCache) {
> > >
> > > cctx.cache().prepareCacheStart(req.startCacheConfiguration(),
> > >                         cacheDesc,
> > >                         nearCfg,
> > >                         evts.topologyVersion(),
> > >                         req.disabledAfterStart());
> > >                 //some code
> > >         }
> > > }
> > > //catch (IgniteCheckedException e) {
> > > //        U.error(log, "Failed to initialize cache. Will try to
> rollback
> > > cache start routine. [cacheName=" + req.cacheName() + ']', e);
> > > //
> > cctx.cache().closeCaches(Collections.singleton(req.cacheName()),
> > > false);
> > > //        cctx.cache().completeCacheStartFuture(req, false, e);
> > > //}
> > > ```
> > > I think that this exception should be propery handled by
> > > GridDhtPartitionsExchangeFuture#onCacheChangeRequest(boolean crd)
> method.
> > > (please take a look at this JIRA ticket:
> > > https://issues.apache.org/jira/browse/IGNITE-1094)
> > >
> > > Best regards,
> > > Slava.
> > >
> > >
> > > пт, 3 авг. 2018 г. в 14:54, Denis Garus <[hidden email]>:
> > >
> > > > Hello, Igniters!
> > > >
> > > >
> > > > If an error occurred during of creation dynamic cache, the
> > > > CacheAffinitySharedManager#processCacheStartRequests method will try
> to
> > > > rollback cache start routine.
> > > > ```
> > > > try {
> > > >         if (startCache) {
> > > >
> > > > cctx.cache().prepareCacheStart(req.startCacheConfiguration(),
> > > >                         cacheDesc,
> > > >                         nearCfg,
> > > >                         evts.topologyVersion(),
> > > >                         req.disabledAfterStart());
> > > >                 //some code
> > > >         }
> > > > }
> > > > catch (IgniteCheckedException e) {
> > > >         U.error(log, "Failed to initialize cache. Will try to
> rollback
> > > > cache start
> > > > routine. " +
> > > >                 "[cacheName=" + req.cacheName() + ']', e);
> > > >
> > > >
> >  cctx.cache().closeCaches(Collections.singleton(req.cacheName()),
> > > > false);
> > > >
> > > >         cctx.cache().completeCacheStartFuture(req, false, e);
> > > > }
> > > > ```
> > > > Assume, that GridDhtPartitionsExchangeFuture will finish without any
> > > error
> > > > because of the exception is just logged.
> > > > Is this way right? What should return the Ignite#createCache method
> in
> > > this
> > > > case?
> > > >
> > > > I can't check what could return in that case because it just doesn't
> > work
> > > > this way now.
> > > > In the further, we're getting the critical error that stops
> > > ExchangeWorker
> > > > in a test environment
> > > > or stops node in a production environment.
> > > >
> > > > Reproducer:
> > > > ```
> > > > package org.apache.ignite.internal.processors.cache;
> > > >
> > > > import org.apache.ignite.IgniteCheckedException;
> > > > import org.apache.ignite.configuration.CacheConfiguration;
> > > > import org.apache.ignite.internal.IgniteEx;
> > > > import org.apache.ignite.internal.util.typedef.internal.U;
> > > > import org.apache.ignite.testframework.GridTestUtils;
> > > > import
> > > > org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
> > > >
> > > > public class CreateCacheFreezeTest extends GridCommonAbstractTest {
> > > >     public void test() throws Exception {
> > > >         IgniteEx ignite = startGrid(0);
> > > >
> > > >         U.registerMBean(ignite.context().config().getMBeanServer(),
> > > > ignite.name(), "FIRST_CACHE",
> > > >
> > > >
> > >
> >
> "org.apache.ignite.internal.processors.cache.CacheLocalMetricsMXBeanImpl",
> > > >             new DummyMBeanImpl(), DummyMBean.class);
> > > >
> > > >         GridTestUtils.assertThrowsWithCause(() -> {
> > > >             ignite.createCache(new
> > CacheConfiguration<>("FIRST_CACHE"));
> > > >
> > > >             return 0;
> > > >         }, IgniteCheckedException.class);
> > > >         //The creation of SECOND_CACHE will hang because of
> > > ExchangeWorker
> > > > is stopped
> > > >         assertNotNull(ignite.createCache(new
> > > > CacheConfiguration<>("SECOND_CACHE")));
> > > >     }
> > > >
> > > >     public interface DummyMBean {
> > > >         void noop();
> > > >     }
> > > >     static class DummyMBeanImpl implements DummyMBean {
> > > >         @Override public void noop() {
> > > >         }
> > > >     }
> > > > }
> > > > ```
> > > >
> > > >
> > > >
> > > > --
> > > > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: Creation of dynamic cache

slava.koptilin
Hello Denis,

> Is it a typo? May be it should be something like:
> grid(0).getOrCreateCache(cfg); //<---- original configuration
Yep, it looks like a typo.

> Can I fix it in my PR?
It would be great :)

Best regards,
Slava.




ср, 8 авг. 2018 г. в 11:09, Denis Garus <[hidden email]>:

> Hi, Slava!
>
> I noticed that in the
> IgniteAbstractDynamicCacheStartFailTest#testCacheStartAfterFailure method
> the second started cache uses new configuration:
> ```
> public void testCacheStartAfterFailure() throws Exception {
> CacheConfiguration cfg = createCacheConfigsWithBrokenAffinityFun(
> false, 1, 0, 1, false).get(0);
>
> GridTestUtils.assertThrows(log, new Callable<Object>() {
> @Override public Object call() throws Exception {
> grid(0).getOrCreateCache(cfg);
> return null;
> }
> }, CacheException.class, null);
>
> // Correct the cache configuration. Default constructor creates a good
> affinity function.
> cfg.setAffinity(new BrokenAffinityFunction());
>
> IgniteCache<Integer, Value> cache =
> grid(0).getOrCreateCache(createCacheConfiguration(EXISTING_CACHE_NAME));
> //<---- new configuration
>
> checkCacheOperations(cache);
> }
> ```
> Is it a typo? May be it should be something like:
>
> grid(0).getOrCreateCache(cfg); //<---- original configuration
>
> Can I fix it in my PR?
>
> вт, 7 авг. 2018 г. в 17:37, Вячеслав Коптилин <[hidden email]>:
>
> > Hi,
> >
> > > I want to add a simulation of IgniteCheckedException in during of cache
> > creation to IgniteAbstractDynamicCacheStartFailTest
> > > and delete the catch block from
> > CacheAffinitySharedManager#processCacheStartRequests method
> > Looks like a plan :)
> >
> > Thanks,
> > S.
> >
> > вт, 7 авг. 2018 г. в 16:42, Denis Garus <[hidden email]>:
> >
> > > Hello, Slava!
> > >
> > > Yes, I agree with you. If we get rid of this catch block, everything
> > works
> > > well.
> > > But, IgniteAbstractDynamicCacheStartFailTest isn't honest enough
> because
> > it
> > > can't expose this failure.
> > > We should simulate not only RuntimeException but IgniteCheckedException
> > as
> > > well.
> > > I want to add a simulation of IgniteCheckedException in during of cache
> > > creation to IgniteAbstractDynamicCacheStartFailTest
> > > and delete the catch block from
> > > CacheAffinitySharedManager#processCacheStartRequests method.
> > >
> > > What are your thoughts?
> > >
> > > вт, 7 авг. 2018 г. в 15:31, Вячеслав Коптилин <
> [hidden email]
> > >:
> > >
> > > > Hello Denis,
> > > >
> > > > It seems we can just comment out the following catch block:
> > > > ```
> > > > try {
> > > >         if (startCache) {
> > > >
> > > > cctx.cache().prepareCacheStart(req.startCacheConfiguration(),
> > > >                         cacheDesc,
> > > >                         nearCfg,
> > > >                         evts.topologyVersion(),
> > > >                         req.disabledAfterStart());
> > > >                 //some code
> > > >         }
> > > > }
> > > > //catch (IgniteCheckedException e) {
> > > > //        U.error(log, "Failed to initialize cache. Will try to
> > rollback
> > > > cache start routine. [cacheName=" + req.cacheName() + ']', e);
> > > > //
> > > cctx.cache().closeCaches(Collections.singleton(req.cacheName()),
> > > > false);
> > > > //        cctx.cache().completeCacheStartFuture(req, false, e);
> > > > //}
> > > > ```
> > > > I think that this exception should be propery handled by
> > > > GridDhtPartitionsExchangeFuture#onCacheChangeRequest(boolean crd)
> > method.
> > > > (please take a look at this JIRA ticket:
> > > > https://issues.apache.org/jira/browse/IGNITE-1094)
> > > >
> > > > Best regards,
> > > > Slava.
> > > >
> > > >
> > > > пт, 3 авг. 2018 г. в 14:54, Denis Garus <[hidden email]>:
> > > >
> > > > > Hello, Igniters!
> > > > >
> > > > >
> > > > > If an error occurred during of creation dynamic cache, the
> > > > > CacheAffinitySharedManager#processCacheStartRequests method will
> try
> > to
> > > > > rollback cache start routine.
> > > > > ```
> > > > > try {
> > > > >         if (startCache) {
> > > > >
> > > > > cctx.cache().prepareCacheStart(req.startCacheConfiguration(),
> > > > >                         cacheDesc,
> > > > >                         nearCfg,
> > > > >                         evts.topologyVersion(),
> > > > >                         req.disabledAfterStart());
> > > > >                 //some code
> > > > >         }
> > > > > }
> > > > > catch (IgniteCheckedException e) {
> > > > >         U.error(log, "Failed to initialize cache. Will try to
> > rollback
> > > > > cache start
> > > > > routine. " +
> > > > >                 "[cacheName=" + req.cacheName() + ']', e);
> > > > >
> > > > >
> > >  cctx.cache().closeCaches(Collections.singleton(req.cacheName()),
> > > > > false);
> > > > >
> > > > >         cctx.cache().completeCacheStartFuture(req, false, e);
> > > > > }
> > > > > ```
> > > > > Assume, that GridDhtPartitionsExchangeFuture will finish without
> any
> > > > error
> > > > > because of the exception is just logged.
> > > > > Is this way right? What should return the Ignite#createCache method
> > in
> > > > this
> > > > > case?
> > > > >
> > > > > I can't check what could return in that case because it just
> doesn't
> > > work
> > > > > this way now.
> > > > > In the further, we're getting the critical error that stops
> > > > ExchangeWorker
> > > > > in a test environment
> > > > > or stops node in a production environment.
> > > > >
> > > > > Reproducer:
> > > > > ```
> > > > > package org.apache.ignite.internal.processors.cache;
> > > > >
> > > > > import org.apache.ignite.IgniteCheckedException;
> > > > > import org.apache.ignite.configuration.CacheConfiguration;
> > > > > import org.apache.ignite.internal.IgniteEx;
> > > > > import org.apache.ignite.internal.util.typedef.internal.U;
> > > > > import org.apache.ignite.testframework.GridTestUtils;
> > > > > import
> > > > >
> org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
> > > > >
> > > > > public class CreateCacheFreezeTest extends GridCommonAbstractTest {
> > > > >     public void test() throws Exception {
> > > > >         IgniteEx ignite = startGrid(0);
> > > > >
> > > > >         U.registerMBean(ignite.context().config().getMBeanServer(),
> > > > > ignite.name(), "FIRST_CACHE",
> > > > >
> > > > >
> > > >
> > >
> >
> "org.apache.ignite.internal.processors.cache.CacheLocalMetricsMXBeanImpl",
> > > > >             new DummyMBeanImpl(), DummyMBean.class);
> > > > >
> > > > >         GridTestUtils.assertThrowsWithCause(() -> {
> > > > >             ignite.createCache(new
> > > CacheConfiguration<>("FIRST_CACHE"));
> > > > >
> > > > >             return 0;
> > > > >         }, IgniteCheckedException.class);
> > > > >         //The creation of SECOND_CACHE will hang because of
> > > > ExchangeWorker
> > > > > is stopped
> > > > >         assertNotNull(ignite.createCache(new
> > > > > CacheConfiguration<>("SECOND_CACHE")));
> > > > >     }
> > > > >
> > > > >     public interface DummyMBean {
> > > > >         void noop();
> > > > >     }
> > > > >     static class DummyMBeanImpl implements DummyMBean {
> > > > >         @Override public void noop() {
> > > > >         }
> > > > >     }
> > > > > }
> > > > > ```
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> > > > >
> > > >
> > >
> >
>