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

Open
#31 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
35/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Active
Tech stack
clickhouse, php
Domain
databases

Research direction

Start by tracing ClickHouse::addBatch(), Accumulator::flush(), insert(), and generateId() to understand how chunk failures and retries affect buffered rows. Reproduce a mid-batch failure and a timeout-after-server-commit if the existing setup permits. Done means failed flush retries do not duplicate rows already committed to ClickHouse, including when a client timeout follows a successful insert.

Written by the indexing model from the issue text.

Description

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.

Dominant language
PHP
Stars
0
Forks
0
Avg merge
1d 22h
Merged PRs (30d)
8

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Similar issues

More PHP issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.