diff --git a/README.md b/README.md index 58ff962..404a7f6 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/reports.go b/reports.go index dfd77a8..24c8d87 100644 --- a/reports.go +++ b/reports.go @@ -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 { diff --git a/reports_test.go b/reports_test.go index 74f2916..911a755 100644 --- a/reports_test.go +++ b/reports_test.go @@ -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]") diff --git a/server.go b/server.go index f26612d..9efeb66 100644 --- a/server.go +++ b/server.go @@ -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 {