PERF: store topic views in a topic view table

* cut down on storage of the work Topic, 3 times per row (in 2 indexes)
* only store one view per user per topic
* only store one view per ip per topic

app/controllers/topics_controller.rb

@@ -396,7 +396,7 @@ class TopicsController < ApplicationController
     end unless request.xhr?
 
     Scheduler::Defer.later "Track Visit" do
-      View.create_for_parent(Topic, topic_id, ip, user_id)
+      TopicViewItem.add(topic_id, ip, user_id)
       if track_visit
         TopicUser.track_visit! topic_id, user_id
       end

app/models/incoming_link.rb

@@ -115,7 +115,6 @@ end
 # Table name: incoming_links
 #
 #  id                  :integer          not null, primary key
-#  topic_id            :integer
 #  created_at          :datetime
 #  user_id             :integer
 #  ip_address          :inet

app/models/incoming_referer.rb

@@ -24,7 +24,6 @@ end
 # Table name: incoming_referers
 #
 #  id                 :integer          not null, primary key
-#  url                :string(1000)     not null
 #  path               :string(1000)     not null
 #  incoming_domain_id :integer          not null
 #

app/models/leader_requirements.rb

@@ -59,7 +59,7 @@ class LeaderRequirements
   end
 
   def topics_viewed_query
-    View.where(user_id: @user.id, parent_type: 'Topic').select('distinct(parent_id)')
+    TopicViewItem.where(user_id: @user.id).select('topic_id')
   end
 
   def topics_viewed

app/models/top_topic.rb

@@ -92,8 +92,8 @@ class TopTopic < ActiveRecord::Base
   end
 
   def self.update_views_count_for(period)
-    sql = "SELECT parent_id as topic_id, COUNT(*) AS count
+    sql = "SELECT topic_id, COUNT(*) AS count
-           FROM views
+           FROM topic_views
            WHERE viewed_at >= :from
            GROUP BY topic_id"
 

app/models/topic_view_item.rb

@@ -0,0 +1,50 @@
+require 'ipaddr'
+
+# awkward TopicView is taken
+class TopicViewItem < ActiveRecord::Base
+  self.table_name = 'topic_views'
+  belongs_to :user
+  validates_presence_of :topic_id, :ip_address, :viewed_at
+
+  def self.add(topic_id, ip, user_id, at=nil)
+    # Only store a view once per day per thing per user per ip
+    redis_key = "view:#{topic_id}:#{Date.today.to_s}"
+    if user_id
+      redis_key << ":user-#{user_id}"
+    else
+      redis_key << ":ip-#{ip}"
+    end
+
+    if $redis.setnx(redis_key, "1")
+      $redis.expire(redis_key, 1.day.to_i)
+
+      TopicViewItem.transaction do
+        at ||= Date.today
+        TopicViewItem.create!(topic_id: topic_id, ip_address: ip, viewed_at: at, user_id: user_id)
+
+        # Update the views count in the parent, if it exists.
+        Topic.where(id: topic_id).update_all 'views = views + 1'
+      end
+    end
+  rescue ActiveRecord::RecordNotUnique
+    # don't care, skip
+  end
+
+end
+
+# == Schema Information
+#
+# Table name: topic_views
+#
+#  topic_id   :integer          not null
+#  viewed_at  :date             not null
+#  user_id    :integer
+#  ip_address :inet             not null
+#
+# Indexes
+#
+#  index_topic_views_on_topic_id_and_viewed_at  (topic_id,viewed_at)
+#  index_topic_views_on_viewed_at_and_topic_id  (viewed_at,topic_id)
+#  ip_address_topic_id_topic_views              (ip_address,topic_id) UNIQUE
+#  user_id_topic_id_topic_views                 (user_id,topic_id) UNIQUE
+#

app/models/user.rb

@@ -86,7 +86,7 @@ class User < ActiveRecord::Base
   before_destroy do
     # These tables don't have primary keys, so destroying them with activerecord is tricky:
     PostTiming.delete_all(user_id: self.id)
-    View.delete_all(user_id: self.id)
+    TopicViewItem.delete_all(user_id: self.id)
   end
 
   # Whether we need to be sending a system message after creation

app/models/user_stat.rb

@@ -13,10 +13,9 @@ class UserStat < ActiveRecord::Base
     # Update denormalized topics_entered
     exec_sql "UPDATE user_stats SET topics_entered = X.c
              FROM
