Просмотр исходного кода

Merge pull request #163 from das7pad/jpa-cut-redis-load-mget

[perf] cut redis load using mget/mset only
Jakob Ackermann 5 лет назад
Родитель
Сommit
31a930da40

+ 47 - 88
services/document-updater/app/js/RedisManager.js

@@ -84,26 +84,20 @@ module.exports = RedisManager = {
         logger.error({ err: error, doc_id, project_id }, error.message)
         logger.error({ err: error, doc_id, project_id }, error.message)
         return callback(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 }))
-      }
-      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
-        return rclient.sadd(
-          keys.docsInProject({ project_id }),
-          doc_id,
+      // update docsInProject set before writing doc contents
+      rclient.sadd(keys.docsInProject({ project_id }), doc_id, (error) => {
+        if (error) return callback(error)
+
+        rclient.mset(
+          {
+            [keys.docLines({ doc_id })]: docLines,
+            [keys.projectKey({ doc_id })]: project_id,
+            [keys.docVersion({ doc_id })]: version,
+            [keys.docHash({ doc_id })]: docHash,
+            [keys.ranges({ doc_id })]: ranges,
+            [keys.pathname({ doc_id })]: pathname,
+            [keys.projectHistoryId({ doc_id })]: projectHistoryId
+          },
           callback
           callback
         )
         )
       })
       })
@@ -124,17 +118,19 @@ module.exports = RedisManager = {
 
 
     let multi = rclient.multi()
     let multi = rclient.multi()
     multi.strlen(keys.docLines({ doc_id }))
     multi.strlen(keys.docLines({ doc_id }))
-    multi.del(keys.docLines({ doc_id }))
-    multi.del(keys.projectKey({ doc_id }))
-    multi.del(keys.docVersion({ doc_id }))
-    multi.del(keys.docHash({ doc_id }))
-    multi.del(keys.ranges({ doc_id }))
-    multi.del(keys.pathname({ doc_id }))
-    multi.del(keys.projectHistoryId({ doc_id }))
-    multi.del(keys.projectHistoryType({ doc_id }))
-    multi.del(keys.unflushedTime({ doc_id }))
-    multi.del(keys.lastUpdatedAt({ doc_id }))
-    multi.del(keys.lastUpdatedBy({ doc_id }))
+    multi.del(
+      keys.docLines({ doc_id }),
+      keys.projectKey({ doc_id }),
+      keys.docVersion({ doc_id }),
+      keys.docHash({ doc_id }),
+      keys.ranges({ doc_id }),
+      keys.pathname({ doc_id }),
+      keys.projectHistoryId({ doc_id }),
+      keys.projectHistoryType({ doc_id }),
+      keys.unflushedTime({ doc_id }),
+      keys.lastUpdatedAt({ doc_id }),
+      keys.lastUpdatedBy({ doc_id })
+    )
     return multi.exec(function (error, response) {
     return multi.exec(function (error, response) {
       if (error != null) {
       if (error != null) {
         return callback(error)
         return callback(error)
@@ -269,48 +265,17 @@ module.exports = RedisManager = {
         projectHistoryId = parseInt(projectHistoryId)
         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
+      )
     })
     })
   },
   },
 
 
@@ -519,19 +484,19 @@ module.exports = RedisManager = {
           return callback(error)
           return callback(error)
         }
         }
         const multi = rclient.multi()
         const multi = rclient.multi()
-        multi.set(keys.docLines({ doc_id }), newDocLines) // index 0
-        multi.set(keys.docVersion({ doc_id }), newVersion) // index 1
-        multi.set(keys.docHash({ doc_id }), newHash) // index 2
+        multi.mset({
+          [keys.docLines({ doc_id })]: newDocLines,
+          [keys.docVersion({ doc_id })]: newVersion,
+          [keys.docHash({ doc_id })]: newHash,
+          [keys.ranges({ doc_id })]: ranges,
+          [keys.lastUpdatedAt({ doc_id })]: Date.now(),
+          [keys.lastUpdatedBy({ doc_id })]: updateMeta && updateMeta.user_id
+        })
         multi.ltrim(
         multi.ltrim(
           keys.docOps({ doc_id }),
           keys.docOps({ doc_id }),
           -RedisManager.DOC_OPS_MAX_LENGTH,
           -RedisManager.DOC_OPS_MAX_LENGTH,
           -1
           -1
         ) // index 3
         ) // index 3
