Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add query blocker tracking based on pg_blocking_pids() #3

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion guides/Linux.md
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ CREATE TABLE "pghero_connection_stats" (
"username" text,
"captured_at" timestamp
);
CREATE INDEX "pghero_connection_stats" ("database", "captured_at");
CREATE INDEX ON "pghero_connection_stats" ("database", "captured_at");
```

Schedule the task below to run once a day.
Expand All @@ -234,6 +234,14 @@ Schedule the task below to run once a day.
sudo pghero run rake pghero:capture_connection_stats
```

## Historical Query Blockers

To track query blockers over time, create a table to store them.

TODO
```sql
CREATE TABLE ...
```

## System Stats

Expand Down
6 changes: 6 additions & 0 deletions guides/Rails.md
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,12 @@ PgHero.drop_user("ganondorf")

## Upgrading

### 2.x.y

New features

Sample blocker queries - TODO

### 2.0.0

New features
Expand Down
47 changes: 38 additions & 9 deletions lib/pghero.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# dependencies
require "active_support"

Expand All @@ -9,8 +11,11 @@
require "pghero/methods/kill"
require "pghero/methods/maintenance"
require "pghero/methods/queries"
require "pghero/methods/query_blockers"
require "pghero/methods/query_blockers_history"
require "pghero/methods/query_stats"
require "pghero/methods/replication"
require "pghero/methods/repository"
require "pghero/methods/sequences"
require "pghero/methods/settings"
require "pghero/methods/space"
Expand All @@ -19,13 +24,14 @@
require "pghero/methods/tables"
require "pghero/methods/users"

require "pghero/base_database"
require "pghero/database"
require "pghero/engine" if defined?(Rails)
require "pghero/pg_const"
require "pghero/repository"
require "pghero/version"

module PgHero
autoload :Connection, "pghero/connection"
autoload :QueryStats, "pghero/query_stats"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to disable autoloading, without requiring these? Are these effectively dead code now?


class Error < StandardError; end
class NotEnabled < Error; end
Expand Down Expand Up @@ -55,7 +61,7 @@ class << self
:query_stats_available?, :query_stats_enabled?, :query_stats_extension_enabled?, :query_stats_readable?,
:rds_stats, :read_iops_stats, :region, :relation_sizes, :replica?, :replication_lag, :replication_lag_stats,
:reset_query_stats, :reset_stats, :running_queries, :secret_access_key, :sequence_danger, :sequences, :settings,
:slow_queries, :space_growth, :ssl_used?, :stats_connection, :suggested_indexes, :suggested_indexes_by_query,
:slow_queries, :space_growth, :ssl_used?, :suggested_indexes, :suggested_indexes_by_query,
:suggested_indexes_enabled?, :system_stats_enabled?, :table_caching, :table_hit_rate, :table_stats,
:total_connections, :transaction_id_danger, :unused_indexes, :unused_tables, :write_iops_stats

Expand Down Expand Up @@ -100,9 +106,10 @@ def config

def databases
@databases ||= begin
repository = PgHero::Repository.new
Hash[
config["databases"].map do |id, c|
[id.to_sym, PgHero::Database.new(id, c)]
[id.to_sym, PgHero::Database.new(id, c, repository)]
end
]
end
Expand All @@ -115,6 +122,7 @@ def primary_database
def capture_query_stats(verbose: false)
each_database do |database|
next unless database.capture_query_stats?

puts "Capturing query stats for #{database.id}..." if verbose
database.capture_query_stats(raise_errors: true)
end
Expand All @@ -134,19 +142,40 @@ def capture_connection_stats(verbose: false)
end
end

def capture_query_blockers(verbose: false)
def capture_query_blockers(verbose: false, filters: nil)
each_database do |database|
next unless database.capture_query_blockers?
if filters && filters.none? { |f| f =~ database.id }
puts "Database #{database.id} did not match any filter; skipping." if verbose
next
elsif !database.capture_query_blockers?
if verbose
puts "Database #{database.id} capture_query_blockers "\
"is disabled via configuration; skipping."
end
next
end

puts "(Simulating) Capturing query blockers for #{database.id}..." if verbose
# TODO: actually implement database.capture_query_blockers
puts "Capturing query blockers for #{database.id}..." if verbose
begin
blocker_sample = database.capture_query_blockers
if verbose
if blocker_sample
puts "Captured blocker sample #{blocker_sample.id} "\
"for #{database.id} with #{blocker_sample.sessions.size} sessions."
else
puts "No blocker sample returned for #{database.id}."
end
end
rescue NotEnabled => e
puts "Query blockers not enabled for #{database.id}: #{e.message}"
end
end
true
end

def analyze_all(**options)
each_database do |database|
next if database.replica?

database.analyze_tables(**options)
end
end
Expand Down
15 changes: 15 additions & 0 deletions lib/pghero/base_database.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# frozen_string_literal: true

module PgHero
class BaseDatabase

include Methods::Basic

# Subclasses must define connection_model returning and ActiveRecord model
# for connection management

def connection
connection_model.connection
end
end
end
5 changes: 0 additions & 5 deletions lib/pghero/connection.rb

This file was deleted.

28 changes: 18 additions & 10 deletions lib/pghero/database.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
# frozen_string_literal: true

require "active_record"

module PgHero
class Database
include Methods::Basic
class Database < BaseDatabase

include Methods::Connections
include Methods::Explain
include Methods::Indexes
include Methods::Kill
include Methods::Maintenance
include Methods::Queries
include Methods::QueryBlockers
include Methods::QueryStats
include Methods::Replication
include Methods::Sequences
Expand All @@ -19,27 +24,24 @@ class Database

attr_reader :id, :config

def initialize(id, config)
def initialize(id, config, repository)
@id = id
@config = config || {}
@repository = repository
end

def name
@name ||= @config["name"] || id.titleize
@name ||= config["name"] || id.titleize
end

def db_instance_identifier
@db_instance_identifier ||= @config["db_instance_identifier"]
@db_instance_identifier ||= config["db_instance_identifier"]
end

def capture_query_stats?
config["capture_query_stats"] != false
end

def capture_query_blockers?
config["capture_query_blockers"] != false
end

def cache_hit_rate_threshold
(config["cache_hit_rate_threshold"] || PgHero.config["cache_hit_rate_threshold"] || PgHero.cache_hit_rate_threshold).to_i
end
Expand Down Expand Up @@ -70,10 +72,12 @@ def index_bloat_bytes

private

attr_reader :repository

def connection_model
@connection_model ||= begin
url = config["url"]
Class.new(PgHero::Connection) do
Class.new(Connection) do
def self.name
"PgHero::Connection::Database#{object_id}"
end
Expand All @@ -87,5 +91,9 @@ def self.name
end
end
end

class Connection < ActiveRecord::Base
self.abstract_class = true
end
end
end
Loading