From fe00bf271df4504a328bc592423396bf153c2f34 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 25 May 2016 22:47:09 +0300 Subject: [PATCH] Update redis-parser to v.2.0.0 Update all code accordingly --- README.md | 87 +++++++++++++++++-------------------- benchmarks/multi_bench.js | 2 +- changelog.md | 14 ++++++ index.js | 15 +++---- lib/customErrors.js | 57 +++++------------------- package.json | 5 ++- test/custom_errors.spec.js | 89 ++++++++++++++++++++++++++++++++++++++ test/node_redis.spec.js | 8 +++- 8 files changed, 171 insertions(+), 106 deletions(-) create mode 100644 test/custom_errors.spec.js diff --git a/README.md b/README.md index afe1c3a..f1680c6 100644 --- a/README.md +++ b/README.md @@ -182,8 +182,8 @@ __Tip:__ If the Redis server runs on the same machine as the client consider usi | port | 6379 | Port of the Redis server | | path | null | The UNIX socket string of the Redis server | | url | null | The URL of the Redis server. Format: `[redis:]//[[user][:password@]][host][:port][/db-number][?db=db-number[&password=bar[&option=value]]]` (More info avaliable at [IANA](http://www.iana.org/assignments/uri-schemes/prov/redis)). | -| parser | hiredis | If hiredis is not installed, automatic fallback to the built-in javascript parser | -| string_numbers | null | Set to `true`, `node_redis` will return Redis number values as Strings instead of javascript Numbers. Useful if you need to handle big numbers (above `Number.MAX_SAFE_INTEGER === 2^53`). Hiredis is incapable of this behavior, so setting this option to `true` will result in the built-in javascript parser being used no matter the value of the `parser` option. | +| parser | javascript | __Deprecated__ Use either the built-in JS parser [`javascript`]() or the native [`hiredis`]() parser. __Note__ `node_redis` < 2.6 uses hiredis as default if installed. This changed in v.2.6.0. | +| string_numbers | null | Set to `true`, `node_redis` will return Redis number values as Strings instead of javascript Numbers. Useful if you need to handle big numbers (above `Number.MAX_SAFE_INTEGER === 2^53`). Hiredis is incapable of this behavior, so setting this option to `true` will result in the built-in javascript parser being used no matter the value of the `parser` option. | | return_buffers | false | If set to `true`, then all replies will be sent to callbacks as Buffers instead of Strings. | | detect_buffers | false | If set to `true`, then replies will be sent to callbacks as Buffers. This option lets you switch between Buffers and Strings on a per-command basis, whereas `return_buffers` applies to every command on a client. __Note__: This doesn't work properly with the pubsub mode. A subscriber has to either always return Strings or Buffers. | | socket_keepalive | true | If set to `true`, the keep-alive functionality is enabled on the underlying socket. | @@ -712,56 +712,47 @@ client.zadd(args, function (err, response) { ## Performance Much effort has been spent to make `node_redis` as fast as possible for common -operations. As pipelining happens naturally from shared connections, overall -efficiency goes up. - -Here are results of `multi_bench.js` which is similar to `redis-benchmark` from the Redis distribution. - -hiredis parser (Lenovo T450s i7-5600U): +operations. ``` -Client count: 1, node version: 4.2.2, server version: 3.0.3, parser: hiredis - PING, 1/1 min/max/avg: 0/ 2/ 0.02/ 2501ms total, 47503.80 ops/sec - PING, batch 50/1 min/max/avg: 0/ 2/ 0.09/ 2501ms total, 529668.13 ops/sec - SET 4B str, 1/1 min/max/avg: 0/ 2/ 0.02/ 2501ms total, 41900.04 ops/sec - SET 4B str, batch 50/1 min/max/avg: 0/ 2/ 0.14/ 2501ms total, 354658.14 ops/sec - SET 4B buf, 1/1 min/max/avg: 0/ 4/ 0.04/ 2501ms total, 23499.00 ops/sec - SET 4B buf, batch 50/1 min/max/avg: 0/ 2/ 0.31/ 2501ms total, 159836.07 ops/sec - GET 4B str, 1/1 min/max/avg: 0/ 4/ 0.02/ 2501ms total, 43489.80 ops/sec - GET 4B str, batch 50/1 min/max/avg: 0/ 2/ 0.11/ 2501ms total, 444202.32 ops/sec - GET 4B buf, 1/1 min/max/avg: 0/ 3/ 0.02/ 2501ms total, 38561.38 ops/sec - GET 4B buf, batch 50/1 min/max/avg: 0/ 2/ 0.11/ 2501ms total, 452139.14 ops/sec - SET 4KiB str, 1/1 min/max/avg: 0/ 2/ 0.03/ 2501ms total, 32990.80 ops/sec - SET 4KiB str, batch 50/1 min/max/avg: 0/ 2/ 0.34/ 2501ms total, 146161.54 ops/sec - SET 4KiB buf, 1/1 min/max/avg: 0/ 1/ 0.04/ 2501ms total, 23294.28 ops/sec - SET 4KiB buf, batch 50/1 min/max/avg: 0/ 2/ 0.36/ 2501ms total, 137584.97 ops/sec - GET 4KiB str, 1/1 min/max/avg: 0/ 2/ 0.03/ 2501ms total, 36350.66 ops/sec - GET 4KiB str, batch 50/1 min/max/avg: 0/ 2/ 0.32/ 2501ms total, 155157.94 ops/sec - GET 4KiB buf, 1/1 min/max/avg: 0/ 4/ 0.02/ 2501ms total, 39776.49 ops/sec - GET 4KiB buf, batch 50/1 min/max/avg: 0/ 2/ 0.32/ 2501ms total, 155457.82 ops/sec - INCR, 1/1 min/max/avg: 0/ 3/ 0.02/ 2501ms total, 43972.41 ops/sec - INCR, batch 50/1 min/max/avg: 0/ 1/ 0.12/ 2501ms total, 425809.68 ops/sec - LPUSH, 1/1 min/max/avg: 0/ 2/ 0.02/ 2501ms total, 38998.40 ops/sec - LPUSH, batch 50/1 min/max/avg: 0/ 4/ 0.14/ 2501ms total, 365013.99 ops/sec - LRANGE 10, 1/1 min/max/avg: 0/ 2/ 0.03/ 2501ms total, 31879.25 ops/sec - LRANGE 10, batch 50/1 min/max/avg: 0/ 1/ 0.32/ 2501ms total, 153698.52 ops/sec - LRANGE 100, 1/1 min/max/avg: 0/ 4/ 0.06/ 2501ms total, 16676.13 ops/sec - LRANGE 100, batch 50/1 min/max/avg: 1/ 6/ 2.03/ 2502ms total, 24520.38 ops/sec - SET 4MiB str, 1/1 min/max/avg: 1/ 6/ 2.11/ 2502ms total, 472.82 ops/sec - SET 4MiB str, batch 20/1 min/max/avg: 85/ 112/ 94.93/ 2563ms total, 210.69 ops/sec - SET 4MiB buf, 1/1 min/max/avg: 1/ 8/ 2.02/ 2502ms total, 490.01 ops/sec - SET 4MiB buf, batch 20/1 min/max/avg: 37/ 52/ 39.48/ 2528ms total, 506.33 ops/sec - GET 4MiB str, 1/1 min/max/avg: 3/ 13/ 5.26/ 2504ms total, 190.10 ops/sec - GET 4MiB str, batch 20/1 min/max/avg: 70/ 106/ 89.36/ 2503ms total, 223.73 ops/sec - GET 4MiB buf, 1/1 min/max/avg: 3/ 11/ 5.04/ 2502ms total, 198.24 ops/sec - GET 4MiB buf, batch 20/1 min/max/avg: 70/ 105/ 88.07/ 2554ms total, 227.09 ops/sec +Lenovo T450s, i7-5600U and 12gb memory +clients: 1, NodeJS: 6.2.0, Redis: 3.2.0, parser: javascript, connected by: tcp + PING, 1/1 avg/max: 0.02/ 5.26 2501ms total, 46916 ops/sec + PING, batch 50/1 avg/max: 0.06/ 4.35 2501ms total, 755178 ops/sec + SET 4B str, 1/1 avg/max: 0.02/ 4.75 2501ms total, 40856 ops/sec + SET 4B str, batch 50/1 avg/max: 0.11/ 1.51 2501ms total, 432727 ops/sec + SET 4B buf, 1/1 avg/max: 0.05/ 2.76 2501ms total, 20659 ops/sec + SET 4B buf, batch 50/1 avg/max: 0.25/ 1.76 2501ms total, 194962 ops/sec + GET 4B str, 1/1 avg/max: 0.02/ 1.55 2501ms total, 45156 ops/sec + GET 4B str, batch 50/1 avg/max: 0.09/ 3.15 2501ms total, 524110 ops/sec + GET 4B buf, 1/1 avg/max: 0.02/ 3.07 2501ms total, 44563 ops/sec + GET 4B buf, batch 50/1 avg/max: 0.10/ 3.18 2501ms total, 473171 ops/sec + SET 4KiB str, 1/1 avg/max: 0.03/ 1.54 2501ms total, 32627 ops/sec + SET 4KiB str, batch 50/1 avg/max: 0.34/ 1.89 2501ms total, 146861 ops/sec + SET 4KiB buf, 1/1 avg/max: 0.05/ 2.85 2501ms total, 20688 ops/sec + SET 4KiB buf, batch 50/1 avg/max: 0.36/ 1.83 2501ms total, 138165 ops/sec + GET 4KiB str, 1/1 avg/max: 0.02/ 1.37 2501ms total, 39389 ops/sec + GET 4KiB str, batch 50/1 avg/max: 0.24/ 1.81 2501ms total, 208157 ops/sec + GET 4KiB buf, 1/1 avg/max: 0.02/ 2.63 2501ms total, 39918 ops/sec + GET 4KiB buf, batch 50/1 avg/max: 0.31/ 8.56 2501ms total, 161575 ops/sec + INCR, 1/1 avg/max: 0.02/ 4.69 2501ms total, 45685 ops/sec + INCR, batch 50/1 avg/max: 0.09/ 3.06 2501ms total, 539964 ops/sec + LPUSH, 1/1 avg/max: 0.02/ 3.04 2501ms total, 41253 ops/sec + LPUSH, batch 50/1 avg/max: 0.12/ 1.94 2501ms total, 425090 ops/sec + LRANGE 10, 1/1 avg/max: 0.02/ 2.28 2501ms total, 39850 ops/sec + LRANGE 10, batch 50/1 avg/max: 0.25/ 1.85 2501ms total, 194302 ops/sec + LRANGE 100, 1/1 avg/max: 0.05/ 2.93 2501ms total, 21026 ops/sec + LRANGE 100, batch 50/1 avg/max: 1.52/ 2.89 2501ms total, 32767 ops/sec + SET 4MiB str, 1/1 avg/max: 5.16/ 15.55 2502ms total, 193 ops/sec + SET 4MiB str, batch 20/1 avg/max: 89.73/ 99.96 2513ms total, 223 ops/sec + SET 4MiB buf, 1/1 avg/max: 2.23/ 8.35 2501ms total, 446 ops/sec + SET 4MiB buf, batch 20/1 avg/max: 41.47/ 50.91 2530ms total, 482 ops/sec + GET 4MiB str, 1/1 avg/max: 2.79/ 10.91 2502ms total, 358 ops/sec + GET 4MiB str, batch 20/1 avg/max: 101.61/118.11 2541ms total, 197 ops/sec + GET 4MiB buf, 1/1 avg/max: 2.32/ 14.93 2502ms total, 430 ops/sec + GET 4MiB buf, batch 20/1 avg/max: 65.01/ 84.72 2536ms total, 308 ops/sec ``` -The hiredis and js parser should most of the time be on the same level. But if you use Redis for big SUNION/SINTER/LRANGE/ZRANGE hiredis is faster. -Therefor the hiredis parser is the default used in node_redis. To use `hiredis`, do: - - npm install hiredis redis - ## Debugging To get debug output run your `node_redis` application with `NODE_DEBUG=redis`. diff --git a/benchmarks/multi_bench.js b/benchmarks/multi_bench.js index c65876b..a79d92e 100644 --- a/benchmarks/multi_bench.js +++ b/benchmarks/multi_bench.js @@ -26,7 +26,7 @@ var run_time = returnArg('time', 2500); // ms var pipeline = returnArg('pipeline', 1); // number of concurrent commands var versions_logged = false; var client_options = { - parser: returnArg('parser', 'hiredis'), + parser: returnArg('parser', 'javascript'), path: returnArg('socket') // '/tmp/redis.sock' }; var small_str, large_str, small_buf, large_buf, very_large_str, very_large_buf; diff --git a/changelog.md b/changelog.md index 168a185..ce2f603 100644 --- a/changelog.md +++ b/changelog.md @@ -1,6 +1,20 @@ Changelog ========= +## v.2.6.0 - 25 Mai, 2016 + +In addition to the pre-releases the following changes exist in v.2.6.0: + +Features + +- Updated [redis-parser](https://github.com/NodeRedis/redis-parser) dependency ([changelog](https://github.com/NodeRedis/redis-parser/releases/tag/v.2.0.0)) + - The JS parser is from now on the new default as it is a lot faster than the hiredis parser + - This is no BC as there is no changed behavior for the user at all but just a performance improvement. Explicitly requireing the Hiredis parser is still possible. + +Deprecations + +- The `parser` option is deprecated and should be removed. The built-in Javascript parser is a lot faster than the hiredis parser and has more features + ## v.2.6.0-2 - 29 Apr, 2016 Features diff --git a/index.js b/index.js index a4957bd..406c8c8 100644 --- a/index.js +++ b/index.js @@ -154,11 +154,11 @@ function RedisClient (options, stream) { this.pipeline = false; this.sub_commands_left = 0; this.times_connected = 0; - this.options = options; this.buffers = options.return_buffers || options.detect_buffers; + this.options = options; this.reply = 'ON'; // Returning replies is the default // Init parser - this.reply_parser = create_parser(this, options); + this.reply_parser = create_parser(this); this.create_stream(); // The listeners will not be attached right away, so let's print the deprecation message while the listener is attached this.on('newListener', function (event) { @@ -184,18 +184,17 @@ util.inherits(RedisClient, EventEmitter); RedisClient.connection_id = 0; function create_parser (self) { - return Parser({ + return new Parser({ returnReply: function (data) { self.return_reply(data); }, returnError: function (err) { // Return a ReplyError to indicate Redis returned an error - self.return_error(new errorClasses.ReplyError(err)); + self.return_error(err); }, returnFatalError: function (err) { // Error out all fired commands. Otherwise they might rely on faulty data. We have to reconnect to get in a working state again // Note: the execution order is important. First flush and emit, then create the stream - err = new errorClasses.ReplyError(err); err.message += '. Please report this.'; self.ready = false; self.flush_and_error({ @@ -209,8 +208,8 @@ function create_parser (self) { self.create_stream(); }, returnBuffers: self.buffers || self.message_buffers, - name: self.options.parser, - stringNumbers: self.options.string_numbers + name: self.options.parser || 'javascript', + stringNumbers: self.options.string_numbers || false }); } @@ -1093,7 +1092,7 @@ exports.RedisClient = RedisClient; exports.print = utils.print; exports.Multi = require('./lib/multi'); exports.AbortError = errorClasses.AbortError; -exports.ReplyError = errorClasses.ReplyError; +exports.ReplyError = Parser.ReplyError; exports.AggregateError = errorClasses.AggregateError; // Add all redis commands / node_redis api to the client diff --git a/lib/customErrors.js b/lib/customErrors.js index 7c6e457..0b192f7 100644 --- a/lib/customErrors.js +++ b/lib/customErrors.js @@ -4,75 +4,42 @@ var util = require('util'); function AbortError (obj) { Error.captureStackTrace(this, this.constructor); - var message; Object.defineProperty(this, 'name', { - get: function () { - return this.constructor.name; - } + value: 'AbortError', + configurable: true, + writable: true }); Object.defineProperty(this, 'message', { - get: function () { - return message; - }, - set: function (msg) { - message = msg; - } + value: obj.message || '', + configurable: true, + writable: true }); for (var keys = Object.keys(obj), key = keys.pop(); key; key = keys.pop()) { this[key] = obj[key]; } - // Explicitly add the message - // If the obj is a error itself, the message is not enumerable - this.message = obj.message; -} - -function ReplyError (obj) { - Error.captureStackTrace(this, this.constructor); - var tmp; - Object.defineProperty(this, 'name', { - get: function () { - return this.constructor.name; - } - }); - Object.defineProperty(this, 'message', { - get: function () { - return tmp; - }, - set: function (msg) { - tmp = msg; - } - }); - this.message = obj.message; } function AggregateError (obj) { Error.captureStackTrace(this, this.constructor); - var tmp; Object.defineProperty(this, 'name', { - get: function () { - return this.constructor.name; - } + value: 'AggregateError', + configurable: true, + writable: true }); Object.defineProperty(this, 'message', { - get: function () { - return tmp; - }, - set: function (msg) { - tmp = msg; - } + value: obj.message || '', + configurable: true, + writable: true }); for (var keys = Object.keys(obj), key = keys.pop(); key; key = keys.pop()) { this[key] = obj[key]; } - this.message = obj.message; } -util.inherits(ReplyError, Error); util.inherits(AbortError, Error); util.inherits(AggregateError, AbortError); module.exports = { - ReplyError: ReplyError, AbortError: AbortError, AggregateError: AggregateError }; diff --git a/package.json b/package.json index 15ef0ba..82e7f2b 100644 --- a/package.json +++ b/package.json @@ -21,12 +21,13 @@ "coverage": "nyc report --reporter=html", "benchmark": "node benchmarks/multi_bench.js", "test": "nyc --cache mocha ./test/*.js ./test/commands/*.js --timeout=8000", - "posttest": "eslint . --fix" + "posttest": "eslint . --fix && npm run coverage", + "compare": "node benchmarks/diff_multi_bench_output.js beforeBench.txt afterBench.txt" }, "dependencies": { "double-ended-queue": "^2.1.0-0", "redis-commands": "^1.2.0", - "redis-parser": "^1.3.0" + "redis-parser": "^2.0.0" }, "engines": { "node": ">=0.10.0" diff --git a/test/custom_errors.spec.js b/test/custom_errors.spec.js new file mode 100644 index 0000000..d7c6ebb --- /dev/null +++ b/test/custom_errors.spec.js @@ -0,0 +1,89 @@ +'use strict'; + +var assert = require('assert'); +var errors = require('../lib/customErrors'); + +describe('errors', function () { + + describe('AbortError', function () { + it('should inherit from Error', function () { + var e = new errors.AbortError({}); + assert.strictEqual(e.message, ''); + assert.strictEqual(e.name, 'AbortError'); + assert.strictEqual(Object.keys(e).length, 0); + assert(e instanceof Error); + assert(e instanceof errors.AbortError); + }); + + it('should list options properties but not name and message', function () { + var e = new errors.AbortError({ + name: 'weird', + message: 'hello world', + property: true + }); + assert.strictEqual(e.message, 'hello world'); + assert.strictEqual(e.name, 'weird'); + assert.strictEqual(e.property, true); + assert.strictEqual(Object.keys(e).length, 1); + assert(e instanceof Error); + assert(e instanceof errors.AbortError); + assert(delete e.name); + assert.strictEqual(e.name, 'Error'); + }); + + it('should change name and message', function () { + var e = new errors.AbortError({ + message: 'hello world', + property: true + }); + assert.strictEqual(e.name, 'AbortError'); + assert.strictEqual(e.message, 'hello world'); + e.name = 'foo'; + e.message = 'foobar'; + assert.strictEqual(e.name, 'foo'); + assert.strictEqual(e.message, 'foobar'); + }); + }); + + describe('AggregateError', function () { + it('should inherit from Error and AbortError', function () { + var e = new errors.AggregateError({}); + assert.strictEqual(e.message, ''); + assert.strictEqual(e.name, 'AggregateError'); + assert.strictEqual(Object.keys(e).length, 0); + assert(e instanceof Error); + assert(e instanceof errors.AggregateError); + assert(e instanceof errors.AbortError); + }); + + it('should list options properties but not name and message', function () { + var e = new errors.AggregateError({ + name: 'weird', + message: 'hello world', + property: true + }); + assert.strictEqual(e.message, 'hello world'); + assert.strictEqual(e.name, 'weird'); + assert.strictEqual(e.property, true); + assert.strictEqual(Object.keys(e).length, 1); + assert(e instanceof Error); + assert(e instanceof errors.AggregateError); + assert(e instanceof errors.AbortError); + assert(delete e.name); + assert.strictEqual(e.name, 'Error'); + }); + + it('should change name and message', function () { + var e = new errors.AggregateError({ + message: 'hello world', + property: true + }); + assert.strictEqual(e.name, 'AggregateError'); + assert.strictEqual(e.message, 'hello world'); + e.name = 'foo'; + e.message = 'foobar'; + assert.strictEqual(e.name, 'foo'); + assert.strictEqual(e.message, 'foobar'); + }); + }); +}); diff --git a/test/node_redis.spec.js b/test/node_redis.spec.js index 8ba91fd..cb36b6c 100644 --- a/test/node_redis.spec.js +++ b/test/node_redis.spec.js @@ -998,8 +998,12 @@ describe('The node_redis client', function () { assert(err instanceof redis.AbortError); error = err.origin; }); - // Fail the set answer. Has no corresponding command obj and will therefore land in the error handler and set - client.reply_parser.execute(new Buffer('a*1\r*1\r$1`zasd\r\na')); + // Make sure we call execute out of the reply + // ready is called in a reply + process.nextTick(function () { + // Fail the set answer. Has no corresponding command obj and will therefore land in the error handler and set + client.reply_parser.execute(new Buffer('a*1\r*1\r$1`zasd\r\na')); + }); }); }); });