Should empty move-in queries emit snapshot-end so clients can observe completion?

Aperta
#4,834 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
35/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
elixir, postgresql

Direzione di ricerca

Start with execute_effect(%AppendMoveInSnapshot{}, acc) in packages/sync-service/lib/electric/shapes/consumer/effects.ex and the consumer tests in packages/sync-service/test/electric/shapes/consumer_test.exs. Read the existing move-in snapshot handling and run mix test test/electric/shapes/consumer_test.exs from packages/sync-service. Done means a successful empty move-in stores a snapshot-end boundary with the expected transaction IDs, and the regression test passes.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

Disclaimer: This issue has been generated completely by GPT-6.1 Sol. I understand the issue only on a high level and haven't studied whether the suggested patch is reasonable.

Summary and scope

Electric 1.8.2 appends snapshot-end after a nonempty move-in query, but omits it when the query successfully returns zero rows. Is this intentional, and if so, how should a client observe completion of an empty move-in?

We found this while implementing scoped move-in confirmation for TanStack/db#2244. We have a consumer-level reproduction of the missing marker and a proposed change below. We have not reproduced a user-visible failure in our existing application. The workflow below is an illustrative use case for the completion signal, not an incident report.

The omission does not by itself mean that the stream is stuck or that subsequent ordinary inserts cannot sync. It matters when a consumer deliberately waits for the results of a particular dependency change before considering that change fully reflected locally.

A realistic use case: accepting access to a shared folder

Consider a document application with these tables:

folders(id)
folder_members(folder_id REFERENCES folders(id), user_id)
documents(id, folder_id REFERENCES folders(id), title)

The app maintains an already-running account-wide document shape:

SELECT * FROM documents
WHERE folder_id IN (
  SELECT folder_id FROM folder_members WHERE user_id = 'current-user'
);

The user accepts an invitation to a shared folder. The API inserts their membership and returns its PostgreSQL transaction ID. The UI wants to keep showing "Loading documents..." until the new access has been reflected in the local document collection, then show either the documents or a trustworthy "This folder is empty" state.

For a nonempty folder, the client must wait for the move-in query's result: the existing collection is also empty for this folder before the documents arrive, so looking at the current row count cannot establish that the folder is empty.

A possible implementation is:

// Illustrative application code; assumes scoped confirmation as in TanStack/db#2244.
// The document shape is already subscribed before access is granted.
const { txid } = await api.acceptFolderInvitation(invitationId);
await documents.utils.awaitTxId(txid);
// Now end the loading state and render the synchronized folder contents.

Equivalently, a custom consumer could wait for the announced move-in query to close rather than using awaitTxId. This is intentionally a wait on a dependency transaction through the dependent shape; whether this is the intended supported contract is part of the question.

The sequence for an empty folder is:

  1. Transaction 150 inserts the user's membership. The folder exists, satisfying the foreign key, but has no documents. No foreign key requires a folder to contain documents.
  2. Electric emits a move-in announcement with txids: [150] for the document shape.
  3. The client records that the membership's document query is pending. Confirming from the announcement would be unsafe for a nonempty folder because its rows are still being fetched.
  4. Electric completes the query with zero rows.
  5. The zero-row branch appends no snapshot-end. A consumer waiting for that boundary has no completion event to consume, so the loading state eventually times out despite the query having finished successfully.

This scenario specifically assumes a live shape established before the membership change. If the app instead starts a fresh shape after accepting access, its normal initial-load completion may be sufficient; that is not the case this report is trying to reproduce.

Also, the currently published TanStack adapter can acknowledge move-in transaction IDs too early. That can mask this missing-boundary case. The proposed scoped-confirmation behavior in TanStack/db#2244 waits for completion rather than resolving from the announcement.

Observed server behavior

The stored log for an empty result contains:

move-in: txids=[150], patterns=[...]
-- query completes with zero rows --
-- no snapshot-end is appended --

The proposed sequence is:

move-in: txids=[150], patterns=[...]
snapshot-end: xmin=..., xmax=..., xip_list=[...]

execute_effect(%AppendMoveInSnapshot{}, acc) in packages/sync-service/lib/electric/shapes/consumer/effects.ex returns early when row_count == 0. That bypasses appending and accounting for the boundary. The branch was also present in upstream main inspected on 2026-10-24.

The missing marker is a stored-log behavior independent of SSE. An SSE up-to-date marks a published batch; it should not be assumed to close this asynchronous move-in query unless that stronger guarantee is part of the protocol. We would welcome clarification if another existing signal is intended for this purpose.

Minimal regression

Add this test inside the existing consumer-test describe block containing consumer startup seeds the stack-scoped subquery index in packages/sync-service/test/electric/shapes/consumer_test.exs. It uses the same existing setup and @shape_with_subquery, as well as that module's aliases and send_stored_move_in_complete helper.

The test drives a dependency move-in carrying transaction 150, supplies an empty asynchronous query result, and inspects actual stored messages. The unpatched zero-row branch omits the expected second message. It requires no UI or TanStack adapter.

