Replace indexes for counting active usersab-admin-page-user-active-count

This adjusts the partial condition for an index. The index is intended
to be used when counting active users with `ghost IS NOT TRUE AND
bot_type IS NULL`.

With the current index, this wasn't working as the partial condition
didn't match the query: `ghost <> TRUE` is not semantically equivalent
to `ghost IS NOT TRUE` (null semantics).

The reason we add an index particularly intended for EE is that the EE
query is going to have the additional part `AND bot_type IS NULL`
whereas the CE query doesn't. Logically, it'd be enough to have an index
for `ghost IS NOT TRUE`. However, on GitLab.com, the query planner makes
poor choices when the additional `AND bot_type IS NULL` part is present:
It goes for the index on `bot_type` and doesn't use the partial index.

Note the existing index isn't being used at all according to GitLab.com
index statistics. Hence we can first remove it and don't have to worry
about the window of time without an index.

Relates to https://gitlab.com/gitlab-org/gitlab-ce/issues/66770

3 files changed, 31 insertions, 2 deletions

diff --git a/changelogs/unreleased/ab-admin-page-user-active-count.yml b/changelogs/unreleased/ab-admin-page-user-active-count.yml
new file mode 100644
index 00000000000..e08715a1586
--- /dev/null
+++ b/changelogs/unreleased/ab-admin-page-user-active-count.yml
@@ -0,0 +1,5 @@
+---
+title: Replace indexes for counting active users
+merge_request: 32538
+author:
+type: performance
diff --git a/db/migrate/20190902131045_replace_indexes_for_counting_active_users.rb b/db/migrate/20190902131045_replace_indexes_for_counting_active_users.rb
new file mode 100644
index 00000000000..2c7c47bee96
--- /dev/null
+++ b/db/migrate/20190902131045_replace_indexes_for_counting_active_users.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+class ReplaceIndexesForCountingActiveUsers < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    remove_concurrent_index_by_name(:users, 'index_users_on_state_and_internal')
+
+    add_concurrent_index(:users, :state, where: 'ghost IS NOT TRUE', name: 'index_users_on_state_and_internal')
+    add_concurrent_index(:users, :state, where: 'ghost IS NOT TRUE AND bot_type IS NULL', name: 'index_users_on_state_and_internal_ee')
+  end
+
+  def down
+    remove_concurrent_index_by_name(:users, 'index_users_on_state_and_internal_ee')
+    remove_concurrent_index_by_name(:users, 'index_users_on_state_and_internal')
+
+    add_concurrent_index(:users, :state, where: 'ghost <> true AND bot_type IS NULL', name: 'index_users_on_state_and_internal')
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 5999a859e77..5b89cdf0b98 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_08_28_083843) do
+ActiveRecord::Schema.define(version: 2019_09_02_131045) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
@@ -3551,7 +3551,8 @@ ActiveRecord::Schema.define(version: 2019_08_28_083843) do
     t.index ["public_email"], name: "index_users_on_public_email", where: "((public_email)::text <> ''::text)"
     t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true
     t.index ["state"], name: "index_users_on_state"
-    t.index ["state"], name: "index_users_on_state_and_internal", where: "((ghost <> true) AND (bot_type IS NULL))"
+    t.index ["state"], name: "index_users_on_state_and_internal", where: "(ghost IS NOT TRUE)"
+    t.index ["state"], name: "index_users_on_state_and_internal_ee", where: "((ghost IS NOT TRUE) AND (bot_type IS NULL))"
     t.index ["username"], name: "index_users_on_username"
     t.index ["username"], name: "index_users_on_username_trigram", opclass: :gin_trgm_ops, using: :gin
   end