zcache - npm Package Compare versions

Comparing version 0.2.10 to 0.2.11

lib/CacheInstance.js

@@ -12,10 +12,4 @@ var events = require('events')
 function CacheInstance() {
-  this._stats = {
-    get: new metrics.Timer,
-    set: new metrics.Timer,
-    mget: new metrics.Timer,
-    mset: new metrics.Timer,
-    del: new metrics.Timer,
-    timeout: new metrics.Counter
-  }
+  this._stats = {}
+  this._timeoutCount = {}
   this._accessCount = 0
@@ -173,25 +167,21 @@ this._hitCount = 0
 /**
- * Update the stats of a certain operation on this cache.
+ * Get the stats of a certain operation.
  *
  * @param {string} op The name of the operation, e.g., get, set, etc.
- * @return {Function} A function that updates the stats and returns the parameter
- *                    that is passed into it. It is handy to chain to a promise.
+ * @return {metrics.Timer}
  */
-CacheInstance.prototype.updateStats = function(op) {
-  var startTime = Date.now()
-  var self = this
-  return function (result) {
-    if (self._stats[op]) self._stats[op].update(Date.now() - startTime)
-    return result
-  }
+CacheInstance.prototype.getStats = function (op) {
+  if (!this._stats[op]) this._stats[op] = new metrics.Timer
+  return this._stats[op]
 }
 
 /**
- * Get the stats of a certain operation.
+ * Get the timeout count of a certain operation.
  *
  * @param {string} op The name of the operation, e.g., get, set, etc.
- * @return {metrics.Timer}
+ * @return {metrics.Count}
  */
-CacheInstance.prototype.getStats = function (op) {
-  return this._stats[op]
+CacheInstance.prototype.getTimeoutCount = function (op) {
+  if (!this._timeoutCount[op]) this._timeoutCount[op] = new metrics.Counter
+  return this._timeoutCount[op]
 }
@@ -198,0 +188,0 @@

lib/RedisConnection.js

@@ -50,5 +50,4 @@ var redis = require('redis')
   this._client.setex(key, Math.floor(maxAgeMs / 1000), val,
-      this._makeNodeResolverWithTimeout(deferred, 'Redis [set] key: ' + key))
+      this._makeNodeResolverWithTimeout(deferred, 'set', 'Redis [set] key: ' + key))
   return deferred.promise
-    .then(this.updateStats('set'))
 }
@@ -71,7 +70,6 @@
   this._client.multi(commands).exec(
-      this._makeNodeResolverWithTimeout(deferred,
+      this._makeNodeResolverWithTimeout(deferred, 'mset',
       'Redis [mset] key.0: ' + items[0].key + ' key.length: ' + items.length))
 
   return deferred.promise
-    .then(this.updateStats('mset'))
 }
@@ -83,5 +81,4 @@
   this._client.del(key,
-      this._makeNodeResolverWithTimeout(deferred, 'Redis [del] key: ' + key))
+      this._makeNodeResolverWithTimeout(deferred, 'del', 'Redis [del] key: ' + key))
   return deferred.promise
-    .then(this.updateStats('del'))
 }
@@ -93,3 +90,2 @@
     .then(returnFirstResult)
-    .then(this.updateStats('get'))
 }
@@ -103,3 +99,3 @@
   this._client.mget(keys,
-      this._makeNodeResolverWithTimeout(deferred,
+      this._makeNodeResolverWithTimeout(deferred, 'mget',
       'Redis [mget] key.0: ' + keys[0] + ' key.length: ' + keys.length))
@@ -119,3 +115,2 @@ return deferred.promise
     .then(this.updateCount())
-    .then(this.updateStats('mget'))
 }
@@ -126,3 +121,3 @@
   var deferred = Q.defer()
-  this._client.info(this._makeNodeResolverWithTimeout(deferred, 'Redis [info]'))
+  this._client.info(deferred.makeNodeResolver())
   return deferred.promise
@@ -196,11 +191,15 @@ .then(function (infoCmdOutput) {
  * A helper that returns a node-style callback function with a specified timeout.
+ * It also records the response time of the request.
  *
  * @param {Promise} deferred A deferred promise.
+ * @param {string} opName The name of the operation. It should be one of these: 'get',
+ *   'mget', 'set', 'mset' and 'del'.
  * @param {string} timeoutMsg The message to throw if timeout happens.
  * @return {function(Object, Object)} A node-style callback function.
  */
-RedisConnection.prototype._makeNodeResolverWithTimeout = function (deferred, timeoutMsg) {
+RedisConnection.prototype._makeNodeResolverWithTimeout = function (deferred, opName, timeoutMsg) {
   // Indicates if this request has already timeout
   var isTimeout = false
-  var timeoutCounter = this._stats.timeout
+  var startTime = Date.now()
+  var self = this
 
@@ -210,6 +209,7 @@ var timeout = setTimeout(function() {
     isTimeout = true
-    timeoutCounter.inc()
+    self.getTimeoutCount(opName).inc()
   }, this._reqTimeoutMs)
 
   return function(err, data) {
+    self.getStats(opName).update(Date.now() - startTime)
     if (!isTimeout) {
@@ -220,4 +220,4 @@ clearTimeout(timeout)
     }
-    //TODO(Xiao): even if it's timeout, we may want to log the error message
-    //if this request finally goes through but fails.
+    // TODO(Xiao): even if it's timeout, we may want to log the error message
+    // if this request finally goes through but fails.
   }
@@ -224,0 +224,0 @@ }

package.json

 {
   "name": "zcache",
   "description": "AWS zone-aware multi-layer cache",
-  "version": "0.2.10",
+  "version": "0.2.11",
   "homepage": "https://github.com/Obvious/zcache",
@@ -6,0 +6,0 @@ "authors": [

test/test_RedisConnection.js

@@ -14,3 +14,2 @@ var zcache = require('../index')
     test.equal(cacheInstance.isAvailable(), true, 'Connection should be available')
-
     cacheInstance.set('abc', '123', 300000)
@@ -71,2 +70,3 @@ .then(function () {
         test.deepEqual(vals[3], undefined)
+
         test.equal(cacheInstance.getStats('set').count(), 1, 'set() is called for once')
@@ -79,2 +79,11 @@ test.equal(cacheInstance.getStats('mset').count(), 1, 'mset() is called for once')
         test.equal(cacheInstance.getHitCount(), 4, 'The number of cache hit is 4')
+
+        // Just print out the stats and spot check by human before we can
+        // figure out a good way to systemtically test the stats.
+        console.log('get:', cacheInstance.getPrettyStatsString('get'))
+        console.log('mget:', cacheInstance.getPrettyStatsString('mget'))
+        console.log('set:', cacheInstance.getPrettyStatsString('set'))
+        console.log('mset:', cacheInstance.getPrettyStatsString('mset'))
+        console.log('del:', cacheInstance.getPrettyStatsString('del'))
+
         return cacheInstance.getServerInfo()
@@ -81,0 +90,0 @@ })

No alert changes

Improved metrics

Total package byte prevSize

96962

increased by0.3%

Worsened metrics

Lines of code

2287

decreased by-0.13%

No dependency changes