Skip to content

Commit 86bb70f

Browse files
erik-the-implementeralcoclaude
authored
Fix flaky PublicationManager relation tracker restart test (#4635)
Fixes #4606. ## Problem The `PublicationManagerTest` "handles relation tracker restart" test is flaky. It stops the `RelationTracker` (the supervisor then restarts it) and relies on `assert_pub_tables` as a "wait for restart" barrier — but it isn't one. The relation is already in the publication and is **not** removed during the restart, so `assert_pub_tables(ctx, [ctx.relation], …)` passes immediately, potentially before the supervisor has restarted and re-registered the process. The subsequent `PublicationManager.remove_shape/2` call (→ `GenServer.call(name(stack_id), …)`) then races with a "no process" exit. ## Fix Wait for the `RelationTracker` to be re-registered, then for it to finish restoring its filters from `ShapeStatus`, before issuing further calls: ```elixir assert wait_until(fn -> is_pid(GenServer.whereis(relation_tracker_name)) end, 2_000) :ok = PublicationManager.wait_for_restore(ctx.stack_id, timeout: 2_000) ``` Rather than add a bespoke helper, this promotes the existing private `wait_until` poller (previously in `shape_cache_test.exs`) into the shared `Support.TestUtils` as `wait_until/2`, and migrates its original call site to the shared version. ## Verification 15/15 seeded runs of the target test pass; the full `publication_manager_test.exs` (26 tests) and the migrated `shape_cache_test.exs` test are green. Test-only change, so no changeset. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Oleksii Sholik <oleksii@sholik.dev> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent f245417 commit 86bb70f

4 files changed

Lines changed: 47 additions & 20 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+
Fix a race condition in the PublicationManager relation tracker restart test that caused intermittent failures.

packages/sync-service/test/electric/replication/publication_manager_test.exs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -652,6 +652,18 @@ defmodule Electric.Replication.PublicationManagerTest do
652652
relation_tracker_name = PublicationManager.RelationTracker.name(ctx.stack_id)
653653
GenServer.stop(relation_tracker_name)
654654

655+
# Wait for the supervisor to restart and re-register the RelationTracker,
656+
# then for it to finish restoring filters from ShapeStatus.
657+
#
658+
# The assert_pub_tables below is NOT a sufficient barrier on its own: the
659+
# relation is already in the publication and is not removed during the
660+
# restart, so the assertion passes immediately - potentially before the
661+
# restarted process has re-registered. Calling remove_shape in that window
662+
# races with a "no process" exit, which is the flakiness this guards
663+
# against.
664+
assert wait_until(fn -> is_pid(GenServer.whereis(relation_tracker_name)) end, 2_000)
665+
:ok = PublicationManager.wait_for_restore(ctx.stack_id, timeout: 2_000)
666+
655667
# After restart, the publication manager should repopulate from ShapeStatus.
656668
# The publication should still have the relation.
657669
assert_pub_tables(ctx, [ctx.relation], 2_000)

packages/sync-service/test/electric/shape_cache_test.exs

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ defmodule Electric.ShapeCacheTest do
2222
assert_shape_cleanup: 1,
2323
patch_shape_status: 1,
2424
patch_shape_cache: 1,
25-
complete_txn_fragment: 3
25+
complete_txn_fragment: 3,
26+
wait_until: 2
2627
]
2728

2829
@stub_inspector Support.StubInspector.new(
@@ -1338,9 +1339,10 @@ defmodule Electric.ShapeCacheTest do
13381339
index_after = SubqueryIndex.for_stack(ctx.stack_id)
13391340
assert index_after != nil
13401341

1341-
assert wait_until(200, fn ->
1342-
SubqueryIndex.has_positions?(index_after, shape_handle)
1343-
end)
1342+
assert wait_until(
1343+
fn -> SubqueryIndex.has_positions?(index_after, shape_handle) end,
1344+
200
1345+
)
13441346
end
13451347

13461348
test "restores shapes with subqueries and their materializers when backup missing", ctx do
@@ -1389,22 +1391,6 @@ defmodule Electric.ShapeCacheTest do
13891391
:ok = stop_supervised(name)
13901392
end
13911393
end
1392-
1393-
defp wait_until(timeout_ms, fun, started_at \\ System.monotonic_time(:millisecond))
1394-
1395-
defp wait_until(timeout_ms, fun, started_at) do
1396-
cond do
1397-
fun.() ->
1398-
true
1399-
1400-
System.monotonic_time(:millisecond) - started_at >= timeout_ms ->
1401-
false
1402-
1403-
true ->
1404-
Process.sleep(10)
1405-
wait_until(timeout_ms, fun, started_at)
1406-
end
1407-
end
14081394
end
14091395

14101396
describe "wait_for_restore eager subquery consumer start" do

packages/sync-service/test/support/test_utils.ex

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,30 @@ defmodule Support.TestUtils do
138138
Electric.StatusMonitor.mark_pg_lock_as_errored(stack_id, error_message)
139139
end
140140

141+
@doc """
142+
Poll `fun` every 10ms until it returns a truthy value or `timeout_ms` elapses.
143+
144+
Returns `true` if the condition was met within the timeout, `false` otherwise.
145+
"""
146+
def wait_until(fun, timeout_ms \\ 500)
147+
when is_function(fun, 0) and is_integer(timeout_ms) do
148+
do_wait_until(fun, timeout_ms, System.monotonic_time(:millisecond))
149+
end
150+
151+
defp do_wait_until(fun, timeout_ms, started_at) do
152+
cond do
153+
fun.() ->
154+
true
155+
156+
System.monotonic_time(:millisecond) - started_at >= timeout_ms ->
157+
false
158+
159+
true ->
160+
Process.sleep(10)
161+
do_wait_until(fun, timeout_ms, started_at)
162+
end
163+
end
164+
141165
def generate_shape(relation, where_clause \\ nil, selected_columns \\ nil) do
142166
all_columns = Enum.uniq(["id", "value", "foo_enum"] ++ (selected_columns || []))
143167
selected_columns = selected_columns || all_columns

0 commit comments

Comments
 (0)