You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@ignite.apache.org by Jon Tricker <jt...@temenos.com> on 2018/06/18 10:23:27 UTC
If a lock is held by another node IgniteCache.isLocalLocked() appears
to return incorrect results.
Additionally if a lock is held by another thread lock.tryLock() appears to block.
Demonstrated by the following code. See header comment for a full description. The error at line 85 should not be printed.
Found on Ignite 2.5.0. Reproducible on Windows and RHL.
Note : The test is technically timing dependant but the sleep in the remote thread is long enough that, on any reasonable system, the parent should check the lock before it dies.
package igniteCacheLockTest;
import org.apache.ignite.Ignition;
import org.apache.ignite.cache.CacheAtomicityMode;
import org.apache.ignite.cache.CacheMode;
import org.apache.ignite.cache.CacheWriteSynchronizationMode;
import org.apache.ignite.configuration.CacheConfiguration;
import org.apache.ignite.configuration.DataRegionConfiguration;
import org.apache.ignite.configuration.DataStorageConfiguration;
import org.apache.ignite.configuration.IgniteConfiguration;
import org.apache.ignite.spi.discovery.tcp.TcpDiscoverySpi;
import org.apache.ignite.spi.discovery.tcp.ipfinder.multicast.TcpDiscoveryMulticastIpFinder;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.locks.Lock;
import org.apache.ignite.Ignite;
import org.apache.ignite.IgniteCache;
import org.apache.ignite.IgniteLock;
/**
* Test demonstrating a cache lock held by one ignite node not being accessible on a different node.
*
* The parent starts an ignite node and creates a cache lock. It then creates a thread which starts a second node, gets a handle to the same
* lock (proven by the fact that the set value can be read back) and locks it.
*
* The parent then uses isLocalLocked() to check if the lock is held. It isn't.
*
* The parent also tries a trylock(). This was done to check if maybe the lock is really held but, maybe, isLocalLocked() is returning
* incorrect information. Incorrectly the, non-blocking, trylock() blocks until the thread dies and then gets the lock.
*
* If the thread is modified to not take the lock (comment out the lock.lock() line). The trylock() gets the lock and returns immediately as
* expected.
*
* There is some interaction between the threads based on the lock location but it is not working as expected.
*/
public class CacheLockTest {
public final static String LOCKNAME = "LOCKNAME";
public final static String CACHENAME = "CACHENAME";
public final static int TESTVALUE = 123;
public static void main(String[] args) {
System.out.println("Starting test");
// Make a uniquely named ignite node.
IgniteConfiguration config1 = new DefaultIgniteConfig();
config1.setIgniteInstanceName("INSTANCE1");
// Start node.
Ignite node1 = Ignition.start(config1);
// Get a reference to the cache
IgniteCache<String, Integer> cache = node1.getOrCreateCache(CACHENAME);
// Write a pattern to the location so we can confirm we are connected to the same cache.
cache.put(LOCKNAME, TESTVALUE);
// Make a lock
Lock lock = cache.lock(LOCKNAME);
// Check is initially unlocked
if (cache.isLocalLocked(LOCKNAME, true)) {
System.out.println("Is initially locked local");
}
if (cache.isLocalLocked(LOCKNAME, false)) {
System.out.println("Is initially locked remote");
}
// Create a remote thread.
TestThread thread = new TestThread(LOCKNAME);
// Run thread. Will get a handle to the lock and lock it.
thread.start();
try {
// Give thread a while to run.
Thread.sleep(5000);
} catch (Exception e) {
System.out.println("Could not sleep.");
}
// Thread should still be alive and have taken the lock. Check it.
if (!cache.isLocalLocked(LOCKNAME, false)) {
System.out.println("ERROR. Thread failed to take lock");
}
// To confirm it is really not locked try to take the lock.
System.out.println("Parent about to do non-blocking trylock().");
if (lock.tryLock()) {
System.out.println("ERROR. Lock was not held.");
// Just took the lock. Tidy.
lock.unlock();
} else {
System.out.println("Lock is held as expected.");
}
// Close node
node1.close();
System.out.println("Test done");
// Run of end and allow thread to self tidy.
thread = null;
}
// Remote thread that takes a lock.
private static class TestThread extends Thread {
private Ignite node2;
private String lockName;
public TestThread(String lockName) {
// Remember lock name.
this.lockName = lockName;
// Start a new, uniquely named, ignite node for the thread,
IgniteConfiguration config2 = new DefaultIgniteConfig();
config2.setIgniteInstanceName("INSTANCE2");
// Start an ignite node. No caches required.
node2 = Ignition.start(config2);
}
public void run() {
// Get a handle to the lock and lock it.
// Get a reference to the cache
IgniteCache<String, Integer> cache = node2.getOrCreateCache(CACHENAME);
// Make a lock
Lock lock = cache.lock(lockName);
// Read the value to check we are talking to the same cache.
int value = cache.get(lockName);
if (TESTVALUE != value) {
System.out.println("Value is wrong = " + value);
}
// Take the lock.
lock.lock();
// Check we got the lock
if (!cache.isLocalLocked(lockName, true)) {
System.out.println("Thread could not get lock");
}
// Wait for long enough for parent to check the lock. i.e. significantly longer than the wait in the parent.
try {
// Give thread a while to run.
System.out.println("Thread sleeping.");
Thread.sleep(20000);
} catch (Exception e) {
System.out.println("Could not sleep.");
}
System.out.println("Thread closing.");
node2.close();
}
}
// A simple, canned, ignite config.
private static class DefaultIgniteConfig extends IgniteConfiguration {
public DefaultIgniteConfig() {
// Some basic config.
setIgniteInstanceName("IGNITE_LOCK_TEST");
// Need a server to connect to. So be a server.
setClientMode(false);
setPeerClassLoadingEnabled(true);
// Set up node discovery
TcpDiscoverySpi discoSpi = new TcpDiscoverySpi();
TcpDiscoveryMulticastIpFinder finder = new TcpDiscoveryMulticastIpFinder();
List<String> addresses = Arrays.asList("127.0.0.1:47500..47502");
finder.setAddresses(addresses);
discoSpi.setIpFinder(finder);
setDiscoverySpi(discoSpi);
// Set up data storage
DataStorageConfiguration storeConfig = new DataStorageConfiguration();
DataRegionConfiguration regionConfig = storeConfig.getDefaultDataRegionConfiguration();
regionConfig.setName("IGNITE_LOCK_REGION");
// For locking should not need persistence.
regionConfig.setPersistenceEnabled(false);
setDataStorageConfiguration(storeConfig);
// Set up a cache. Same for both instances.
CacheConfiguration<String, Integer> cacheConfig = new CacheConfiguration<String, Integer>();
cacheConfig.setName(CACHENAME);
cacheConfig.setCacheMode(CacheMode.REPLICATED);
cacheConfig.setAtomicityMode(CacheAtomicityMode.TRANSACTIONAL);
// Not sure we need the following
cacheConfig.setWriteSynchronizationMode(CacheWriteSynchronizationMode.FULL_SYNC);
cacheConfig.setBackups(0);
this.setCacheConfiguration(cacheConfig);
}
}
}
The information in this e-mail and any attachments is confidential and may be legally privileged. It is intended solely for the addressee or addressees. Any use or disclosure of the contents of this e-mail/attachments by a not intended recipient is unauthorized and may be unlawful. If you have received this e-mail in error please notify the sender. Please note that any views or opinions presented in this e-mail are solely those of the author and do not necessarily represent those of TEMENOS. We recommend that you check this e-mail and any attachments against viruses. TEMENOS accepts no liability for any damage caused by any malicious code or virus transmitted by this e-mail.
Re: If a lock is held by another node IgniteCache.isLocalLocked()
appears to return incorrect results.
Posted by aealexsandrov <ae...@gmail.com>.
Hi,
I filed next issue:
https://issues.apache.org/jira/browse/IGNITE-8833
Thank you,
Andrei
--
Sent from: http://apache-ignite-users.70518.x6.nabble.com/
Re: If a lock is held by another node IgniteCache.isLocalLocked()
appears to return incorrect results.
Posted by aealexsandrov <ae...@gmail.com>.
Hi,
It looks like an issue. Let me investigate it and create an issue.
BR,
Andrei
--
Sent from: http://apache-ignite-users.70518.x6.nabble.com/
RE: If a lock is held by another node IgniteCache.isLocalLocked()
appears to return incorrect results.
Posted by Jon Tricker <jt...@temenos.com>.
Replying to my own post but the following appears to be a workround. Replaces IgniteCache.isLocalLocked(xxx, false) with a local isRemoteLocked() method. This uses trylock() with a short timeout to check if the lock is held. See comments in code for more details.
package igniteCacheLockTest;
import org.apache.ignite.Ignition;
import org.apache.ignite.cache.CacheAtomicityMode;
import org.apache.ignite.cache.CacheMode;
import org.apache.ignite.cache.CacheWriteSynchronizationMode;
import org.apache.ignite.configuration.CacheConfiguration;
import org.apache.ignite.configuration.DataRegionConfiguration;
import org.apache.ignite.configuration.DataStorageConfiguration;
import org.apache.ignite.configuration.IgniteConfiguration;
import org.apache.ignite.spi.discovery.tcp.TcpDiscoverySpi;
import org.apache.ignite.spi.discovery.tcp.ipfinder.multicast.TcpDiscoveryMulticastIpFinder;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.Lock;
import org.apache.ignite.Ignite;
import org.apache.ignite.IgniteCache;
import org.apache.ignite.IgniteLock;
/**
* Test demonstrating a cache lock held by one ignite node not being accessible
* on a different node.
*
* The parent starts an ignite node and creates a cache lock. It then creates a
* thread which starts a second node, gets a handle to the same lock (proven by
* the fact that the set value can be read back) and locks it.
*
* The parent then uses isLocalLocked() to check if the lock is held. It isn't.
*
* The parent also tries a trylock(). This was done to check if maybe the lock
* is really held but, maybe, isLocalLocked() is returning incorrect
* information. Incorrectly the, non-blocking, trylock() blocks until the thread
* dies and then gets the lock.
*
* If the thread is modified to not take the lock (comment out the lock.lock()
* line). The trylock() gets the lock and returns immediately as expected.
*
* There is some interaction between the threads based on the lock location but
* it is not working as expected.
*/
public class CacheLockTest {
public final static String LOCKNAME = "LOCKNAME";
public final static String CACHENAME = "CACHENAME";
public final static int TESTVALUE = 123;
public static void main(String[] args) {
System.out.println("Starting test");
// Make a uniquely named ignite node.
IgniteConfiguration config1 = new DefaultIgniteConfig();
config1.setIgniteInstanceName("INSTANCE1");
// Start node.
Ignite node1 = Ignition.start(config1);
// Get a reference to the cache
IgniteCache<String, Integer> cache = node1.getOrCreateCache(CACHENAME);
// Write a pattern to the location so we can confirm we are connected to the
// same cache.
cache.put(LOCKNAME, TESTVALUE);
// Make a lock
Lock lock = cache.lock(LOCKNAME);
// Check is initially unlocked
if (cache.isLocalLocked(LOCKNAME, true)) {
System.out.println("Is initially locked local");
}
if (isRemoteLocked(lock)) {
System.out.println("Is initially locked remote");
}
// Create a remote thread.
TestThread thread = new TestThread(LOCKNAME);
// Run thread. Will get a handle to the lock and lock it.
thread.start();
try {
// Give thread a while to run.
Thread.sleep(5000);
} catch (Exception e) {
System.out.println("Could not sleep.");
}
// Thread should still be alive and have taken the lock. Check it.
if (!isRemoteLocked(lock)) {
System.out.println("ERROR. Thread failed to take lock");
}
// To confirm it is really not locked try to take the lock.
//
// Not much point in doing this. Trylock(), with timeout, has already been
// tested in isRemoteLocked() and trylock(), without timeout, blocks.
/*
System.out.println("Parent about to do non-blocking trylock().");
if (lock.tryLock()) {
System.out.println("ERROR. Lock was not held.");
// Just took the lock. Tidy.
lock.unlock();
} else {
System.out.println("Lock is held as expected.");
}
*/
// Close node
node1.close();
System.out.println("Test done");
// Run of end and allow thread to self tidy.
thread = null;
}
// Since IgniteCache.inLocalLocked(..., false) does not appear to detect locks
// held by remote nodes a function that does.
private static boolean isRemoteLocked(Lock lock) {
boolean res = false;
try {
// Would like to use trylock(). However if the lock is held by a remote this
// appears to block. With a short timeout it works.
//
// The length of the timeout seems important. If the following is
// changed to MICROSECONDS (which makes negligible difference to the tests timing) it again blocks.
if (res = lock.tryLock(1L, TimeUnit.MILLISECONDS)) {
// Not remotely held. But now we hold it. So unlock again.
lock.unlock();
}
} catch (Exception e) {
System.out.println("ERROR : Trylock() threw " + e.getMessage());
}
// If we locked it then it was NOT remote locked. Return the inverse of res.
return !res;
}
// Remote thread that takes a lock.
private static class TestThread extends Thread {
private Ignite node2;
private String lockName;
public TestThread(String lockName) {
// Remember lock name.
this.lockName = lockName;
// Start a new, uniquely named, ignite node for the thread,
IgniteConfiguration config2 = new DefaultIgniteConfig();
config2.setIgniteInstanceName("INSTANCE2");
// Start an ignite node. No caches required.
node2 = Ignition.start(config2);
}
public void run() {
// Get a handle to the lock and lock it.
// Get a reference to the cache
IgniteCache<String, Integer> cache = node2.getOrCreateCache(CACHENAME);
// Make a lock
Lock lock = cache.lock(lockName);
// Read the value to check we are talking to the same cache.
int value = cache.get(lockName);
if (TESTVALUE != value) {
System.out.println("Value is wrong = " + value);
}
// Take the lock.
lock.lock();
// Check we got the lock
if (!cache.isLocalLocked(lockName, true)) {
System.out.println("Thread could not get lock");
}
// Wait for long enough for parent to check the lock. i.e. significantly longer
// than the wait in the parent.
try {
// Give thread a while to run.
System.out.println("Thread sleeping.");
Thread.sleep(20000);
} catch (Exception e) {
System.out.println("Could not sleep.");
}
System.out.println("Thread closing.");
node2.close();
}
}
// A simple, canned, ignite config.
private static class DefaultIgniteConfig extends IgniteConfiguration {
public DefaultIgniteConfig() {
// Some basic config.
setIgniteInstanceName("IGNITE_LOCK_TEST");
// Need a server to connect to. So be a server.
setClientMode(false);
setPeerClassLoadingEnabled(true);
// Set up node discovery
TcpDiscoverySpi discoSpi = new TcpDiscoverySpi();
TcpDiscoveryMulticastIpFinder finder = new TcpDiscoveryMulticastIpFinder();
List<String> addresses = Arrays.asList("127.0.0.1:47500..47502");
finder.setAddresses(addresses);
discoSpi.setIpFinder(finder);
setDiscoverySpi(discoSpi);
// Set up data storage
DataStorageConfiguration storeConfig = new DataStorageConfiguration();
DataRegionConfiguration regionConfig = storeConfig.getDefaultDataRegionConfiguration();
regionConfig.setName("IGNITE_LOCK_REGION");
// For locking should not need persistence.
regionConfig.setPersistenceEnabled(false);
setDataStorageConfiguration(storeConfig);
// Set up a cache. Same for both instances.
CacheConfiguration<String, Integer> cacheConfig = new CacheConfiguration<String, Integer>();
cacheConfig.setName(CACHENAME);
cacheConfig.setCacheMode(CacheMode.REPLICATED);
cacheConfig.setAtomicityMode(CacheAtomicityMode.TRANSACTIONAL);
// Not sure we need the following
cacheConfig.setWriteSynchronizationMode(CacheWriteSynchronizationMode.FULL_SYNC);
cacheConfig.setBackups(0);
this.setCacheConfiguration(cacheConfig);
}
}
}
From: Jon Tricker [mailto:jtricker@temenos.com]
Sent: 18 June 2018 11:23
To: user@ignite.apache.org
Cc: Ted Jolly <tj...@temenos.com>; Herve Aubert <ha...@temenos.com>; Arjun Venkataraj <av...@temenos.com>
Subject: If a lock is held by another node IgniteCache.isLocalLocked() appears to return incorrect results.
Additionally if a lock is held by another thread lock.tryLock() appears to block.
Demonstrated by the following code. See header comment for a full description. The error at line 85 should not be printed.
Found on Ignite 2.5.0. Reproducible on Windows and RHL.
Note : The test is technically timing dependant but the sleep in the remote thread is long enough that, on any reasonable system, the parent should check the lock before it dies.
package igniteCacheLockTest;
import org.apache.ignite.Ignition;
import org.apache.ignite.cache.CacheAtomicityMode;
import org.apache.ignite.cache.CacheMode;
import org.apache.ignite.cache.CacheWriteSynchronizationMode;
import org.apache.ignite.configuration.CacheConfiguration;
import org.apache.ignite.configuration.DataRegionConfiguration;
import org.apache.ignite.configuration.DataStorageConfiguration;
import org.apache.ignite.configuration.IgniteConfiguration;
import org.apache.ignite.spi.discovery.tcp.TcpDiscoverySpi;
import org.apache.ignite.spi.discovery.tcp.ipfinder.multicast.TcpDiscoveryMulticastIpFinder;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.locks.Lock;
import org.apache.ignite.Ignite;
import org.apache.ignite.IgniteCache;
import org.apache.ignite.IgniteLock;
/**
* Test demonstrating a cache lock held by one ignite node not being accessible on a different node.
*
* The parent starts an ignite node and creates a cache lock. It then creates a thread which starts a second node, gets a handle to the same
* lock (proven by the fact that the set value can be read back) and locks it.
*
* The parent then uses isLocalLocked() to check if the lock is held. It isn't.
*
* The parent also tries a trylock(). This was done to check if maybe the lock is really held but, maybe, isLocalLocked() is returning
* incorrect information. Incorrectly the, non-blocking, trylock() blocks until the thread dies and then gets the lock.
*
* If the thread is modified to not take the lock (comment out the lock.lock() line). The trylock() gets the lock and returns immediately as
* expected.
*
* There is some interaction between the threads based on the lock location but it is not working as expected.
*/
public class CacheLockTest {
public final static String LOCKNAME = "LOCKNAME";
public final static String CACHENAME = "CACHENAME";
public final static int TESTVALUE = 123;
public static void main(String[] args) {
System.out.println("Starting test");
// Make a uniquely named ignite node.
IgniteConfiguration config1 = new DefaultIgniteConfig();
config1.setIgniteInstanceName("INSTANCE1");
// Start node.
Ignite node1 = Ignition.start(config1);
// Get a reference to the cache
IgniteCache<String, Integer> cache = node1.getOrCreateCache(CACHENAME);
// Write a pattern to the location so we can confirm we are connected to the same cache.
cache.put(LOCKNAME, TESTVALUE);
// Make a lock
Lock lock = cache.lock(LOCKNAME);
// Check is initially unlocked
if (cache.isLocalLocked(LOCKNAME, true)) {
System.out.println("Is initially locked local");
}
if (cache.isLocalLocked(LOCKNAME, false)) {
System.out.println("Is initially locked remote");
}
// Create a remote thread.
TestThread thread = new TestThread(LOCKNAME);
// Run thread. Will get a handle to the lock and lock it.
thread.start();
try {
// Give thread a while to run.
Thread.sleep(5000);
} catch (Exception e) {
System.out.println("Could not sleep.");
}
// Thread should still be alive and have taken the lock. Check it.
if (!cache.isLocalLocked(LOCKNAME, false)) {
System.out.println("ERROR. Thread failed to take lock");
}
// To confirm it is really not locked try to take the lock.
System.out.println("Parent about to do non-blocking trylock().");
if (lock.tryLock()) {
System.out.println("ERROR. Lock was not held.");
// Just took the lock. Tidy.
lock.unlock();
} else {
System.out.println("Lock is held as expected.");
}
// Close node
node1.close();
System.out.println("Test done");
// Run of end and allow thread to self tidy.
thread = null;
}
// Remote thread that takes a lock.
private static class TestThread extends Thread {
private Ignite node2;
private String lockName;
public TestThread(String lockName) {
// Remember lock name.
this.lockName = lockName;
// Start a new, uniquely named, ignite node for the thread,
IgniteConfiguration config2 = new DefaultIgniteConfig();
config2.setIgniteInstanceName("INSTANCE2");
// Start an ignite node. No caches required.
node2 = Ignition.start(config2);
}
public void run() {
// Get a handle to the lock and lock it.
// Get a reference to the cache
IgniteCache<String, Integer> cache = node2.getOrCreateCache(CACHENAME);
// Make a lock
Lock lock = cache.lock(lockName);
// Read the value to check we are talking to the same cache.
int value = cache.get(lockName);
if (TESTVALUE != value) {
System.out.println("Value is wrong = " + value);
}
// Take the lock.
lock.lock();
// Check we got the lock
if (!cache.isLocalLocked(lockName, true)) {
System.out.println("Thread could not get lock");
}
// Wait for long enough for parent to check the lock. i.e. significantly longer than the wait in the parent.
try {
// Give thread a while to run.
System.out.println("Thread sleeping.");
Thread.sleep(20000);
} catch (Exception e) {
System.out.println("Could not sleep.");
}
System.out.println("Thread closing.");
node2.close();
}
}
// A simple, canned, ignite config.
private static class DefaultIgniteConfig extends IgniteConfiguration {
public DefaultIgniteConfig() {
// Some basic config.
setIgniteInstanceName("IGNITE_LOCK_TEST");
// Need a server to connect to. So be a server.
setClientMode(false);
setPeerClassLoadingEnabled(true);
// Set up node discovery
TcpDiscoverySpi discoSpi = new TcpDiscoverySpi();
TcpDiscoveryMulticastIpFinder finder = new TcpDiscoveryMulticastIpFinder();
List<String> addresses = Arrays.asList("127.0.0.1:47500..47502");
finder.setAddresses(addresses);
discoSpi.setIpFinder(finder);
setDiscoverySpi(discoSpi);
// Set up data storage
DataStorageConfiguration storeConfig = new DataStorageConfiguration();
DataRegionConfiguration regionConfig = storeConfig.getDefaultDataRegionConfiguration();
regionConfig.setName("IGNITE_LOCK_REGION");
// For locking should not need persistence.
regionConfig.setPersistenceEnabled(false);
setDataStorageConfiguration(storeConfig);
// Set up a cache. Same for both instances.
CacheConfiguration<String, Integer> cacheConfig = new CacheConfiguration<String, Integer>();
cacheConfig.setName(CACHENAME);
cacheConfig.setCacheMode(CacheMode.REPLICATED);
cacheConfig.setAtomicityMode(CacheAtomicityMode.TRANSACTIONAL);
// Not sure we need the following
cacheConfig.setWriteSynchronizationMode(CacheWriteSynchronizationMode.FULL_SYNC);
cacheConfig.setBackups(0);
this.setCacheConfiguration(cacheConfig);
}
}
}
The information in this e-mail and any attachments is confidential and may be legally privileged. It is intended solely for the addressee or addressees. Any use or disclosure of the contents of this e-mail/attachments by a not intended recipient is unauthorized and may be unlawful. If you have received this e-mail in error please notify the sender. Please note that any views or opinions presented in this e-mail are solely those of the author and do not necessarily represent those of TEMENOS. We recommend that you check this e-mail and any attachments against viruses. TEMENOS accepts no liability for any damage caused by any malicious code or virus transmitted by this e-mail.
The information in this e-mail and any attachments is confidential and may be legally privileged. It is intended solely for the addressee or addressees. Any use or disclosure of the contents of this e-mail/attachments by a not intended recipient is unauthorized and may be unlawful. If you have received this e-mail in error please notify the sender. Please note that any views or opinions presented in this e-mail are solely those of the author and do not necessarily represent those of TEMENOS. We recommend that you check this e-mail and any attachments against viruses. TEMENOS accepts no liability for any damage caused by any malicious code or virus transmitted by this e-mail.