Merge pull request #1331 from rbrw/ticket/master/merge-2.3.1-fact-pat…

…hs-etc

(maint) Merge 2.3.1 into master (fact paths, etc.)

documentation/release_notes.markdown

@@ -302,6 +302,73 @@ derive data from a PuppetDB export tarball.
 
 TODO
 
+2.3.1
+-----
+
+PuppetDB 2.3.1 is a backwards-compatible bugfix release that may
+notably increase performance for larger installations.
+
+### Upgrading
+
+* For the best-possible performance and scaling capacity, we recommend
+  PostgreSQL version 9.4 or newer with the [`pg_trgm`][pg_trgm]
+  extension enabled, as explained [here][configure_postgres], and we
+  have officially deprecated versions earlier than 9.2.  HSQLDB is
+  only recommended for local development because it has a number of
+  scaling and operational issues.
+
+* Make sure that all of your PuppetDB instances are shut down, and
+  only upgrade one at a time.
+
+* Make sure to upgrade your puppetdb-terminus package (on the host
+  where your Puppet master lives), and restart your master service.
+
+### Contributors
+
+John Duarte, Ken Barber, Matthaus Owens, Nick Fagerlund, Russell Mull,
+Wyatt Alt, Rob Browning, Ryan Senior, juniorsysadmin
+
+### Changes
+
+* The tk-jetty9-version has been upgraded from 1.2.0 to 1.3.0.
+  ([PDB-1307](https://tickets.puppetlabs.com/browse/PDB-1307))
+
+#### Bug Fixes and Maintenance
+
+* We have reworked out table schema and fact value garbage
+  collection to ensure that concurrent updates cannot lead
+  to fact GC failures.  Before this fix PuppetDB, under high load,
+  could end up in a situation which would produce PostgreSQL log
+  messages like this:
+
+    ERROR:  update or delete on table "fact_paths" violates foreign key constraint "fact_values_path_id_fk" on table "fact_values"
+    DETAIL:  Key (id)=(11) is still referenced from table "fact_values".
+    STATEMENT:  COMMIT
+
+  The fix for this problem may also significantly increase PuppetDB's
+  performance in some situations.
+  ([PDB-1031](https://tickets.puppetlabs.com/browse/PDB-1031)
+   [PDB-1224](https://tickets.puppetlabs.com/browse/PDB-1224)
+   [PDB-1225](https://tickets.puppetlabs.com/browse/PDB-1225)
+   [PDB-1226](https://tickets.puppetlabs.com/browse/PDB-1226)
+   [PDB-1227](https://tickets.puppetlabs.com/browse/PDB-1227))
+
+* Report timestamps should be hashed consistently now.
+  ([PDB-1286](https://tickets.puppetlabs.com/browse/PDB-1286))
+
+* The facter requirement has been removed from the RPM specfile.
+  ([PDB-1303](https://tickets.puppetlabs.com/browse/PDB-1303))
+
+#### Documentation
+
+* The documentation has been updated for Puppet 4.
+  ([PDB-1305](https://tickets.puppetlabs.com/browse/PDB-1305))
+
+* Omitted .html extensions have been added to some the links in the
+  2.3.0 release notes.
+
+* The documentation sidebar (TOC) is now hosted in the PuppetDB
+  repository. ([PDB-1319](https://tickets.puppetlabs.com/browse/PDB-1319))
 
 2.3.0
 -----
@@ -383,35 +450,35 @@ Ryan Senior, and Wyatt Alt
 #### Documentation
 
 * The documentation for the `<=` and `>=`
-  [operators](./api/query/v2/operators) has been fixed (the
+  [operators](./api/query/v2/operators.html) has been fixed (the
   descriptions were incorrectly reversed).
 
 * The firewall and SELinux requirements have been documented
-  [here](./connect_puppet_master).
+  [here](./connect_puppet_master.html).
   ([PDB-137](https://tickets.puppetlabs.com/browse/PDB-137))
 
 * Broken links have been fixed in the
-  [connection](./connect_puppet_master) and [commands](./api/commands)
+  [connection](./connect_puppet_master.html) and [commands](./api/commands.html)
   documentation.
 
 * A missing `-L` option has been added to a curl invocation
-  [here](./install_from_souce).
+  [here](./install_from_souce.html).
 
 * An incorrect reference to "Java" has been changed to "JVM" in the
-  [configuration](./configure) documentation.
+  [configuration](./configure.html) documentation.
 
 * The relationship between "MQ depth" and "Command Queue depth" has
-  been clarified in the [tuning and maintenance](./maintain_and_tune)
+  been clarified in the [tuning and maintenance](./maintain_and_tune.html)
   documentation.
 
 * An example that uses curl with SSL to communicate with Puppet
-  Enterprise has been added to the [curl](./api/query/curl)
+  Enterprise has been added to the [curl](./api/query/curl.html)
   documentation.
 
 * Some minor edits have been made to the
-  [fact-contents](./api/query/v4/fact-contents),
-  [connection](./connect_puppet_master), and
-  [KahaDB Corruption](./trouble_kahadb_corruption) documentation.
+  [fact-contents](./api/query/v4/fact-contents.html),
+  [connection](./connect_puppet_master.html), and
+  [KahaDB Corruption](./trouble_kahadb_corruption.html) documentation.
 
 #### Testing
 

documentation/repl.markdown

@@ -53,8 +53,7 @@ Within the REPL, you can interactively execute PuppetDB's functions. For example
     nil
     user=> (use 'clojure.java.jdbc)
     nil
-    user=> (with-connection (:database configuration)
-             (garbage-collect!))
+    user=> (garbage-collect! (:database configuration))
     (0)
 
 Redefining Functions

documentation/scaling_recommendations.markdown

@@ -54,7 +54,7 @@ Use one of the following rules of thumb to choose an initial heap size; afterwar
 * If you are using PostgreSQL, allocate 128 MB of memory as a base, plus 1 MB for each Puppet node in your infrastructure.
 * If you are using the embedded database, allocate at least 1 GB of heap.
 
-Bottleneck: Node Checkin Interval
+Bottleneck: Node Check In Interval
 -----
 
 The more frequently your Puppet nodes check in, the heavier the load on your PuppetDB server.

project.clj

@@ -10,7 +10,7 @@
    :sign-releases false})
 
 (def tk-version "1.1.0")
-(def tk-jetty9-version "1.2.0")
+(def tk-jetty9-version "1.3.0")
 (def ks-version "1.0.0")
 
 (defproject puppetlabs/puppetdb pdb-version

puppet/lib/puppet/indirector/facts/puppetdb.rb

@@ -11,7 +11,7 @@ class Puppet::Node::Facts::Puppetdb < Puppet::Indirector::REST
 
   def get_trusted_info(node)
     trusted = Puppet.lookup(:trusted_information) do
-      Puppet::Context::TrustedInformation.local(request.node)
+      Puppet::Context::TrustedInformation.local(node)
     end
     trusted.to_h
   end

puppet/spec/unit/indirector/facts/puppetdb_spec.rb

@@ -110,15 +110,30 @@ def save
   end
 
   describe "#get_trusted_info" do
-
     it 'should return trusted data' do
+      node = Puppet::Node.new('my_certname')
+      trusted = subject.get_trusted_info(node)
+      expect(trusted).to eq({'authenticated'=>"local", 'certname'=>'testing', 'extensions'=>{}})
+    end
 
-      if Puppet::Util::Puppetdb.puppet3compat?
-        Puppet[:trusted_node_data] = true
+    it 'should return trusted data when falling back to the node' do
+      # This removes :trusted_information from the global context, triggering our fallback code.
+      if Puppet.methods.include? :rollback_context
+        Puppet.rollback_context('initial testing state')
+      else
+        Puppet.pop_context # puppet 3.5.1
       end
 
-      node = Puppet::Node.new("my_certname")
-      expect(subject.get_trusted_info(node)).to eq({"authenticated"=>"local", "certname"=>"testing", "extensions"=>{}})
+      node = Puppet::Node.new('my_certname', :parameters => {'clientcert' => 'trusted_certname'})
+      trusted = subject.get_trusted_info(node)
+
+      expect(trusted).to eq({'authenticated'=>'local', 'certname'=>'trusted_certname', 'extensions'=>{}})
+
+      # Put the context back the way the test harness expects
+      Puppet.push_context({}, 'context to make the tests happy')
+      if Puppet.methods.include? :mark_context
+        Puppet.mark_context('initial testing state')
+      end
     end
   end
 

src/puppetlabs/puppetdb/cli/services.clj

@@ -146,9 +146,8 @@
   {:pre [(map? db)]}
   (try
     (kitchensink/demarcate
-     "database garbage collection"
-     (with-transacted-connection db
-       (scf-store/garbage-collect!)))
+      "database garbage collection"
+      (scf-store/garbage-collect! db))
     (catch Exception e
       (log/error e "Error during garbage collection"))))
 

src/puppetlabs/puppetdb/facts.clj

@@ -2,7 +2,7 @@
   (:require [puppetlabs.kitchensink.core :as kitchensink]
             [puppetlabs.puppetdb.cheshire :as json]
             [schema.core :as s]
-            [puppetlabs.puppetdb.schema :as pls]
+            [puppetlabs.puppetdb.schema :as pls :refer [defn-validated]]
             [clojure.string :as str]
             [puppetlabs.puppetdb.scf.hash :as hash]
             [puppetlabs.puppetdb.scf.storage-utils :as sutils]
@@ -17,21 +17,30 @@
 (def fact-path
   [fact-path-element])
 
-(def fact-path-map
+(def pathmap-schema
   {:path s/Str
-   :depth s/Int
    :name s/Str
-   :value_hash s/Str
+   :depth s/Int})
+
+(def fact-set
+  {s/Str s/Any})
+
+(def facts-schema
+  {:certname String
+   :values fact-set
+   :timestamp pls/Timestamp
+   :environment (s/maybe s/Str)
+   :producer_timestamp (s/either (s/maybe s/Str) pls/Timestamp)})
+
+(def valuemap-schema
+  {:value_hash s/Str
    :value_float (s/maybe Double)
    :value_string (s/maybe s/Str)
    :value_integer (s/maybe s/Int)
    :value_boolean (s/maybe s/Bool)
    :value_json (s/maybe s/Str)
    :value_type_id s/Int})
 
-(def fact-set
-  {s/Str s/Any})
-
 ;; GLOBALS
 
 (def factpath-delimiter
@@ -123,40 +132,39 @@
    (nil? data) 4
    (coll? data) 5))
 
-(defn factmap-express-node
-  "Convert data and path into a node definition ready for storage."
-  [data path]
-  (let [type-id (value-type-id data)
-        initial-map {:path (factpath-to-string path)
-                     :name (first path)
-                     :depth (dec (count path))
-                     :value_type_id type-id
-                     :value_hash (hash/generic-identity-hash data)
+(defn value->valuemap
+  [value]
+  ;; Used by migration-legacy, so copy this function there before
+  ;; making backward-incompatible changes.
+  (let [type-id (value-type-id value)
+        initial-map {:value_type_id type-id
+                     :value_hash (hash/generic-identity-hash value)
                      :value_string nil
                      :value_integer nil
                      :value_float nil
                      :value_boolean nil
                      :value_json nil}]
-    (if (nil? data)
+    (if (nil? value)
       initial-map
       (let [value-keyword (case type-id
                             0 :value_string
                             1 :value_integer
                             2 :value_float
                             3 :value_boolean
                             5 :value_json)
-            data (if (coll? data)
-                   (sutils/db-serialize data)
-                   data)]
-        (assoc initial-map value-keyword data)))))
-
-(defn factmap-to-paths*
-  "Recursive function, when given some structured data it will descend into
-   children building up the path until an outer leaf is reached, returning the
-   final built up list of paths as a result."
-  ([data] (factmap-to-paths* data [] []))
-  ;; We specifically do not validate with schema here, for performance.
-  ([data mem path]
+            value (if (coll? value)
+                    (sutils/db-serialize value)
+                    value)]
+        (assoc initial-map value-keyword value)))))
+
+(defn flatten-facts-with
+  "Returns a collection of (leaf-fn path leaf) for all of the paths
+  represented by facts."
+  ;; Used by migration-legacy, so copy this function there before
+  ;; making backward-incompatible changes.
+  ([leaf-fn facts] (flatten-facts-with leaf-fn facts [] []))
+  ;; We intentionally do not validate with schema here, for performance.
+  ([leaf-fn data mem path]
      (let [depth (dec (count path))]
        (if (coll? data)
          ;; Branch
@@ -171,21 +179,31 @@
                     ;; thus allowing us to store the top level for each
                     ;; fact.
                     fp (if (= depth 0)
-                         (conj mem (factmap-express-node data path))
+                         (conj mem (leaf-fn path data))
                          mem)]
-               (let [new-fp (factmap-to-paths* v fp (conj path k))]
+               (let [new-fp (flatten-facts-with leaf-fn v fp (conj path k))]
                  (if (empty? remaining)
                    new-fp
                    (recur (first remaining)
                           (next remaining)
                           new-fp))))))
          ;; Leaf
-         (conj mem (factmap-express-node data path))))))
+         (conj mem (leaf-fn path data))))))
+
+(defn-validated path->pathmap :- pathmap-schema
+  [path :- fact-path]
+  ;; Used by migration-legacy, so copy this function there before
+  ;; making backward-incompatible changes.
+  {:path (factpath-to-string path)
+   :name (first path)
+   :depth (dec (count path))})
 
-(pls/defn-validated factmap-to-paths :- [fact-path-map]
-  "Converts a map of facts to a list of `fact-path-map`s."
-  [hash :- fact-set]
-  (factmap-to-paths* hash))
+(defn-validated facts->paths-and-valuemaps
+  :- [(s/pair fact-path "path" valuemap-schema "valuemap")]
+  "Returns [path valuemap] pairs for all
+  facts. i.e. ([\"foo#~bar\" vm] ...)"
+  [facts :- fact-set]
+  (flatten-facts-with (fn [fp leaf] [fp (value->valuemap leaf)]) facts))
 
 (pls/defn-validated unstringify-value
   "Converts a stringified value from the database into its real value and type.

src/puppetlabs/puppetdb/query.clj

@@ -364,7 +364,7 @@
                       FROM factsets fs
                         INNER JOIN facts as f on fs.id = f.factset_id
                         INNER JOIN fact_values as fv on f.fact_value_id = fv.id
-                        INNER JOIN fact_paths as fp on fv.path_id = fp.id
+                        INNER JOIN fact_paths as fp on f.fact_path_id = fp.id
                         INNER JOIN value_types as vt on vt.id=fv.value_type_id
                         LEFT OUTER JOIN environments as env on fs.environment_id = env.id
                       WHERE depth = 0) AS facts