user3490755
user3490755

Reputation: 995

NodeJS getting stuck in callback

Im having troubles where my node app all a sudden starts to consume a lot of CPU. Im suspecting that the function below gets stuck somehow..

Client.prototype.countActiveChatsRedis = function (userID, agentID, obj, callback) {
    var count = 0;

    pub.keys("widgetActive:" + userID + ":*", function(err, key) {

        if(err !== null) {
            console.log("Redis error..... --> " + err);
            callback(count, obj);           
        }

        if(key && key.length > 0) {
            pub.mget(key, function(err, data) {
                if(data) {
                    for(var i = 0; i < data.length;i++) {
                        if(data[i]) {
                            var arr = data[i].split(",");

                            if(arr[2] == agentID) {
                                if (Number(arr[3]) > 0) {
                                    count++;
                                }                           
                            }
                        }
                    }

                    callback(count, obj);
                }
            });         
        } else {
            callback(count, obj);           
        }
    });
}

Any ideas what the problem could be? Any case where it could avoid sending the callback?

This function runs around 50 times per second.

Upvotes: 0

Views: 1725

Answers (3)

Ben
Ben

Reputation: 5074

It's stucked because in the case where there is no data, no callback is being called.

pub.mget(key, function(err, data) {
   if(data) {
     for(var i = 0; i < data.length;i++) {
        if(data[i]) {
          var arr = data[i].split(",");

          if(arr[2] == agentID) {
            if (Number(arr[3]) > 0) {
               count++;
            }                           
          }
        }
     }

     // callback(count, obj);  // <==== move this callback outside of if (data)
   }
   callback(count, obj);  // this ensures that callback always gets called

 });       

Upvotes: 0

mesh
mesh

Reputation: 113

It is bad practice to use KEYS in a production environment. To quote the Redis master himself:

Warning: consider KEYS as a command that should only be used in production environments with extreme care. It may ruin performance when it is executed against large databases. This command is intended for debugging and special operations, such as changing your keyspace layout. Don't use KEYS in your regular application code. If you're looking for a way to find keys in a subset of your keyspace, consider using SCAN or sets.

Whenever you add a key with this prefix, just add it to a set called "widgetActive" the user's id or any other data you need. you can also use HASH if you need to save some data for each entry.

Upvotes: 3

Martin Golpashin
Martin Golpashin

Reputation: 1062

you should always return your callbacks to make sure that your function terminates correctly and returns the control to the calling context:

Client.prototype.countActiveChatsRedis = function (userID, agentID, obj, callback) {
var count = 0;

pub.keys("widgetActive:" + userID + ":*", function(err, key) {

    if(err !== null) {
        console.log("Redis error..... --> " + err);
        return callback(count, obj);           
    }

    if(key && key.length > 0) {
        pub.mget(key, function(err, data) {
            if(data) {
                for(var i = 0; i < data.length;i++) {
                    if(data[i]) {
                        var arr = data[i].split(",");

                        if(arr[2] == agentID) {
                            if (Number(arr[3]) > 0) {
                                count++;
                            }                           
                        }
                    }
                }

                return callback(count, obj);
            }
        });         
    } else {
        return callback(count, obj);           
    }
});
}

Upvotes: 0

Related Questions