electric-sql / electric-sql/electric

Evict stale partition mappings for vanished/detached relations in Partitions.handle_relation/2

Open
#4,583 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.