Skip to content

Commit 35d204e

Browse files
magnetisedalco
andauthored
chore: Remove unused storage callbacks (#3717)
Now that SQLite has all the shape data we don't need the storage callbacks that retreive it, and also do not need to serialize the shape to disk at all. I've kept the `get_all_stored_shape_handles/1` callback to allow for some kind of async reconciliation process that can detect and remove orphaned storage instances. --------- Co-authored-by: Oleksii Sholik <oleksii@sholik.dev>
1 parent dfcfa40 commit 35d204e

File tree

7 files changed

+2
-215
lines changed

7 files changed

+2
-215
lines changed

packages/sync-service/config/runtime.exs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -129,13 +129,6 @@ storage_spec =
129129
"fast_file" ->
130130
{Electric.ShapeCache.PureFileStorage, storage_dir: shape_path}
131131

132-
"crashing_file" ->
133-
num_calls_until_crash =
134-
env!("CRASHING_FILE_ELECTRIC_STORAGE__NUM_CALLS_UNTIL_CRASH", :integer)
135-
136-
{Electric.ShapeCache.CrashingFileStorage,
137-
storage_dir: shape_path, num_calls_until_crash: num_calls_until_crash}
138-
139132
_ ->
140133
raise Dotenvy.Error, message: "storage must be one of: MEMORY, FAST_FILE, LEGACY_FILE"
141134
end

packages/sync-service/lib/electric/shape_cache/crashing_file_storage.ex

Lines changed: 0 additions & 76 deletions
This file was deleted.

packages/sync-service/lib/electric/shape_cache/in_memory_storage.ex

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -130,12 +130,6 @@ defmodule Electric.ShapeCache.InMemoryStorage do
130130
@impl Electric.ShapeCache.Storage
131131
def get_all_stored_shape_handles(_opts), do: {:ok, MapSet.new()}
132132

133-
@impl Electric.ShapeCache.Storage
134-
def get_stored_shapes(_opts, _shape_handles), do: %{}
135-
136-
@impl Electric.ShapeCache.Storage
137-
def metadata_backup_dir(_opts), do: nil
138-
139133
@impl Electric.ShapeCache.Storage
140134
def get_total_disk_usage(_opts), do: 0
141135

packages/sync-service/lib/electric/shape_cache/pure_file_storage.ex

Lines changed: 2 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ defmodule Electric.ShapeCache.PureFileStorage do
3838
alias Electric.ShapeCache.PureFileStorage.Snapshot
3939
alias Electric.ShapeCache.PureFileStorage.WriteLoop
4040
alias Electric.ShapeCache.Storage
41-
alias Electric.Shapes.Shape
4241

4342
import LogOffset
4443
import Electric.ShapeCache.PureFileStorage.SharedRecords
@@ -94,9 +93,6 @@ defmodule Electric.ShapeCache.PureFileStorage do
9493

9594
@clean_cache_keys storage_meta_keys() -- @read_path_keys
9695

97-
# Directory for storing metadata
98-
@metadata_storage_dir ".meta"
99-
10096
def shared_opts(opts) do
10197
stack_id = Keyword.fetch!(opts, :stack_id)
10298
storage_dir = Keyword.get(opts, :storage_dir, "./shapes")
@@ -205,37 +201,6 @@ defmodule Electric.ShapeCache.PureFileStorage do
205201
|> Enum.into(MapSet.new())}
206202
end
207203

