Skip to content

Commit e6171a9

Browse files
robacourtclaude
andauthored
Fix O(n) shape removal from the subquery index (#4594)
## Summary Removing a shape from the subquery index in the where-clause filter had become **O(n)** in the number of shapes — and **O(shapes × values)** once subqueries were seeded. Shape removal runs on the replication path and blocks it, so slow removal caused WAL lag in production (the "Materializer shape invalidated" path). This restores **O(1)** removal (as in v1.5). Fixes #4279 ## Problem The subquery index is a single ETS `:bag` table. Several deletions used `:ets.match_delete/2` with a **wildcard in the key position** (e.g. `{:membership, handle, :_, :_}`, `{:node_positive_member, node, :_}`). A `:bag` (hash) table can only use its key index when the key is **fully bound**, so a partially-bound key forces a **full-table scan** that grows with the number of shapes. The worst offender was `delete_node_members`, which scanned every shape's per-`(node, value)` routing row on each removal — O(shapes × values). ## Solution Switch the table from `:bag` to **`:ordered_set`** and lift the discriminating fields (`value`, `shape`, `branch`, `next_cond`) out of the value tuples into the **keys**. In an `:ordered_set`, a delete whose key has a bound **prefix** is range-limited (not a full scan), and a point delete by a fully-bound key is O(log n). Concretely: - `unregister_shape/2` replaces its full-table `match_delete` scans with prefix-bounded `select_delete`s. - `remove_shape/5` point-deletes the shape's node rows, and **derives** its `node_*_member` deletions from the removed shape's own membership rows (scoped by `subquery_ref`), applying exact point-deletes. This touches only the removed shape's rows — never the other shapes sharing a value. Removal is now **O(V_shape · log n)** — proportional to the removed shape's own view size, independent of the total number of shapes, the number of shapes on a node, and the number of shapes sharing a value. This is a pure internal storage change: every public function and all externally-observable behavior is preserved. The per-change routing hot path (`affected_shapes`) and the exact-evaluation `member?` path are equal-or-faster (the `:ordered_set` prefix `select` projects in C); table memory is roughly flat (measured slightly lower in a microbenchmark). > Note: a dedicated **memory** reduction (interning / row-shrinking) is intentionally **out of scope** here — this PR is the removal-complexity fix only, kept minimal and behavior-preserving. Memory can be tackled separately. ## Test plan - [x] `:performance` **flatness** tests prove removal is independent of each dimension — total shapes, shapes-on-node, and shapes-per-value — by measuring removal at two well-separated sizes and asserting the delta stays within noise (observed delta 0 at 1k vs 20k). - [x] **Positive control**: removal cost *does* scale with the removed shape's own view size (pins the complexity class as O(V_shape), so a regression can't hide under a flat budget). - [x] **No-orphan guard**: a full `Filter.remove_shape` of a seeded shape, and draining a shared node one-by-one, leaves the subquery index byte-identical to its pre-add state. - [x] Existing `subquery_index` / `filter` / `consumer` / `shape_log_collector` suites green; full `mix test` green. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent d2418d6 commit e6171a9

4 files changed

Lines changed: 362 additions & 117 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@core/sync-service": patch
3+
---
4+
5+
Restore O(1) removal of subquery shapes from the where-clause filter. The v1.6 subquery index removed shapes with full-table ETS scans, so removing a shape cost O(total shapes × values) and blocked replication processing — causing WAL lag when many seeded subquery shapes were present. The index now uses an `:ordered_set` with prefix-bounded deletes, making shape removal independent of the total number of shapes, the number of shapes on a node, and the number of shapes sharing a value.

packages/sync-service/lib/electric/shapes/filter/indexes/subquery_index.ex

Lines changed: 119 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,10 @@ defmodule Electric.Shapes.Filter.Indexes.SubqueryIndex do
4747
def new(opts \\ []) do
4848
case Keyword.get(opts, :stack_id) do
4949
nil ->
50-
:ets.new(:subquery_index, [:bag, :public])
50+
:ets.new(:subquery_index, [:ordered_set, :public])
5151

5252
stack_id ->
53-
:ets.new(table_name(stack_id), [:bag, :public, :named_table])
53+
:ets.new(table_name(stack_id), [:ordered_set, :public, :named_table])
5454
end
5555
end
5656

@@ -94,10 +94,10 @@ defmodule Electric.Shapes.Filter.Indexes.SubqueryIndex do
9494
"""
9595
@spec unregister_shape(t(), term()) :: :ok
9696
def unregister_shape(table, shape_handle) do
97-
:ets.match_delete(table, {{:membership, shape_handle, :_, :_}, true})
98-
:ets.match_delete(table, {{:polarity, shape_handle, :_}, :_})
99-
:ets.match_delete(table, {{:shape_node, shape_handle}, :_})
100-
:ets.match_delete(table, {{:shape_dep_node, shape_handle, :_}, :_})
97+
delete_by_key_prefix(table, {:membership, shape_handle, :_, :_})
98+
delete_by_key_prefix(table, {:polarity, shape_handle, :_})
99+
delete_by_key_prefix(table, {:shape_node, shape_handle, :_, :_})
100+
delete_by_key_prefix(table, {:shape_dep_node, shape_handle, :_, :_, :_})
101101
:ets.delete(table, {:fallback, shape_handle})
102102
:ok
103103
end
@@ -130,27 +130,27 @@ defmodule Electric.Shapes.Filter.Indexes.SubqueryIndex do
130130

131131
:ets.insert(
132132
table,
133-
{{:node_shape, node_id},
134-
{shape_id, optimisation.dep_index, optimisation.polarity, next_condition_id, branch_key}}
133+
{{:node_shape, node_id, shape_id, branch_key},
134+
{optimisation.dep_index, optimisation.polarity, next_condition_id}}
135135
)
136136

137137
if optimisation.polarity == :negated do
138-
:ets.insert(table, {{:node_negated_shape, node_id}, {shape_id, next_condition_id}})
138+
:ets.insert(table, {{:node_negated_shape, node_id, shape_id, next_condition_id}, true})
139139
end
140140

141141
:ets.insert(
142142
table,
143-
{{:shape_node, shape_id},
144-
{node_id, optimisation.dep_index, optimisation.polarity, next_condition_id, branch_key}}
143+
{{:shape_node, shape_id, node_id, branch_key},
144+
{optimisation.dep_index, optimisation.polarity, next_condition_id}}
145145
)
146146

147147
:ets.insert(
148148
table,
149-
{{:shape_dep_node, shape_id, optimisation.dep_index},
150-
{node_id, optimisation.polarity, next_condition_id, branch_key}}
149+
{{:shape_dep_node, shape_id, optimisation.dep_index, node_id, branch_key},
150+
{optimisation.polarity, next_condition_id}}
151151
)
152152

153-
:ets.insert(table, {{:node_fallback, node_id}, {shape_id, next_condition_id}})
153+
:ets.insert(table, {{:node_fallback, node_id, shape_id, next_condition_id}, true})
154154
:ok
155155
end
156156

@@ -181,31 +181,24 @@ defmodule Electric.Shapes.Filter.Indexes.SubqueryIndex do
181181
branch_key
182182
)
183183

184-
:ets.match_delete(
184+
delete_node_members(
185185
table,
186-
{{:node_shape, node_id}, {shape_id, dep_index, polarity, next_condition_id, branch_key}}
186+
node_id,
187+
shape_id,
188+
polarity,
189+
next_condition_id,
190+
optimisation.subquery_ref
187191
)
188192

193+
:ets.delete(table, {:node_shape, node_id, shape_id, branch_key})
194+
189195
if polarity == :negated do
190-
:ets.match_delete(
191-
table,
192-
{{:node_negated_shape, node_id}, {shape_id, next_condition_id}}
193-
)
196+
:ets.delete(table, {:node_negated_shape, node_id, shape_id, next_condition_id})
194197
end
195198

196-
:ets.match_delete(
197-
table,
198-
{{:shape_node, shape_id}, {node_id, dep_index, polarity, next_condition_id, branch_key}}
199-
)
200-
201-
:ets.match_delete(
202-
table,
203-
{{:shape_dep_node, shape_id, dep_index},
204-
{node_id, polarity, next_condition_id, branch_key}}
205-
)
206-
207-
:ets.match_delete(table, {{:node_fallback, node_id}, {shape_id, next_condition_id}})
208-
delete_node_members(table, node_id, shape_id, polarity, next_condition_id)
199+
:ets.delete(table, {:node_fallback, node_id, shape_id, next_condition_id})
200+
:ets.delete(table, {:shape_node, shape_id, node_id, branch_key})
201+
:ets.delete(table, {:shape_dep_node, shape_id, dep_index, node_id, branch_key})
209202

210203
if node_empty?(table, node_id) do
211204
:ets.delete(table, {:node_meta, node_id})
@@ -237,7 +230,7 @@ defmodule Electric.Shapes.Filter.Indexes.SubqueryIndex do
237230

238231
for {node_id, _dep_index, _polarity, _next_condition_id, _branch_key} <-
239232
nodes_for_shape(table, shape_handle) do
240-
:ets.match_delete(table, {{:node_fallback, node_id}, {shape_handle, :_}})
233+
delete_by_key_prefix(table, {:node_fallback, node_id, shape_handle, :_})
241234
end
242235

243236
:ok
@@ -250,19 +243,8 @@ defmodule Electric.Shapes.Filter.Indexes.SubqueryIndex do
250243
def add_value(table, shape_handle, subquery_ref, dep_index, value) do
251244
for {node_id, polarity, next_condition_id, _branch_key} <-
252245
nodes_for_shape_dependency(table, shape_handle, dep_index) do
253-
case polarity do
254-
:positive ->
255-
:ets.insert(
256-
table,
257-
{{:node_positive_member, node_id, value}, {shape_handle, next_condition_id}}
258-
)
259-
260-
:negated ->
261-
:ets.insert(
262-
table,
263-
{{:node_negated_member, node_id, value}, {shape_handle, next_condition_id}}
264-
)
265-
end
246+
tag = if polarity == :positive, do: :node_positive_member, else: :node_negated_member
247+
:ets.insert(table, {{tag, node_id, value, shape_handle, next_condition_id}, true})
266248
end
267249

268250
:ets.insert(table, {{:membership, shape_handle, subquery_ref, value}, true})
@@ -276,19 +258,8 @@ defmodule Electric.Shapes.Filter.Indexes.SubqueryIndex do
276258
def remove_value(table, shape_handle, subquery_ref, dep_index, value) do
277259
for {node_id, polarity, next_condition_id, _branch_key} <-
278260
nodes_for_shape_dependency(table, shape_handle, dep_index) do
279-
case polarity do
280-
:positive ->
281-
:ets.match_delete(
282-
table,
283-
{{:node_positive_member, node_id, value}, {shape_handle, next_condition_id}}
284-
)
285-
286-
:negated ->
287-
:ets.match_delete(
288-
table,
289-
{{:node_negated_member, node_id, value}, {shape_handle, next_condition_id}}
290-
)
291-
end
261+
tag = if polarity == :positive, do: :node_positive_member, else: :node_negated_member
262+
:ets.delete(table, {tag, node_id, value, shape_handle, next_condition_id})
292263
end
293264

294265
:ets.delete(table, {:membership, shape_handle, subquery_ref, value})
@@ -305,16 +276,15 @@ defmodule Electric.Shapes.Filter.Indexes.SubqueryIndex do
305276
candidates =
306277
case evaluate_node_lhs(table, node_id, record) do
307278
{:ok, typed_value} ->
308-
positive =
309-
values_for_key(table, {:node_positive_member, node_id, typed_value}) |> MapSet.new()
279+
positive = members_for(table, :node_positive_member, node_id, typed_value)
310280

311281
negated =
312282
MapSet.difference(
313-
values_for_key(table, {:node_negated_shape, node_id}) |> MapSet.new(),
314-
values_for_key(table, {:node_negated_member, node_id, typed_value}) |> MapSet.new()
283+
negated_shapes_for(table, node_id),
284+
members_for(table, :node_negated_member, node_id, typed_value)
315285
)
316286

317-
fallback = values_for_key(table, {:node_fallback, node_id}) |> MapSet.new()
287+
fallback = fallback_for(table, node_id)
318288

319289
positive
320290
|> MapSet.union(negated)
@@ -407,61 +377,104 @@ defmodule Electric.Shapes.Filter.Indexes.SubqueryIndex do
407377
end
408378
end
409379

410-
defp delete_node_members(table, node_id, shape_id, polarity, next_condition_id) do
411-
case polarity do
412-
:positive ->
413-
:ets.match_delete(
414-
table,
415-
{{:node_positive_member, node_id, :_}, {shape_id, next_condition_id}}
416-
)
380+
# Range-bounded delete of every row whose key matches `key_pattern` (a tuple with a
381+
# bound prefix and `:_` wildcards in trailing positions). On an :ordered_set this is
382+
# O(log n + matched), never a full scan.
383+
defp delete_by_key_prefix(table, key_pattern) do
384+
:ets.select_delete(table, [{{key_pattern, :_}, [], [true]}])
385+
end
417386

418-
:negated ->
419-
:ets.match_delete(
420-
table,
421-
{{:node_negated_member, node_id, :_}, {shape_id, next_condition_id}}
422-
)
387+
# Delete this shape's node-local member rows for this node by enumerating the shape's
388+
# own values (scoped to the node's subquery_ref) from its membership rows and
389+
# point-deleting each. O(V_node · log n); touches only this shape's rows.
390+
#
391+
# INVARIANT (the safety of this approach rests on it): a node-member row is always a
392+
# subset of the shape's membership values — `add_value/5` and `remove_value/5` write
393+
# and remove the membership row alongside the node-member rows — so enumerating
394+
# membership finds every node-member row to delete. This requires membership rows to
395+
# outlive node removal, and to not be mutated concurrently:
396+
# * Ordering: `Filter.remove_shape/2` runs this (via `remove_shape/5`) BEFORE
397+
# `unregister_shape/2` deletes the `:membership` rows, so they are still present.
398+
# * No concurrent writer: the shape's consumer is the only process that writes
399+
# membership/node-member rows, and it is stopped synchronously before
400+
# `Filter.remove_shape/2` runs (`ShapeCleaner.remove_shape_immediate/3`), so no
401+
# `add_value`/`remove_value` can race this.
402+
# The "no orphan rows" test guards exactly this path. Reordering cleanup to remove
403+
# from the filter before stopping the consumer would reintroduce orphaned node-member
404+
# rows.
405+
defp delete_node_members(table, node_id, shape_id, polarity, next_condition_id, subquery_ref) do
406+
tag =
407+
case polarity do
408+
:positive -> :node_positive_member
409+
:negated -> :node_negated_member
410+
end
411+
412+
values =
413+
:ets.select(table, [
414+
{{{:membership, shape_id, subquery_ref, :"$1"}, :_}, [], [:"$1"]}
415+
])
416+
417+
for value <- values do
418+
:ets.delete(table, {tag, node_id, value, shape_id, next_condition_id})
423419
end
420+
421+
:ok
422+
end
423+
424+
defp members_for(table, tag, node_id, value) do
425+
:ets.select(table, [
426+
{{{tag, node_id, value, :"$1", :"$2"}, :_}, [], [{{:"$1", :"$2"}}]}
427+
])
428+
|> MapSet.new()
429+
end
430+
431+
defp negated_shapes_for(table, node_id) do
432+
:ets.select(table, [
433+
{{{:node_negated_shape, node_id, :"$1", :"$2"}, :_}, [], [{{:"$1", :"$2"}}]}
434+
])
435+
|> MapSet.new()
436+
end
437+
438+
defp fallback_for(table, node_id) do
439+
:ets.select(table, [
440+
{{{:node_fallback, node_id, :"$1", :"$2"}, :_}, [], [{{:"$1", :"$2"}}]}
441+
])
442+
|> MapSet.new()
443+
end
444+
445+
defp all_node_shapes(table, node_id) do
446+
:ets.select(table, [
447+
{{{:node_shape, node_id, :"$1", :_}, {:_, :_, :"$2"}}, [], [{{:"$1", :"$2"}}]}
448+
])
449+
|> MapSet.new()
424450
end
425451

426452
defp nodes_for_shape(table, shape_handle) do
427-
table
428-
|> :ets.lookup({:shape_node, shape_handle})
429-
|> Enum.map(&elem(&1, 1))
453+
:ets.select(table, [
454+
{{{:shape_node, shape_handle, :"$1", :"$2"}, {:"$3", :"$4", :"$5"}}, [],
455+
[{{:"$1", :"$3", :"$4", :"$5", :"$2"}}]}
456+
])
430457
end
431458

432459
defp nodes_for_shape_dependency(table, shape_handle, dep_index) do
433-
table
434-
|> :ets.lookup({:shape_dep_node, shape_handle, dep_index})
435-
|> Enum.map(&elem(&1, 1))
460+
:ets.select(table, [
461+
{{{:shape_dep_node, shape_handle, dep_index, :"$1", :"$2"}, {:"$3", :"$4"}}, [],
462+
[{{:"$1", :"$3", :"$4", :"$2"}}]}
463+
])
436464
end
437465

438466
defp node_shape_entry_for_shape(table, shape_id, node_id, branch_key) do
439-
table
440-
|> nodes_for_shape(shape_id)
441-
|> Enum.find_value(fn
442-
{^node_id, dep_index, polarity, next_condition_id, ^branch_key} ->
443-
{dep_index, polarity, next_condition_id}
444-
445-
_ ->
446-
nil
447-
end)
467+
case :ets.lookup(table, {:shape_node, shape_id, node_id, branch_key}) do
468+
[{_, {dep_index, polarity, next_condition_id}}] -> {dep_index, polarity, next_condition_id}
469+
[] -> nil
470+
end
448471
end
449472

450473
defp node_empty?(table, node_id) do
451-
:ets.lookup(table, {:node_shape, node_id}) == []
452-
end
453-
454-
defp all_node_shapes(table, node_id) do
455-
table
456-
|> :ets.lookup({:node_shape, node_id})
457-
|> Enum.reduce(MapSet.new(), fn
458-
{{:node_shape, ^node_id}, {shape_id, _dep_index, _polarity, next_condition_id, _branch_key}},
459-
acc ->
460-
MapSet.put(acc, {shape_id, next_condition_id})
461-
462-
_, acc ->
463-
acc
464-
end)
474+
case :ets.select(table, [{{{:node_shape, node_id, :_, :_}, :_}, [], [true]}], 1) do
475+
:"$end_of_table" -> true
476+
_ -> false
477+
end
465478
end
466479

467480
defp evaluate_node_lhs(table, node_id, record) do
@@ -485,12 +498,6 @@ defmodule Electric.Shapes.Filter.Indexes.SubqueryIndex do
485498
end
486499
end
487500

488-
defp values_for_key(table, key) do
489-
table
490-
|> :ets.lookup(key)
491-
|> Enum.map(&elem(&1, 1))
492-
end
493-
494501
defp shape_ready?(table, shape_handle) do
495502
not fallback?(table, shape_handle)
496503
end

0 commit comments

Comments
 (0)