1

I'm using this fairly straightforward means to match two live players :

class Seek
  def self.create(uuid)
    if opponent = REDIS.spop("seeks")
      Game.start(uuid, opponent)
    else
      REDIS.sadd("seeks", uuid)
    end
  end

  def self.remove(uuid)
    REDIS.srem("seeks", uuid)
  end
end

Then when my game starts, I simply do Seek.create(uuid).

There's very little niche problem I get where sometimes two people Seek at the same time. I'm guessing that Redis.spop("seeks") returns nil for both players, and converts then adds them both to REDIS.sadd("seeks", uuid) . And then they both wait indefinitely ( unless of course another player comes along ).

My situation seems like a rather rare case, but I'm curious if my seek.rb file could be written in a better way to prevent this.

Trip
  • 26,756
  • 46
  • 158
  • 277
  • How about using [Mutex#synchronize](https://ruby-doc.org/core-2.2.0/Mutex.html#method-i-synchronize)? [synchronized-method-for-concurrency-in-ruby](https://stackoverflow.com/questions/14090731/synchronized-method-for-concurrency-in-ruby) – max pleaner Jun 03 '17 at 04:52

2 Answers2

1

Your problem is that there's a race condition between SPOP and SADD. You should run these two commands in a transaction. With Redis, you can achieve that with Lua scripting, which ensures the whole script runs atomically on the server side.

-- transaction.lua
redis.replicate_commands() -- see https://redis.io/commands/eval#replicating-commands-instead-of-scripts for details

local uuid = ARGV[1]    -- pass uuid by script's arguments
local member = redis.call('SPOP', 'seeks')
if (member) then
    return member    -- get an exist uuid
else
    redis.call('SADD', 'seeks', uuid)   -- add it to the set
end   -- the whole script runs in a transaction
for_stack
  • 21,012
  • 4
  • 35
  • 48
1

I am hoping you have monitored your logs. Apart from using transactions, you can also use a spin lock to handle race conditions in redis. You can refer this article for further details : http://hoyvinglavin.com/2012/04/29/redis-as-a-mutex-service/. But typically, this is how you would have modeled your code to solve the problem at hand.

class Seek

  def self.create(uuid)

    if opponent = REDIS.spop("seeks")
      Game.start(uuid, opponent)
    else
      #Check if key(lock) exists in redis. If not then proceed ahead
      #Set key(acquire lock). Raise exception in case redis set key does not return true
      REDIS.sadd("seeks", uuid) #Perform your operation
      #Delete key(release lock)
    end
  end

  def self.remove(uuid)
    REDIS.srem("seeks", uuid)
  end
end
Egalitarian
  • 2,168
  • 7
  • 24
  • 33