electric-sql / electric-sql/electric
Evict stale partition mappings for vanished/detached relations in Partitions.handle_relation/2
- Dominant language
- TypeScript
- Stars
- 10.4k
- Forks
- 375
- Avg merge
- 3d 1h
- Merged PRs (30d)
- 18
Description
## Summary
`Electric.Shapes.Partitions.handle_relation/2` does not evict stale entries from the `partitions` map when a relation stops being a tracked partition. Because `handle_txn_fragment/2` expands changes by `{schema, table}` **name**, a stale mapping can silently mis-route changes to the wrong root shape after a drop/detach + same-name recreation.
Surfaced by a Codex review on #4565 (commit `a4dcc7d61`), but the behavior is pre-existing — that PR only made the `:table_not_found` path reachable without crashing.
## The bug
The partition map maps `partition_table → [root_table]`, and expansion looks it up by name:
```elixir
defp expand_change(%{relation: relation} = change, state) do
[change | state.partitions |> Map.get(relation, []) |> Enum.map(&%{change | relation: &1})]
end
```
Two branches of `handle_relation/2` return the state unchanged without clearing a previously-tracked entry for that name:
```elixir
# packages/sync-service/lib/electric/shapes/partitions.ex
{:ok, _} ->
{:ok, state} # relation no longer has a parent (detached / never partitioned)
:table_not_found ->
# the table was dropped (or dropped and recreated under a new oid) ...
{:ok, state} # added in #4565
```
If a tracked partition `P` of root `R` is dropped (or detached) and a **new table with the same `{schema, table}` name** is later created as a **non-partition**, the stale `partitions[P] = [R]` entry persists. Changes to the new `P` then get duplicated onto the old root shape `R`, which is incorrect.
## Why it's narrow
It requires all of:
- an active shape on the partitioned root `R`,
- partition `P` dropped/detached,
- a new relation recreated with the *exact same* schema-qualified name,
- recreated **as a plain/non-partition table** (if recreated as a partition, the `{:ok, %{parent: ...}}` branch overwrites the stale entry, so no bug),
- changes flowing to the recreated table.
## Fix considerations
- Evict the vanished/detached relation from `partitions` in **both** the `:table_not_found` and the no-parent `{:ok, _}` branches — fixing only one (as the inline suggestion did) leaves the identical hole in the other.
- Eviction must be **ownership-aware**. Entries in `partitions` come from two places with different bookkeeping:
- `add_shape/3` — also tracked in `partition_ownership` and counted in `active`,
- `handle_relation/2` discovery — *not* tracked in `partition_ownership`.
A blind `Map.delete/2` on a relation message could desync `partitions` from `partition_ownership` / `active` for shape-owned children, so the cleanup needs to account for ownership (likely reuse / extend `clean_up_partitions/1` semantics rather than delete directly).
- Add a regression test that reproduces drop/detach → recreate-as-plain-table → asserts the change is **not** duplicated to the old root shape.
## References
- PR #4565 — "Don't crash ShapeLogCollector on introspection failures in the replication path"
- File: `packages/sync-service/lib/electric/shapes/partitions.ex` (`handle_relation/2`, `expand_change/2`)
Contributor guide
Assessment
This issue has not been assessed yet.