PostHog / PostHog/posthog

fix(signals): document_embeddings re-emissions can land out of order

Open
#73,705 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
39.9k
Forks
3.4k
Avg merge
6h 51m
Merged PRs (30d)
232

Description

Problem

emit_embedding_request (posthog/api/embedding_worker.py) produces to document_embeddings_input without a Kafka key:

producer = get_producer(topic=KAFKA_DOCUMENT_EMBEDDINGS_INPUT_TOPIC)
return producer.produce(topic=KAFKA_DOCUMENT_EMBEDDINGS_INPUT_TOPIC, data=payload)

Unkeyed produces get no partition affinity, so two messages for the same document_id can be processed by the worker in either order. The destination table is a ReplacingMergeTree whose winner is chosen by inserted_at, which the worker stamps at insert time rather than the caller stamping at emit time. A slower earlier request can therefore be inserted after a newer one and win.

The consequence that matters: a re-emission carrying live content can supersede a tombstone that was emitted after it, making a retracted document visible again.

Scope

This is a property of the shared embedding pipeline, not of any one caller. Anything that re-emits the same document_id is exposed:

  • soft_delete_report_signals and soft_delete_scout_signal in signals, which have had this exposure since they shipped
  • report-level documents added in #73704, which retract on deletion, on an unsafe safety verdict, and on an unreviewed edit
  • any other product re-emitting a stable document_id

Products currently writing to this table include error tracking, session replay, and AI observability, so a fix needs those owners involved.

Possible directions

  1. Key the produce by document_id. Gives per-key partition ordering, which fixes the common case as long as the worker preserves per-partition order.
  2. Carry a caller-supplied monotonic revision and make it the ver column for the ReplacingMergeTree, so the winner is decided by the emitter rather than by worker completion time. More invasive, but robust to worker concurrency.

Option 2 is the only one that is correct under a worker that processes a partition concurrently, so it is worth confirming the worker's behavior before choosing.

Context

Raised during review of #73704. Deliberately not fixed there, because it predates that change and the fix touches shared infrastructure used by several products.

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.

Research direction

Start in posthog/api/embedding_worker.py at emit_embedding_request, then trace the document_embeddings_input consumer and the destination ReplacingMergeTree behavior. Confirm whether the worker processes a partition concurrently before choosing between keyed production and caller-supplied revisions. Done means re-emissions for one document cannot let an older live request supersede a later tombstone, including callers such as the signals and report-level document paths.

Written by the indexing model from the issue text.

Assessment

Tech stack
clickhouse, kafka, python
Domain
backend, data-engineering, distributed-systems
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.