Hi, I had an impression that current behavior is required by jcache, but now I can not find anything about this neither in spec nor in jcache tck tests. So I think we can change current behavior.
Thanks On Tue, Aug 29, 2017 at 9:48 PM, Nikolay Izhikov <nizhikov....@gmail.com> wrote: > Yakov. > > I found following description in jcache javadoc [1] - > > === > Returns: > true if the evaluation passes, otherwise false. > *The effect of returning true is that listener will be invoked* > === > > JSR doesn't specify how filter exception has to be handled. > As far as I can understand *only* way to pass filter is return true from > `evaluate`. > > I think we has to change current behavior. > > Should I file a ticket? > > [1] https://static.javadoc.io/javax.cache/cache-api/1.0.0/javax/ > cache/event/CacheEntryEventFilter.html#evaluate(javax.cache. > event.CacheEntryEvent) > > > 29.08.2017 17:09, Yakov Zhdanov пишет: > > Igniters, >> >> Does anyone else see potential issues on user side with current approach? >> >> Sam, is this JCache requirement? >> >> --Yakov >> >> 2017-08-29 15:16 GMT+03:00 Nikolay Izhikov <nizhikov....@gmail.com>: >> >> Yakov. >>> >>> I think exception equals `true` is intended behavior. >>> >>> Filter evaluation implementation from master - [1] >>> Test from master to check filter exception(without explicit asserts >>> checking listeners call) - [2] >>> >>> Here is my quick test with asserts on listener call after filter >>> exception: >>> >>> ``` >>> package org.apache.ignite.internal.processors.cache.query.continuous; >>> >>> //... imports >>> >>> public class GridCacheContinuousQueryFilterExceptionTest extends >>> GridCacheContinuousQueryAbstractSelfTest implements Serializable { >>> /** >>> * @throws Exception If failed. >>> */ >>> public void testListenerAfterFilterException() throws Exception { >>> IgniteCache<Integer, Integer> cache = >>> grid(0).cache(DEFAULT_CACHE_NAME); >>> >>> ContinuousQuery<Integer, Integer> qry = new ContinuousQuery<>(); >>> >>> final CountDownLatch latch = new CountDownLatch(100); >>> >>> qry.setLocalListener(new CacheEntryUpdatedListener<Integer, >>> Integer>() { >>> @Override public void onUpdated(Iterable<CacheEntryEvent<? >>> extends Integer, ? extends Integer>> evts) { >>> for (CacheEntryEvent<? extends Integer, ? extends >>> Integer> >>> evt : evts) >>> latch.countDown(); >>> } >>> }); >>> >>> qry.setRemoteFilter(new CacheEntryEventSerializableFil >>> ter<Integer, >>> Integer>() { >>> @Override public boolean evaluate(CacheEntryEvent<? extends >>> Integer, ? extends Integer> evt) { >>> throw new RuntimeException("Test error."); >>> } >>> }); >>> >>> try (QueryCursor<Cache.Entry<Integer, Integer>> ignored = >>> cache.query(qry)) { >>> for (int i = 0; i < 100; i++) >>> cache.put(i, i); >>> >>> assertTrue(latch.await(10, SECONDS)); >>> } >>> } >>> >>> @Override protected CacheMode cacheMode() { >>> return CacheMode.REPLICATED; >>> } >>> >>> @Override protected int gridCount() { >>> return 1; >>> } >>> } >>> ``` >>> >>> [1] https://github.com/apache/ignite/blob/master/modules/core/ >>> src/main/java/org/apache/ignite/internal/processors/cache/ >>> query/continuous/CacheContinuousQueryHandler.java#L791 >>> >>> [2] https://github.com/apache/ignite/blob/master/modules/core/ >>> src/test/java/org/apache/ignite/internal/processors/cache/ >>> query/continuous/GridCacheContinuousQueryAbstractSelfTest.java#L359 >>> >>> >>> 29.08.2017 14:46, Yakov Zhdanov пишет: >>> >>> If filter throws exception entry would be passed to listener. >>> >>>> >>>>> >>>> this is strange. Imagine a filter that very rarely throws some runtime >>>> exception due to external or environmental reasons, but in case of >>>> normal >>>> execution filter evaluates to false. In case of error entry is passed >>>> to a >>>> local listener which can lead to some serious consequences and >>>> inconsistencies in business logic. We probably need to send entry with a >>>> notion that there was an error on server. >>>> >>>> --Yakov >>>> >>>> >>>> >>
