(PDB-4832) Add test for migration 76 in isolation

Zak-Kent · Zak-Kent · commit 0ef1f1a8138e · 2020-07-31T14:53:02.000-07:00
This commit tests the case where the old version of migration 74 has
already been applied and the idx_report_id index wasn't added to the
partitions. In this case migration 76 will add the index to any existing
partitions. Changing the behavior of migration 74 after the fact should
be safe because the index creation is guarded by 'if not exists' in both
migration 74 and 76. If we've already migrated past 74 before the change
then the index will get added in 76. If someone applies both 74 and 76
at the same time 76 will then become a no-op.
diff --git a/src/puppetlabs/puppetdb/scf/partitioning.clj b/src/puppetlabs/puppetdb/scf/partitioning.clj
@@ -111,6 +111,12 @@
       (format "CREATE UNIQUE INDEX IF NOT EXISTS resource_events_hash_%s ON %s (event_hash)"
               iso-week-year full-table-name)])))
 
+;; This var is used in testing to simulate migration 74 being applied without
+;; adding the idx_reports_id index to partitions. Changing this behavior in
+;; migration 74 should be safe because the index creation is guarded by
+;; 'if not exists' in both the changed migration 74 and in the newer 76.
+(def ^:dynamic add-report-id-idx? true)
+
 (defn create-reports-partition
   "Creates a partition in the reports table"
   [date]
@@ -131,35 +137,38 @@
                     " FOREIGN KEY (status_id) REFERENCES report_statuses(id) ON DELETE CASCADE")
                iso-week-year)])
     (fn [full-table-name iso-week-year]
-      [(format "CREATE UNIQUE INDEX IF NOT EXISTS idx_reports_id_%s ON %s USING btree (id)"
-               iso-week-year full-table-name)
-       (format "CREATE INDEX IF NOT EXISTS idx_reports_compound_id_%s ON %s USING btree (producer_timestamp, certname, hash) WHERE (start_time IS NOT NULL)"
-               iso-week-year full-table-name)
-       (format "CREATE INDEX IF NOT EXISTS idx_reports_noop_pending_%s ON %s USING btree (noop_pending) WHERE (noop_pending = true)"
-               iso-week-year full-table-name)
-       (format "CREATE INDEX IF NOT EXISTS idx_reports_prod_%s ON %s USING btree (producer_id)"
-               iso-week-year full-table-name)
-       (format "CREATE INDEX IF NOT EXISTS idx_reports_producer_timestamp_%s ON %s USING btree (producer_timestamp)"
-               iso-week-year full-table-name)
-       (format "CREATE INDEX IF NOT EXISTS idx_reports_producer_timestamp_by_hour_certname_%s ON %s USING btree (date_trunc('hour'::text, timezone('UTC'::text, producer_timestamp)), producer_timestamp, certname)"
-               iso-week-year full-table-name)
-       (format "CREATE INDEX IF NOT EXISTS reports_cached_catalog_status_on_fail_%s ON %s USING btree (cached_catalog_status) WHERE (cached_catalog_status = 'on_failure'::text)"
-               iso-week-year full-table-name)
-       (format "CREATE INDEX IF NOT EXISTS reports_catalog_uuid_idx_%s ON %s USING btree (catalog_uuid)"
-               iso-week-year full-table-name)
-       (format "CREATE INDEX IF NOT EXISTS reports_certname_idx_%s ON %s USING btree (certname)"
-               iso-week-year full-table-name)
-       (format "CREATE INDEX IF NOT EXISTS reports_end_time_idx_%s ON %s USING btree (end_time)"
-               iso-week-year full-table-name)
-       (format "CREATE INDEX IF NOT EXISTS reports_environment_id_idx_%s ON %s USING btree (environment_id)"
-               iso-week-year full-table-name)
-       (format "CREATE UNIQUE INDEX IF NOT EXISTS reports_hash_expr_idx_%s ON %s USING btree (encode(hash, 'hex'::text))"
-               iso-week-year full-table-name)
-       (format "CREATE INDEX IF NOT EXISTS reports_job_id_idx_%s ON %s USING btree (job_id) WHERE (job_id IS NOT NULL)"
-               iso-week-year full-table-name)
-       (format "CREATE INDEX IF NOT EXISTS reports_noop_idx_%s ON %s USING btree (noop) WHERE (noop = true)"
-               iso-week-year full-table-name)
-       (format "CREATE INDEX IF NOT EXISTS reports_status_id_idx_%s ON %s USING btree (status_id)"
-               iso-week-year full-table-name)
-       (format "CREATE INDEX IF NOT EXISTS reports_tx_uuid_expr_idx_%s ON %s USING btree (((transaction_uuid)::text))"
-               iso-week-year full-table-name)])))
+      (let [indexes
+            [(format "CREATE INDEX IF NOT EXISTS idx_reports_compound_id_%s ON %s USING btree (producer_timestamp, certname, hash) WHERE (start_time IS NOT NULL)"
+                     iso-week-year full-table-name)
+             (format "CREATE INDEX IF NOT EXISTS idx_reports_noop_pending_%s ON %s USING btree (noop_pending) WHERE (noop_pending = true)"
+                     iso-week-year full-table-name)
+             (format "CREATE INDEX IF NOT EXISTS idx_reports_prod_%s ON %s USING btree (producer_id)"
+                     iso-week-year full-table-name)
+             (format "CREATE INDEX IF NOT EXISTS idx_reports_producer_timestamp_%s ON %s USING btree (producer_timestamp)"
+                     iso-week-year full-table-name)
+             (format "CREATE INDEX IF NOT EXISTS idx_reports_producer_timestamp_by_hour_certname_%s ON %s USING btree (date_trunc('hour'::text, timezone('UTC'::text, producer_timestamp)), producer_timestamp, certname)"
+                     iso-week-year full-table-name)
+             (format "CREATE INDEX IF NOT EXISTS reports_cached_catalog_status_on_fail_%s ON %s USING btree (cached_catalog_status) WHERE (cached_catalog_status = 'on_failure'::text)"
+                     iso-week-year full-table-name)
+             (format "CREATE INDEX IF NOT EXISTS reports_catalog_uuid_idx_%s ON %s USING btree (catalog_uuid)"
+                     iso-week-year full-table-name)
+             (format "CREATE INDEX IF NOT EXISTS reports_certname_idx_%s ON %s USING btree (certname)"
+                     iso-week-year full-table-name)
+             (format "CREATE INDEX IF NOT EXISTS reports_end_time_idx_%s ON %s USING btree (end_time)"
+                     iso-week-year full-table-name)
+             (format "CREATE INDEX IF NOT EXISTS reports_environment_id_idx_%s ON %s USING btree (environment_id)"
+                     iso-week-year full-table-name)
+             (format "CREATE UNIQUE INDEX IF NOT EXISTS reports_hash_expr_idx_%s ON %s USING btree (encode(hash, 'hex'::text))"
+                     iso-week-year full-table-name)
+             (format "CREATE INDEX IF NOT EXISTS reports_job_id_idx_%s ON %s USING btree (job_id) WHERE (job_id IS NOT NULL)"
+                     iso-week-year full-table-name)
+             (format "CREATE INDEX IF NOT EXISTS reports_noop_idx_%s ON %s USING btree (noop) WHERE (noop = true)"
+                     iso-week-year full-table-name)
+             (format "CREATE INDEX IF NOT EXISTS reports_status_id_idx_%s ON %s USING btree (status_id)"
+                     iso-week-year full-table-name)
+             (format "CREATE INDEX IF NOT EXISTS reports_tx_uuid_expr_idx_%s ON %s USING btree (((transaction_uuid)::text))"
+                     iso-week-year full-table-name)]]
+        (if add-report-id-idx?
+          (conj indexes (format "CREATE UNIQUE INDEX IF NOT EXISTS idx_reports_id_%s ON %s USING btree (id)"
+                                iso-week-year full-table-name))
+          indexes)))))
diff --git a/test/puppetlabs/puppetdb/scf/migrate_test.clj b/test/puppetlabs/puppetdb/scf/migrate_test.clj
@@ -20,6 +20,7 @@
             [puppetlabs.puppetdb.testutils.db :refer [*db* with-test-db]]
             [puppetlabs.puppetdb.scf.hash :as shash]
             [puppetlabs.puppetdb.time :refer [ago days now to-timestamp]]
