[perf] switch write sequence for doc contents and doc tracking

Doc contents are added only after the tracking has been setup.
All read paths on the tracking have been checked to gracefully handle
 the case of existing doc_id but missing doc contents.

- getDoc: -1 operation

REF: 0a2b47c660c60b95e360d8f3b3e30b862ceb6d79

services/document-updater/app/js/RedisManager.js

@@ -84,28 +84,23 @@ module.exports = RedisManager = {
         logger.error({ err: error, doc_id, project_id }, error.message)
         return callback(error)
       }
-      const multi = rclient.multi()
-      multi.set(keys.docLines({ doc_id }), docLines)
-      multi.set(keys.projectKey({ doc_id }), project_id)
-      multi.set(keys.docVersion({ doc_id }), version)
-      multi.set(keys.docHash({ doc_id }), docHash)
-      if (ranges != null) {
-        multi.set(keys.ranges({ doc_id }), ranges)
-      } else {
-        multi.del(keys.ranges({ doc_id }))
-      }
-      multi.set(keys.pathname({ doc_id }), pathname)
-      multi.set(keys.projectHistoryId({ doc_id }), projectHistoryId)
-      return multi.exec(function (error, result) {
-        if (error != null) {
-          return callback(error)
+      // update docsInProject set before writing doc contents
+      rclient.sadd(keys.docsInProject({ project_id }), doc_id, (error) => {
+        if (error) return callback(error)
+
+        const multi = rclient.multi()
+        multi.set(keys.docLines({ doc_id }), docLines)
+        multi.set(keys.projectKey({ doc_id }), project_id)
+        multi.set(keys.docVersion({ doc_id }), version)
+        multi.set(keys.docHash({ doc_id }), docHash)
+        if (ranges != null) {
+          multi.set(keys.ranges({ doc_id }), ranges)
+        } else {
+          multi.del(keys.ranges({ doc_id }))
         }
-        // update docsInProject set
-        return rclient.sadd(
-          keys.docsInProject({ project_id }),
-          doc_id,
-          callback
-        )
+        multi.set(keys.pathname({ doc_id }), pathname)
+        multi.set(keys.projectHistoryId({ doc_id }), projectHistoryId)
+        multi.exec(callback)
       })
     })
   },
@@ -269,48 +264,17 @@ module.exports = RedisManager = {
         projectHistoryId = parseInt(projectHistoryId)
       }
 
-      // doc is not in redis, bail out
-      if (docLines == null) {
-        return callback(
-          null,
-          docLines,
-          version,
-          ranges,
-          pathname,
-          projectHistoryId,
-          unflushedTime,
-          lastUpdatedAt,
-          lastUpdatedBy
-        )
-      }
-
-      // doc should be in project set, check if missing (workaround for missing docs from putDoc)
-      return rclient.sadd(keys.docsInProject({ project_id }), doc_id, function (
-        error,
-        result
-      ) {
-        if (error != null) {
-          return callback(error)
-        }
-        if (result !== 0) {
-          // doc should already be in set
-          logger.error(
-            { project_id, doc_id, doc_project_id },
-            'doc missing from docsInProject set'
-          )
-        }
-        return callback(
-          null,
-          docLines,
-          version,
-          ranges,
-          pathname,
-          projectHistoryId,
-          unflushedTime,
-          lastUpdatedAt,
-          lastUpdatedBy
-        )
-      })
+      callback(
+        null,
+        docLines,
+        version,
+        ranges,
+        pathname,
+        projectHistoryId,
+        unflushedTime,
+        lastUpdatedAt,
+        lastUpdatedBy
+      )
     })
   },
 

services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js

@@ -182,12 +182,6 @@ describe('RedisManager', function () {
           .should.equal(true)
       })
 
-      it('should check if the document is in the DocsIn set', function () {
-        return this.rclient.sadd
-          .calledWith(`DocsIn:${this.project_id}`)
-          .should.equal(true)
-      })
-
       it('should return the document', function () {
         return this.callback
           .calledWithExactly(
@@ -209,78 +203,6 @@ describe('RedisManager', function () {
       })
     })
 
-    describe('when the document is not present', function () {
-      beforeEach(function () {
-        this.rclient.mget = sinon
-          .stub()
-          .yields(null, [
-            null,
-            null,
-            null,
-            null,
-            null,
-            null,
-            null,
-            null,
-            null,
-            null
-          ])
-        this.rclient.sadd = sinon.stub().yields()
-        return this.RedisManager.getDoc(
-          this.project_id,
-          this.doc_id,
-          this.callback
-        )
-      })
-
-      it('should not check if the document is in the DocsIn set', function () {
-        return this.rclient.sadd
-          .calledWith(`DocsIn:${this.project_id}`)
-          .should.equal(false)
-      })
-
-      it('should return an empty result', function () {
-        return this.callback
-          .calledWithExactly(null, null, 0, {}, null, null, null, null, null)
-          .should.equal(true)
-      })
-
-      return it('should not log any errors', function () {
-        return this.logger.error.calledWith().should.equal(false)
-      })
-    })
-
-    describe('when the document is missing from the DocsIn set', function () {
-      beforeEach(function () {
-        this.rclient.sadd = sinon.stub().yields(null, 1)
-        return this.RedisManager.getDoc(
-          this.project_id,
-          this.doc_id,
-          this.callback
-        )
-      })
-
-      it('should log an error', function () {
-        return this.logger.error.calledWith().should.equal(true)
-      })
-
-      return it('should return the document', function () {
-        return this.callback
-          .calledWithExactly(
-            null,
-            this.lines,
-            this.version,
-            this.ranges,
-            this.pathname,
-            this.projectHistoryId,
-            this.unflushed_time,
-            this.lastUpdatedAt,
-            this.lastUpdatedBy
-          )
-          .should.equal(true)
-      })
-    })
-
     describe('with a corrupted document', function () {
       beforeEach(function () {
         this.badHash = 'INVALID-HASH-VALUE'