Rebuild trigram indexes.ab-db14-rebuild-trigrm-indexes

This was necessary due to a postgres bug with GIN indexes that might
leave invalid entries behind.

From PostgreSQL 9.6.7 Release Notes [1]:

> Ensure that vacuum will always clean up the pending-insertions list of a
> GIN index (Masahiko Sawada)

> This is necessary to ensure that dead index entries get removed. The old
> code got it backwards, allowing vacuum to skip the cleanup if some other
> process were running cleanup concurrently, thus risking invalid entries
> being left behind in the index.

[1] https://www.postgresql.org/docs/9.6/static/release-9-6-7.html

Closes https://gitlab.com/gitlab-com/database/issues/14.

6 files changed, 170 insertions, 1 deletions

diff --git a/changelogs/unreleased/ab-db14-rebuild-trigrm-indexes.yml b/changelogs/unreleased/ab-db14-rebuild-trigrm-indexes.yml
new file mode 100644
index 00000000000..0111cd36fa4
--- /dev/null
+++ b/changelogs/unreleased/ab-db14-rebuild-trigrm-indexes.yml
@@ -0,0 +1,5 @@
+---
+title: Rebuild trigram indexes.
+merge_request: 19424
+author:
+type: other
diff --git a/db/post_migrate/20180605124335_rebuild_trigram_indexes.rb b/db/post_migrate/20180605124335_rebuild_trigram_indexes.rb
new file mode 100644
index 00000000000..13e4f7c31bf
--- /dev/null
+++ b/db/post_migrate/20180605124335_rebuild_trigram_indexes.rb
@@ -0,0 +1,85 @@
+class RebuildTrigramIndexes < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  DOWNTIME = false
+
+  TRIGRAM_INDEXES = {
+      issues: %i(title description),
+      merge_requests: %i(title description),
+      milestones: %i(title description),
+      namespaces: %i(name path),
+      notes: %i(note),
+      projects: %i(name path description),
+      snippets: %i(title file_name),
+      users: %i(username name email),
+    }.freeze
+
+  def self.trigram_indexes
+    TRIGRAM_INDEXES.map do |key, index_names|
+      index_names.map { |name| [key, name] }
+    end.flatten(1)
+  end
+
+  def up
+    return unless Gitlab::Database.postgresql?
+
+    create_trigrams_extension
+
+    unless trigrams_enabled?
+      raise 'You must enable the pg_trgm extension. You can do so by running ' \
+        '"CREATE EXTENSION pg_trgm;" as a PostgreSQL super user, this must be ' \
+        'done for every GitLab database. For more information see ' \
+        'http://www.postgresql.org/docs/current/static/sql-createextension.html'
+    end
+
+    self.class.trigram_indexes.each_with_index do |(table, column), i|
+      delay = (i+1)*6.hours
+      BackgroundMigrationWorker.perform_in(delay, Gitlab::BackgroundMigration::RebuildTrigramIndex, [table, column])
+    end
+  end
+
+  def down
+    return unless Gitlab::Database.postgresql?
+
+    self.class.trigram_indexes.each do |(table, column)|
+      index_name = "index_#{table}_on_#{column}_trigram"
+      index_name_old = "#{index_name}_old"
+
+      # Clean up any left-over "_old" indexes
+      # (This is useful in case the migration is aborted)
+      if index_exists_by_name?(table, index_name_old)
+        if index_exists_by_name?(table, index_name)
+          # Both indexes exist - remove the newly created one
+          remove_concurrent_index_by_name table, index_name
+        end
+
+        rename_index table, index_name_old, index_name
+      end
+    end
+  end
+
+  private
+
+  def trigrams_enabled?
+    res = execute("SELECT true AS enabled FROM pg_available_extensions WHERE name = 'pg_trgm' AND installed_version IS NOT NULL;")
+    row = res.first
+
+    check = if Gitlab.rails5?
+              true
+            else
+              't'
+            end
+    row && row['enabled'] == check ? true : false
+  end
+
+  def create_trigrams_extension
+    # This may not work if the user doesn't have permission. We attempt in
+    # case we do have permission, particularly for test/dev environments.
+    begin
+      enable_extension 'pg_trgm'
+    rescue
+    end
+  end
+end
diff --git a/lib/gitlab/background_migration/rebuild_trigram_index.rb b/lib/gitlab/background_migration/rebuild_trigram_index.rb
new file mode 100644
index 00000000000..d4dd0d721ab
--- /dev/null
+++ b/lib/gitlab/background_migration/rebuild_trigram_index.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+module Gitlab
+  module BackgroundMigration
+    # Rebuilds an existing trigram index
+    class RebuildTrigramIndex < ActiveRecord::Migration
+      include Gitlab::Database::MigrationHelpers
+
+      def perform(table, column)
+        return unless Gitlab::Database.postgresql?
+
+        disable_statement_timeout
+
+        index_name = "index_#{table}_on_#{column}_trigram"
+        index_name_old = "#{index_name}_old"
+
+        if index_exists_by_name?(table, index_name)
+          if index_exists_by_name?(table, index_name_old)
+            raise "Failed to rename existing index since this index already exists: #{index_name_old}" \
+              "Consider removing the index and re-run the migration."
+          end
+
+          rename_index table, index_name, index_name_old
+        end
+
+        execute "CREATE INDEX CONCURRENTLY #{index_name} ON #{table} USING gin(#{column} gin_trgm_ops);"
+
+        if index_exists_by_name?(table, index_name_old)
+          remove_concurrent_index_by_name table, index_name_old
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/background_migration/rebuild_trigram_index_spec.rb b/spec/lib/gitlab/background_migration/rebuild_trigram_index_spec.rb
new file mode 100644
index 00000000000..575b4db6e3a
--- /dev/null
+++ b/spec/lib/gitlab/background_migration/rebuild_trigram_index_spec.rb
@@ -0,0 +1,31 @@
+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::RebuildTrigramIndex, :migration, schema: 20180608201435, if: Gitlab::Database.postgresql? do
+  subject { migration.perform(table, column) }
+  let(:table) { "issues" }
+  let(:column) { "description" }
+  let(:migration) { described_class.new }
+
+  let(:index_name) { "index_#{table}_on_#{column}_trigram" }
+  let(:index_name_old) { "#{index_name}_old" }
+
+  it 'renames the existing index' do
+    expect(migration).to receive(:rename_index).with(table, index_name, index_name_old).and_call_original
+
+    subject
+  end
+
+  it 'creates a new index' do
+    allow(migration).to receive(:execute)
+
+    expect(migration).to receive(:execute).with("CREATE INDEX CONCURRENTLY #{index_name} ON #{table} USING gin(#{column} gin_trgm_ops);").and_call_original
+
+    subject
+  end
+
+  it 'drops the renamed index' do
+    expect(migration).to receive(:remove_concurrent_index_by_name).with(table, index_name_old).and_call_original
+
+    subject
+  end
+end
diff --git a/spec/migrations/rebuild_trigram_indexes_spec.rb b/spec/migrations/rebuild_trigram_indexes_spec.rb
new file mode 100644
index 00000000000..8bb65961334
--- /dev/null
+++ b/spec/migrations/rebuild_trigram_indexes_spec.rb
@@ -0,0 +1,15 @@
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20180605124335_rebuild_trigram_indexes')
+
+describe RebuildTrigramIndexes, :migration, :sidekiq, if: Gitlab::Database.postgresql? do
+  it 'correctly schedules the background migrations' do
+    Sidekiq::Testing.fake! do
+      migrate!
+
+      RebuildTrigramIndexes.trigram_indexes.each do |(table, column)|
+        expect(Gitlab::BackgroundMigration::RebuildTrigramIndex).to be_scheduled_migration(table.to_s, column.to_s)
+      end
+      expect(BackgroundMigrationWorker.jobs.size).to eq(RebuildTrigramIndexes.trigram_indexes.size)
+    end
+  end
+end
diff --git a/spec/support/matchers/background_migrations_matchers.rb b/spec/support/matchers/background_migrations_matchers.rb
index f4127efc6ae..b832d613a68 100644
--- a/spec/support/matchers/background_migrations_matchers.rb
+++ b/spec/support/matchers/background_migrations_matchers.rb
@@ -16,7 +16,7 @@ RSpec::Matchers.define :be_scheduled_migration do |*expected|
   match do |migration|
     BackgroundMigrationWorker.jobs.any? do |job|
       args = job['args'].size == 1 ? [BackgroundMigrationWorker.jobs[0]['args'][0], []] : job['args']
-      args == [migration, expected]
+      args == [migration.to_s, expected]
     end
   end