Skip to content

Commit

Permalink
Simplify the presentation
Browse files Browse the repository at this point in the history
Present a explanation box recommending to use CONCURRENTLY with DROP
INDEX. In Active Record, this requires two things:
disable_ddl_transaction! once at the top of the migration file, and
"algorithm: :concurrently" option added to every remove_index line

Since this is a sensible default when PostgreSQL 14 or greater is in
use, since it can avoid downtime from an index drop, make it the
default.
Remove the ability to disable it since it seems unnecessary in
retrospect.

Only show the explanation box and only add the option to
remove_index when version 14 or greater is in use
  • Loading branch information
andyatkinson committed Jan 3, 2024
1 parent c8d32a7 commit 36eaedf
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 10 deletions.
1 change: 0 additions & 1 deletion app/controllers/pg_hero/home_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ def index
@unused_indexes = @database.unused_indexes(max_scans: 0)
end

@remove_index_concurrently = PgHero.remove_index_concurrently
@show_migrations = PgHero.show_migrations
end

Expand Down
13 changes: 11 additions & 2 deletions app/helpers/pg_hero/home_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,24 @@ def pghero_js_value(value)
json_escape(value.to_json(root: false)).html_safe
end

def pghero_remove_index(query, use_concurrently: false)
def pghero_drop_idx_concurrently_explanation
if @database.drop_idx_concurrently_supported?
ret = "<h2>Tip: Use CONCURRENTLY for DROP INDEX</h2>"
ret << "<ul><li>Add <code>disable_ddl_transaction!</code> to your migration</li>"
ret << "<li>Add <code>algorithm: :concurrently</code> to each <code>remove_index</code></li></ul>"
ret.html_safe
end
end

def pghero_remove_index(query)
if query[:columns]
columns = query[:columns].map(&:to_sym)
columns = columns.first if columns.size == 1
end
ret = String.new("remove_index #{query[:table].to_sym.inspect}")
ret << ", name: #{(query[:name] || query[:index]).to_s.inspect}"
ret << ", column: #{columns.inspect}" if columns
ret << ", algorithm: concurrently" if use_concurrently
ret << ", algorithm: concurrently" if @database.drop_idx_concurrently_supported?
ret
end

Expand Down
9 changes: 4 additions & 5 deletions app/views/pg_hero/home/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -390,13 +390,12 @@

<div id="migration2" style="display: none;">
<pre>rails generate migration remove_unneeded_indexes</pre>
<% if @remove_index_concurrently %>
<p>Paste the line below to <a href="" >Disable DDL transactions</a></p>
<pre style="overflow: scroll; white-space: pre; word-break: normal;">disable_ddl_transaction!</pre>
<% if @database.drop_idx_concurrently_supported? %>
<p><%= pghero_drop_idx_concurrently_explanation %></p>
<% end %>
<p>And paste <% if @remove_index_concurrently %>(using <a href="https://www.postgresql.org/docs/current/sql-dropindex.html">CONCURRENTLY</a>)<% end %></p>
<p>And paste</p>
<pre style="overflow: scroll; white-space: pre; word-break: normal;"><% @duplicate_indexes.each do |query| %>
<%= pghero_remove_index(query[:unneeded_index], use_concurrently: @remove_index_concurrently) %><% end %></pre>
<%= pghero_remove_index(query[:unneeded_index]) %><% end %></pre>
</div>

<table class="table duplicate-indexes">
Expand Down
3 changes: 3 additions & 0 deletions app/views/pg_hero/home/space.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@

<div id="migration" style="display: none;">
<pre>rails generate migration remove_unused_indexes</pre>
<% if @database.drop_idx_concurrently_supported? %>
<p><%= pghero_drop_idx_concurrently_explanation %></p>
<% end %>
<p>And paste</p>
<pre style="overflow: scroll; white-space: pre; word-break: normal;"><% @unused_indexes.sort_by { |q| [-q[:size_bytes], q[:index]] }.each do |query| %>
<%= pghero_remove_index(query) %><% end %></pre>
Expand Down
3 changes: 1 addition & 2 deletions lib/pghero.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class NotEnabled < Error; end

# settings
class << self
attr_accessor :long_running_query_sec, :slow_query_ms, :slow_query_calls, :explain_timeout_sec, :total_connections_threshold, :cache_hit_rate_threshold, :env, :show_migrations, :config_path, :filter_data, :remove_index_concurrently
attr_accessor :long_running_query_sec, :slow_query_ms, :slow_query_calls, :explain_timeout_sec, :total_connections_threshold, :cache_hit_rate_threshold, :env, :show_migrations, :config_path, :filter_data
end
self.long_running_query_sec = (ENV["PGHERO_LONG_RUNNING_QUERY_SEC"] || 60).to_i
self.slow_query_ms = (ENV["PGHERO_SLOW_QUERY_MS"] || 20).to_i
Expand All @@ -53,7 +53,6 @@ class << self
self.show_migrations = true
self.config_path = ENV["PGHERO_CONFIG_PATH"] || "config/pghero.yml"
self.filter_data = ENV["PGHERO_FILTER_DATA"].to_s.size > 0
self.remove_index_concurrently = ENV["PGHERO_REMOVE_INDEX_CONCURRENTLY"] || true

class << self
extend Forwardable
Expand Down
4 changes: 4 additions & 0 deletions lib/pghero/methods/basic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ def quote_ident(value)
connection.quote_column_name(value)
end

def drop_idx_concurrently_supported?
server_version_num >= 140000
end

private

def select_all(sql, conn: nil, query_columns: [])
Expand Down

0 comments on commit 36eaedf

Please sign in to comment.