Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IMap.tryLockAndGet: concurrency error when map has MapStore #268

Closed
linus-eivind opened this issue Sep 10, 2012 · 2 comments

Comments

Projects
None yet
2 participants
@linus-eivind
Copy link

commented Sep 10, 2012

Environment:
hazelcast v2.3.1 (hazelcast-all-2.3.1.jar)
groboutils v5 (groboutils-5-core.jar)

When multiple threads access an IMap (through a single client), the map behaves erroneously when configured with a map-store.

Without the map-store, tryLockAndGet are synchronized so that only one thread is given the lock on a given key at any time. But with a map-store enabled, tryLockAndGet gives the lock on a given key to multiple threads simultaneously.

Earlier documentations (http://www.hazelcast.com/docs/1.9.4/manual/multi_html/ch15.html#InternalsThreads) stated that there is no need to synchronize access to maps, queues, etc. But this paragraph is taken out in the later documentation versions. Is the 1.9.4 documentation on synchronized access still valid?

I'll attach a test-case that shows the map-store problem.

@linus-eivind

This comment has been minimized.

Copy link
Author

commented Sep 10, 2012

OK, found no way to attach a file, so I'll paste it in here. Dependencies are:

hazelcast-all-2.3.1.jar
groboutils-5-core.jar
junit-4.9.jar

package com.hazelcast.test;

import static org.junit.Assert.fail;

import java.io.Serializable;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;

import net.sourceforge.groboutils.junit.v1.MultiThreadedTestRunner;
import net.sourceforge.groboutils.junit.v1.TestRunnable;

import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;

import com.hazelcast.client.ClientConfig;
import com.hazelcast.client.HazelcastClient;
import com.hazelcast.config.GroupConfig;
import com.hazelcast.config.InMemoryXmlConfig;
import com.hazelcast.core.Hazelcast;
import com.hazelcast.core.HazelcastInstance;
import com.hazelcast.core.IMap;
import com.hazelcast.core.MapLoader;

public class TryLockAndGetTest {

    protected static String HAZELCAST_WITH_MAPLOADER_HOST = "127.0.0.1";
    protected static String HAZELCAST_WITH_MAPLOADER_PORT = "5701";
    protected static String HAZELCAST_WITHOUT_MAPLOADER_HOST = "127.0.0.1";
    protected static String HAZELCAST_WITHOUT_MAPLOADER_PORT = "5702";
    protected static String HAZELCAST_MAP_NAME = "testMap";

    protected static String HAZELCAST_AUTH_GROUP = "dev-test";
    protected static String HAZELCAST_AUTH_PASSWORD = "dev-pass";

    protected static HazelcastInstance hazelcastWithMapLoaderServerInstance = null;
    protected static HazelcastInstance hazelcastWithoutMapLoaderServerInstance = null;
    private HazelcastClient clientToMapWithMapStore = null;
    private HazelcastClient clientToMapWithoutMapStore = null;
    protected IMap<String, Serializable> mapWithMapStore = null;
    protected IMap<String, Serializable> mapWithoutMapStore = null;

    public static final String HAZELCAST_WITH_MAPLOADER_CONFIG_XML =
            "<hazelcast xsi:schemaLocation=\"http://www.hazelcast.com/schema/config hazelcast-basic.xsd\" xmlns=\"http://www.hazelcast.com/schema/config\" xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\">"+
            "   <group>"+
            "       <name>dev-test</name>"+
            "       <password>dev-pass</password>"+
            "   </group>"+
            "   <network>"+
            "       <port auto-increment=\"false\">"+HAZELCAST_WITH_MAPLOADER_PORT+"</port>"+
            "       <join>"+
            "           <multicast enabled=\"false\"/>"+
            "           <tcp-ip enabled=\"false\" />"+
            "           <aws enabled=\"false\" />"+
            "       </join>"+
            "       <interfaces enabled=\"false\" />"+
            "       <symmetric-encryption enabled=\"false\" />"+
            "       <asymmetric-encryption enabled=\"false\" />"+
            "   </network>"+
            "   <executor-service>"+
            "       <core-pool-size>16</core-pool-size>"+
            "       <max-pool-size>64</max-pool-size>"+
            "       <keep-alive-seconds>60</keep-alive-seconds>"+
            "   </executor-service>"+
            "   <map name=\""+HAZELCAST_MAP_NAME+"\">"+
            "       <time-to-live-seconds>3600</time-to-live-seconds>"+
            "       <max-idle-seconds>0</max-idle-seconds>"+
            "       <eviction-policy>NONE</eviction-policy>"+
            "       <map-store enabled=\"true\">"+
            "           <class-name>com.hazelcast.test.TryLockAndGetTest$DoNothingMapLoader</class-name>"+
            "           <properties>"+
            "               <property name=\"createdirifmissing\">true</property>"+
            "               <property name=\"directory\">/tmp/hazelcast/1/"+HAZELCAST_MAP_NAME+"</property>"+
            "               <property name=\"name\">"+HAZELCAST_MAP_NAME+"</property>"+
            "           </properties>"+
            "       </map-store>"+
            "   </map>"+
            "</hazelcast>";
    public static final String HAZELCAST_WITHOUT_MAPLOADER_CONFIG_XML =
            "<hazelcast xsi:schemaLocation=\"http://www.hazelcast.com/schema/config hazelcast-basic.xsd\" xmlns=\"http://www.hazelcast.com/schema/config\" xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\">"+
            "   <group>"+
            "       <name>dev-test</name>"+
            "       <password>dev-pass</password>"+
            "   </group>"+
            "   <network>"+
            "       <port auto-increment=\"false\">"+HAZELCAST_WITHOUT_MAPLOADER_PORT+"</port>"+
            "       <join>"+
            "           <multicast enabled=\"false\" />"+
            "           <tcp-ip enabled=\"false\" />"+
            "           <aws enabled=\"false\" />"+
            "       </join>"+
            "       <interfaces enabled=\"false\" />"+
            "       <symmetric-encryption enabled=\"false\" />"+
            "       <asymmetric-encryption enabled=\"false\" />"+
            "   </network>"+
            "   <executor-service>"+
            "       <core-pool-size>16</core-pool-size>"+
            "       <max-pool-size>64</max-pool-size>"+
            "       <keep-alive-seconds>60</keep-alive-seconds>"+
            "   </executor-service>"+
            "   <map name=\""+HAZELCAST_MAP_NAME+"\">"+
            "       <time-to-live-seconds>3600</time-to-live-seconds>"+
            "       <max-idle-seconds>0</max-idle-seconds>"+
            "       <eviction-policy>NONE</eviction-policy>"+
            "   </map>"+
            "</hazelcast>";

    @BeforeClass
    public static void start_hazelcast_server() {
        InMemoryXmlConfig config1 = new InMemoryXmlConfig(HAZELCAST_WITH_MAPLOADER_CONFIG_XML);
        hazelcastWithMapLoaderServerInstance = Hazelcast.newHazelcastInstance(config1);
        InMemoryXmlConfig config2 = new InMemoryXmlConfig(HAZELCAST_WITHOUT_MAPLOADER_CONFIG_XML);
        hazelcastWithoutMapLoaderServerInstance = Hazelcast.newHazelcastInstance(config2);
    }

    @AfterClass
    public static void stop_hazelcast_server() {
        if(hazelcastWithMapLoaderServerInstance!=null)
            hazelcastWithMapLoaderServerInstance.getLifecycleService().kill();
        if(hazelcastWithoutMapLoaderServerInstance!=null)
            hazelcastWithoutMapLoaderServerInstance.getLifecycleService().kill();
    }

    @Before
    public void start_hazelcast_clients_and_get_map() throws Exception {
        ClientConfig config1 = new ClientConfig();
        GroupConfig groupConfig1 = new GroupConfig(HAZELCAST_AUTH_GROUP, HAZELCAST_AUTH_PASSWORD);
        config1.setGroupConfig(groupConfig1);
        List<String> urlList1 = new ArrayList<String>();
        urlList1.add(HAZELCAST_WITH_MAPLOADER_HOST+":"+HAZELCAST_WITH_MAPLOADER_PORT);
        config1.setAddresses(urlList1);
        clientToMapWithMapStore = HazelcastClient.newHazelcastClient(config1);
        mapWithMapStore = clientToMapWithMapStore.getMap(HAZELCAST_MAP_NAME);
        mapWithMapStore.size(); // Trigger map initialization

        ClientConfig config2 = new ClientConfig();
        GroupConfig groupConfig2 = new GroupConfig(HAZELCAST_AUTH_GROUP, HAZELCAST_AUTH_PASSWORD);
        config2.setGroupConfig(groupConfig2);
        List<String> urlList2 = new ArrayList<String>();
        urlList2.add(HAZELCAST_WITHOUT_MAPLOADER_HOST+":"+HAZELCAST_WITHOUT_MAPLOADER_PORT);
        config2.setAddresses(urlList2);
        clientToMapWithoutMapStore = HazelcastClient.newHazelcastClient(config2);
        mapWithoutMapStore = clientToMapWithoutMapStore.getMap(HAZELCAST_MAP_NAME);
        mapWithoutMapStore.size(); // Trigger map initialization

    }

    @After
    public void stop_hazelcast_client() {
        if(clientToMapWithMapStore!=null)
            clientToMapWithMapStore.shutdown();
        if(clientToMapWithoutMapStore!=null)
            clientToMapWithoutMapStore.shutdown();
    }

    @Test
    public void check_that_tryLockAndGet_is_synchronized_towards_map_with_mapstore() {
        String key = "testKey";
        TryLockAndGetRunnable[] lockTasks = {
                new TryLockAndGetRunnable(1, mapWithMapStore, key, 10, 1000),
                new TryLockAndGetRunnable(2, mapWithMapStore, key, 10, 1000),
                new TryLockAndGetRunnable(3, mapWithMapStore, key, 10, 1000),
                new TryLockAndGetRunnable(4, mapWithMapStore, key, 10, 1000)
        };
        MultiThreadedTestRunner mttr = new MultiThreadedTestRunner(lockTasks);
        try {
            mttr.runTestRunnables();
            int numberOfThreadsThatGotTheLock = 0;
            for(TryLockAndGetRunnable task : lockTasks) {
                if(task.gotTheLock())
                    numberOfThreadsThatGotTheLock++;
            }
            if(numberOfThreadsThatGotTheLock!=1)
                fail(numberOfThreadsThatGotTheLock+" threads got the lock, should be 1");
        } catch (Throwable e) {
            fail(e.getMessage());
        }
    }

    @Test
    public void check_that_tryLockAndGet_is_synchronized_towards_map_without_mapstore() {
        String key = "testKey";
        TryLockAndGetRunnable[] lockTasks = {
                new TryLockAndGetRunnable(1, mapWithoutMapStore, key, 10, 1000),
                new TryLockAndGetRunnable(2, mapWithoutMapStore, key, 10, 1000),
                new TryLockAndGetRunnable(3, mapWithoutMapStore, key, 10, 1000),
                new TryLockAndGetRunnable(4, mapWithoutMapStore, key, 10, 1000)
        };
        MultiThreadedTestRunner mttr = new MultiThreadedTestRunner(lockTasks);
        try {
            mttr.runTestRunnables();
            int numberOfThreadsThatGotTheLock = 0;
            for(TryLockAndGetRunnable task : lockTasks) {
                if(task.gotTheLock())
                    numberOfThreadsThatGotTheLock++;
            }
            if(numberOfThreadsThatGotTheLock!=1)
                fail(numberOfThreadsThatGotTheLock+" threads got the lock, should be 1");
        } catch (Throwable e) {
            fail(e.getMessage());
        }
    }

    /**
     * GroboUtils runnable. Tries to get the lock on a key. The lock attempt is with a timeout.
     * If it gets the lock, it holds it for a while, and then releases it.
     */
    static class TryLockAndGetRunnable extends TestRunnable {
        int clientNumber = -1;
        IMap<String, Serializable> map = null;
        String key = null;
        long timeout = -1;
        long holdLockMillis = -1;
        boolean gotTheLock = false;

        public TryLockAndGetRunnable(int clientNumber, IMap<String, Serializable> map, String key, long timeoutMillis, long holdLockMillis) {
            this.clientNumber = clientNumber;
            this.map = map;
            this.key = key;
            this.timeout = timeoutMillis;
            this.holdLockMillis = holdLockMillis;
        }

        public boolean gotTheLock() {
            return gotTheLock;
        }

        @Override
        public void runTest() throws Throwable {
            try {
                map.tryLockAndGet(key, timeout, TimeUnit.MILLISECONDS);
                gotTheLock = true;
                if(holdLockMillis>0) {
                    try {
                        Thread.sleep(holdLockMillis);
                    } catch (InterruptedException ie) {}
                }
                try {
                    map.unlock(key);
                } catch(Exception e) {
                    System.out.println("Failed unlocking key ["+key+"]: "+e.getMessage());
                }
            } catch (TimeoutException te) {
            }
        }
    }

    /**
     * Dummy MapLoader implementation that does nothing
     */
    public static class DoNothingMapLoader implements MapLoader<String, Serializable> {

        @Override
        public Serializable load(String arg0) {
            return null;
        }

        @Override
        public Map<String, Serializable> loadAll(Collection<String> arg0) {
            return null;
        }

        @Override
        public Set<String> loadAllKeys() {
            return null;
        }

    }
}
@mdogan

This comment has been minimized.

Copy link
Member

commented Sep 10, 2012

Thanks for the report and unit test. We will work on it.

@ghost ghost assigned mdogan Sep 10, 2012

@mdogan mdogan closed this in 7c4cfb2 Sep 22, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.