supabase / supavisor / 37803781336

81%

Build Type

push

github

Committed by web-flow

Commit Message

Clear the secrets for validation on a pool shutdown (#1208)

Invalidate client authentication globally on a pool shutdown

It only invalidates the `:secrets_for_validation` if there's no sibling
pool for the same tenant and DB user; i.e. the cache is NOT cleared if
there's another pool for the same tenant and DB user but different
mode/db/upstream_tls/search_path settings.

The PR also adds a missing logs.

Caveats:
* on each pool shutdown RCP multicall is made once or twice: to retrieve
pools' PIDs and then optionally to invalidate the cache globally

EDIT 1:

New approach has been proposed: instead of checking and invalidating
globally via `:erpc`, we take advantage of the `:syn`'s `:tenants`
process group to which we join every pool keyed by `{tenant, user}`.
When a pool shuts down, we check the group for members — if there're
some we keep the cache, otherwise, we invalidate. The invalidation is
run locally.

The process still involves inter-cluster comms on the shutdown as we
check the remote pools liveness via `erpc` but the impact is way lower
as we already have a list of pools and we don't have to check each node.

The downside of invalidating locally is that it might fail (e.g. due to
the erpc-base liveness check) at one node while succeed on the other.
But we'd be facing the same risk with the previous approach.

EDIT 2:

We slightly changed the approach by using a separate `tenant_pools`
scope as the `Supavisor.Terminator` unregisters a pool being drained and
shutdown early from the `:tenants` scpoe to allow for the pool to be
created on another node.



EDIT 3 (race-freedom of the last-member check):

We dropped the `:erpc` liveness check. It isn't needed, and it broke the
node-down case.

`:syn` calls `on_process_left/5` serially in each node's scope process,
after removing the leaving pid from the group, and every node gets a
callback for every leaving pool. `invalidate_local/2` only clears that
node's cache, so each node decides on its o... (continued)

Coverage Stats

29 of 30 new or added lines in 3 files covered. (96.67%)

4 existing lines in 4 files now uncovered.

3191 of 3924 relevant lines covered (81.32%)

48564.55 hits per line