From 236c755d62f1e491ffbe4f4f616722d67c659229 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Mon, 3 Dec 2018 08:35:09 +1100
Subject: [PATCH] FIX: do not store key tracking last seen time indefinitely

UserStat has some special logic to keep adding time read if repeat calls
are made in intervals less than 100 seconds. This is called regularly
when we update read timings on a topic.

We only need to cache this key in redis for 100 seconds, however previously
we would keep it forever, 1 key per user. This has potential of bloating
a very large amount of keys for no longer active users in redis.
---
 app/models/user_stat.rb       | 2 +-
 spec/models/user_stat_spec.rb | 9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/app/models/user_stat.rb b/app/models/user_stat.rb
index 65f48d62dc3..c4806af20b7 100644
--- a/app/models/user_stat.rb
+++ b/app/models/user_stat.rb
@@ -101,7 +101,7 @@ class UserStat < ActiveRecord::Base
   end
 
   def self.cache_last_seen(id, val)
-    $redis.set(last_seen_key(id), val)
+    $redis.setex(last_seen_key(id), MAX_TIME_READ_DIFF, val)
   end
 
   protected
diff --git a/spec/models/user_stat_spec.rb b/spec/models/user_stat_spec.rb
index 43e73819a14..be844c015d1 100644
--- a/spec/models/user_stat_spec.rb
+++ b/spec/models/user_stat_spec.rb
@@ -76,6 +76,15 @@ describe UserStat do
     let(:user) { Fabricate(:user) }
     let(:stat) { user.user_stat }
 
+    it 'always expires redis key' do
+      # this tests implementation which is not 100% ideal
+      # that said, redis key leaks are not good
+      stat.update_time_read!
+      ttl = $redis.ttl(UserStat.last_seen_key(user.id))
+      expect(ttl).to be > 0
+      expect(ttl).to be <= UserStat::MAX_TIME_READ_DIFF
+    end
+
     it 'makes no changes if nothing is cached' do
       $redis.del(UserStat.last_seen_key(user.id))
       stat.update_time_read!