Skip to content

WPB-22953: migrate connections to PostGreSQL - #5626

Merged
blackheaven merged 6 commits into
developfrom
gdifolco/WPB-22953-migration-postgres-connections
Oct 30, 2026
Merged

blackheaven merged 6 commits into
developfrom
gdifolco/WPB-22953-migration-postgres-connections

Conversation

@blackheaven

@blackheaven blackheaven commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

https://wearezeta.atlassian.net/browse/WPB-22953

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@blackheaven
blackheaven requested review from a team as code owners October 21, 2026 11:15
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Oct 21, 2026
@blackheaven
blackheaven force-pushed the gdifolco/WPB-22953-migration-postgres-connections branch from 13fc4ad to 6a9d599 Compare October 21, 2026 22:41
@VeryMilkyJoe VeryMilkyJoe self-assigned this Oct 22, 2026
[resultlessStatement|INSERT INTO connection ("left", "right", status, last_update, conv)
VALUES ($1 :: uuid, $2 :: uuid, $3 :: int4, $4 :: timestamptz, $5 :: uuid?)
ON CONFLICT ("left", "right") DO UPDATE
SET status = ($3 :: int4), last_update = ($4 :: timestamptz), conv = ($5 :: uuid?)

@VeryMilkyJoe VeryMilkyJoe Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to upsert like this, what if there is a newer connection in postgres than in cassandra, wouldnt we want the newer data (i.e. the newer timestamp)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added a timestamp, not sure it fixes your concern, WDYT?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think it does. I think it is only an edge case anyways but it never hurts to be careful.

@VeryMilkyJoe VeryMilkyJoe removed their assignment Oct 23, 2026
@blackheaven
blackheaven force-pushed the gdifolco/WPB-22953-migration-postgres-connections branch from f001ebc to b5a1f1d Compare October 23, 2026 16:19
@blackheaven
blackheaven force-pushed the gdifolco/WPB-22953-migration-postgres-connections branch from b5a1f1d to 3bba55b Compare October 29, 2026 16:15

@VeryMilkyJoe VeryMilkyJoe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me :)

@blackheaven
blackheaven force-pushed the gdifolco/WPB-22953-migration-postgres-connections branch from 3bba55b to ee55dcb Compare October 30, 2026 14:30
@blackheaven
blackheaven force-pushed the gdifolco/WPB-22953-migration-postgres-connections branch from ee55dcb to 89b1b15 Compare October 30, 2026 15:12
…s store in brig-integration

testAllConnectionsPaging wrote remote rows straight to Cassandra while brig
under PostgresqlStorage reads them from Postgres, so the test saw 5 instead
of 8. Branch the seeding on postgresMigration.connections.
@blackheaven
blackheaven force-pushed the gdifolco/WPB-22953-migration-postgres-connections branch from af97020 to 92f7950 Compare October 30, 2026 18:06
LookupLocalConnectionsPage's marker lambda bound the status Int32 (second
row field) instead of the right-user UUID, so every continuation request
failed to decode the paging state, treated it as absent, and re-returned
the full result set. Encode/decode now use the explicit ASCII UUID codec,
mirroring the remote marker.
@blackheaven
blackheaven merged commit f265f55 into develop Oct 30, 2026
10 checks passed
@blackheaven
blackheaven deleted the gdifolco/WPB-22953-migration-postgres-connections branch October 30, 2026 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants