I've introduced a new API as discussed in #10. There is a problem though, the tests always fail for me, with or without this PR. I'm running it with go test ./redsync/mutext_test.go and it fails with:

--- FAIL: TestMutex (24.60s)
        mutex_test.go:84: failed to acquire lock
FAIL
FAIL    command-line-arguments  24.670s

I'm not sure why this is happenning. Any ideas?

The godoc example (and all other New* functions) require passing Redis credentials/connections everytime when acquiring a mutex.

Instead of that, an API that separates connections from mutexes seems much more usable. For example:

// redsync.New returns *RedSync
redsync := redsync.New([]net.Addr{
    &net.TCPAddr{Port: 6379},
    &net.TCPAddr{Port: 6380},
})

lock, err := redsync.Lock("foo")
if err != nil {
    // success!
}

redsync.Unlock(lock)

This way, a single RedSync instance could be shared across the codebase without dealing with connection parameters everywhere. What do you think?

Dear Authors,

Thanks for your effort in implementing the redsync algorithm in go. I am trying to use your implementation in a distributed system where multiple nodes might request for the same named resource. In that case, though the mutexes created for the same named resource are different, the lock should still succeed for one and only one node.

The delScript inside Lock() prevents this from happening. I am trying to understand why you would need that in the first place?

( I'd like to simulate the following behaviour at the redis-server:

127.0.0.1:6379> get uuid1 (nil) 127.0.0.1:6379> set uuid1 2345 NX PX 200000 OK 127.0.0.1:6379> get uuid1 "2345" 127.0.0.1:6379> set uuid1 1234 NX PX 200000 (nil) 127.0.0.1:6379> get uuid1 "2345"

A get fetching a non-nil value should suffice, the value needn't be equal )

Thanks, Smita

Why caller should call Touch() ? What if Touch() is hidden by Lock()

In Lock(): run a goruntine to call Touch() regularly. Then caller can only focus on Lock() and Unlock()

There's no quorum code and the initialization assumes that you can successfully connect to all the Redis servers, some of which may be unavailable.

The Ruby implementation seems to be a bit more complete.

Any plan to fix these?

This simple example fails with "failed to acquire lock"

I am using go tip: go version devel +811b785 Wed Feb 10 14:42:30 2016 +0000 linux/amd64 package main

import ( "log" "net"

"github.com/hjr265/redsync.go/redsync"

)

func main() { m, err := redsync.NewMutex("foo", []net.Addr{&net.TCPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 6379}}) if err != nil { log.Fatal(err) } err = m.Lock() if err != nil { log.Fatal(err) } m.Unlock()

}

The following changes have been introduced to the library in this pull request:

1. Adding touch function to redsync Locker interface: Touch() function resets the expiry of a valid (key,value) on the Redis cluster. It does this using the touchScript. Touch script assumes a valid (key,value) pair which is valid at the time "GET" is invoked at the particular node in the Redis cluster. If found, the timer value is reset to original slice. Else, the (key,value) is not created. Touch() returns the status of touch, and therefore, the validity of the mutex of the caller. The function returns true if expiry at atleast Quorum number of nodes was successfully reset. This means the lock is valid at atleast Quorum nodes, and therefore is valid and held by the caller. Else, the caller does not have a valid mutex (possibly timed out).
2. Changing Unlock() to return status of unlock of a mutex: Similar to the touch function, the Unlock() function's signature has been changed to return the status of Unlock(). If Unlock() successfully found and deleted the (key,value) at atleast Quorum number of nodes, then Unlock() was successful on a valid mutex. Else, the mutex was invalid (possibly had timed out), therefore, Unlock() was unsuccessful.

It turns out the latest Godep https://github.com/tools/godep/ does not allow for imported repos to be of the form *.go, meaning redsync.go breaks it. Would be good to copy the repo into a different one with a supported Godep name. The name of the newer package under development https://github.com/redsync/redsync/ seems fine so I look forward to that being available!

This PR includes:

• [x] Refactoring to make code pass glint
• [x] Change library from radix to redigo, which includes using EVALSHA (fixes #2)
• [x] Use tempredis to automatically start redis servers for testing
• [x] Add NewMutexWithPool so redigo's Pool of hosts can be used, allowing reusing existing redis connections.
• [x] Allow redis authentication using redis.Pool Dial. See redis.Pool example

@hjr265 let me know what you think.

Hi hjr265,

as I have known, the redis set operation with "NX" will promise distributed lock, reply, err := redis.String(conn.Do("SET", m.name, value, "NX", "PX", int(m.expiry/time.Millisecond)))

so it confused me, why the Lock and Unlock method need m.nodem.Lock? and will the m.nodem.Lock lead to deadlock?

func (m *Mutex) Lock() error {
	m.nodem.Lock()
	defer m.nodem.Unlock()
        ...
}

Hi Mahmud, Since connection pool parameters are configurable in the redigo library [https://github.com/garyburd/redigo/blob/master/redis/pool.go], isn't it then better to take these also as a parameter when creating a new mutex?

In my particular case, I hit a problem with the MaxActive pool value set to 1. Running a load of 1000+ requests per parameter, the average wait time for a Lock() to return successfully was about 9.8 seconds. I ran it with (an extreme) MaxActive = 0, and the value came down to 6.6ms!

IMO, application should allow parallel unrelated requests to go through and related requests should be synchronized. The library should take this input and work with it. Defaults may be used (like the constants defined currently) in the absence of application-defined values.

What do you think? May I go ahead release a pull request for the same?

Call me crazy but if this is for distributed locking, then why is it impossible to unlock from another process?

https://github.com/hjr265/redsync.go/blob/master/redsync/mutex.go#L281-L284

If i'm calling Lock() from one daemon, the key gets added to redis as you'd expect. I call Unlock() in another daemon with the same named mutex, pool, etc, this is impossible because 'value' will obviously not be set. Am I missing something, is it not supposed to be used this way?