+            [puppetlabs.puppetdb.scf.partitioning :as part]
             [clojure.string :as str])
   (:import (java.time ZoneId ZonedDateTime)
            (java.sql Timestamp)))
@@ -1474,3 +1475,24 @@
         (apply-migration-for-testing! 76)
         ;; migration 76 should be a no-op
         (check-idx-reports-id)))))
+
+(deftest migration-76-adds-report-id-idx-when-not-added-by-migration-74
+  (testing "All report paritions have idx_reports_id index when old version of 74 applied"
+    (jdbc/with-db-connection *db*
+      (clear-db-for-testing!)
+      ;; don't add the idx_reports_id index when fast forwarding past migration 74
+      (binding [part/add-report-id-idx? false]
+        (fast-forward-to-migration! 75))
+      (let [assert-no-index (fn [index indexes]
+                              (is (nil? (some #(str/includes? % index) indexes))))]
+        ;; check that idx_reports_id wasn't added by migration 74
+        (dorun (->> (utils/partition-names "reports")
+                    (map utils/table-indexes)
+                    (map (partial assert-no-index "idx_reports_id")))))
+      (apply-migration-for-testing! 76)
+      (let [assert-index-exists (fn [index indexes]
+                                  (is (some #(str/includes? % index) indexes)))]
+        ;; check that idx_reports_id is now present in all paritions
+        (dorun (->> (utils/partition-names "reports")
+                    (map utils/table-indexes)
+                    (map (partial assert-index-exists "idx_reports_id"))))))))
