Sum /events aggregates instead of keeping one backend's row
An extract carrying a ["function", ...] column returns counts, not events, so the union's verbatim-record key folded two backends' identical rows into one number. - Route /events through the central aggregate guard with its own fan-out path - Drop its unsummed opt-out so the route-table property tests cover it - Document the combined path on /events
This commit is contained in:
@@ -30,7 +30,7 @@ not PQL) is forwarded verbatim.
|
||||
| `GET /pdb/query/v4/fact-names` | Fan out to all and serve the **union** of the flat name arrays, deduped and re-sorted, re-paged across backends, plus the `pdbmux_source` name while injection is on. `order_by` is only valid on `name`. |
|
||||
| `GET /pdb/query/v4/resources` | An `extract` query with a `function` column is fanned out and **combined**; any other query is an unmerged pass-through. |
|
||||
| `GET /pdb/query/v4/reports` | Fan out to all and serve the **union**, deduped by report `hash`, re-ordered and re-paged across backends. |
|
||||
| `GET /pdb/query/v4/events` | Fan out to all and serve the **union**, deduped by record identity, re-ordered and re-paged. |
|
||||
| `GET /pdb/query/v4/events` | Fan out to all and serve the **union**, deduped by record identity, re-ordered and re-paged. An `extract` query with a `function` column is **combined** instead. |
|
||||
| `GET /pdb/query/v4/event-counts` | Fan out to all and **sum** each subject's counts into one row per subject. |
|
||||
| `GET /pdb/query/v4/aggregate-event-counts` | Fan out to all and **sum** the summary object's counts. |
|
||||
| `GET /pdb/query/v4/reports/<hash>/{events,logs,metrics}` | Ask every backend; serve the answer from whichever backend actually holds that report. `404` when none does. |
|
||||
@@ -89,14 +89,14 @@ paths add two more headers `pdbmux` sets itself, `X-Cache` and `Age` — see
|
||||
- **Aggregates** — `extract`/`group_by` rows are counts, not records, so each
|
||||
backend returns a partial answer that has to be **combined**, not deduped. This
|
||||
covers `/event-counts`, `/aggregate-event-counts`, and any `/reports`,
|
||||
`/nodes`, `/resources`, `/facts` or `/facts/<name>[/<value>]` query whose
|
||||
`extract` carries a `["function", ...]` column.
|
||||
`/events`, `/nodes`, `/resources`, `/facts` or `/facts/<name>[/<value>]` query
|
||||
whose `extract` carries a `["function", ...]` column.
|
||||
- The grouping key is the row's full set of non-aggregate columns: for
|
||||
`/reports`, `/nodes`, `/resources`, `/facts` and `/facts/<name>` they come
|
||||
from the query — the plain `extract` fields, the row-function columns and
|
||||
any `group_by` clause — and for the event-count endpoints from the row
|
||||
itself (`subject_type`/`subject`, or `summarize_by`), whose remaining fields
|
||||
are all counts.
|
||||
`/reports`, `/events`, `/nodes`, `/resources`, `/facts` and `/facts/<name>`
|
||||
they come from the query — the plain `extract` fields, the row-function
|
||||
columns and any `group_by` clause — and for the event-count endpoints from
|
||||
the row itself (`subject_type`/`subject`, or `summarize_by`), whose
|
||||
remaining fields are all counts.
|
||||
- On `/nodes`, `/facts` and `/facts/<name>[/<value>]` this takes precedence
|
||||
over the `certname` merge: an aggregate row has no `certname`, so deduping
|
||||
would collapse every backend's rows into one backend's numbers. A query with
|
||||
@@ -157,8 +157,9 @@ paths add two more headers `pdbmux` sets itself, `X-Cache` and `Age` — see
|
||||
fetched and the window cut after the fold, which an aggregate's row count —
|
||||
one per distinct group value — keeps affordable. `include_total` still
|
||||
reports the merged group count.
|
||||
- A `/reports` query with no `function` column is a projection of real reports,
|
||||
not an aggregate, and stays on the union path.
|
||||
- A `/reports` or `/events` query with no `function` column is a projection of
|
||||
real records, not an aggregate, and stays on the union path — so an event
|
||||
stored identically in two backends is still served once.
|
||||
- `include_total=true` on a combined endpoint reports the **merged** row count,
|
||||
not the sum of the backends' `X-Records`, since shared keys collapse.
|
||||
|
||||
|
||||
@@ -649,6 +649,72 @@ func countsByName(t *testing.T, rows []map[string]any) map[string]int {
|
||||
return out
|
||||
}
|
||||
|
||||
// An events count row is an aggregate, not an event, so the union's
|
||||
// verbatim-record dedupe would fold two backends' equal counts into one number.
|
||||
func TestEventsAggregatesAreSummed(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
const q = `["extract",[["function","count"]]]`
|
||||
|
||||
wantA := backendCount(ctx, t, h.a, eventsPath, q)
|
||||
wantB := backendCount(ctx, t, h.b, eventsPath, q)
|
||||
if wantA == 0 || wantB == 0 {
|
||||
t.Fatalf("the fixture gives %s %d and %s %d events, so a sum proves nothing", h.a.name, wantA, h.b.name, wantB)
|
||||
}
|
||||
|
||||
resp := get(t, eventsPath, query(q))
|
||||
if got := countOf(t, resp.rows(t)); got != wantA+wantB {
|
||||
t.Fatalf("/events count = %d, want %d (%s=%d + %s=%d)", got, wantA+wantB, h.a.name, wantA, h.b.name, wantB)
|
||||
}
|
||||
if got := resp.header.Get(backendsHeader); got != "2/2" {
|
||||
t.Errorf("%s = %q, want %q", backendsHeader, got, "2/2")
|
||||
}
|
||||
|
||||
t.Run("grouped counts are summed per key", func(t *testing.T) {
|
||||
const grouped = `["extract",[["function","count"],"certname"],["group_by","certname"]]`
|
||||
want := map[string]int{}
|
||||
for _, b := range []*backend{h.a, h.b} {
|
||||
for cn, n := range countsByCertname(t, b.query(ctx, t, eventsPath, query(grouped))) {
|
||||
want[cn] += n
|
||||
}
|
||||
}
|
||||
// shared reports to both backends, so its row is the one a dedupe would
|
||||
// leave frozen at a single backend's number.
|
||||
if want[nodeShared] < 2 {
|
||||
t.Fatalf("%s has %d events across the estate, so its merged row cannot show a sum", nodeShared, want[nodeShared])
|
||||
}
|
||||
got := countsByCertname(t, get(t, eventsPath, query(grouped)).rows(t))
|
||||
if !reflect.DeepEqual(got, want) {
|
||||
t.Errorf("grouped /events counts = %v, want %v", got, want)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("a query with no function column stays on the union", func(t *testing.T) {
|
||||
rows := get(t, eventsPath, nil).rows(t)
|
||||
if len(rows) != wantA+wantB {
|
||||
t.Fatalf("/events returned %d records, want %d: the union dropped or duplicated events", len(rows), wantA+wantB)
|
||||
}
|
||||
for _, row := range rows {
|
||||
if _, ok := row["resource_title"].(string); !ok {
|
||||
t.Fatalf("/events record is not an event: %v", row)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// countsByCertname reads a ["function","count"] + group_by "certname" result set.
|
||||
func countsByCertname(t *testing.T, rows []map[string]any) map[string]int {
|
||||
t.Helper()
|
||||
out := map[string]int{}
|
||||
for _, row := range rows {
|
||||
certname, ok := row["certname"].(string)
|
||||
if !ok {
|
||||
t.Fatalf("grouped aggregate row has no certname: %v", row)
|
||||
}
|
||||
out[certname] = int(aggregateNumber(t, row, "count"))
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// The provenance fact must name the backend whose data won, and must be absent
|
||||
// from the query shapes it would corrupt.
|
||||
func TestSourceFactInjectionAndGating(t *testing.T) {
|
||||
|
||||
@@ -105,7 +105,6 @@ func TestQueryRoutes_UnsummedRoutesAreTheKnownOnes(t *testing.T) {
|
||||
want := []string{
|
||||
aggregateEventCountsPath,
|
||||
eventCountsPath,
|
||||
eventsPath,
|
||||
factNamesPath,
|
||||
reportsPath + "/<hash>/<sub>",
|
||||
}
|
||||
|
||||
@@ -151,6 +151,7 @@ var queryRoutes = []route{
|
||||
// /resources has no cross-backend record identity, so only its aggregates merge.
|
||||
{name: resourcesPath, matches: pathIs(resourcesPath), fanOut: resourcesPath, serve: (*Server).proxyUnmerged},
|
||||
{name: reportsPath, matches: pathIs(reportsPath), fanOut: reportsPath, serve: (*Server).serveReports},
|
||||
{name: eventsPath, matches: pathIs(eventsPath), fanOut: eventsPath, serve: (*Server).serveEvents},
|
||||
{name: factsPath + "/<name>", matches: isFactsSubPath, serve: (*Server).serveFactsByName},
|
||||
{
|
||||
name: factNamesPath,
|
||||
@@ -158,12 +159,6 @@ var queryRoutes = []route{
|
||||
serve: (*Server).serveFactNames,
|
||||
unsummed: "the union dedupes names across backends, so a count of it is not the sum of the backends' counts",
|
||||
},
|
||||
{
|
||||
name: eventsPath,
|
||||
matches: pathIs(eventsPath),
|
||||
serve: (*Server).serveEvents,
|
||||
unsummed: "unioned on the verbatim record; summing aggregates here would change what the route answers, so it is a change of its own",
|
||||
},
|
||||
{
|
||||
name: eventCountsPath,
|
||||
matches: pathIs(eventCountsPath),
|
||||
@@ -452,6 +447,11 @@ func (s *Server) serveReports(w http.ResponseWriter, r *http.Request) {
|
||||
s.serveUnion(w, r, reportsPath, reportKey)
|
||||
}
|
||||
|
||||
// Only a plain query reaches here: openvoxdb serves events from the same generic
|
||||
// query engine as every other entity, so an extract carrying a ["function", ...]
|
||||
// column is an aggregate handleQuery has already diverted —
|
||||
// src/puppetlabs/puppetdb/http/handlers.clj:178-189 and
|
||||
// src/puppetlabs/puppetdb/query_eng/engine.clj:1120-1210,1889-1911,2719-2733.
|
||||
func (s *Server) serveEvents(w http.ResponseWriter, r *http.Request) {
|
||||
s.serveUnion(w, r, eventsPath, rawKey)
|
||||
}
|
||||
|
||||
@@ -596,6 +596,44 @@ func TestHandler_EventsDedupedByIdentity(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// An events aggregate row is a count, not an event, so the union's
|
||||
// verbatim-record key would collapse two backends' identical rows into one.
|
||||
func TestHandler_EventsAggregatesAreCombined(t *testing.T) {
|
||||
a := newFakeBackend(t, `[]`, `[]`)
|
||||
a.bodies[eventsPath] = `[{"count":5}]`
|
||||
b := newFakeBackend(t, `[]`, `[]`)
|
||||
b.bodies[eventsPath] = `[{"count":5}]`
|
||||
srv := newTestServer(testConfig(a.srv.URL, b.srv.URL, mergeStatic))
|
||||
|
||||
rec := doGet(t, srv.Handler(), eventsPath, `["extract",[["function","count"]]]`)
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status %d: %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if got := counts(t, rec.Body.Bytes(), "count"); !slices.Equal(got, []float64{10}) {
|
||||
t.Errorf("count = %v, want [10]; the identical rows were deduped instead of added", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandler_EventsGroupedAggregatesCombinePerKey(t *testing.T) {
|
||||
a := newFakeBackend(t, `[]`, `[]`)
|
||||
a.bodies[eventsPath] = `[{"status":"success","count":5,"max":7},{"status":"failure","count":1,"max":2}]`
|
||||
b := newFakeBackend(t, `[]`, `[]`)
|
||||
b.bodies[eventsPath] = `[{"status":"success","count":5,"max":3}]`
|
||||
srv := newTestServer(testConfig(a.srv.URL, b.srv.URL, mergeStatic))
|
||||
|
||||
const q = `["extract",[["function","count"],["function","max","line"],"status"],["group_by","status"]]`
|
||||
rec := doGet(t, srv.Handler(), eventsPath, q)
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status %d: %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if got := counts(t, rec.Body.Bytes(), "count"); !slices.Equal(got, []float64{10, 1}) {
|
||||
t.Errorf("counts = %v, want [10 1]", got)
|
||||
}
|
||||
if got := counts(t, rec.Body.Bytes(), "max"); !slices.Equal(got, []float64{7, 2}) {
|
||||
t.Errorf("max = %v, want [7 2]; the column was combined by the wrong operation", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandler_ReportSubResourceFromHoldingBackend(t *testing.T) {
|
||||
// Only a holds report r1, so its logs come from a; an unmerged pass-through
|
||||
// to whichever backend answered first could have 404'd.
|
||||
|
||||
Reference in New Issue
Block a user