Entry listener with predicate should be notified on value entry/exit from predicate value space #8340

vbekiaris · 2016-06-07T15:50:04Z

This PR introduces filtering strategies in map event publisher, to facilitate building a 'query cache' style listener with predicate on IMap . Default filtering strategy maintains Hazelcast entry events behavior. A new filtering strategy has been introduced which processes UPDATED entry events and, depending on listener's predicate, may alter the type of event, to match exit/entry of value from predicate value space.

An EE counterpart is required.

devOpsHazelcast · 2016-06-07T16:23:21Z

Test PASSed.

devOpsHazelcast · 2016-06-08T07:29:52Z

Test FAILed.

devOpsHazelcast · 2016-06-08T12:30:49Z

Test FAILed.

vbekiaris · 2016-06-08T13:41:34Z

IssuesTest::testIssue321_3 fails due to expectation that an entry event which include values will be published before the same event is published to another listener who excludes values. Since there are no event delivery order guarantees, changing the test to verify one event with value and another event without value are received by the listener, in any order.

devOpsHazelcast · 2016-06-08T14:16:37Z

Test PASSed.

ahmetmircik · 2016-06-10T09:30:20Z

hazelcast/src/main/java/com/hazelcast/map/impl/event/MapEventPublisherImpl.java

+                                     Object mergingValue) {
+
+        Map<Integer, EntryEventData> eventDataIncludingValues = new Int2ObjectHashMap<EntryEventData>(EVENT_DATA_MAP_CAPACITY);
+        Map<Integer, EntryEventData> eventDataExcludingValues = new Int2ObjectHashMap<EntryEventData>(EVENT_DATA_MAP_CAPACITY);


We can defer Int2ObjectHashMap creation at a later point, and can do it when needed, now for ex., if i have value-included event registrations and don't have any value-excluded ones, there will be always two Int2ObjectHashMap instance creation although one is sufficient. This event publishing part is on hot-path and it is better to create possible minimum number of objects.

Good point, deferred map creation until actually needed.

devOpsHazelcast · 2016-06-14T18:02:48Z

Test FAILed.

devOpsHazelcast · 2016-06-14T18:41:06Z

Test PASSed.

ahmetmircik · 2016-06-21T11:11:17Z

hazelcast/src/main/java/com/hazelcast/map/impl/event/MapEventPublisherImpl.java

+                                     Object mergingValue) {
+
+        Map<Integer, EntryEventData> eventDataIncludingValues = null;
+        Map<Integer, EntryEventData> eventDataExcludingValues = null;


isn't an ArrayList cheaper than Map to hold event data in terms of number of created objects and calculations needed for hash? Seems an ArrayList is a better fit here and no need a Map

The need here is to maintain a mapping from event type of event-to-be-published to the EntryEventData object (which can be reused across events published to listeners). At the moment, given the filtering strategies implemented in this PR, in the worst case for an UPDATED event we might create 2 more entries for event types REMOVED and ADDED (when the QueryCacheNaturalFilteringStrategy is used and appropriately configured listeners with predicates are registered).
On the other hand, with the default filtering strategy, there is no need to use a map at all, we just need to maintain two possible forms of the EntryEventData (with & without values), so in this case the Maps are indeed overkill.
Caching EntryEventData is not a direct concern of the chosen filtering strategy, however the caching implementation can be optimized depending on the chosen filtering strategy, so I will try something like introducing a new interface EntryEventDataCache, with an instance obtainable from the FilteringStrategy itself:

interface EntryEventDataCache { EntryEventData getOrCreateEventData(mapName, caller, dataKey, value, oldValue, mergingValue, eventTypeForPublishing); } interface FilteringStrategy { .... EntryEventDataCache getEntryEventDataCache(); }

devOpsHazelcast · 2016-06-22T10:19:30Z

Test FAILed.

devOpsHazelcast · 2016-06-22T10:34:22Z

Test FAILed.

devOpsHazelcast · 2016-06-22T11:43:47Z

Test PASSed.

devOpsHazelcast · 2016-06-22T11:47:45Z

Test PASSed.

ahmetmircik · 2016-06-22T12:41:32Z

Maybe you want to remove some unused methods in MapEventPublisherImpl, stumbled them when i get your branch my local,

other than that LGTM 👍

vbekiaris · 2016-06-22T12:55:09Z

Good catch, thanks for pointing that out.

devOpsHazelcast · 2016-06-22T13:07:44Z

Test FAILed.

vbekiaris · 2016-06-22T14:36:50Z

The MapEventPublisherImplTest committed previously erroneously tests that EntryEvent objects received by listeners are the same, however this is not the case: even though a single EntryEventData object is used to dispatch the event to listeners, ultimately a new DataAwareEntryEvent object is delivered to each listener. Working on replacing this test with another one that actually tests the operation of each EntryEventDataCache implementation.

devOpsHazelcast · 2016-06-23T09:25:09Z

Test PASSed.

vbekiaris · 2016-06-28T12:44:21Z

run-lab-run

devOpsHazelcast · 2016-06-28T13:20:32Z

Test PASSed.

tombujok · 2016-06-29T09:01:22Z

hazelcast/src/main/java/com/hazelcast/map/impl/event/DefaultEntryEventFilteringStrategy.java

+    @Override
+    public int doFilter(EventFilter filter, Data dataKey, Object dataOldValue, Object dataValue, EntryEventType eventType,
+                        String mapNameOrNull) {
+            if (filter instanceof MapPartitionLostEventFilter) {


Why can the mapName be null?

tombujok · 2016-06-29T09:37:25Z

👍 good job (some minor comments)

vbekiaris · 2016-06-29T12:31:07Z

Thanks @ahmetmircik & @tombujok, I pushed updated javadoc & fixed the argument name that was leftover as mapNameOrNull. When green, will squash & commit.

devOpsHazelcast · 2016-06-29T13:04:50Z

Test PASSed.

tombujok · 2016-06-29T14:31:33Z

hazelcast/src/main/java/com/hazelcast/map/impl/event/AbstractFilteringStrategy.java

+    private Extractors getExtractorsForMapName(String mapName) {
+        if (mapName == null) {
+            return Extractors.empty();
+        }


Aha, one more thing. If the mapName can't be null (since you changed it in the last commit) is this null check here necessary?

This null check was introduced in faa6428 but I don't think there are any chances mapName could be null as all code paths arrive at this point via one of publishEvent variants and the callers are various map operations, which should have a non-null map name.

devOpsHazelcast · 2016-07-04T23:43:21Z

Test FAILed.

…ring strategy maintains Hazelcast entry events behavior. New filtering strategy introduced which processes UPDATED entry events and, depending on listener's predicate, may alter the type event, to match exit/entry of value from predicate value space.

…ng strategy

vbekiaris · 2016-07-06T07:29:24Z

Unrelated test failure com.hazelcast.durableexecutor.DurableExecutorServiceTest.testStatsIssue2039:

java.util.concurrent.ExecutionException: java.util.concurrent.RejectedExecutionException: Capacity[1] is reached!!! 
    at com.hazelcast.durableexecutor.impl.TaskRingBuffer.findEmptySpot(TaskRingBuffer.java:70)
    at com.hazelcast.durableexecutor.impl.TaskRingBuffer.add(TaskRingBuffer.java:60)
    at com.hazelcast.durableexecutor.impl.DurableExecutorContainer.execute(DurableExecutorContainer.java:59)
    at com.hazelcast.durableexecutor.impl.operations.TaskOperation.run(TaskOperation.java:47)
    at com.hazelcast.spi.impl.operationservice.impl.OperationRunnerImpl.run(OperationRunnerImpl.java:182)
    at com.hazelcast.spi.impl.operationexecutor.impl.OperationThread.process(OperationThread.java:122)
    at com.hazelcast.spi.impl.operationexecutor.impl.OperationThread.run(OperationThread.java:102)
    at ------ submitted from ------.(Unknown Source)
    at com.hazelcast.spi.impl.operationservice.impl.InvocationFuture.resolve(InvocationFuture.java:111)
    at com.hazelcast.spi.impl.operationservice.impl.InvocationFuture.resolveAndThrow(InvocationFuture.java:74)
    at com.hazelcast.spi.impl.AbstractInvocationFuture.get(AbstractInvocationFuture.java:158)
    at com.hazelcast.durableexecutor.impl.DurableExecutorServiceProxy.submitToPartition(DurableExecutorServiceProxy.java:243)
    at com.hazelcast.durableexecutor.impl.DurableExecutorServiceProxy.submit(DurableExecutorServiceProxy.java:131)
    at com.hazelcast.durableexecutor.DurableExecutorServiceTest.testStatsIssue2039(DurableExecutorServiceTest.java:452)

vbekiaris · 2016-07-06T07:29:30Z

run-lab-run

devOpsHazelcast · 2016-07-06T08:05:17Z

Test PASSed.

devOpsHazelcast · 2016-07-06T11:08:02Z

Test PASSed.

devOpsHazelcast · 2016-07-07T10:16:21Z

Test PASSed.

vbekiaris added the Team: Core label Jun 7, 2016

vbekiaris added this to the 3.7 milestone Jun 7, 2016

vbekiaris self-assigned this Jun 7, 2016

vbekiaris force-pushed the master-event-predicate branch from 04430c5 to 700a17c Compare June 8, 2016 07:19

vbekiaris force-pushed the master-event-predicate branch from 700a17c to 188c68d Compare June 8, 2016 12:21

vbekiaris changed the title ~~[DONT MERGE] Entry listener with predicate should be notified on value entry/exit from predicate value space~~ Entry listener with predicate should be notified on value entry/exit from predicate value space Jun 8, 2016

vbekiaris force-pushed the master-event-predicate branch from 188c68d to 605076a Compare June 8, 2016 13:42

ahmetmircik reviewed Jun 10, 2016
View reviewed changes

vbekiaris force-pushed the master-event-predicate branch from 605076a to 5cae199 Compare June 14, 2016 17:47

vbekiaris force-pushed the master-event-predicate branch from 5cae199 to ddd6f6a Compare June 14, 2016 18:07

ahmetmircik reviewed Jun 21, 2016
View reviewed changes

vbekiaris force-pushed the master-event-predicate branch from e7e2188 to d58b51b Compare June 22, 2016 10:21

vbekiaris force-pushed the master-event-predicate branch 2 times, most recently from d58c92b to 49c8653 Compare June 22, 2016 11:11

vbekiaris added the Type: Defect label Jun 22, 2016

vbekiaris force-pushed the master-event-predicate branch from 49c8653 to ae93547 Compare June 22, 2016 12:54

vbekiaris force-pushed the master-event-predicate branch from ae93547 to 42809d9 Compare June 23, 2016 08:49

tombujok reviewed Jun 29, 2016
View reviewed changes

vbekiaris added 2 commits July 6, 2016 10:26

EntryEventData caching implementation is now provided by each filteri…

c4eb15e

…ng strategy

vbekiaris force-pushed the master-event-predicate branch from a9b7d9a to 08f0f91 Compare July 6, 2016 10:31

Updated javadoc & argument names

2fa7d9a

vbekiaris force-pushed the master-event-predicate branch from 08f0f91 to 2fa7d9a Compare July 7, 2016 09:40

vbekiaris merged commit c57d334 into hazelcast:master Jul 7, 2016

vbekiaris deleted the master-event-predicate branch July 7, 2016 12:12

mmedenjak mentioned this pull request Feb 8, 2018

Map EntryListener doesn't fire events when entries change #1118

Open

mmedenjak added the Source: Internal PR or issue was opened by an employee label Apr 13, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Entry listener with predicate should be notified on value entry/exit from predicate value space #8340

Entry listener with predicate should be notified on value entry/exit from predicate value space #8340

vbekiaris commented Jun 7, 2016

devOpsHazelcast commented Jun 7, 2016

devOpsHazelcast commented Jun 8, 2016

devOpsHazelcast commented Jun 8, 2016

vbekiaris commented Jun 8, 2016

devOpsHazelcast commented Jun 8, 2016

ahmetmircik Jun 10, 2016

vbekiaris Jun 14, 2016

devOpsHazelcast commented Jun 14, 2016

devOpsHazelcast commented Jun 14, 2016

ahmetmircik Jun 21, 2016

vbekiaris Jun 21, 2016

devOpsHazelcast commented Jun 22, 2016

devOpsHazelcast commented Jun 22, 2016

devOpsHazelcast commented Jun 22, 2016

devOpsHazelcast commented Jun 22, 2016

ahmetmircik commented Jun 22, 2016

vbekiaris commented Jun 22, 2016

devOpsHazelcast commented Jun 22, 2016

vbekiaris commented Jun 22, 2016

devOpsHazelcast commented Jun 23, 2016

vbekiaris commented Jun 28, 2016

devOpsHazelcast commented Jun 28, 2016

tombujok Jun 29, 2016

tombujok commented Jun 29, 2016

vbekiaris commented Jun 29, 2016

devOpsHazelcast commented Jun 29, 2016

tombujok Jun 29, 2016 •

edited

vbekiaris Jul 4, 2016

devOpsHazelcast commented Jul 4, 2016

vbekiaris commented Jul 6, 2016

vbekiaris commented Jul 6, 2016

devOpsHazelcast commented Jul 6, 2016

devOpsHazelcast commented Jul 6, 2016

devOpsHazelcast commented Jul 7, 2016

Entry listener with predicate should be notified on value entry/exit from predicate value space #8340

Entry listener with predicate should be notified on value entry/exit from predicate value space #8340

Conversation

vbekiaris commented Jun 7, 2016

devOpsHazelcast commented Jun 7, 2016

devOpsHazelcast commented Jun 8, 2016

devOpsHazelcast commented Jun 8, 2016

vbekiaris commented Jun 8, 2016

devOpsHazelcast commented Jun 8, 2016

ahmetmircik Jun 10, 2016

Choose a reason for hiding this comment

vbekiaris Jun 14, 2016

Choose a reason for hiding this comment

devOpsHazelcast commented Jun 14, 2016

devOpsHazelcast commented Jun 14, 2016

ahmetmircik Jun 21, 2016

Choose a reason for hiding this comment

vbekiaris Jun 21, 2016

Choose a reason for hiding this comment

devOpsHazelcast commented Jun 22, 2016

devOpsHazelcast commented Jun 22, 2016

devOpsHazelcast commented Jun 22, 2016

devOpsHazelcast commented Jun 22, 2016

ahmetmircik commented Jun 22, 2016

vbekiaris commented Jun 22, 2016

devOpsHazelcast commented Jun 22, 2016

vbekiaris commented Jun 22, 2016

devOpsHazelcast commented Jun 23, 2016

vbekiaris commented Jun 28, 2016

devOpsHazelcast commented Jun 28, 2016

tombujok Jun 29, 2016

Choose a reason for hiding this comment

tombujok commented Jun 29, 2016

vbekiaris commented Jun 29, 2016

devOpsHazelcast commented Jun 29, 2016

tombujok Jun 29, 2016 • edited

Choose a reason for hiding this comment

vbekiaris Jul 4, 2016

Choose a reason for hiding this comment

devOpsHazelcast commented Jul 4, 2016

vbekiaris commented Jul 6, 2016

vbekiaris commented Jul 6, 2016

devOpsHazelcast commented Jul 6, 2016

devOpsHazelcast commented Jul 6, 2016

devOpsHazelcast commented Jul 7, 2016

tombujok Jun 29, 2016 •

edited