-            (SELECT v.user_id,
+            (SELECT v.user_id, COUNT(topic_id) AS c
-                    COUNT(DISTINCT parent_id) AS c
+             FROM topic_views AS v
-             FROM views AS v
+             WHERE v.user_id IN (
-             WHERE parent_type = 'Topic' AND v.user_id IN (
                 SELECT u1.id FROM users u1 where u1.last_seen_at > :seen_at
              )
              GROUP BY v.user_id) AS X

app/models/view.rb

@@ -1,51 +0,0 @@
-require 'ipaddr'
-
-class View < ActiveRecord::Base
-  belongs_to :parent, polymorphic: true
-  belongs_to :user
-  validates_presence_of :parent_type, :parent_id, :ip_address, :viewed_at
-
-  def self.create_for_parent(parent_class, parent_id, ip, user_id)
-    # Only store a view once per day per thing per user per ip
-    redis_key = "view:#{parent_class.name}:#{parent_id}:#{Date.today.to_s}"
-    if user_id
-      redis_key << ":user-#{user_id}"
-    else
-      redis_key << ":ip-#{ip}"
-    end
-
-    if $redis.setnx(redis_key, "1")
-      $redis.expire(redis_key, 1.day.to_i)
-
-      View.transaction do
-        View.create!(parent_id: parent_id, parent_type: parent_class.to_s, ip_address: ip, viewed_at: Date.today, user_id: user_id)
-
-        # Update the views count in the parent, if it exists.
-        if parent_class.columns_hash["views"]
-          parent_class.where(id: parent_id).update_all 'views = views + 1'
-        end
-      end
-    end
-  end
-
-  def self.create_for(parent, ip, user=nil)
-    user_id = user.id if user
-    create_for_parent(parent.class, parent.id, ip, user_id)
-  end
-end
-
-# == Schema Information
-#
-# Table name: views
-#
-#  parent_id   :integer          not null
-#  parent_type :string(50)       not null
-#  viewed_at   :date             not null
-#  user_id     :integer
-#  ip_address  :inet             not null
-#
-# Indexes
-#
-#  index_views_on_parent_id_and_parent_type              (parent_id,parent_type)
-#  index_views_on_user_id_and_parent_type_and_parent_id  (user_id,parent_type,parent_id)
-#

db/migrate/20140804072504_views_to_topic_views.rb

@@ -0,0 +1,11 @@
+class ViewsToTopicViews < ActiveRecord::Migration
+  def change
+    remove_column :views, :parent_type
+    rename_column :views, :parent_id, :topic_id
+
+    rename_table :views, :topic_views
+
+    add_index :topic_views, [:topic_id]
+    add_index :topic_views, [:user_id, :topic_id]
+  end
+end

db/migrate/20140804075613_normalize_topic_view_data_and_index.rb

@@ -0,0 +1,39 @@
+class NormalizeTopicViewDataAndIndex < ActiveRecord::Migration
+  def change
+    remove_index :topic_views, [:topic_id]
+    remove_index :topic_views, [:user_id, :topic_id]
+
+    execute 'CREATE TEMPORARY TABLE tmp_views_user(user_id int, topic_id int, viewed_at date, ip_address inet)'
+
+    execute 'INSERT INTO tmp_views_user(user_id, topic_id, ip_address, viewed_at)
+      SELECT user_id, topic_id, min(ip_address::varchar)::inet, min(viewed_at)
+      FROM topic_views
+      WHERE user_id IS NOT NULL
+      GROUP BY user_id, topic_id
+    '
+
+    execute 'CREATE TEMPORARY TABLE tmp_views_ip(topic_id int, viewed_at date, ip_address inet)'
+
+    execute 'INSERT INTO tmp_views_ip(topic_id, ip_address, viewed_at)
+      SELECT topic_id, ip_address, min(viewed_at)
+      FROM topic_views
+      WHERE user_id IS NULL
+      GROUP BY user_id, topic_id, ip_address
+    '
+
+    execute 'truncate table topic_views'
+
+    execute 'INSERT INTO topic_views(user_id, topic_id, ip_address, viewed_at)
+    SELECT user_id, topic_id, ip_address, viewed_at FROM tmp_views_user
+    UNION ALL
+    SELECT NULL, topic_id, ip_address, viewed_at FROM tmp_views_ip
+    '
+
+
+    execute 'CREATE UNIQUE INDEX user_id_topic_id_topic_views ON topic_views(user_id, topic_id) WHERE user_id IS NOT NULL'
+    execute 'CREATE UNIQUE INDEX ip_address_topic_id_topic_views ON topic_views(ip_address, topic_id) WHERE user_id IS NULL'
+
+    add_index :topic_views, [:topic_id, :viewed_at]
+    add_index :topic_views, [:viewed_at, :topic_id]
+  end
+end

spec/controllers/topics_controller_spec.rb

@@ -587,7 +587,7 @@ describe TopicsController do
     end
 
     it 'records a view' do
-      lambda { xhr :get, :show, topic_id: topic.id, slug: topic.slug }.should change(View, :count).by(1)
+      lambda { xhr :get, :show, topic_id: topic.id, slug: topic.slug }.should change(TopicViewItem, :count).by(1)
     end
 
     it 'records incoming links' do

spec/models/leader_requirements_spec.rb

@@ -10,7 +10,7 @@ describe LeaderRequirements do
   end
 
   def make_view(id, at, user_id)
-    View.create!(parent_id: id, parent_type: 'Topic', ip_address: '11.22.33.44', viewed_at: at, user_id: user_id)
+    TopicViewItem.add(id, '11.22.33.44', user_id, at)
   end
 
   describe "requirements" do

spec/models/topic_view_item_spec.rb

@@ -0,0 +1,5 @@
+require 'spec_helper'
+
+describe TopicView do
+
+end

spec/models/user_stat_spec.rb

@@ -26,7 +26,7 @@ describe UserStat do
 
       context 'with a view' do
         let(:topic) { Fabricate(:topic) }
-        let!(:view) { View.create_for(topic, '127.0.0.1', user) }
+        let!(:view) { TopicViewItem.add(topic.id, '127.0.0.1', user.id) }
 
         before do
           user.update_column :last_seen_at, 1.second.ago
@@ -39,7 +39,7 @@ describe UserStat do
         end
 
         it "won't record a second view as a different topic" do
-          View.create_for(topic, '127.0.0.1', user)
+          TopicViewItem.add(topic.id, '127.0.0.1', user.id)
           UserStat.update_view_counts
           stat.reload
           stat.topics_entered.should == 1

spec/models/view_spec.rb

@@ -1,13 +0,0 @@
-require 'spec_helper'
-
-describe View do
-
-  it { should belong_to :parent }
-  it { should belong_to :user }
-  it { should validate_presence_of :parent_type }
-  it { should validate_presence_of :parent_id }
-  it { should validate_presence_of :ip_address }
-  it { should validate_presence_of :viewed_at }
-
-
-end