Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: tune timeouts and add logging to table trigger setup process #252

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion services/data/postgres_async_db.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import asyncio
import psycopg2
import psycopg2.extras
import os
Expand Down Expand Up @@ -79,6 +80,7 @@ async def _init(self, db_conf: DBConfiguration, create_triggers=DB_TRIGGER_CREAT
minsize=db_conf.pool_min,
maxsize=db_conf.pool_max,
timeout=db_conf.timeout,
options=db_conf.options,
echo=AIOPG_ECHO)

for table in self.tables:
Expand Down Expand Up @@ -366,6 +368,8 @@ async def create_if_missing(db: _AsyncPostgresDB, table_name, command):
@staticmethod
async def create_trigger_if_missing(db: _AsyncPostgresDB, table_name, trigger_name, commands=[]):
"executes the commands only if a trigger with the given name does not already exist on the table"
# NOTE: keep a relatively low timeout for the setup queries,
Copy link
Contributor

Choose a reason for hiding this comment

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

How would we set the timeout here differently than the other statements. I agree that these should have a very quick timeout and others can potentially be longer.

# as these should not take long to begin with, and we want to release as soon as possible in case of errors
with (await db.pool.cursor()) as cur:
try:
await cur.execute(
Expand All @@ -378,9 +382,14 @@ async def create_trigger_if_missing(db: _AsyncPostgresDB, table_name, trigger_na
(table_name, trigger_name),
)
trigger_exist = bool(cur.rowcount)
if not trigger_exist:
if trigger_exist:
db.logger.info("Trigger {} already exists, skipping.".format(trigger_name))
else:
db.logger.info("Creating trigger for table: {}".format(trigger_name))
for command in commands:
await cur.execute(command)
except (asyncio.CancelledError, asyncio.TimeoutError):
db.logger.warning("Trigger creation timed out, no triggers were set up for table: {}".format(table_name))
finally:
cur.close()

Expand Down
2 changes: 1 addition & 1 deletion services/ui_backend_service/ui_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ def main():
for sig in (signal.SIGTERM, signal.SIGHUP, signal.SIGINT):
loop.add_signal_handler(sig, lambda sig=sig: async_loop_signal_handler(sig))

the_app = app(loop, DBConfiguration())
the_app = app(loop, DBConfiguration(statement_timeout=60))
handler = web.AppRunner(the_app)
loop.run_until_complete(handler.setup())
f = loop.create_server(handler.server, DEFAULT_SERVICE_HOST, DEFAULT_SERVICE_PORT)
Expand Down
5 changes: 4 additions & 1 deletion services/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ class DBConfiguration(object):
pool_max: int = None # aiopg default: 10

timeout: int = None # aiopg default: 60 (seconds)
options: str = None # psycopg2 configuration options

_dsn: str = None

Expand All @@ -182,7 +183,9 @@ def __init__(self,
prefix="MF_METADATA_DB_",
pool_min: int = 1,
pool_max: int = 10,
timeout: int = 60):
timeout: int = 60,
statement_timeout: int = None):
self.options = "-c statement_timeout={}".format(statement_timeout * 1000) if statement_timeout else None

self._dsn = os.environ.get(prefix + "DSN", dsn)
# Check if it is a BAD DSN String.
Expand Down