-        if (ranges != null) {
-          multi.set(keys.ranges({ doc_id }), ranges) // index 4
-        } else {
-          multi.del(keys.ranges({ doc_id })) // also index 4
-        }
         // push the ops last so we can get the lengths at fixed index position 7
         // push the ops last so we can get the lengths at fixed index position 7
         if (jsonOps.length > 0) {
         if (jsonOps.length > 0) {
           multi.rpush(keys.docOps({ doc_id }), ...Array.from(jsonOps)) // index 5
           multi.rpush(keys.docOps({ doc_id }), ...Array.from(jsonOps)) // index 5
@@ -555,12 +520,6 @@ module.exports = RedisManager = {
           // hasn't been modified before (the content in mongo has been
           // hasn't been modified before (the content in mongo has been
           // valid up to this point). Otherwise leave it alone ("NX" flag).
           // valid up to this point). Otherwise leave it alone ("NX" flag).
           multi.set(keys.unflushedTime({ doc_id }), Date.now(), 'NX')
           multi.set(keys.unflushedTime({ doc_id }), Date.now(), 'NX')
-          multi.set(keys.lastUpdatedAt({ doc_id }), Date.now()) // index 8
-          if (updateMeta != null ? updateMeta.user_id : undefined) {
-            multi.set(keys.lastUpdatedBy({ doc_id }), updateMeta.user_id) // index 9
-          } else {
-            multi.del(keys.lastUpdatedBy({ doc_id })) // index 9
-          }
         }
         }
         return multi.exec(function (error, result) {
         return multi.exec(function (error, result) {
           let docUpdateCount
           let docUpdateCount
@@ -572,7 +531,7 @@ module.exports = RedisManager = {
             docUpdateCount = undefined // only using project history, don't bother with track-changes
             docUpdateCount = undefined // only using project history, don't bother with track-changes
           } else {
           } else {
             // project is using old track-changes history service
             // project is using old track-changes history service
-            docUpdateCount = result[7] // length of uncompressedHistoryOps queue (index 7)
+            docUpdateCount = result[4]
           }
           }
 
 
           if (
           if (

+ 188 - 0
services/document-updater/benchmarks/multi_vs_mget_mset.rb

@@ -0,0 +1,188 @@
+require "benchmark"
+require "redis"
+
+N = (ARGV.first || 1).to_i
+DOC_ID = (ARGV.last || "606072b20bb4d3109fb5b122")
+
+@r = Redis.new
+
+
+def get
+  @r.get("doclines:{#{DOC_ID}}")
+  @r.get("DocVersion:{#{DOC_ID}}")
+  @r.get("DocHash:{#{DOC_ID}}")
+  @r.get("ProjectId:{#{DOC_ID}}")
+  @r.get("Ranges:{#{DOC_ID}}")
+  @r.get("Pathname:{#{DOC_ID}}")
+  @r.get("ProjectHistoryId:{#{DOC_ID}}")
+  @r.get("UnflushedTime:{#{DOC_ID}}")
+  @r.get("lastUpdatedAt:{#{DOC_ID}}")
+  @r.get("lastUpdatedBy:{#{DOC_ID}}")
+end
+
+def mget
+  @r.mget(
+    "doclines:{#{DOC_ID}}",
+    "DocVersion:{#{DOC_ID}}",
+    "DocHash:{#{DOC_ID}}",
+    "ProjectId:{#{DOC_ID}}",
+    "Ranges:{#{DOC_ID}}",
+    "Pathname:{#{DOC_ID}}",
+    "ProjectHistoryId:{#{DOC_ID}}",
+    "UnflushedTime:{#{DOC_ID}}",
+    "lastUpdatedAt:{#{DOC_ID}}",
+    "lastUpdatedBy:{#{DOC_ID}}",
+    )
+end
+
+def set
+  @r.set("doclines:{#{DOC_ID}}", "[\"@book{adams1995hitchhiker,\",\"  title={The Hitchhiker's Guide to the Galaxy},\",\"  author={Adams, D.},\",\"  isbn={9781417642595},\",\"  url={http://books.google.com/books?id=W-xMPgAACAAJ},\",\"  year={1995},\",\"  publisher={San Val}\",\"}\",\"\"]")
+  @r.set("DocVersion:{#{DOC_ID}}", "0")
+  @r.set("DocHash:{#{DOC_ID}}", "0075bb0629c6c13d0d68918443648bbfe7d98869")
+  @r.set("ProjectId:{#{DOC_ID}}", "606072b20bb4d3109fb5b11e")
+  @r.set("Ranges:{#{DOC_ID}}", "")
+  @r.set("Pathname:{#{DOC_ID}}", "/references.bib")
+  @r.set("ProjectHistoryId:{#{DOC_ID}}", "")
+  @r.set("UnflushedTime:{#{DOC_ID}}", "")
+  @r.set("lastUpdatedAt:{#{DOC_ID}}", "")
+  @r.set("lastUpdatedBy:{#{DOC_ID}}", "")
+end
+
+def mset
+  @r.mset(
+    "doclines:{#{DOC_ID}}", "[\"@book{adams1995hitchhiker,\",\"  title={The Hitchhiker's Guide to the Galaxy},\",\"  author={Adams, D.},\",\"  isbn={9781417642595},\",\"  url={http://books.google.com/books?id=W-xMPgAACAAJ},\",\"  year={1995},\",\"  publisher={San Val}\",\"}\",\"\"]",
+    "DocVersion:{#{DOC_ID}}", "0",
+    "DocHash:{#{DOC_ID}}", "0075bb0629c6c13d0d68918443648bbfe7d98869",
+    "ProjectId:{#{DOC_ID}}", "606072b20bb4d3109fb5b11e",
+    "Ranges:{#{DOC_ID}}", "",
+    "Pathname:{#{DOC_ID}}", "/references.bib",
+    "ProjectHistoryId:{#{DOC_ID}}", "",
+    "UnflushedTime:{#{DOC_ID}}", "",
+    "lastUpdatedAt:{#{DOC_ID}}", "",
+    "lastUpdatedBy:{#{DOC_ID}}", "",
+    )
+end
+
+
+def benchmark_multi_get(benchmark, i)
+  benchmark.report("#{i}: multi get") do
+    N.times do
+      @r.multi do
+        get
+      end
+    end
+  end
+end
+
+def benchmark_mget(benchmark, i)
+  benchmark.report("#{i}: mget") do
+    N.times do
+      mget
+    end
+  end
+end
+
+def benchmark_multi_set(benchmark, i)
+  benchmark.report("#{i}: multi set") do
+    N.times do
+      @r.multi do
+        set
+      end
+    end
+  end
+end
+
+def benchmark_mset(benchmark, i)
+  benchmark.report("#{i}: mset") do
+    N.times do
+      mset
+    end
+  end
+end
+
+
+# init
+set
+
+Benchmark.bmbm do |benchmark|
+  3.times do |i|
+    benchmark_multi_get(benchmark, i)
+    benchmark_mget(benchmark, i)
+    benchmark_multi_set(benchmark, i)
+    benchmark_mset(benchmark, i)
+  end
+end
+
+
+
+=begin
+# Results
+
+I could not max out the redis-server process with this benchmark.
+The ruby process hit 100% of a modern i7 CPU thread and the redis-server process
+ barely hit 50% of a CPU thread.
+
+Based on the timings below, mget is about 3 times faster and mset about 4 times
+ faster than multiple get/set commands in a multi.
+=end
+
+=begin
+$ redis-server --version
+Redis server v=5.0.7 sha=00000000:0 malloc=jemalloc-5.2.1 bits=64 build=636cde3b5c7a3923
+$ ruby multi_vs_mget_mset.rb 100000
+Rehearsal ------------------------------------------------
+0: multi get  12.132423   4.246689  16.379112 ( 16.420069)
+0: mget        4.499457   0.947556   5.447013 (  6.274883)
+0: multi set  12.685936   4.495241  17.181177 ( 17.225984)
+0: mset        2.543401   0.913448   3.456849 (  4.554799)
+1: multi get  13.397207   4.581881  17.979088 ( 18.027755)
+1: mget        4.551287   1.160531   5.711818 (  6.579168)
+1: multi set  13.018957   4.927175  17.946132 ( 17.987502)
+1: mset        2.561096   1.048416   3.609512 (  4.780087)
+2: multi get  13.224422   5.014475  18.238897 ( 18.284152)
+2: mget        4.664434   1.051083   5.715517 (  6.592088)
+2: multi set  12.972284   4.600422  17.572706 ( 17.613185)
+2: mset        2.621344   0.984123   3.605467 (  4.766855)
+------------------------------------- total: 132.843288sec
+
+                   user     system      total        real
+0: multi get  13.341552   4.900892  18.242444 ( 18.289912)
+0: mget        5.056534   0.960954   6.017488 (  6.971189)
+0: multi set  12.989880   4.823793  17.813673 ( 17.858393)
+0: mset        2.543434   1.025352   3.568786 (  4.723040)
+1: multi get  13.059379   4.674345  17.733724 ( 17.777859)
+1: mget        4.698754   0.915637   5.614391 (  6.489614)
+1: multi set  12.608293   4.729163  17.337456 ( 17.372993)
+1: mset        2.645290   0.940584   3.585874 (  4.744134)
+2: multi get  13.678224   4.732373  18.410597 ( 18.457525)
+2: mget        4.716749   1.072064   5.788813 (  6.697683)
+2: multi set  13.058710   4.889801  17.948511 ( 17.988742)
+2: mset        2.311854   0.989166   3.301020 (  4.346467)
+=end
+
+=begin
+# multi get/set run at about O(65'000) operations per second
+$ redis-cli info | grep 'instantaneous_ops_per_sec'
+instantaneous_ops_per_sec:65557
+
+# mget runs at about O(15'000) operations per second
+$ redis-cli info | grep 'instantaneous_ops_per_sec'
+instantaneous_ops_per_sec:14580
+
+# mset runs at about O(20'000) operations per second
+$ redis-cli info | grep 'instantaneous_ops_per_sec'
+instantaneous_ops_per_sec:20792
+
+These numbers are pretty reasonable:
+multi: 100'000 * 12 ops / 18s = 66'666 ops/s
+mget : 100'000 *  1 ops  /  7s = 14'285 ops/s
+mset : 100'000 *  1 ops  /  5s = 20'000 ops/s
+
+
+
+Bonus: Running three benchmarks in parallel on different keys.
+multi get: O(125'000) ops/s and 80% CPU load of redis-server
+multi set: O(130'000) ops/s and 90% CPU load of redis-server
+mget     : O( 30'000) ops/s and 70% CPU load of redis-server
+mset     : O( 40'000) ops/s and 90% CPU load of redis-server
+=end

+ 77 - 236
services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js

@@ -153,7 +153,6 @@ describe('RedisManager', function () {
           this.projectHistoryId.toString(),
           this.projectHistoryId.toString(),
           this.unflushed_time
           this.unflushed_time
         ])
         ])
-      return (this.rclient.sadd = sinon.stub().yields(null, 0))
     })
     })
 
 
     describe('successfully', function () {
     describe('successfully', function () {
@@ -182,12 +181,6 @@ describe('RedisManager', function () {
           .should.equal(true)
           .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 () {
       it('should return the document', function () {
         return this.callback
         return this.callback
           .calledWithExactly(
           .calledWithExactly(
@@ -209,78 +202,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 () {
     describe('with a corrupted document', function () {
       beforeEach(function () {
       beforeEach(function () {
         this.badHash = 'INVALID-HASH-VALUE'
         this.badHash = 'INVALID-HASH-VALUE'
@@ -547,6 +468,7 @@ describe('RedisManager', function () {
       this.project_update_list_length = sinon.stub()
       this.project_update_list_length = sinon.stub()
 
 
       this.RedisManager.getDocVersion = sinon.stub()
       this.RedisManager.getDocVersion = sinon.stub()
+      this.multi.mset = sinon.stub()
       this.multi.set = sinon.stub()
       this.multi.set = sinon.stub()
       this.multi.rpush = sinon.stub()
       this.multi.rpush = sinon.stub()
       this.multi.expire = sinon.stub()
       this.multi.expire = sinon.stub()
@@ -555,9 +477,6 @@ describe('RedisManager', function () {
       this.multi.exec = sinon
       this.multi.exec = sinon
         .stub()
         .stub()
         .callsArgWith(0, null, [
         .callsArgWith(0, null, [
-          this.hash,
-          null,
-          null,
           null,
           null,
           null,
           null,
           null,
           null,
@@ -602,27 +521,16 @@ describe('RedisManager', function () {
             .should.equal(true)
             .should.equal(true)
         })
         })
 
 
-        it('should set the doclines', function () {
-          return this.multi.set
-            .calledWith(`doclines:${this.doc_id}`, JSON.stringify(this.lines))
-            .should.equal(true)
-        })
-
-        it('should set the version', function () {
-          return this.multi.set
-            .calledWith(`DocVersion:${this.doc_id}`, this.version)
-            .should.equal(true)
-        })
-
-        it('should set the hash', function () {
-          return this.multi.set
-            .calledWith(`DocHash:${this.doc_id}`, this.hash)
-            .should.equal(true)
-        })
-
-        it('should set the ranges', function () {
-          return this.multi.set
-            .calledWith(`Ranges:${this.doc_id}`, JSON.stringify(this.ranges))
+        it('should set most details in a single MSET call', function () {
+          this.multi.mset
+            .calledWith({
+              [`doclines:${this.doc_id}`]: JSON.stringify(this.lines),
+              [`DocVersion:${this.doc_id}`]: this.version,
+              [`DocHash:${this.doc_id}`]: this.hash,
+              [`Ranges:${this.doc_id}`]: JSON.stringify(this.ranges),
+              [`lastUpdatedAt:${this.doc_id}`]: Date.now(),
+              [`lastUpdatedBy:${this.doc_id}`]: 'last-author-fake-id'
+            })
             .should.equal(true)
             .should.equal(true)
         })
         })
 
 
@@ -632,18 +540,6 @@ describe('RedisManager', function () {
             .should.equal(true)
             .should.equal(true)
         })
         })
 
 
-        it('should set the last updated time', function () {
-          return this.multi.set
-            .calledWith(`lastUpdatedAt:${this.doc_id}`, Date.now())
-            .should.equal(true)
-        })
-
-        it('should set the last updater', function () {
-          return this.multi.set
-            .calledWith(`lastUpdatedBy:${this.doc_id}`, 'last-author-fake-id')
-            .should.equal(true)
-        })
-
         it('should push the doc op into the doc ops list', function () {
         it('should push the doc op into the doc ops list', function () {
           return this.multi.rpush
           return this.multi.rpush
             .calledWith(
             .calledWith(
@@ -825,8 +721,15 @@ describe('RedisManager', function () {
       })
       })
 
 
       return it('should still set the doclines', function () {
       return it('should still set the doclines', function () {
-        return this.multi.set
-          .calledWith(`doclines:${this.doc_id}`, JSON.stringify(this.lines))
+        this.multi.mset
+          .calledWith({
+            [`doclines:${this.doc_id}`]: JSON.stringify(this.lines),
+            [`DocVersion:${this.doc_id}`]: this.version,
+            [`DocHash:${this.doc_id}`]: this.hash,
+            [`Ranges:${this.doc_id}`]: JSON.stringify(this.ranges),
+            [`lastUpdatedAt:${this.doc_id}`]: Date.now(),
+            [`lastUpdatedBy:${this.doc_id}`]: 'last-author-fake-id'
+          })
           .should.equal(true)
           .should.equal(true)
       })
       })
     })
     })
@@ -848,15 +751,16 @@ describe('RedisManager', function () {
         )
         )
       })
       })
 
 
-      it('should not set the ranges', function () {
-        return this.multi.set
-          .calledWith(`Ranges:${this.doc_id}`, JSON.stringify(this.ranges))
-          .should.equal(false)
-      })
-
-      return it('should delete the ranges key', function () {
-        return this.multi.del
-          .calledWith(`Ranges:${this.doc_id}`)
+      it('should set empty ranges', function () {
+        this.multi.mset
+          .calledWith({
+            [`doclines:${this.doc_id}`]: JSON.stringify(this.lines),
+            [`DocVersion:${this.doc_id}`]: this.version,
+            [`DocHash:${this.doc_id}`]: this.hash,
+            [`Ranges:${this.doc_id}`]: null,
+            [`lastUpdatedAt:${this.doc_id}`]: Date.now(),
+            [`lastUpdatedBy:${this.doc_id}`]: 'last-author-fake-id'
+          })
           .should.equal(true)
           .should.equal(true)
       })
       })
     })
     })
@@ -944,15 +848,16 @@ describe('RedisManager', function () {
         )
         )
       })
       })
 
 
-      it('should set the last updater to null', function () {
-        return this.multi.del
-          .calledWith(`lastUpdatedBy:${this.doc_id}`)
-          .should.equal(true)
-      })
-
-      return it('should still set the last updated time', function () {
-        return this.multi.set
-          .calledWith(`lastUpdatedAt:${this.doc_id}`, Date.now())
+      it('should unset last updater', function () {
+        this.multi.mset
+          .calledWith({
+            [`doclines:${this.doc_id}`]: JSON.stringify(this.lines),
+            [`DocVersion:${this.doc_id}`]: this.version,
+            [`DocHash:${this.doc_id}`]: this.hash,
+            [`Ranges:${this.doc_id}`]: JSON.stringify(this.ranges),
+            [`lastUpdatedAt:${this.doc_id}`]: Date.now(),
+            [`lastUpdatedBy:${this.doc_id}`]: undefined
+          })
           .should.equal(true)
           .should.equal(true)
       })
       })
     })
     })
@@ -960,16 +865,14 @@ describe('RedisManager', function () {
 
 
   describe('putDocInMemory', function () {
   describe('putDocInMemory', function () {
     beforeEach(function () {
     beforeEach(function () {
-      this.multi.set = sinon.stub()
+      this.rclient.mset = sinon.stub().yields(null)
       this.rclient.sadd = sinon.stub().yields()
       this.rclient.sadd = sinon.stub().yields()
-      this.multi.del = sinon.stub()
       this.lines = ['one', 'two', 'three', 'これは']
       this.lines = ['one', 'two', 'three', 'これは']
       this.version = 42
       this.version = 42
       this.hash = crypto
       this.hash = crypto
         .createHash('sha1')
         .createHash('sha1')
         .update(JSON.stringify(this.lines), 'utf8')
         .update(JSON.stringify(this.lines), 'utf8')
         .digest('hex')
         .digest('hex')
-      this.multi.exec = sinon.stub().callsArgWith(0, null, [this.hash])
       this.ranges = { comments: 'mock', entries: 'mock' }
       this.ranges = { comments: 'mock', entries: 'mock' }
       return (this.pathname = '/a/b/c.tex')
       return (this.pathname = '/a/b/c.tex')
     })
     })
@@ -988,45 +891,17 @@ describe('RedisManager', function () {
         )
         )
       })
       })
 
 
-      it('should set the lines', function () {
-        return this.multi.set
-          .calledWith(`doclines:${this.doc_id}`, JSON.stringify(this.lines))
-          .should.equal(true)
-      })
-
-      it('should set the version', function () {
-        return this.multi.set
-          .calledWith(`DocVersion:${this.doc_id}`, this.version)
-          .should.equal(true)
-      })
-
-      it('should set the hash', function () {
-        return this.multi.set
-          .calledWith(`DocHash:${this.doc_id}`, this.hash)
-          .should.equal(true)
-      })
-
-      it('should set the ranges', function () {
-        return this.multi.set
-          .calledWith(`Ranges:${this.doc_id}`, JSON.stringify(this.ranges))
-          .should.equal(true)
-      })
-
-      it('should set the project_id for the doc', function () {
-        return this.multi.set
-          .calledWith(`ProjectId:${this.doc_id}`, this.project_id)
-          .should.equal(true)
-      })
-
-      it('should set the pathname for the doc', function () {
-        return this.multi.set
-          .calledWith(`Pathname:${this.doc_id}`, this.pathname)
-          .should.equal(true)
-      })
-
-      it('should set the projectHistoryId for the doc', function () {
-        return this.multi.set
-          .calledWith(`ProjectHistoryId:${this.doc_id}`, this.projectHistoryId)
+      it('should set all the details in a single MSET call', function () {
+        this.rclient.mset
+          .calledWith({
+            [`doclines:${this.doc_id}`]: JSON.stringify(this.lines),
+            [`ProjectId:${this.doc_id}`]: this.project_id,
+            [`DocVersion:${this.doc_id}`]: this.version,
+            [`DocHash:${this.doc_id}`]: this.hash,
+            [`Ranges:${this.doc_id}`]: JSON.stringify(this.ranges),
+            [`Pathname:${this.doc_id}`]: this.pathname,
+            [`ProjectHistoryId:${this.doc_id}`]: this.projectHistoryId
+          })
           .should.equal(true)
           .should.equal(true)
       })
       })
 
 
@@ -1055,17 +930,19 @@ describe('RedisManager', function () {
         )
         )
       })
       })
 
 
-      it('should delete the ranges key', function () {
-        return this.multi.del
-          .calledWith(`Ranges:${this.doc_id}`)
+      it('should unset ranges', function () {
+        this.rclient.mset
+          .calledWith({
+            [`doclines:${this.doc_id}`]: JSON.stringify(this.lines),
+            [`ProjectId:${this.doc_id}`]: this.project_id,
+            [`DocVersion:${this.doc_id}`]: this.version,
+            [`DocHash:${this.doc_id}`]: this.hash,
+            [`Ranges:${this.doc_id}`]: null,
+            [`Pathname:${this.doc_id}`]: this.pathname,
+            [`ProjectHistoryId:${this.doc_id}`]: this.projectHistoryId
+          })
           .should.equal(true)
           .should.equal(true)
       })
       })
-
-      return it('should not set the ranges', function () {
-        return this.multi.set
-          .calledWith(`Ranges:${this.doc_id}`, JSON.stringify(this.ranges))
-          .should.equal(false)
-      })
     })
     })
 
 
     describe('with null bytes in the serialized doc lines', function () {
     describe('with null bytes in the serialized doc lines', function () {
@@ -1148,33 +1025,21 @@ describe('RedisManager', function () {
         .should.equal(true)
         .should.equal(true)
     })
     })
 
 
-    it('should delete the lines', function () {
-      return this.multi.del
-        .calledWith(`doclines:${this.doc_id}`)
-        .should.equal(true)
-    })
-
-    it('should delete the version', function () {
+    it('should delete the details in a singe call', function () {
       return this.multi.del
       return this.multi.del
-        .calledWith(`DocVersion:${this.doc_id}`)
-        .should.equal(true)
-    })
-
-    it('should delete the hash', function () {
-      return this.multi.del
-        .calledWith(`DocHash:${this.doc_id}`)
-        .should.equal(true)
-    })
-
-    it('should delete the unflushed time', function () {
-      return this.multi.del
-        .calledWith(`UnflushedTime:${this.doc_id}`)
-        .should.equal(true)
-    })
-
-    it('should delete the project_id for the doc', function () {
-      return this.multi.del
-        .calledWith(`ProjectId:${this.doc_id}`)
+        .calledWith(
+          `doclines:${this.doc_id}`,
+          `ProjectId:${this.doc_id}`,
+          `DocVersion:${this.doc_id}`,
+          `DocHash:${this.doc_id}`,
+          `Ranges:${this.doc_id}`,
+          `Pathname:${this.doc_id}`,
+          `ProjectHistoryId:${this.doc_id}`,
+          `ProjectHistoryType:${this.doc_id}`,
+          `UnflushedTime:${this.doc_id}`,
+          `lastUpdatedAt:${this.doc_id}`,
+          `lastUpdatedBy:${this.doc_id}`
+        )
         .should.equal(true)
         .should.equal(true)
     })
     })
 
 
@@ -1183,30 +1048,6 @@ describe('RedisManager', function () {
         .calledWith(`DocsIn:${this.project_id}`, this.doc_id)
         .calledWith(`DocsIn:${this.project_id}`, this.doc_id)
         .should.equal(true)
         .should.equal(true)
     })
     })
-
-    it('should delete the pathname for the doc', function () {
-      return this.multi.del
-        .calledWith(`Pathname:${this.doc_id}`)
-        .should.equal(true)
-    })
-
-    it('should delete the pathname for the doc', function () {
-      return this.multi.del
-        .calledWith(`ProjectHistoryId:${this.doc_id}`)
-        .should.equal(true)
-    })
-
-    it('should delete lastUpdatedAt', function () {
-      return this.multi.del
-        .calledWith(`lastUpdatedAt:${this.doc_id}`)
-        .should.equal(true)
-    })
-
-    return it('should delete lastUpdatedBy', function () {
-      return this.multi.del
-        .calledWith(`lastUpdatedBy:${this.doc_id}`)
-        .should.equal(true)
-    })
   })
   })
 
 
   describe('clearProjectState', function () {
   describe('clearProjectState', function () {