Skip to content

Commit 714c1fb

Browse files
author
pixie-agent
committed
fix(ae): remaining CodeRabbit Majors (#53 followup)
- controller: don't fan out Pixie rows when attribution Sink.Write fails (avoids orphaned rows; release in-flight slot, non-fatal). (controller.go:347) - controller.Rehydrate: re-arm rev-1 pushPixieRows for restored windows so a restart doesn't silently miss post-restart Pixie data. (controller.go:255) - passthrough.pull: per-table timeout context so a hung dependency can't stall the sweep / delay shutdown (covers serial+concurrent ticks). (passthrough.go:147) - schema: TTL (30d) on ae_reconcile to cap append-only growth. (schema.sql:485) Verified no-change-needed: adaptive_attribution ORDER BY (hostname, anomaly_hash) is safe — anomaly_hash already encodes namespace+pod (anomaly/hash.go), so rows never collapse across ns/pod. (schema.sql:430) Already on ae-prod (no-op): filter timer leak, stats.py EXACT guard, sink async, watermark paging, pixieapi WithDirectTLSSkipVerify, http.Server timeouts.
1 parent e6a6237 commit 714c1fb

3 files changed

Lines changed: 43 additions & 4 deletions

File tree

src/vizier/services/adaptive_export/internal/clickhouse/schema.sql

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,10 @@ CREATE TABLE IF NOT EXISTS forensic_db.ae_reconcile (
482482
hostname String
483483
) ENGINE = MergeTree
484484
PARTITION BY toYYYYMMDD(ts)
485-
ORDER BY (table_name, ts);
485+
ORDER BY (table_name, ts)
486+
-- append-only debug log; cap growth so long reconcile runs don't accumulate
487+
-- unbounded storage (CodeRabbit). 30d matches the pixie observation tables.
488+
TTL toDateTime(ts) + INTERVAL 30 DAY DELETE;
486489

487490
-- dx_attack_graph — dx evidence-graph edge list: one row per directed hop of an
488491
-- investigation (delivery/egress/execution/exfil/pivot), read by the Pixie

src/vizier/services/adaptive_export/internal/controller/controller.go

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -254,12 +254,33 @@ func (c *Controller) Rehydrate(ctx context.Context) error {
254254
return err
255255
}
256256
c.mu.Lock()
257-
defer c.mu.Unlock()
257+
var resume []sink.AttributionRow
258258
for i := range rows {
259259
row := rows[i]
260260
c.active[row.AnomalyHash] = &row
261+
// Rev-1: a restart restored the window but no pushPixieRows goroutine —
262+
// without this, post-restart Pixie data is silently missed until another
263+
// event for the same hash arrives (CodeRabbit). Re-arm the fan-out for
264+
// each restored window, mirroring handle()'s spawn (in-flight guarded).
265+
if c.querier != nil && len(c.cfg.PushPixieTables) > 0 && !c.inFlight[row.AnomalyHash] {
266+
c.inFlight[row.AnomalyHash] = true
267+
resume = append(resume, row)
268+
}
269+
}
270+
c.mu.Unlock()
271+
for i := range resume {
272+
r := resume[i]
273+
go func() {
274+
defer func() {
275+
c.mu.Lock()
276+
delete(c.inFlight, r.AnomalyHash)
277+
c.mu.Unlock()
278+
}()
279+
c.pushPixieRows(ctx, r)
280+
}()
261281
}
262-
log.WithField("rehydrated", len(rows)).Info("controller: active set restored")
282+
log.WithFields(log.Fields{"rehydrated": len(rows), "resumed": len(resume)}).
283+
Info("controller: active set restored")
263284
return nil
264285
}
265286

@@ -331,7 +352,17 @@ func (c *Controller) handle(ctx context.Context, ev kubescape.Event) {
331352
c.mu.Unlock()
332353

333354
if err := c.sink.Write(ctx, []sink.AttributionRow{snapshot}); err != nil {
334-
log.WithError(err).Warn("controller: sink write failed")
355+
// Attribution persistence failed → do NOT fan out, or we'd write Pixie
356+
// rows with no persisted attribution anchor (orphaned rows, CodeRabbit).
357+
// Non-fatal (system-stability rule): release the reserved in-flight slot
358+
// and return; a later event for the same hash retries.
359+
log.WithError(err).Warn("controller: sink write failed — skipping fan-out")
360+
if spawn {
361+
c.mu.Lock()
362+
delete(c.inFlight, hash)
363+
c.mu.Unlock()
364+
}
365+
return
335366
}
336367
if c.cfg.OnAttribution != nil {
337368
c.cfg.OnAttribution(snapshot.Namespace, snapshot.Pod, snapshot.TEnd)

src/vizier/services/adaptive_export/internal/passthrough/passthrough.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,11 @@ func (l *Loop) tickConcurrent(ctx context.Context, sliceStart, sliceEnd time.Tim
252252
// sink, and recorder are all pool/HTTP-backed and concurrency-safe, and
253253
// each call touches a different forensic_db.<table>.
254254
func (l *Loop) pull(ctx context.Context, table, src string, sliceStart, sliceEnd time.Time) {
255+
// Bound this table's external query+write+record so a hung dependency can't
256+
// stall the whole sweep or delay shutdown (CodeRabbit). Derived per-table
257+
// from the parent ctx; covers both the serial and concurrent tick paths.
258+
ctx, cancel := context.WithTimeout(ctx, l.cfg.Refresh)
259+
defer cancel()
255260
rows, err := l.q.Query(ctx, src)
256261
if err != nil {
257262
log.WithError(err).WithField("table", table).Warn("ADAPTIVE_PASSTHROUGH: pixie query failed")

0 commit comments

Comments
 (0)