abelaska / jedis-lock Goto Github PK

It is possible that code between lock() and release() execute for a very long time for some reason.
And in this case, release() could still del the lockKey which could be already acquired by other application.

Maybe it is a good idea that don't del lockkey if it is expired.

fake code like this:

public synchronized void release(Jedis jedis) {
            if(isExpired()){   //  isExpired() 
                 locked = false;
                 return;
        }

    if (locked) {
        jedis.del(lockKey);
        locked = false;
    }
}

I was going through the source code and found that the key is set using setnx(key, value). But that does not set the auto expiry time which has been passed to JedisLock. I was asking because the description of the lock implementation at http://redis.io/topics/distlock specifies to take an auto-release lock (with a specified expiry time).

So the Jedis equivalent of this command "SET resource_name my_random_value NX PX 30000" would probably be something like...

set(lockKey, , "NX", "PX", expiryTimeMillis)

Can someone explain me if my understanding is correct.

Hi there,
I tried to use this locking feature along with Jedis and Spring-data-redis and ran into the following issue described in the below stackoverflow issue.

https://stackoverflow.com/questions/47763245/cannot-get-jedis-connection-could-not-get-a-resource-from-the-pool

I believe it is caused when I try to create a separate instance of Jedis to lock instead of one created in the pool by spring-data-redis.

Hi! In the acquire method when the lock is expired, if one thread, like thread A, has getSet the lock in redis but doesn't get into subsequent statements while another thread, like thread B, changed the value of the lock in redis, then thread A will fail to reentry the lock because the value of the lock in redis is not same as what thread A has.And thread B will get the lock next turn while Thread A still owns it.
I think I can change the code as below to solve such problem:

            if (currentLock.isExpiredOrMine(lockUUID)) {
                String oldValueStr = jedis.getSet(lockKeyPath, newLock.toString());
                if (oldValueStr != null && oldValueStr.equals(currentValueStr)) {
                    jedis.set(lockKeyPath, newLock.toString());
                    this.lock = newLock;
                    return true;
                }
            }

I have a redis cluster so I use Jedis Cluster for connecting to redis.

Jedis jedis = new Jedis("localhost");
JedisLock lock = new JedisLock(jedis, "lockname", 10000, 30000);

Since JedisLock requires Jedis object and not JedisCluster. how do i use it to use Jedis l;ick functionality in redis clsuter

JedisLock.acquire() has hard coded Thread.sleep(100);

It would be nice if that sleep were configurable,
also sleep time could increase from step to step
if a resource is not available for a long time

All the JedisLock APIs seems to work with Jedis instance. Is there any support of JedisCluster instance?

final String currentValueStr = jedis.get(lockKeyPath);

        final Lock currentLock = Lock.fromString(currentValueStr);


        if (currentLock.isExpiredOrMine(lockUUID)) {

            String oldValueStr = jedis.getSet(lockKeyPath, newLock.toString());

if two thread in different jvm enter < if (currentLock.isExpiredOrMine(lockUUID)) { > in the same 。one thread will get lock ,because getset is atomic ,but the next thread will change the lockKeyPath's value. so,

the later thread2 will always think isMine ,so will always update lockKeyPath's value ,so the lock get by thread1 will neve expire ,

Now, it use:

    if (jedis.setnx(lockKeyPath, newLock.toString()) == 1) {
        this.lock = newLock;
        return true;
    }

to get lock. But it doesn't set expire time. Is it proper to set it after getting lock?

    if (jedis.setnx(lockKeyPath, newLock.toString()) == 1) {
        jedis.expire(lockKeyPath, expireTime);  
        this.lock = newLock;
        return true;
    }

if (currentLock.isExpiredOrMine(lockUUID)) {
//when two process run to here,the first one get the lock,and return true,but he get the lock,but the second one will grab it;
//the second one goto next recycle and get lock too,he really got the lock.
//there is two process get the lock.....and when first one will release the second one's lock,you know then will always recyle like this.
String oldValueStr = jedis.getSet(lockKeyPath, newLock.toString());
if (oldValueStr != null && oldValueStr.equals(currentValueStr)) {
this.lock = newLock;
return true;
}
}
}
i try to fix this , i think we should use watch to keep it in transaction