208-
def get_stored_shapes(stack_opts, shape_handles) do
209-
Task.Supervisor.async_stream(
210-
stack_task_supervisor(stack_opts.stack_id),
211-
shape_handles,
212-
fn handle ->
213-
shape_opts = for_shape(handle, stack_opts)
214-
215-
case read_shape_definition(shape_opts) do
216-
{:ok, shape} ->
217-
{handle, {:ok, shape}}
218-
219-
_ ->
220-
Logger.warning(
221-
"Failed to read shape definition for shape #{handle}, removing it from disk"
222-
)
223-
224-
cleanup!(shape_opts)
225-
{handle, {:error, :failed_to_recover_shape}}
226-
end
227-
end,
228-
timeout: :infinity,
229-
ordered: false
230-
)
231-
|> Enum.map(fn {:ok, res} -> res end)
232-
|> Map.new()
233-
end
234-
235-
def metadata_backup_dir(%{base_path: base_path}) do
236-
Path.join([base_path, @metadata_storage_dir, "backups"])
237-
end
238-
239204
def drop_all_ets_entries(stack_id) do
240205
try do
241206
:ets.delete_all_objects(stack_ets(stack_id))
@@ -523,7 +488,7 @@ defmodule Electric.ShapeCache.PureFileStorage do
523488
def init_writer!(shape_opts, shape_definition) do
524489
table = :ets.new(:in_memory_storage, [:ordered_set, :protected])
525490

526-
{initial_acc, suffix} = initialise_filesystem!(shape_opts, shape_definition)
491+
{initial_acc, suffix} = initialise_filesystem!(shape_opts)
527492

