Merge pull request #4004 from rbrw/pdb-5672-analyze-parent-partitions

(PDB-5672) Periodically analyze reports table

ext/bin/prep-debianish-root

@@ -30,12 +30,13 @@ apt-get-update() {
 install-pg() {
     local need_pgdg=
     for pkg in "$@"; do
-        if ! apt-cahe show "$pkg"; then
+        if ! apt-cache show "$pkg"; then
             need_pgdg=1
             break
         fi
     done
     if test "$need_pgdg"; then
+        apt install -y lsb-release gnupg2
         curl https://www.postgresql.org/media/keys/ACCC4CF8.asc | sudo apt-key add -
         echo "deb http://apt.postgresql.org/pub/repos/apt $(lsb_release -sc)-pgdg main" \
              > /etc/apt/sources.list.d/pdb-pgdg.list

src/puppetlabs/puppetdb/cli/services.clj

@@ -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]]
@@ -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}
@@ -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 []

src/puppetlabs/puppetdb/query/monitor.clj

@@ -355,7 +355,7 @@
        (reset! exit true)
        (.wakeup selector)
        (if timeout-ms
-         (.join thread timeout-ms)
+         (.join thread ^long timeout-ms)
          (.join thread))
        (not (.isAlive thread))))))
 

src/puppetlabs/puppetdb/scf/storage.clj

@@ -373,7 +373,7 @@
   "Returns the id (primary key) from the table for the row with col = name."
   [table col name]
   (->> {:select :id :from table :where [:= col name]}
-       hsql/format (select-one! (jdbc/connection)  :id)))
+       hsql/format (select-one! (jdbc/connection) :id)))
 
 (defn environment-id [name] (named-row-id :environments :environment name))
 (defn certname-id [name] (named-row-id :certnames :certname name))

test/puppetlabs/puppetdb/cli/services_test.clj

@@ -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]
@@ -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))))))