Skip to content

Use tx-pooler for pg connection and add index#33

Open
roeierez wants to merge 2 commits into
mainfrom
tx-pooler
Open

Use tx-pooler for pg connection and add index#33
roeierez wants to merge 2 commits into
mainfrom
tx-pooler

Conversation

@roeierez
Copy link
Copy Markdown
Member

Improving performance by:

  1. Add missing index on records(user_id, revision)
  2. Disable prepare statement mode in postgress (so we can use stateless tx pooloer which scales a lot better)
  3. To be able to seamlessly rollback if needed added two DB env variables. the migration db still need to be session pooler on some platforms as it uses advisory locks.
  4. Allow configure max connections on the pool (right now we are using default which is a bit low).

@roeierez roeierez requested review from JssDWt and dangeross April 23, 2026 14:59
Comment thread syncer_server.go
var err error

if config.PgDatabaseUrl != "" {
if config.PgRuntimeUrl != "" {
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.

Perhaps we're the only users of data-sync, but do you think it maybe safer to fail fast if (the old) DATABASE_URL is set and DATABASE_RUNTIME_URL not set?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That will change the semantic of which data source to use. I honestly think it is ok and it will allow us rollback fast if needed.

Copy link
Copy Markdown
Contributor

@dangeross dangeross Apr 24, 2026

Choose a reason for hiding this comment

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

My concern is more for other users of it, on update if they don't change their env config they'll switch into sqlite storage

Comment thread syncer_server.go
// pooler (e.g. Supavisor :6543), since golang-migrate needs a session.
migrationURL := config.PgMigrationUrl
if migrationURL == "" {
migrationURL = config.PgRuntimeUrl
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.

Should we log a warning that the DATABASE_RUNTIME_URL will be used?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added log.

@@ -0,0 +1 @@
CREATE INDEX IF NOT EXISTS idx_records_user_revision ON records (user_id, revision);
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.

Claude suggests using CREATE INDEX CONCURRENTLY as it otherwise takes exclusive access, blocking other uses of records until complete. Depends how large the table is I guess

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Makes sense. Added

@roeierez roeierez requested a review from dangeross April 24, 2026 11:59
@@ -0,0 +1 @@
CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_records_user_revision ON records (user_id, revision);
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.

What is the reason to change this index? I'm asking because the query performance advisor doesn't flag this as a suggestion. It does suggest these for about 5% performance gain.

CREATE INDEX ON public.records USING btree (user_id);

and

CREATE INDEX ON public.user_revisions USING btree (user_id);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants