Node-redis: SETNX/SET with 'NX' option not atomic?

[tciuro]

Hello,

I thought that setnx() or set with the NX option was atomic, but what I'm seeing doesn't match that expectation.

Setup:

Node v0.10.32
redis 0.12.1
Redis 2.8.12

My node app uses the cluster API. On start, there are several maintenance functions that need to run. In order to avoid more than one worker doing the same thing, I have a setup function that uses setnx (I've also tried set with the NX option) to grant that responsibility to the first worker asking for it:

Manager.prototype.once = function (label, cb) {
    console.log('[' + label + '] setting: ' + 'delegation:' + label + ' -> ' + cluster.worker.id);

    this.redis.set('delegation:' + label, cluster.worker.id, 'NX', function (err, reply) {
        if (err) {
            console.error('[' + label + ' - ' + cluster.worker.id + '] setnx error: ' + err);
            return cb(err);
        }

        console.log('[' + label + '] reply: ' + 'delegation:' + label + ' -> ' + reply);

        if (reply) {
            // I won, so execute!
            console.log('[' + label + ' - ' + cluster.worker.id + '] setnx set!: ' + reply);
            return cb();
        } else {
            // I lost, some other process has already done it; do nothing
            console.log('[' + label + ' - ' + cluster.worker.id + '] setnx conflict!...');
            return cb({
                status: 409,
                message: 'Conflict: another process has processed label: ' + label
            });
        }
    });
};

Assume 8 workers. Unless I'm mistaken, I should see 1 setnx set!: OK and 7 setnx conflict!... because only the first worker would be successfully setting the value. But this is not what I'm seeing. Instead, I see this:

[socketioTraffic] setting: delegation:socketioTraffic -> 3
[socketioTraffic] setting: delegation:socketioTraffic -> 4
[socketioTraffic] setting: delegation:socketioTraffic -> 1
[socketioTraffic] setting: delegation:socketioTraffic -> 6
[socketioTraffic] setting: delegation:socketioTraffic -> 8
[socketioTraffic] setting: delegation:socketioTraffic -> 2
[socketioTraffic] setting: delegation:socketioTraffic -> 7
[socketioTraffic] setting: delegation:socketioTraffic -> 5
[socketioTraffic] reply: delegation:socketioTraffic -> OK
[socketioTraffic - 3] setnx set!: OK
[socketioTraffic] reply: delegation:socketioTraffic -> OK
[socketioTraffic - 4] setnx set!: OK
[socketioTraffic] reply: delegation:socketioTraffic -> null
[socketioTraffic - 2] setnx conflict!...
[socketioTraffic] reply: delegation:socketioTraffic -> null
[socketioTraffic - 1] setnx conflict!...
[socketioTraffic] reply: delegation:socketioTraffic -> OK
[socketioTraffic - 6] setnx set!: OK
[socketioTraffic] reply: delegation:socketioTraffic -> OK
[socketioTraffic - 8] setnx set!: OK
[socketioTraffic] reply: delegation:socketioTraffic -> null
[socketioTraffic - 7] setnx conflict!...
[socketioTraffic] reply: delegation:socketioTraffic -> OK
[socketioTraffic - 5] setnx set!: OK

So, the 8 workers are trying to set the value 1 for key delegation:socketioTraffic, 5 of them succeed and 3 fail. Any idea why this is happening?

[brycebaril]

I am unable reproduce this with a quick test script... Can you do a couple things:

1. make sure that there isn't some code somewhere else that is clearing this value
2. run with the redis client with debug_mode set to true

My test script if you want to try yourself (manually switch to sync or async to confirm they both work):

var redis = require("redis")
//redis.debug_mode = true

var client = redis.createClient()
var key = "fooNX"

function sync() {
  for (var i = 0; i < 10; i++) {
    (function () {
      var j = i
      client.set(key, 1, "NX", function (err, reply) {
        console.log("try %s", j)
        console.log(err)
        console.log(reply)
      })
    })()
  } 
} 

var c = 0
function async() {
  client.set(key, 1, "NX", function (err, reply) {
    if (++c < 10) {
      async() 
    } 
    console.log("try %s", c)
    console.log(err)
    console.log(reply)
  })
} 

client.del(key, function (err, reply) {
  async()
})

[tciuro]

@brycebaril aaaarrrrghh. You are right. The culprit is your #1 suggestion: the key was being cleared somewhere else. Ugh. Thanks for taking the time to prove me wrong.