-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Override global statement timeout when creating indexes in Postgres #16085
Conversation
# override the global statement timeout to avoid accidentally squashing | ||
# a long-running index creation process | ||
timeout_sql = "SET LOCAL statement_timeout = 0" | ||
c.execute(timeout_sql) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to set this back to the default, as we're not in a transaction when creating indices I think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the autocommit being on the reason that this isn't in a transaction? The statement timeout only applies for a given transaction - if we are not in a transaction then I wonder if it will have any effect. I will do some digging in the docs but they were pretty scarce on information when I looked into this the first time. Do we have any Postgres/psychopg wizards around who might have some arcane insight into this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the autocommit being on the reason that this isn't in a transaction?
Yes
The statement timeout only applies for a given transaction - if we are not in a transaction then I wonder if it will have any effect.
https://www.postgresql.org/docs/current/sql-set.html says that LOCAL
doesn't work outside a transaction, and so we'd need to use SESSION
3502fea
to
c18bc5c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise looks good
# mypy ignore - `statement_timeout` is defined on PostgresEngine | ||
default_timeout = self.db_pool.engine.statement_timeout # type: ignore[attr-defined] | ||
undo_timeout_sql = f"SET statement_timeout = {default_timeout}" | ||
c.execute(undo_timeout_sql) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be in finally
section, so if happens even if index creation raises an exception
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
#15853 added a global timeout for long-running Postgres statements, but this may interfere with long-running index creation. This PR adds a local statement timeout to override the global timeout and disable it when creating an index in the background. Fixes #15942.