From 9b2886979a7dd27b216857ad81ba7a757397a9dc Mon Sep 17 00:00:00 2001 From: rhymes Date: Tue, 14 Jan 2020 18:03:38 +0100 Subject: [PATCH] ActiveRecord count optimizations (#5478) [deploy] * Do not load messages in memory, just to count them * Add Model.estimated_count * Use Model.estimated_count in task record_db_table_counts * Use size and estimated_count * Use table_name so that .estimated_count works with all models * Add comment to explain the .load.size + .each pattern --- app/labor/email_logic.rb | 20 +++++++++++----- app/models/application_record.rb | 24 +++++++++++++++++++ app/views/internal/users/_notes.erb | 6 ++++- app/views/moderations/mod.html.erb | 8 +++---- app/views/partnerships/index.html.erb | 2 +- .../stories/_sign_in_invitation.html.erb | 2 +- app/views/users/_account.html.erb | 2 +- lib/tasks/fetch.rake | 18 ++++---------- spec/labor/email_logic_spec.rb | 2 -- spec/models/application_record_spec.rb | 10 ++++++++ 10 files changed, 65 insertions(+), 29 deletions(-) create mode 100644 spec/models/application_record_spec.rb diff --git a/app/labor/email_logic.rb b/app/labor/email_logic.rb index 49e45d68e..9433ea4c6 100644 --- a/app/labor/email_logic.rb +++ b/app/labor/email_logic.rb @@ -1,6 +1,5 @@ class EmailLogic - attr_reader :open_percentage, :last_email_sent_at, - :days_until_next_email, :articles_to_send + attr_reader :open_percentage, :last_email_sent_at, :days_until_next_email, :articles_to_send def initialize(user) @user = user @@ -28,13 +27,19 @@ class EmailLogic def get_articles_to_send fresh_date = get_fresh_date + articles = if user_has_followings? + experience_level_rating = (@user.experience_level || 5) + experience_level_rating_min = experience_level_rating - 3.6 + experience_level_rating_max = experience_level_rating + 3.6 + @user.followed_articles. where("published_at > ?", fresh_date). where(published: true, email_digest_eligible: true). where.not(user_id: @user.id). where("score > ?", 12). - where("experience_level_rating > ? AND experience_level_rating < ?", (@user.experience_level || 5) - 3.6, (@user.experience_level || 5) + 3.6). + where("experience_level_rating > ? AND experience_level_rating < ?", + experience_level_rating_min, experience_level_rating_max). order("score DESC"). limit(8) else @@ -46,7 +51,9 @@ class EmailLogic order("score DESC"). limit(8) end + @ready_to_receive_email = false if articles.length < 3 + articles end @@ -62,10 +69,11 @@ class EmailLogic def get_open_rate past_sent_emails = @user.email_messages.where(mailer: "DigestMailer#digest_email").limit(10) - # Will stick with 50% open rate if @user has no/not-enough email digest history - return 0.5 if past_sent_emails.length < 10 - past_sent_emails_count = past_sent_emails.count + + # Will stick with 50% open rate if @user has no/not-enough email digest history + return 0.5 if past_sent_emails_count < 10 + past_opened_emails_count = past_sent_emails.where("opened_at IS NOT NULL").count past_opened_emails_count / past_sent_emails_count end diff --git a/app/models/application_record.rb b/app/models/application_record.rb index 10a4cba84..9dec3eb36 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -1,3 +1,27 @@ class ApplicationRecord < ActiveRecord::Base self.abstract_class = true + + QUERY_ESTIMATED_COUNT = <<~SQL.squish.freeze + SELECT ( + (reltuples / GREATEST(relpages, 1)) * + (pg_relation_size($1) / (GREATEST(current_setting('block_size')::integer, 1))) + )::bigint AS count + FROM pg_class WHERE relname = $2; + SQL + + # Computes an estimated count of the number of rows using stats collected by VACUUM + # inspired by + # and + def self.estimated_count + query_name = "SQL COUNT ESTIMATE: #{table_name}" + table_name_attr = ActiveRecord::Relation::QueryAttribute.new( + "relname", table_name, ActiveRecord::Type::String.new + ) + + result = connection.exec_query( + QUERY_ESTIMATED_COUNT, query_name, [table_name_attr, table_name_attr] + ) + + result.first["count"] + end end diff --git a/app/views/internal/users/_notes.erb b/app/views/internal/users/_notes.erb index d7f303561..804f8f493 100644 --- a/app/views/internal/users/_notes.erb +++ b/app/views/internal/users/_notes.erb @@ -1,7 +1,11 @@

Notes

- <% if @user.notes.count > 0 %> + + <%# notes are going to be iterated upon in the following line, + thus we can preload them, to avoid a second SELECT COUNT(*) + that would instead happen when ActiveRecord calls .each on a collection #%> + <% if @user.notes.load.size > 0 %> <% @user.notes.each do |note| %>

<%= note.created_at.strftime("%d %B %Y %H:%M UTC") %> by <%= User.find(note.author_id).username if note.author_id.present? %> - <% if !note.reason.blank? %><%= note.reason %>: diff --git a/app/views/moderations/mod.html.erb b/app/views/moderations/mod.html.erb index 4bdfc01df..7b2457465 100644 --- a/app/views/moderations/mod.html.erb +++ b/app/views/moderations/mod.html.erb @@ -28,7 +28,7 @@

All negative reactions are private.

-
+

Use Thumbsdown to move this content "down" for any reason (quality, usefulness, etc.).

@@ -53,13 +53,13 @@ <% end %> <%= f.hidden_field :article_id, value: @moderatable.id %> <% if current_user.any_admin? || @tag_moderator_tags.any? { |tag| @moderatable.tag_list.include?(tag.name) } %> -
+
<%= f.radio_button :adjustment_type, "removal", required: true %> <%= f.label :adjustment_type, "Remove", value: "removal" %>
<% end %> - <% if @moderatable.tag_list.count < 4 %> -
+ <% if @moderatable.tag_list.size < 4 %> +
<%= f.radio_button :adjustment_type, "addition", required: true %> <%= f.label :adjustment_type, "Add", value: "addition" %>
diff --git a/app/views/partnerships/index.html.erb b/app/views/partnerships/index.html.erb index 49f7da32e..d16110eb3 100644 --- a/app/views/partnerships/index.html.erb +++ b/app/views/partnerships/index.html.erb @@ -10,7 +10,7 @@

Partner With " />

🚀 Reach

- DEV serves millions of unique visitors per month. This highly-targeted developer audience is comprised of registered members (<%= User.count %>) and visitors from the open web — many of whom visit DEV as part of their daily routine. You may be interested in the public analytics available on SimilarWeb. + DEV serves millions of unique visitors per month. This highly-targeted developer audience is comprised of registered members (<%= User.estimated_count %>) and visitors from the open web — many of whom visit DEV as part of their daily routine. You may be interested in the public analytics available on SimilarWeb.

❤️ Community

diff --git a/app/views/stories/_sign_in_invitation.html.erb b/app/views/stories/_sign_in_invitation.html.erb index f28e83cb2..862fbd3b7 100644 --- a/app/views/stories/_sign_in_invitation.html.erb +++ b/app/views/stories/_sign_in_invitation.html.erb @@ -3,7 +3,7 @@ " alt="rainbow DEV logo" />

- DEV is a community of
<%= number_with_delimiter User.count %> amazing humans who code. + DEV is a community of
<%= number_with_delimiter User.estimated_count %> amazing humans who code.

diff --git a/app/views/users/_account.html.erb b/app/views/users/_account.html.erb index 37b90e132..046e6ea24 100644 --- a/app/views/users/_account.html.erb +++ b/app/views/users/_account.html.erb @@ -53,7 +53,7 @@ <% end %>

Danger Zone

-<% if @user.identities.count == 2 %> +<% if @user.identities.size == 2 %>

Remove OAuth Associations

You can remove one of your authentication methods. We'll still need one to authenticate you. diff --git a/lib/tasks/fetch.rake b/lib/tasks/fetch.rake index bbf98e70c..f2af0e882 100644 --- a/lib/tasks/fetch.rake +++ b/lib/tasks/fetch.rake @@ -113,19 +113,11 @@ task fix_credits_count_cache: :environment do end task record_db_table_counts: :environment do - table_names = %w[users articles organizations comments podcasts classified_listings page_views] - table_names.each do |table_name| - estimate = ActiveRecord::Base.connection.execute("SELECT reltuples::bigint AS estimate FROM pg_class where relname='#{table_name}'").first["estimate"] - Rails.logger.info( - "db_table_size", - table_info: { - table_name: table_name, - table_size: estimate - }, - ) - DataDogStatsClient.gauge( - "postgres.db_table_size", estimate, tags: { table_name: table_name } - ) + models = [User, Article, Organization, Comment, Podcast, ClassifiedListing, PageView] + models.each do |model| + estimate = model.estimated_count + Rails.logger.info("db_table_size", table_info: { table_name: model.table_name, table_size: estimate }) + DataDogStatsClient.gauge("postgres.db_table_size", estimate, tags: { table_name: model.table_name }) end end diff --git a/spec/labor/email_logic_spec.rb b/spec/labor/email_logic_spec.rb index 13424d484..ab7a56cfd 100644 --- a/spec/labor/email_logic_spec.rb +++ b/spec/labor/email_logic_spec.rb @@ -3,8 +3,6 @@ require "rails_helper" RSpec.describe EmailLogic, type: :labor do let(:user) { create(:user) } - # TODO: improve this test suite, and improve it's speed - describe "#analyze" do context "when user is brand new with no-follow" do it "returns 0.5 for open_percentage" do diff --git a/spec/models/application_record_spec.rb b/spec/models/application_record_spec.rb new file mode 100644 index 000000000..edf394b5d --- /dev/null +++ b/spec/models/application_record_spec.rb @@ -0,0 +1,10 @@ +require "rails_helper" + +# ApplicationRecord is an abstract class, tests will use one of the core models +RSpec.describe ApplicationRecord, type: :model do + describe ".estimated_count" do + it "does not raise errors if there are no rows" do + expect { User.estimated_count }.not_to raise_error + end + end +end