Skip to content

ingest: poison-drop counter lumps three conditions under reason=unpairable #567

Description

@EricAndrechek

What

internal/ingest.parseMsg rejects five distinct conditions, but collapses three of them onto the single reason string "unpairable":

worker.go condition reason passed
422 envelope is not valid JSON malformed
428 declared format is unknown or absent (a pre-v2 message) unknown_format
435 no columns, or no row unpairable
449 a column name repeats unpairable
456 row length disagrees with the column list unpairable

reason reaches an operator in exactly two places (rejectPoison, worker.go:668): the reason label on wavehouse_ingest_poison_dropped_total, and the ERROR log. So on the DLQ-off path, the counter cannot distinguish "the producer sent a row of the wrong length" from "the producer repeated a column name" — two different bugs with two different owners, sliceable only by reading logs.

On the DLQ-on path the classification survives, because parkOnDLQ carries the per-condition detail string into X-DLQ-Error. This is only a gap where the DLQ is switched off for the table — which is also the path where the message is permanently dropped, so it is the case with the least other evidence left behind.

Why now

PR #554 briefly had a read-side counter (wavehouse_sse_rows_unpairable_total) whose reason label did make these distinctions — empty_row, duplicate_column, length_mismatch, undecodable_row. That counter was removed in 750a4ae as redundant with the write side: the worker refuses the identical conditions on the identical bytes, and does it first.

That reasoning holds, but it does mean the finer classification now exists nowhere in metrics. The right home for it is the worker, which is the only consumer that actually parks the message — not a second counter on the read path.

Suggested change

Pass a specific reason at each of the three sites (empty_envelope, duplicate_column, length_mismatch), leaving detail as-is. The label set stays small and bounded, so cardinality is a non-issue.

Worth checking whether anything already alerts on reason="unpairable" before splitting it, since the label value would stop appearing.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions