PERF: store env samples separately from message data

lib/logster/base_store.rb

@@ -14,7 +14,7 @@ def save(message)
     end
 
     # Modify the saved message to the given one (identified by message.key) and bump it to the top of the latest list
-    def replace_and_bump(message)
+    def replace_and_bump(message, save_env: true)
       not_implemented
     end
 
@@ -40,7 +40,17 @@ def clear_all
     end
 
     # Get a message by its message_key
-    def get(message_key)
+    def get(message_key, load_env: true)
+      not_implemented
+    end
+
+    # Get a group of messages by their message_keys
+    def bulk_get(message_keys)
+      not_implemented
+    end
+
+    # Get a message's env by its message_key
+    def get_env(message_key)
       not_implemented
     end
 
@@ -104,13 +114,14 @@ def report(severity, progname, msg, opts = {})
 
       if Logster.config.allow_grouping
         key = self.similar_key(message)
-        similar = get key if key
+        similar = get(key, load_env: false) if key
       end
 
       if similar
-        similar.merge_similar_message(message)
+        similar.env = get_env(similar.key) if similar.count < Logster::MAX_GROUPING_LENGTH
+        save_env = similar.merge_similar_message(message)
 
-        replace_and_bump similar
+        replace_and_bump(similar, save_env: save_env)
         similar
       else
         save message

lib/logster/message.rb

