Member/CycleGenerator: better delete_cycles errors; UUID-based advisory lock

delete_cycles returns first error for debugging. Advisory lock key
derived from member id (first 8 bytes of UUID hex) to reduce
phash2 collision risk; fallback to phash2 on invalid UUID.
This commit is contained in:
Moritz 2026-03-04 17:08:32 +01:00
parent ea350ab315
commit 1ce9915c7d
Signed by: moritz
GPG key ID: 1020A035E5DD0824
2 changed files with 27 additions and 8 deletions

View file

@ -37,6 +37,7 @@ defmodule Mv.Membership.Member do
data_layer: AshPostgres.DataLayer, data_layer: AshPostgres.DataLayer,
authorizers: [Ash.Policy.Authorizer] authorizers: [Ash.Policy.Authorizer]
import Bitwise
require Ash.Query require Ash.Query
import Ash.Expr import Ash.Expr
alias Ecto.Adapters.SQL, as: EctoSQL alias Ecto.Adapters.SQL, as: EctoSQL
@ -906,6 +907,25 @@ defmodule Mv.Membership.Member do
end) end)
end end
# Returns a deterministic 64-bit key for pg_advisory_xact_lock from a member id (UUID string).
# Reduces collision risk vs phash2 when multiple members are locked.
@doc false
def advisory_lock_key_for_member_id(member_id) when is_binary(member_id) do
hex = String.replace(member_id, "-", "")
if String.length(hex) >= 16 do
first_8_hex = String.slice(hex, 0, 16)
bin = Base.decode16!(first_8_hex, case: :lower)
decoded = :binary.decode_unsigned(bin, :big)
# Postgres bigint is signed 64-bit; keep in non-negative range
rem(decoded, 1 <<< 63)
else
:erlang.phash2(member_id)
end
rescue
ArgumentError -> :erlang.phash2(member_id)
end
# Regenerates cycles when membership fee type changes # Regenerates cycles when membership fee type changes
# Deletes future unpaid cycles and regenerates them with the new type/amount # Deletes future unpaid cycles and regenerates them with the new type/amount
# Uses advisory lock to prevent concurrent modifications # Uses advisory lock to prevent concurrent modifications
@ -915,7 +935,7 @@ defmodule Mv.Membership.Member do
# Uses system actor for cycle regeneration (mandatory side effect) # Uses system actor for cycle regeneration (mandatory side effect)
def regenerate_cycles_on_type_change(member, _opts \\ []) do def regenerate_cycles_on_type_change(member, _opts \\ []) do
today = Date.utc_today() today = Date.utc_today()
lock_key = :erlang.phash2(member.id) lock_key = advisory_lock_key_for_member_id(member.id)
# Use advisory lock to prevent concurrent deletion and regeneration # Use advisory lock to prevent concurrent deletion and regeneration
# This ensures atomicity when multiple updates happen simultaneously # This ensures atomicity when multiple updates happen simultaneously
@ -1025,18 +1045,17 @@ defmodule Mv.Membership.Member do
end end
end end
# Deletes cycles and returns :ok if all succeeded, {:error, reason} otherwise # Deletes cycles and returns :ok if all succeeded, {:error, reason} otherwise.
# Uses system actor for authorization to ensure deletion always works # Returns the first error for debugging; uses system actor for authorization.
defp delete_cycles(cycles_to_delete, actor_opts) do defp delete_cycles(cycles_to_delete, actor_opts) do
delete_results = delete_results =
Enum.map(cycles_to_delete, fn cycle -> Enum.map(cycles_to_delete, fn cycle ->
Ash.destroy(cycle, actor_opts) Ash.destroy(cycle, actor_opts)
end) end)
if Enum.any?(delete_results, &match?({:error, _}, &1)) do case Enum.find(delete_results, &match?({:error, _}, &1)) do
{:error, :deletion_failed} {:error, reason} -> {:error, reason}
else nil -> :ok
:ok
end end
end end

View file

@ -112,7 +112,7 @@ defmodule Mv.MembershipFees.CycleGenerator do
end end
defp do_generate_cycles_with_lock(member, today, false, opts) do defp do_generate_cycles_with_lock(member, today, false, opts) do
lock_key = :erlang.phash2(member.id) lock_key = Member.advisory_lock_key_for_member_id(member.id)
Repo.transaction(fn -> Repo.transaction(fn ->
EctoSQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) EctoSQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key])