fix(sync-service): drop subquery shapes on restart instead of restoring them#4735
Open
robacourt wants to merge 4 commits into
Open
fix(sync-service): drop subquery shapes on restart instead of restoring them#4735robacourt wants to merge 4 commits into
robacourt wants to merge 4 commits into
Conversation
…ng them Restoring shapes that involve subqueries consistently across a restart has proven too fragile: the outer shape's on-disk view and its dependency materializer can diverge, producing incorrect results (or a 409 must-refetch) after the server comes back up. Rather than keep chasing those consistency bugs, drop every shape involved in a subquery on startup and let clients re-request them from scratch. On boot, `drop_subquery_shapes/1` (replacing `eagerly_start_subquery_shape_consumers/1`) removes the transitive closure `[handle | shape_dependencies_handles]` for each shape with a non-empty `shape_dependencies`, before ShapeLogCollector starts dispatching events so no change ever reaches a dropped shape. The targeted regression `oracle_restore_test.exs` bug 1 now passes; the restart property test survives many more restart cycles before hitting a separate, pre-existing cold subquery snapshot bug. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for electric-next ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4735 +/- ##
=======================================
Coverage 60.04% 60.05%
=======================================
Files 397 397
Lines 43766 43766
Branches 12588 12590 +2
=======================================
+ Hits 26278 26282 +4
+ Misses 17407 17403 -4
Partials 81 81
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The sync service now removes every shape involved in a subquery on startup to prevent consistency issues.
…tore The ShapeLogCollector restores routing indexes independently from ShapeStatus in handle_continue(:restore_shapes), and it starts before ShapeCache. The startup drop removed subquery shapes from ShapeStatus/storage but relied on ShapeCleaner -> ShapeLogCollector.remove_shape to clear the collector routing; that call goes through the RequestBatcher, which deliberately returns before the removal is applied. So a restart could reinstate dropped subquery handles into the collector's routing indexes and mark_as_ready could run before the async cleanup landed. Skip the whole subquery hierarchy in the collector's restore too, using a shared ShapeStatus.subquery_shape_handles/1 helper (also used by the ShapeCache drop), so a dropped handle is never added to routing in the first place. Add a deterministic test that restarts only the collector and asserts the subquery hierarchy is absent from ShapeLogCollector.active_shapes/1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Restoring shapes that involve subqueries consistently across a server restart has proven too fragile: keeping an outer shape's on-disk view in sync with its dependency materializer across a restart could diverge, producing incorrect results (or a
409 must-refetch) after the server comes back up.Rather than keep chasing those restore-path consistency bugs, this drops every shape involved in a subquery on startup and lets clients re-request them from scratch.
Changes
eagerly_start_subquery_shape_consumers/1withdrop_subquery_shapes/1inshape_cache.ex, called fromhandle_continue(:wait_for_restore, …)beforeShapeLogCollector.mark_as_ready/1, so no change ever reaches a dropped shape.[handle | shape_dependencies_handles]for every shape with a non-emptyshape_dependencies, via a singleShapeCleaner.remove_shapes/2call. This covers nested subqueries automatically: an intermediate dependency that itself has a subquery matches the same filter and contributes its own dependencies.restore_shape_and_dependencies/3path (used bystart_consumer_for_handleand fresh creation) is untouched; after startup there are no subquery shapes left to restore.shape_cache_test.exsto assert the drop behaviour (subquery shape + its materializers are gone after restart) instead of restore.Test Plan
mix test test/electric/shape_cache_test.exs— passes (drop-on-restart + guard tests).oracle_restore_test.exsbug 1 (subquery-shape-diverges-after-restart regression) now passes with the full stack.Notes
Supersedes the restore path added in #4666 / #4715 for the subquery-restart case. The remaining
oracle_property_testfailures under load are pre-existing and unrelated to this change (a checker-timeout/liveness artifact plus a separate 409 flake); they reproduce withRESTART_SERVER_EVERY=0.Generated with Claude Code