fix: never dedupe hash-less report rows across backends
An extract/count()/group_by query returns synthetic rows with no report
hash, which reportKey fell back to keying by verbatim raw bytes. Two
backends emitting a byte-identical aggregate row (e.g.
{"status":"changed","count":1}) therefore collapsed into one, silently
undercounting the merged result and contradicting the documented
guarantee that no backend's rows are dropped.
Give the mergeUnion key func an ok return: false means the record has no
dedupe identity and is always kept. reportKey returns ok=false for
hash-less rows; hash-keyed report dedupe and event verbatim-identity
dedupe are unchanged.
Add TestMergeUnion_IdenticalHashlessRowsAreNotCollapsed covering the
collision case, and reword the README line to say aggregate rows pass
through even when byte-identical.
This commit is contained in:
@@ -59,7 +59,7 @@ unknown fields survive untouched.
|
||||
on `hash`; events, which carry no id of their own, dedupe on the verbatim
|
||||
record (a node briefly reporting to both PuppetDBs stores identical records in
|
||||
each). Records the merge cannot key — `extract`/`group_by` aggregate rows — are
|
||||
all kept rather than collapsed, so no backend's rows are silently dropped;
|
||||
never deduped, so every backend's rows pass through even when byte-identical;
|
||||
summing those aggregates across backends is not implemented yet.
|
||||
|
||||
### Paging and ordering on the merged endpoints
|
||||
|
||||
+14
-11
@@ -13,17 +13,19 @@ import (
|
||||
// Reports and events are immutable history, so a certname that migrated between
|
||||
// PuppetDBs legitimately has records in both and the union — not a per-node
|
||||
// winner — is the correct merged view. results must be ordered by precedence;
|
||||
// the first backend holding a key supplies the record.
|
||||
func mergeUnion(results []backendResult, key func(record) string) []json.RawMessage {
|
||||
// the first backend holding a key supplies the record. A key func returning
|
||||
// ok=false means the record has no dedupe identity and is always kept.
|
||||
func mergeUnion(results []backendResult, key func(record) (string, bool)) []json.RawMessage {
|
||||
seen := make(map[string]bool)
|
||||
out := []json.RawMessage{}
|
||||
for _, res := range results {
|
||||
for _, rec := range res.records {
|
||||
k := key(rec)
|
||||
if seen[k] {
|
||||
continue
|
||||
if k, ok := key(rec); ok {
|
||||
if seen[k] {
|
||||
continue
|
||||
}
|
||||
seen[k] = true
|
||||
}
|
||||
seen[k] = true
|
||||
out = append(out, rec.Raw)
|
||||
}
|
||||
}
|
||||
@@ -32,18 +34,19 @@ func mergeUnion(results []backendResult, key func(record) string) []json.RawMess
|
||||
|
||||
// reportKey identifies a report by its content hash, which PuppetDB guarantees
|
||||
// is unique per report. An `extract`/`group_by` query returns synthetic rows
|
||||
// with no hash, so those fall back to raw identity and are all kept.
|
||||
func reportKey(rec record) string {
|
||||
// with no hash and no identity — two backends can emit byte-identical aggregate
|
||||
// rows that both count — so those are never deduped.
|
||||
func reportKey(rec record) (string, bool) {
|
||||
if rec.Hash == "" {
|
||||
return rawKey(rec)
|
||||
return "", false
|
||||
}
|
||||
return "hash\x00" + rec.Hash
|
||||
return "hash\x00" + rec.Hash, true
|
||||
}
|
||||
|
||||
// rawKey identifies a record by its verbatim JSON. Events carry no unique id,
|
||||
// but two byte-identical events from the same PuppetDB serialiser describe the
|
||||
// same resource change, so raw equality is a safe dedupe key.
|
||||
func rawKey(rec record) string { return "raw\x00" + string(rec.Raw) }
|
||||
func rawKey(rec record) (string, bool) { return "raw\x00" + string(rec.Raw), true }
|
||||
|
||||
// orderField is one entry of PuppetDB's order_by param.
|
||||
type orderField struct {
|
||||
|
||||
@@ -35,6 +35,16 @@ func TestMergeUnion_HashlessRowsAreAllKept(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestMergeUnion_IdenticalHashlessRowsAreNotCollapsed(t *testing.T) {
|
||||
// Two backends can legitimately produce the same aggregate row; collapsing
|
||||
// them as duplicates undercounts the merged result.
|
||||
same := `{"status":"changed","count":1}`
|
||||
merged := mergeUnion([]backendResult{recs(t, "old", same), recs(t, "new", same)}, reportKey)
|
||||
if len(merged) != 2 {
|
||||
t.Errorf("expected both backends' aggregate rows, got %d: %v", len(merged), merged)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMergeUnion_EventsDedupeOnRawIdentity(t *testing.T) {
|
||||
same := event("h1", "r1", "Package[nginx]")
|
||||
other := event("h1", "r1", "Service[nginx]")
|
||||
|
||||
@@ -127,7 +127,7 @@ func (s *Server) serveMerged(w http.ResponseWriter, r *http.Request, path string
|
||||
// and events — and serves the deduped union of every backend. Because each
|
||||
// backend ordered and paged only its own slice, the union is re-ordered and
|
||||
// re-paged here from the client's order_by/limit/offset.
|
||||
func (s *Server) serveUnion(w http.ResponseWriter, r *http.Request, path string, key func(record) string) {
|
||||
func (s *Server) serveUnion(w http.ResponseWriter, r *http.Request, path string, key func(record) (string, bool)) {
|
||||
in := r.URL.Query()
|
||||
page, err := parsePaging(in)
|
||||
if err != nil {
|
||||
|
||||
Reference in New Issue
Block a user