Multimap entry listener called twice

[tshipilov]

Test to reproduce the problem

    @Test
    public void testError2() throws InterruptedException {
        String mapName = "map";
        long key = 1L;
        String value = "value";

        HazelcastInstance instance1 = Hazelcast.newHazelcastInstance();
        HazelcastInstance instance2 = Hazelcast.newHazelcastInstance();

        CountingEntryListener<Object, Object> listener = new CountingEntryListener<Object, Object>();

        MultiMap<Object, Object> map = instance1.getMultiMap(mapName);
        map.addEntryListener(listener, true);

        TransactionContext ctx1 = instance2.newTransactionContext();
        ctx1.beginTransaction();
        ctx1.getMultiMap(mapName).put(key, value);
        ctx1.commitTransaction();


        TransactionContext ctx2 = instance2.newTransactionContext();
        ctx2.beginTransaction();
        ctx2.getMultiMap(mapName).remove(key, value);
        ctx2.commitTransaction();

        Thread.sleep(100);

        assertEquals(1, listener.addedCount);
        assertEquals(1, listener.removedCount);
    }

    private class CountingEntryListener<K,V> extends EntryAdapter<K,V> {
        int addedCount = 0;
        int removedCount = 0;

        @Override
        public void entryAdded(EntryEvent<K, V> event) {
            addedCount++;
        }

        @Override
        public void entryRemoved(EntryEvent<K, V> event) {
            removedCount++;
        }
    }

[tshipilov]

java.lang.AssertionError:
Expected :1
Actual :2

[mdogan]

@gurbuzali can you please look at this?

[gurbuzali]

a fix on the way, the one @tshipilov mentioned

[tshipilov]

I'm sorry. I already deleted my comments about fix for 993, because it breaks multimap functionality since all Txn_Operation classes are not implementing BackupAwareOperation. For instance, after this fix other nodes will not be updated on add/remove. You should make Txn_Operation implement BackupAwareOperation.
Please revert your changes in TxnCommitOperation.

[gurbuzali]

what makes you think other nodes will not be updated on add/remove ?
here is the test and it gets updated at each add/remove:

@Test
public void testListener() throws InterruptedException {
    String mapName = "mm";
    long key = 1L;
    String value = "value";
    String value2 = "value2";
    final Config config = new Config();
    final TestHazelcastInstanceFactory factory = createHazelcastInstanceFactory(2);
    HazelcastInstance instance1 = factory.newHazelcastInstance(config);
    HazelcastInstance instance2 = factory.newHazelcastInstance(config);

    CountingEntryListener<Object, Object> listener = new CountingEntryListener<Object, Object>();

    MultiMap<Object, Object> map = instance1.getMultiMap(mapName);
    map.addEntryListener(listener, true);

    TransactionContext ctx1 = instance2.newTransactionContext();
    ctx1.beginTransaction();
    ctx1.getMultiMap(mapName).put(key, value);
    ctx1.commitTransaction();

    TransactionContext ctx2 = instance2.newTransactionContext();
    ctx2.beginTransaction();
    ctx2.getMultiMap(mapName).remove(key, value);
    ctx2.commitTransaction();

    TransactionContext ctx3 = instance2.newTransactionContext();
    ctx3.beginTransaction();
    ctx3.getMultiMap(mapName).put(key, value2);
    ctx3.commitTransaction();

    TransactionContext ctx4 = instance1.newTransactionContext();
    ctx4.beginTransaction();
    ctx4.getMultiMap(mapName).remove(key, value2);
    ctx4.commitTransaction();

    Thread.sleep(100);

    assertEquals(2, listener.addedCount);
    assertEquals(2, listener.removedCount);
}

[tshipilov]

My other tests break after this change. Just debug it. In this cycle backupOpList become empty because all Txn*Operation are not instance of BackupAwareOperation:

       List<Operation> backupOpList = new ArrayList<Operation>();
        for (Operation operation : opList) {
            if (operation instanceof BackupAwareOperation){
                backupOpList.add(operation);
            }
        }

[tshipilov]

As a quick and dirty workaround I commented afterRun() call in com.hazelcast.multimap.txn.TxnCommitBackupOperation#run().

    public void run() throws Exception {
        for (Operation op: opList){
            op.setNodeEngine(getNodeEngine()).setServiceName(getServiceName()).setPartitionId(getPartitionId());
            op.beforeRun();
            op.run();
            //op.afterRun();
        }
        getOrCreateContainer().unlock(dataKey, caller, threadId);
    }

It works, but I think it is better to make Txn*Operation implement BackupAwareOperation.

[gurbuzali]

Yes, you are correct, better to implement BackupAwareOperation.
Here is the fix: https://github.com/hazelcast/hazelcast/pull/994/files

[tshipilov]

Thank you for quick resolution! Could you tell me when it is going to be available in official build?