528493
register_with_stack(
529494
shape_opts,
@@ -569,7 +534,7 @@ defmodule Electric.ShapeCache.PureFileStorage do
569534
writer_state(state, writer_acc: WriteLoop.flush_and_close_all(acc, state))
570535
end
571536

572-
defp initialise_filesystem!(%__MODULE__{} = opts, shape_definition) do
537+
def initialise_filesystem!(%__MODULE__{} = opts) do
573538
on_disk_version = read_metadata!(opts, :version)
574539
new? = is_nil(on_disk_version)
575540

@@ -585,7 +550,6 @@ defmodule Electric.ShapeCache.PureFileStorage do
585550

586551
if initialize? do
587552
create_directories!(opts)
588-
write_shape_definition!(opts, shape_definition)
589553
end
590554

591555
suffix =
@@ -747,24 +711,6 @@ defmodule Electric.ShapeCache.PureFileStorage do
747711
:ets.update_element(stack_ets, handle, updates)
748712
end
749713

750-
defp write_shape_definition!(%__MODULE__{} = opts, shape_definition) do
751-
write!(
752-
shape_metadata_path(opts, "shape_definition.json"),
753-
Jason.encode!(shape_definition),
754-
[:raw]
755-
)
756-
end
757-
758-
defp read_shape_definition(%__MODULE__{} = opts) do
759-
path = shape_metadata_path(opts, "shape_definition.json")
760-
761-
with {:ok, contents} <- File.open(path, [:read, :raw, :read_ahead], &IO.binread(&1, :eof)),
762-
{:ok, decoded} <- Jason.decode(if(is_binary(contents), do: contents, else: "")),
763-
{:ok, rebuilt} <- Shape.from_json_safe(decoded) do
764-
{:ok, rebuilt}
765-
end
766-
end
767-
768714
defp last_snapshot_chunk(%__MODULE__{} = opts),
769715
do: read_cached_metadata(opts, :last_snapshot_chunk)
770716

packages/sync-service/lib/electric/shape_cache/storage.ex

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,6 @@ defmodule Electric.ShapeCache.Storage do
5353
@callback get_all_stored_shape_handles(compiled_opts()) ::
5454
{:ok, MapSet.t(shape_handle())} | {:error, term()}
5555

56-
@doc "Retrieve stored shapes for given shape handles"
57-
@callback get_stored_shapes(compiled_opts(), Enumerable.t(shape_handle())) ::
58-
%{shape_handle() => {:ok, shape_def :: Shape.t()} | {:error, term()}}
59-
60-
@doc "Get the directory where metadata backups are stored."
61-
@callback metadata_backup_dir(compiled_opts()) :: String.t() | nil
62-
6356
@doc "Get the total disk usage for all shapes"
6457
@callback get_total_disk_usage(compiled_opts()) :: non_neg_integer()
6558

@@ -260,16 +253,6 @@ defmodule Electric.ShapeCache.Storage do
260253
mod.get_all_stored_shape_handles(opts)
261254
end
262255

263-
@impl __MODULE__
264-
def get_stored_shapes({mod, opts}, shape_handles) do
265-
mod.get_stored_shapes(opts, shape_handles)
266-
end
267-
268-
@impl __MODULE__
269-
def metadata_backup_dir({mod, opts}) do
270-
mod.metadata_backup_dir(opts)
271-
end
272-
273256
@impl __MODULE__
274257
def get_total_disk_usage({mod, opts}) do
275258
mod.get_total_disk_usage(opts)

packages/sync-service/test/electric/shape_cache/storage_implementations_test.exs

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1187,46 +1187,6 @@ defmodule Electric.ShapeCache.StorageImplimentationsTest do
11871187
end
11881188
end
11891189

1190-
describe "#{module_name}.get_stored_shapes/1" do
1191-
@describetag skip_initialise: true
1192-
setup :start_storage
1193-
1194-
test "retrieves no shapes if no shapes persisted", %{storage_base: storage_base} do
1195-
assert %{} = Storage.get_stored_shapes(storage_base, [])
1196-
end
1197-
1198-
test "retrieves stored shapes", %{storage: opts, storage_base: storage_base} do
1199-
_writer = Storage.init_writer!(opts, @shape)
1200-
invalid_handle = "invalid-handle"
1201-
1202-
assert %{@shape_handle => {:ok, parsed}, ^invalid_handle => {:error, _}} =
1203-
Storage.get_stored_shapes(storage_base, [@shape_handle, invalid_handle])
1204-
1205-
assert @shape == parsed
1206-
end
1207-
1208-
test "restores shape snapshot started flag", %{storage: opts, storage_base: storage_base} do
1209-
_writer = Storage.init_writer!(opts, @shape)
1210-
:ok = Storage.mark_snapshot_as_started(opts)
1211-
1212-
assert %{@shape_handle => {:ok, parsed}} =
1213-
Storage.get_stored_shapes(storage_base, [@shape_handle])
1214-
1215-
assert @shape == parsed
1216-
end
1217-
end
1218-
1219-
describe "#{module_name}.metadata_backup_dir/1" do
1220-
@describetag skip_initialise: true
1221-
setup :start_storage
1222-
1223-
test "returns metadata backup directory", %{storage_base: storage_base} do
1224-
assert dir = Storage.metadata_backup_dir(storage_base)
1225-
assert is_binary(dir)
1226-
assert Path.type(dir) == :absolute
1227-
end
1228-
end
1229-
12301190
describe "#{module_name}.cleanup!/1" do
12311191
setup :start_storage
12321192

@@ -1299,7 +1259,6 @@ defmodule Electric.ShapeCache.StorageImplimentationsTest do
12991259

13001260
Storage.cleanup_all!(storage_base)
13011261
assert Storage.get_total_disk_usage(storage_base) == 0
1302-
refute File.dir?(Storage.metadata_backup_dir(storage_base))
13031262
end
13041263

13051264
test "should handle entire base directory already missing", %{storage_base: storage_base} do

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

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -77,18 +77,6 @@ defmodule Support.TestStorage do
7777
Storage.get_all_stored_shape_handles(storage)
7878
end
7979

80-
@impl Electric.ShapeCache.Storage
81-
def get_stored_shapes({parent, _init, storage}, shape_handles) do
82-
send(parent, {__MODULE__, :get_stored_shapes, shape_handles})
83-
Storage.get_stored_shapes(storage, shape_handles)
84-
end
85-
86-
@impl Electric.ShapeCache.Storage
87-
def metadata_backup_dir({parent, _init, storage}) do
88-
send(parent, {__MODULE__, :metadata_backup_dir})
89-
Storage.metadata_backup_dir(storage)
90-
end
91-
9280
@impl Electric.ShapeCache.Storage
9381
def get_total_disk_usage({parent, _init, storage}) do
9482
send(parent, {__MODULE__, :get_total_disk_usage})

0 commit comments

Comments
 (0)