test "consumer closes an empty move-in snapshot with its transaction IDs", ctx do
      parent = self()

      Repatch.patch(
        Electric.Shapes.Consumer.Effects,
        :query_move_in_async,
        [mode: :shared],
        fn _task_sup, _consumer_state, _buffering_state, consumer_pid ->
          send(parent, {:query_requested, consumer_pid})
          :ok
        end
      )

      Support.TestUtils.activate_mocks_for_descendant_procs(Consumer)

      {shape_handle, _} =
        ShapeCache.get_or_create_shape_handle(@shape_with_subquery, ctx.stack_id)

      :started = ShapeCache.await_snapshot_start(shape_handle, ctx.stack_id)

      {:ok, shape} = Electric.Shapes.fetch_shape_by_handle(ctx.stack_id, shape_handle)
      [dep_handle] = shape.shape_dependencies_handles

      consumer_pid = Consumer.whereis(ctx.stack_id, shape_handle)
      ref = Shapes.Consumer.register_for_changes(ctx.stack_id, shape_handle)

      assert :ok = LsnTracker.broadcast_last_seen_lsn(ctx.stack_id, 100)

      send(
        consumer_pid,
        {:materializer_changes, dep_handle,
         %{
           move_in: [{1, "1"}],
           move_out: [],
           txids: [150]
         }}
      )

      assert_receive {:query_requested, ^consumer_pid}

      send(consumer_pid, {:pg_snapshot_known, {100, 300, []}})

      shape_storage = Storage.for_shape(shape_handle, ctx.storage)

      send_stored_move_in_complete(
        consumer_pid,
        shape_storage,
        [],
        Lsn.from_integer(100)
      )

      assert_receive {^ref, :new_changes, _offset}, @receive_timeout

      assert [
               %{"headers" => %{"event" => "move-in", "txids" => [150]}},
               %{
                 "headers" => %{
                   "control" => "snapshot-end",
                   "xmin" => "100",
                   "xmax" => "300",
                   "xip_list" => []
                 }
               }
             ] = get_log_items_from_storage(LogOffset.last_before_real_offsets(), shape_storage)
    end

Run in packages/sync-service:

mix test test/electric/shapes/consumer_test.exs

Proposed fix

If each announced move-in query is intended to have an observable completion, append snapshot-end after a successful query, including an empty one. Count that control message in the change count, encoded byte size, and pending written offset, just as for nonempty snapshots.

diff --git a/packages/sync-service/lib/electric/shapes/consumer/effects.ex b/packages/sync-service/lib/electric/shapes/consumer/effects.ex
--- a/packages/sync-service/lib/electric/shapes/consumer/effects.ex
+++ b/packages/sync-service/lib/electric/shapes/consumer/effects.ex
@@ -159,27 +159,26 @@
 
     state = %{state | writer: writer, latest_offset: inserted_offset}
 
-    if row_count == 0 do
-      %{acc | state: state}
-    else
-      snapshot_end =
-        snapshot
-        |> snapshot_end_message()
-        |> Jason.encode!()
-
-      {{_, offset}, writer} =
-        ShapeCache.Storage.append_control_message!(snapshot_end, state.writer)
-
-      state = %{state | writer: writer, latest_offset: offset}
-
-      %{
-        acc
-        | state: state,
-          num_changes: acc.num_changes + row_count + 1,
-          total_size: acc.total_size + row_bytes + byte_size(snapshot_end),
-          pending_written_offset: state.latest_offset
-      }
-    end
+    # empty move-ins also need a closing boundary so clients can
+    # confirm their explicit txids without waiting for an unrelated snapshot.
+    # Remove this patch when upstream always emits snapshot-end for move-ins.
+    snapshot_end =
+      snapshot
+      |> snapshot_end_message()
+      |> Jason.encode!()
+
+    {{_, offset}, writer} =
+      ShapeCache.Storage.append_control_message!(snapshot_end, state.writer)
+
+    state = %{state | writer: writer, latest_offset: offset}
+
+    %{
+      acc
+      | state: state,
+        num_changes: acc.num_changes + row_count + 1,
+        total_size: acc.total_size + row_bytes + byte_size(snapshot_end),
+        pending_written_offset: state.latest_offset
+    }
   end
 
   defp execute_effect(%NotifyFlushed{log_offset: log_offset}, acc) do

The consumer regression was rerun successfully with this change on 2026-10-24. A separate controlled adapter regression verifies that the proposed scoped-confirmation behavior can acknowledge an empty move-in when its closing marker is supplied. These tests establish the protocol behavior; they do not constitute a reproduction of the illustrative folder UI.

Related reports and intended contract

  • #3569 requested move-in visibility boundaries for transaction mapping; #3576 introduced snapshot-end after move-ins. Neither explicitly reports the zero-row omission.
  • #4304 added the originating transaction IDs to move-in/out control messages.
  • TanStack/db#2244 concerns an adapter treating a scoped snapshot's database visibility metadata as global confirmation. This report addresses the server's missing completion marker, rather than asking it to provide global confirmation.

Should every announced, successful move-in query have a closing boundary, including an empty result? Is waiting for the originating membership transaction through the dependent document shape an intended use of move-in transaction IDs? If empty results intentionally have no boundary, what protocol evidence should a consumer use to distinguish an empty completed query from one whose results are still pending?

Lingua principale
TypeScript
Stelle
10.4k
Fork
399
Metriche di merge delle PR
Nessuna PR unita negli ultimi 30g

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di electric-sql/electric

Tutte le issue di electric-sql/electric

Issue simili

Altre issue su TypeScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.