Skip to content

Failed flush retries re-insert already-committed rows (duplicate usage events) #31

Description

@loks0n

Summary

When a flush fails partway through, the retry re-inserts rows that already landed in ClickHouse, producing duplicate rows (and over-counted usage). Three behaviors combine to cause this, observed on 0.14.0:

  1. ClickHouse::addBatch() is chunked but all-or-nothing. It splits the batch into 1000-row INSERTs and only returns true after every chunk succeeds. If chunk N fails, chunks 1..N-1 are already committed server-side, but the caller sees a thrown exception.

  2. Accumulator::flush() retains the whole buffer on failure. Buffer entries are only cleared when addBatch() returns true, so after a mid-batch failure the next flush re-sends all entries — including the ones whose chunks already succeeded.

  3. Inserts are not idempotent. insert()'s own comment says so: MergeTree has no row-level dedup, and each retry calls generateId() again, so re-sent rows get fresh ids and can't be deduplicated by block hash either.

There's a second path to the same outcome with no chunking involved: a client-side timeout on an insert that the server actually completed (we observed a burst of Operation timed out after 30s in production while the server was demonstrably healthy and ingesting). The retry then duplicates the full batch.

Observed in production

Appwrite Cloud stats-usage workers logging, e.g.:

ClickHouse insert failed: Operation timed out
  [Operation: addBatch(), Table: projects_usage_events, Query: INSERT INTO projects_usage_events (1000 rows)]
ClickHouse insert failed: Connection reset by peer
  [Operation: addBatch(), Table: projects_usage_events, Query: INSERT INTO projects_usage_events (890 rows)]

The 890 rows failure is a tail chunk — the preceding 1000-row chunks of that same addBatch() call had already been inserted, and were re-inserted on the next flush.

Suggested directions

  • Make Accumulator::flush()/addBatch() clear buffer entries per successful chunk rather than per call, so a tail-chunk failure doesn't re-send committed chunks; and/or
  • Make retries idempotent: deterministic row ids derived from the buffered entry (not generateId() at encode time) plus stable insert blocks, so ClickHouse's insert_deduplicate block-hash dedup can absorb replays (or a ReplacingMergeTree/dedup-on-read scheme).

The timeout-after-server-commit case can only be fully solved by idempotency, not by smarter chunk bookkeeping.

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

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions