Skip to content

Commit

Permalink
(PDB-5672) Analyze reports every hour
Browse files Browse the repository at this point in the history
postgres (up to at least 16) never analyzes partitioned table parents
Nearly fixed in 14, but removed before release:
  https://www.postgresql.org/about/news/postgresql-14-beta-1-released-2213/
  https://www.postgresql.org/about/news/postgresql-14-rc-1-released-2309/

Assumes the analysis runs quickly enough to avoid needing any enforced
serialization, and to avoid (with the current single threaded
executor) ever delaying gc enough to matter.
  • Loading branch information
rbrw committed Sep 27, 2024
1 parent 443ca1e commit 8d1f620
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 2 deletions.
1 change: 1 addition & 0 deletions project.clj
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@
8 "-Djava.security.properties==dev-resources/jdk8-fips-security"
11 "-Djava.security.properties==dev-resources/jdk11on-fips-security"
17 "-Djava.security.properties==dev-resources/jdk11on-fips-security"
21 "-Djava.security.properties==dev-resources/jdk11on-fips-security"
(throw (ex-info "Unsupported major Java version. Expects 8, 11, or 17."
{:major feature :minor interim})))))}]
:kondo {:dependencies [[clj-kondo "2024.05.24"]]}
Expand Down
32 changes: 30 additions & 2 deletions src/puppetlabs/puppetdb/cli/services.clj
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,15 @@
data may linger in the database. We periodically sweep the
database, compacting it and performing regular cleanup so we can
maintain acceptable performance."
(:require [clojure.string :as str]
(:require [clojure.java.jdbc :as sql]
[clojure.string :as str]
[clojure.tools.logging :as log]
[metrics.counters :as counters :refer [counter inc!]]
[metrics.gauges :refer [gauge-fn]]
[metrics.timers :refer [time! timer]]
[metrics.reporters.jmx :as jmx-reporter]
[murphy :refer [try! with-final]]
[next.jdbc :as nxt]
[puppetlabs.i18n.core :refer [trs tru]]
[puppetlabs.kitchensink.core :as kitchensink]
[puppetlabs.puppetdb.cli.tk-util :refer [run-tk-cli-cmd]]
Expand Down Expand Up @@ -939,6 +941,21 @@
(catch InterruptedException _
(log/info (trs "Garbage collector interrupted")))))))

(defn analyze-partitioned-tables [db shutdown-for-ex]
(with-nonfatal-exceptions-suppressed
(with-monitored-execution shutdown-for-ex
(try!
(jdbc/with-db-connection db
(jdbc/with-db-transaction []
(let [c (sql/db-connection jdbc/*db*)]
(doseq [table ["reports" "resource_events"]]
(try!
(nxt/execute! c [(str "analyze " table)])
(catch InterruptedException _
(log/info (trs (str table " analysis interrupted")))))))))
(catch Exception ex
(log/error ex))))))

(defn start-garbage-collection
"Starts garbage collection of the databases represented in db-configs"
[{:keys [clean-lock] :as _context}
Expand All @@ -952,7 +969,18 @@
(schedule-with-fixed-delay sched #(invoke-periodic-gc db cfg request
shutdown-for-ex
clean-lock lock-status)
(to-millis interval) (to-millis interval)))))))
(to-millis interval) (to-millis interval))))
;; pg (up to at least 16) never analyzes partitioned table parents
;; Nearly fixed in 14, but removed before release:
;; https://www.postgresql.org/about/news/postgresql-14-beta-1-released-2213/
;; https://www.postgresql.org/about/news/postgresql-14-rc-1-released-2309/
;;
;; Assumes the analysis runs quickly enough to avoid needing any
;; enforced serialization, and to avoid (with the current single
;; threaded executor) ever delaying gc enough to matter.
(let [hourly (* 60 60 1000)
analyze #(analyze-partitioned-tables db shutdown-for-ex)]
(schedule-with-fixed-delay sched analyze 0 hourly)))))


(defn database-lock-status []
Expand Down
28 changes: 28 additions & 0 deletions test/puppetlabs/puppetdb/cli/services_test.clj
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
(ns puppetlabs.puppetdb.cli.services-test
(:require [clojure.set :refer [subset?]]
[next.jdbc.plan :refer [select-one!]]
[puppetlabs.http.client.sync :as pl-http]
[puppetlabs.puppetdb.cli.util :refer [err-exit-status]]
[puppetlabs.puppetdb.command.constants :as cmd-consts]
Expand Down Expand Up @@ -620,3 +621,30 @@
:db-lock-status db-lock-status})
(is (= 1 (count (jdbc/query ["SELECT * FROM reports_latest"]))))
(is (empty? (jdbc/query ["SELECT * FROM reports_historical"])))))))))

(deftest reports-analysis
;; For now, just test for the initial invocation
(let [start (now)
is-analyzed
(fn is-analyzed
([table] (is-analyzed table 0))
([table i]
(let [r (jdbc/with-db-transaction []
(->> [(str "select last_analyze, last_autoanalyze"
" from pg_stat_user_tables where relname = '" table "'")]
(select-one! (jdbc/connection) [:last_analyze :last_autoanalyze])))
last-ms (some-> r :last_analyze .getTime time/from-long)]
(if (and last-ms (nil? (:last_autoanalyze r)))
(do
(is (= nil (:last_autoanalyze r)))
(is (time/after? last-ms start)))
(if (= i 100)
(is false (str table " was eventually analyzed"))
(do
(Thread/sleep 100)
(is-analyzed table (inc i))))))))]
(svc-utils/with-puppetdb-instance
(doseq [parent ["reports" "reports_historical" "reports_latest"
"resource_events" "resource_events_historical" "resource_events_latest"]]
(testing (str parent " analysis times")
(is-analyzed parent))))))

0 comments on commit 8d1f620

Please sign in to comment.