Skip to content

Better caching and indexing of broadcast messages

yorickpeterse-staging requested to merge broadcast-messages-cache into master

This cleans up BroadcastMessage so we cache data in a better way, adds an index for better querying, and adds some NOT NULL constraints that were missing. These are some fairly minor changes but they help get rid of unnecessary queries to the broadcast_messages table. In particular on production we sometimes do 40-50 sequential reads in a minute on this table. This isn't that bad since we only have 35 rows, but we can very easily turn this into an index scan once every few weeks at most.

Database Checklist

When adding migrations:

  • Updated db/schema.rb
  • Added a down method so the migration can be reverted
  • Added the output of the migration(s) to the MR body
  • Added the execution time of the migration(s) to the MR body
  • Made sure the migration won't interfere with a running GitLab cluster, for example by disabling transactions for long running migrations

When adding or modifying queries:

  • Included the raw SQL queries of the relevant queries
  • Included the output of EXPLAIN ANALYZE and execution timings of the relevant queries
  • Added tests for the relevant changes

When adding indexes:

  • Described the need for these indexes in the MR body
  • Made sure existing indexes can not be reused instead

Query Output

These are taken from my development environment.

The old query is as follows:

SELECT "broadcast_messages".* 
FROM "broadcast_messages" 
WHERE (ends_at > '2017-08-09 14:07:30.607384' AND starts_at <= '2017-08-09 14:07:30.607384')  
ORDER BY "broadcast_messages"."id" DESC, "broadcast_messages"."created_at" ASC, "broadcast_messages"."id" ASC

And its plan:

                                                                              QUERY PLAN                                                                              
----------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Sort  (cost=10000000017.42..10000000017.54 rows=46 width=168) (actual time=0.014..0.014 rows=0 loops=1)
   Sort Key: id DESC, created_at
   Sort Method: quicksort  Memory: 25kB
   ->  Seq Scan on broadcast_messages  (cost=10000000000.00..10000000016.15 rows=46 width=168) (actual time=0.004..0.004 rows=0 loops=1)
         Filter: ((ends_at > '2017-08-09 14:07:30.607384'::timestamp without time zone) AND (starts_at <= '2017-08-09 14:07:30.607384'::timestamp without time zone))
 Planning time: 0.141 ms
 Execution time: 0.054 ms

One thing that stands out here is redundant ordering due to the use of order and not reorder, this is fixed in this MR.

The new query:

 SELECT "broadcast_messages".* 
FROM "broadcast_messages" 
WHERE (ends_at > '2017-08-09 14:11:10.801179' AND starts_at <= '2017-08-09 14:11:10.801179')  
ORDER BY "broadcast_messages"."id" ASC

And its plan:

                                                                                      QUERY PLAN                                                                                       
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Sort  (cost=14.25..14.36 rows=46 width=168) (actual time=0.006..0.006 rows=0 loops=1)
   Sort Key: id
   Sort Method: quicksort  Memory: 25kB
   ->  Index Scan using index_broadcast_messages_on_starts_at_and_ends_at_and_id on broadcast_messages  (cost=0.15..12.98 rows=46 width=168) (actual time=0.003..0.003 rows=0 loops=1)
         Index Cond: ((starts_at <= '2017-08-09 14:11:10.801179'::timestamp without time zone) AND (ends_at > '2017-08-09 14:11:10.801179'::timestamp without time zone))
 Planning time: 0.103 ms
 Execution time: 0.028 ms

Migration Output

== 20170809133343 AddBroadcastMessagesIndex: migrating ========================
-- transaction_open?()
   -> 0.0000s
-- execute("SET statement_timeout TO 0")
   -> 0.0002s
-- add_index(:broadcast_messages, [:starts_at, :ends_at, :id], {:algorithm=>:concurrently})
   -> 0.0189s
== 20170809133343 AddBroadcastMessagesIndex: migrated (0.0194s) ===============

== 20170809134534 AddBroadcastMessageNotNullConstraints: migrating ============
-- change_column_null(:broadcast_messages, :starts_at, false)
   -> 0.0005s
-- change_column_null(:broadcast_messages, :ends_at, false)
   -> 0.0003s
-- change_column_null(:broadcast_messages, :created_at, false)
   -> 0.0003s
-- change_column_null(:broadcast_messages, :updated_at, false)
   -> 0.0003s
-- change_column_null(:broadcast_messages, :message_html, false)
   -> 0.0002s
== 20170809134534 AddBroadcastMessageNotNullConstraints: migrated (0.0018s) ===

This is from my local environment. Since this table is tiny these migrations are expected to only take a few seconds.

General Checklist

Merge request reports