Dedupe multiple destinations on key lookups after failed proxy

esatterwhite · May 27, 2015 · 441a439 · 441a439
1 parent 0abc631
commit 441a439
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 4 deletions.
diff --git a/lib/request-proxy/send.js b/lib/request-proxy/send.js
@@ -162,13 +162,13 @@ RequestProxySend.prototype.handleSuccess = function handleSuccess(res1, res2, ca
 };
 
 RequestProxySend.prototype.lookupKeys = function lookupKeys(keys) {
-    var dests = [];
+    var dests = {};
 
     for (var i = 0; i < keys.length; i++) {
-        dests.push(this.ringpop.lookup(keys[i]));
+        dests[this.ringpop.lookup(keys[i])] = true;
     }
 
-    return dests;
+    return Object.keys(dests);
 };
 
 RequestProxySend.prototype.rerouteRetry = function rerouteRetry(newDest, callback) {

diff --git a/test/integration/proxy-test.js b/test/integration/proxy-test.js
@@ -33,8 +33,8 @@ var retrySchedule = [0, 0.01, 0.02];
 
 function scheduleTicker(ringpop, onScheduled) {
     return function tickIt() {
-        ringpop.timers.advance(10000);
         onScheduled();
+        ringpop.timers.advance(10000);
     };
 }
 
@@ -522,6 +522,47 @@ test('aborts retry because keys diverge', function t(assert) {
     });
 });
 
+test('retries multiple keys w/ same dest', function t(assert) {
+    assert.plan(4);
+
+    var numRetries = 0;
+
+    var cluster = allocCluster({
+        useFakeTimers: true
+    }, function onReady() {
+        // Make node two refuse initial request
+        cluster.two.ring.checksum = cluster.one.ring.checksum + 1;
+
+        cluster.one.on('requestProxy.retryAborted', function onRetryAborted() {
+            assert.fail('retry aborted');
+        });
+
+        cluster.one.on('requestProxy.retryAttempted', function onRetryAttempted() {
+            numRetries++;
+        });
+
+        cluster.one.on('requestProxy.retryScheduled', function onRetryScheduled() {
+            cluster.one.timers.advance(10000);
+        });
+
+        cluster.requestAll({
+            keys: [cluster.keys.two, cluster.keys.two, cluster.keys.two],
+            host: 'one',
+            maxRetries: 5,
+            retrySchedule: [1]
+        }, function onRequest(err, responses) {
+            assert.ifError(err, 'no error occurs');
+            assert.equal(responses.length, 1, 'one response');
+            assert.equal(responses[0].res.statusCode, 500, '500 status code');
+
+            assert.equal(numRetries, 5, 'retried much');
+
+            cluster.destroy();
+            assert.end();
+        });
+    });
+});
+
 test('reroutes retry to local', function t(assert) {
     assert.plan(3);