WPB-28377: migrate gundeck presence from redis to PostGreSQL - #5493
WPB-28377: migrate gundeck presence from redis to PostGreSQL#5493blackheaven wants to merge 4 commits into
Conversation
zebot
added
the
ok-to-test
88e6bed to
0d5db01
Compare
0d5db01 to
6fa7155
Compare
6fa7155 to
de7d9d6
Compare
|
|
||
|
|
||
| postgresql: | ||
| host: postgres | ||
| port: "5432" | ||
| user: wire-server | ||
| dbname: backendA | ||
| password: posty-the-gres |
There was a problem hiding this comment.
This is not right, the federation-v0 code still needs redis.
There was a problem hiding this comment.
Restored the redis config and services for the federation docker-compose parties (v0 and v1, and also v2 which had the same problem — all three run released wire-server images whose gundeck still requires the redis block and a redis server), plus the redis values block for the fed-v0 helm party. Byte-identical to what develop had.
|
|
||
|
|
||
| postgresql: | ||
| host: postgres | ||
| port: "5432" | ||
| user: wire-server | ||
| dbname: backendA | ||
| password: posty-the-gres |
There was a problem hiding this comment.
same as federation-v0, this still needs redis.
There was a problem hiding this comment.
Same fix: redis service + config restored (byte-identical to develop). v2 restored as well since it has the identical problem.
| postgresql: | ||
| host: postgresql | ||
| port: "5432" | ||
| user: wire-server | ||
| dbname: backendA | ||
| password: posty-the-gres |
There was a problem hiding this comment.
Same as docker setup, this would still require redis.
There was a problem hiding this comment.
Restored: redis-v2 service in federation-v2.yaml and the original redis block (including its TLS settings) in federation-v2/gundeck.yaml, verbatim from develop. Also restored deploy/dockerephemeral/docker/redis-ca.pem which the tlsCa path references.
| conn_id text NOT NULL, | ||
| resource text NOT NULL, | ||
| client_id text, | ||
| created_at bigint NOT NULL, |
There was a problem hiding this comment.
Switched to timestamptz: the migration now declares created_at timestamptz, and Gundeck.Presence.Data passes/read UTCTime with :: timestamptz statement annotations (wire format stays epoch-millis; conversion helpers msToUtc/utcToMs are exact — milliseconds nest inside timestamptz microseconds). Edited in place since it's unreleased.
| deleteStale = | ||
| [resultlessStatement| | ||
| DELETE FROM presence | ||
| WHERE created_at < ($1 :: int8) |
There was a problem hiding this comment.
We should create an index for this.
There was a problem hiding this comment.
Added: CREATE INDEX presence_created_at_idx ON presence (created_at) in the same migration (naming follows the conversation_codes_expires_at_idx precedent); it backs the weekly cleanup delete.
| statement | ||
| (toUUID (userId p), connIdText (connId p), fromIntegral (ms (createdAt p))) | ||
| deleteOne |
There was a problem hiding this comment.
Sending n statements is not good. Can we do this in one query for all the presences?
Makes me wonder if we should have a "presence id", so we can precisely delete the presences for which cannon says they're not actually present. WDYT?
There was a problem hiding this comment.
Done: deleteAll is now a single statement — DELETE FROM presence p USING unnest($1 :: uuid[], $2 :: text[], $3 :: timestamptz[]) AS d (...) WHERE ... AND p.created_at <= d.created_at — so it's one round trip while keeping the per-row compare-and-delete guard (a newer re-registration with the same conn id must not be deleted by a stale disconnect; empty input still short-circuits).
On the presence id: I'd rather not, at least not in this PR. Cannon already knows exactly which (user_id, conn_id) pairs dropped, and that pair is the row's primary key — so a surrogate id wouldn't add precision cannon doesn't already have. The reconnect race (same conn id re-registered between gundeck's presence list and cannon's negative-ack) is guarded by the created_at compare-and-delete, the same semantics the redis implementation had. A presence id would also change Wire.API.Presence (wire format + golden schema + cannon interop) for no behavioral gain. Happy to follow up separately if you still want it.
- Restore redis config/services for the federation docker-compose parties (v0/v1/v2) and the fed-v0 helm values: those parties run released wire-server images whose gundeck still requires a redis server. - presence.created_at: bigint epoch millis -> timestamptz; boundary conversion in Gundeck.Presence.Data via msToUtc/utcToMs (exact, no second truncation). Migration edited in place (unreleased). - Add presence_created_at_idx to back the weekly cleanup delete. - Replace the per-presence delete loop with a single DELETE ... USING unnest(...) statement, keeping the per-row created_at <= compare-and-delete guard (reconnect-race safety). - docker-compose.yaml: drop the now-empty top-level volumes key left by the redis-cluster removal (broke compose parsing). - Regenerate postgres-schema.sql (timestamptz column + new index).
c4cb60e to
586fac9
Compare
3 participants
https://wearezeta.atlassian.net/browse/WPB-28377
Checklist
changelog.d