elad / node-cluster-socket.io

Writeup on how to make node.js cluster and socket.io play nice
421 stars 63 forks source link

worker_index doesn't work for ipv6 #6

Open barisusakli opened 9 years ago

barisusakli commented 9 years ago

With an ip address of ::ffff:127.0.0.1 worker_index returns NaN

I changed it to :

var worker_index = function(ip, len) {
    var s = '';
    for (var i = 0, _len = ip.length; i < _len; i++) {
         if (parseInt(ip[i], 10)) {
              s += ip[i];
         }
     }

     return Number(s) % len || 0;
};

Not sure how bad that effects performance.

elad commented 9 years ago

Why not if (ip[i] >= '0' && ip[i] <= '9') instead?

barisusakli commented 9 years ago

Yeah I guess that works as well not sure which one is faster.

barisusakli commented 9 years ago

Although that will skip [a-f] on ipv6 addresses which might effect distribution. (mine does skip them as well) :laughing:

elad commented 9 years ago

We'll have to adjust the other hashing functions to also work for IPv6 and then compare...

etergen commented 9 years ago

As a solution to the IPv6 problem, I'm using the following code:

function getWorker(ip, len) {
  var _ip = ip.split(/['.'|':']/),
      arr = [];

  for (el in _ip) {
    if (_ip[el] == '') {
      arr.push(0);
    } else {
      arr.push(parseInt(_ip[el], 16));
  }

  return Number(arr.join('')) % len;
}

Please let me know your thoughts

uqee commented 9 years ago

Guys, have a look at: https://github.com/uqee/sticky-cluster for ipv6 support.

elad commented 9 years ago

@uqee we can easily make the example code in this module work with IPv6 if we replace the embedded hashing function with string-hash like in your project and then I believe performance will be comparable.

uqee commented 9 years ago

@elad, yes, you're definitely right

wvffle commented 8 years ago

What about this one? Would it damage the performance?

var worker_index = function(ip, len) {
        var s = '';
        for (var i = 0; i < ip.length; i++) {
            if (!isNaN(ip[i])) {
                s += ip[i];
            }
        }
        return Number(s) % len;
    };
Gnucki commented 8 years ago

I use that code to handle IPv4 and IPv6. Maybe I missed something but it seems much faster.

var worker_index = function(ip, len) {
    return Number('0' + s.replace(/[abcdef:.]/g, '')) % len;
};

http://jsperf.com/ip-string-replace

basickarl commented 7 years ago

Was there an official answer to this btw?