@@ -35,7 +35,7 @@ def initialize(severity, progname, message, timestamp = nil, key = nil, count: 1
       @first_timestamp = nil
     end
 
-    def to_h
+    def to_h(exclude_env: false)
       h = {
         message: @message,
         progname: @progname,
@@ -44,19 +44,18 @@ def to_h
         key: @key,
         backtrace: @backtrace,
         count: @count,
-        env: @env,
         protected: @protected
       }
 
-      if @first_timestamp
-        h[:first_timestamp] = @first_timestamp
-      end
+      h[:first_timestamp] = @first_timestamp if @first_timestamp
+      h[:env] = @env unless exclude_env
 
       h
     end
 
     def to_json(opts = nil)
-      JSON.fast_generate(to_h, opts)
+      exclude_env = Hash === opts && opts.delete(:exclude_env)
+      JSON.fast_generate(to_h(exclude_env: exclude_env), opts)
     end
 
     def self.from_json(json)
@@ -135,14 +134,15 @@ def merge_similar_message(other)
       self.timestamp = [self.timestamp, other.timestamp].max
 
       self.count += other.count || 1
-      return if self.count >= Logster::MAX_GROUPING_LENGTH
+      return false if self.count > Logster::MAX_GROUPING_LENGTH
 
       other_env = JSON.load JSON.fast_generate other.env
       if Array === self.env
         Array === other_env ? self.env.concat(other_env) : self.env << other_env
       else
         Array === other_env ? self.env = [self.env, *other_env] : self.env = [self.env, other_env]
       end
+      true
     end
 
     def self.populate_from_env(env)

lib/logster/redis_store.rb

@@ -144,18 +144,20 @@ def save(message)
     def delete(msg)
       @redis.multi do
         @redis.hdel(hash_key, msg.key)
+        @redis.hdel(env_key, msg.key)
         @redis.hdel(grouping_key, msg.grouping_key)
         @redis.lrem(list_key, -1, msg.key)
       end
     end
 
-    def replace_and_bump(message)
+    def replace_and_bump(message, save_env: true)
       # TODO make it atomic
       exists = @redis.hexists(hash_key, message.key)
       return false unless exists
 
       @redis.multi do
-        @redis.hset(hash_key, message.key, message.to_json)
+        @redis.hset(hash_key, message.key, message.to_json(exclude_env: true))
+        @redis.hset(env_key, message.key, (message.env || {}).to_json) if save_env
         @redis.lrem(list_key, -1, message.key)
         @redis.rpush(list_key, message.key)
       end
@@ -201,12 +203,11 @@ def latest(opts={})
       begin
         keys = @redis.lrange(list_key, start, finish) || []
         break unless keys and keys.count > 0
-        rows = @redis.hmget(hash_key, keys)
+        rows = bulk_get(keys)
 
         temp = []
 
-        rows.each do |s|
-          row = Message.from_json(s)
+        rows.each do |row|
           break if before && before == row.key
           row = nil if severity && !severity.include?(row.severity)
 
@@ -236,10 +237,14 @@ def clear
       keys = @redis.smembers(protected_key) || []
       if keys.empty?
         @redis.del(hash_key)
+        @redis.del(env_key)
       else
         protected = @redis.mapped_hmget(hash_key, *keys)
+        protected_env = @redis.mapped_hmget(env_key, *keys)
         @redis.del(hash_key)
+        @redis.del(env_key)
         @redis.mapped_hmset(hash_key, protected)
+        @redis.mapped_hmset(env_key, protected_env)
 
         sorted = protected
           .values
@@ -264,15 +269,39 @@ def clear_all
       @redis.del(list_key)
       @redis.del(protected_key)
       @redis.del(hash_key)
+      @redis.del(env_key)
       @redis.del(grouping_key)
       @redis.del(solved_key)
     end
 
-    def get(message_key)
+    def get(message_key, load_env: true)
       json = @redis.hget(hash_key, message_key)
       return nil unless json
 
-      Message.from_json(json)
+      message = Message.from_json(json)
+      if load_env
+        message.env = get_env(message_key)
+      end
+      message
+    end
+
+    def bulk_get(message_keys)
+      envs = @redis.hmget(env_key, message_keys)
+      @redis.hmget(hash_key, message_keys).map!.with_index do |json, ind|
+        message =  Message.from_json(json)
+        env = envs[ind]
+        if !message.env || message.env.size == 0
+          env = env && env.size > 0 ? ::JSON.parse(env) : {}
+          message.env = env
+        end
+        message
+      end
+    end
+
+    def get_env(message_key)
+      json = @redis.hget(env_key, message_key)
+      return {} if !json || json.size == 0
+      JSON.parse(json)
     end
 
     def protect(message_key)
@@ -323,8 +352,7 @@ def clear_solved(count = nil)
         start = count ? 0 - count : 0
         message_keys = @redis.lrange(list_key, start, -1 ) || []
 
-        @redis.hmget(hash_key, message_keys).each do |json|
-          message =  Message.from_json(json)
+        bulk_get(message_keys).each do |message|
           unless (ignores & (message.solved_keys || [])).empty?
             delete message
           end
@@ -339,6 +367,7 @@ def trim
           unless @redis.sismember(protected_key, removed_key)
             rmsg = get removed_key
             @redis.hdel(hash_key, rmsg.key)
+            @redis.hdel(env_key, rmsg.key)
             @redis.hdel(grouping_key, rmsg.grouping_key)
             break
           else
@@ -352,7 +381,8 @@ def trim
     end
 
     def update_message(message)
-      @redis.hset(hash_key, message.key, message.to_json)
+      @redis.hset(hash_key, message.key, message.to_json(exclude_env: true))
+      @redis.hset(env_key, message.key, (message.env || {}).to_json)
       if message.protected
         @redis.sadd(protected_key, message.key)
       else
@@ -506,6 +536,10 @@ def hash_key
       @hash_key ||= "__LOGSTER__MAP"
     end
 
+    def env_key
+      @env_key ||= "__LOGSTER__ENV_MAP"
+    end
+
     def protected_key
       @saved_key ||= "__LOGSTER__SAVED"
     end

test/logster/test_message.rb

@@ -14,14 +14,12 @@ def test_merge_similar
 
     msg1.merge_similar_message(msg2)
 
-    msg1 = Logster::Message.from_json(msg1.to_json)
-
     assert_equal(20, msg1.timestamp)
     assert_equal(10, msg1.first_timestamp)
 
     assert Array === msg1.env
     assert_equal(msg1.env.size, 2)
-    assert({ "a" => "1", "b" => "2" } <= msg1.env[0])
+    assert({ a: "1", b: "2" } <= msg1.env[0])
     assert({ "a" => "2", "c" => "3" } <= msg1.env[1])
   end
 
@@ -33,13 +31,12 @@ def test_merge_messages_both_with_array_envs
     msg2.env = [{ e: "ee", f: "ff" }, { g: "gg", h: "hh" }]
 
     msg1.merge_similar_message(msg2) 
-    msg1 = Logster::Message.from_json(msg1.to_json)
 
     # new env should be an array, but it should never have
     # another array of envs within itself (hence flatten(1))
     assert_equal(msg1.env.size, 4)
-    assert_equal(msg1.env.map(&:keys).flatten(1), %w{a b c d e f g h})
-    assert_equal(msg1.env.map(&:values).flatten(1), %w{aa bb cc dd ee ff gg hh})
+    assert_equal(msg1.env.map(&:keys).flatten(1).map(&:to_s), %w{a b c d e f g h})
+    assert_equal(msg1.env.map(&:values).flatten(1).map(&:to_s), %w{aa bb cc dd ee ff gg hh})
   end
 
   def test_merge_messages_one_with_array_envs
@@ -50,11 +47,10 @@ def test_merge_messages_one_with_array_envs
     msg2.env = { e: "ee", f: "ff" }
 
     msg1.merge_similar_message(msg2) 
-    msg1 = Logster::Message.from_json(msg1.to_json)
 
     assert_equal(msg1.env.size, 3)
-    assert_equal(msg1.env.map(&:keys).flatten(1), %w{a b c d e f})
-    assert_equal(msg1.env.map(&:values).flatten(1), %w{aa bb cc dd ee ff})
+    assert_equal(msg1.env.map(&:keys).flatten(1).map(&:to_s), %w{a b c d e f})
+    assert_equal(msg1.env.map(&:values).flatten(1).map(&:to_s), %w{aa bb cc dd ee ff})
   end
 
   def test_adds_application_version
@@ -76,7 +72,6 @@ def test_merging_sums_count_for_both_messages
     assert_equal(msg1.grouping_key, msg2.grouping_key)
 
     msg1.merge_similar_message(msg2)
-    msg1 = Logster::Message.from_json(msg1.to_json)
     assert_equal(msg1.count, 15 + 13)
   end
 
@@ -85,7 +80,6 @@ def test_populate_from_env_works_on_array
     hash = { "custom_key" => "key" }
     msg.populate_from_env([hash])
 
-    msg = Logster::Message.from_json(msg.to_json)
     assert Array === msg.env
     assert_equal(msg.env.size, 1)
     assert hash <= msg.env[0]
@@ -100,9 +94,8 @@ def test_ensure_env_samples_dont_exceed_50
     assert_equal(msg1.grouping_key, msg2.grouping_key)
 
     msg1.merge_similar_message(msg2)
-    msg1 = Logster::Message.from_json(msg1.to_json)
 
     assert_equal(63, msg1.count) # update count
-    assert_equal([{ "a" => 1 }], msg1.env) # but don't merge msg2's env into msg1's
+    assert_equal([{ a: 1 }], msg1.env) # but don't merge msg2's env into msg1's
   end
 end