node_redis_cluster
node_redis_cluster copied to clipboard
[*] changed algorithm for hashing to native redis crc16 CCIT Xmodem
Not cool, doing the V8 - Native ... Native - V8 rountrips makes computing the crc16 significantly slower than computing it in JS, bringing native code in this module will make it impossible to use in environments where people do not have build tools (which is a lot), and if the crc16 algorithm has a problem there must be a fix we can apply to it, replacing it for a native C crc16 doesn't seem a good idea.
There is no solution in JS for utf-8 multibyte crc16...
Try this test:
assert.equal(crc.crc16(new Buffer('123456789')).toString(16), '31c3'); assert.equal(crc.crc16(new Buffer('привет')) % 16384, 7365); assert.equal(crc.crc16(new Buffer('nht.reach.accounts:زووم')) % 16384, 4107);
I checked 5 JS libraries and all checked libs doing it wrong...
check issue #9
I understand that, still don't think it justifies bringing a native module to the dependencies. Options are 1) fix one of the 5 libraries that don't work 2) accept this as a caveat and tell people to use ASCII keys, shouldn't be too hard.
Ok, I'll try to fix it
Arseniy Pavlenko | CTO
M. +972-54-5417509
scrnz.com
On Fri, Jan 23, 2015 at 1:32 PM, João Jerónimo [email protected] wrote:
I understand that, still don't think it justifies bringing a native module to the dependencies. Options are 1) fix one of the 5 libraries that don't work 2) accept this as a caveat and tell people to use ASCII keys, shouldn't be too hard.
Reply to this email directly or view it on GitHub: https://github.com/joaojeronimo/node_redis_cluster/pull/10#issuecomment-71181388