electric-sql / electric-sql/electric

Race condition in drop_replication_slot_on_stop: cast may not arrive before terminate

Open
#4,200 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

There is a race condition in `Electric.Connection.Manager.drop_replication_slot_on_stop/1` where the cast message may not be processed before the process terminates, causing the replication slot to not be dropped.

## The race

In `packages/sync-service/lib/electric/connection/manager.ex`:

1. **Line 122**: State initializes with `drop_slot_requested: false`
2. **Line 189**: `drop_replication_slot_on_stop(manager)` does `GenServer.cast(manager, :drop_replication_slot_on_stop)` — returns immediately
3. **Line 680**: `handle_cast(:drop_replication_slot_on_stop, state)` flips `drop_slot_requested: true`
4. **Line 874**: `terminate/2` reads `state.drop_slot_requested` and conditionally calls `drop_publication/1` and `drop_slot/1`

The caller (e.g., `TenantManager.do_stop_tenant`) calls `drop_replication_slot_on_termination` (a cast → returns immediately) and *immediately* calls `DynamicTenantSupervisor.stop_tenant` which invokes `DynamicSupervisor.terminate_child`.

**The problem:** The cast and the supervisor's terminate signal are sent from *two different processes* (the TenantManager GenServer vs. the DynamicSupervisor). Erlang's mailbox preserves send order only for messages between the *same* sender pair. There is no happens-before between messages from different senders.

If the supervisor's terminate signal arrives before the cast is processed, `terminate/2` runs with `drop_slot_requested: false` and **the slot is NOT dropped**.

## Impact

The existing manual `clean?: true` path does this same dance today. Either:
- (a) In practice the cast usually wins because of typical scheduling latencies, or
- (b) The manual path silently misses slot drops sometimes and nobody noticed (operators just retry)

This becomes more critical for automated slot cleanup (e.g., when WAL retention exceeds thresholds) where silent failures are harder to detect.

## Proposed fix

Add a synchronous variant `drop_replication_slot_on_stop_sync/1` using `GenServer.call` that flips the flag and returns only after the cast is processed:

```elixir
@spec drop_replication_slot_on_stop_sync(GenServer.server()) :: :ok
def drop_replication_slot_on_stop_sync(manager) do
GenServer.call(manager, :drop_replication_slot_on_stop)
end

def handle_call(:drop_replication_slot_on_stop, _from, state) do
{:reply, :ok, %{state | drop_slot_requested: true}}
end
```

This ensures the flag is set before the caller proceeds to terminate the process.

## References

- Investigation from alco-agent-tasks#37 (section 1.8)
- File: `packages/sync-service/lib/electric/connection/manager.ex`

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.