fix: clean up keys in worker (#31)

screwdriver-cd · Jun 12, 2018 · f6839b1 · f6839b1
1 parent b1a6e9e
commit f6839b1
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 7 deletions.
diff --git a/index.js b/index.js
@@ -311,6 +311,7 @@ class ExecutorQueue extends Executor {
             blockedBy: blockedBy.toString()
         }]);
         const deleteKey = `deleted_${jobId}_${buildId}`;
+        let started = true;
 
         // This is to prevent the case where a build is aborted while still in buildQueue
         // The job might be picked up by the worker, so it's not deleted from buildQueue here
@@ -319,16 +320,16 @@ class ExecutorQueue extends Executor {
         await this.redisBreaker.runCommand('set', deleteKey, '');
         await this.redisBreaker.runCommand('expire', deleteKey, EXPIRE_TIME);
 
-        if (numDeleted !== 0) {
-            // Build hadn't been started, "start" event was removed from queue
-            return this.redisBreaker.runCommand('hdel', this.buildConfigTable, buildId);
+        if (numDeleted !== 0) { // build hasn't started
+            started = false;
         }
 
         // "start" event has been processed, need worker to stop the executor
         return this.queueBreaker.runCommand('enqueue', this.buildQueue, 'stop', [{
             buildId,
             jobId,
-            blockedBy: blockedBy.toString()
+            blockedBy: blockedBy.toString(),
+            started // call executor.stop if the job already started
         }]);
     }
 

diff --git a/test/index.test.js b/test/index.test.js
@@ -342,24 +342,25 @@ describe('index test', () => {
 
         it('removes a start event from the queue and the cached buildconfig', () => {
             const deleteKey = `deleted_${jobId}_${buildId}`;
+            const stopConfig = Object.assign({ started: false }, partialTestConfigToString);
 
             return executor.stop(partialTestConfig).then(() => {
                 assert.calledOnce(queueMock.connect);
                 assert.calledWith(queueMock.del, 'builds', 'start', [partialTestConfigToString]);
-                assert.calledWith(redisMock.hdel, 'buildConfigs', buildId);
                 assert.calledWith(redisMock.set, deleteKey, '');
                 assert.calledWith(redisMock.expire, deleteKey, 1800);
-                assert.notCalled(queueMock.enqueue);
+                assert.calledWith(queueMock.enqueue, 'builds', 'stop', [stopConfig]);
             });
         });
 
         it('adds a stop event to the queue if no start events were removed', () => {
             queueMock.del.yieldsAsync(null, 0);
+            const stopConfig = Object.assign({ started: true }, partialTestConfigToString);
 
             return executor.stop(partialTestConfig).then(() => {
                 assert.calledOnce(queueMock.connect);
                 assert.calledWith(queueMock.del, 'builds', 'start', [partialTestConfigToString]);
-                assert.calledWith(queueMock.enqueue, 'builds', 'stop', [partialTestConfigToString]);
+                assert.calledWith(queueMock.enqueue, 'builds', 'stop', [stopConfig]);
             });
         });