-
Notifications
You must be signed in to change notification settings - Fork 26
Harden the platform against deploy races and request storms #901
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
Changes from all commits
4a1a697
2062f4b
52038c5
b9f0393
db8dda5
06b08b8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| import zlib | ||
|
|
||
| from django.core.management import BaseCommand, call_command | ||
| from django.db import connections | ||
|
|
||
|
|
||
| MIGRATION_LOCK_NAMESPACE = 0x54414C59 # "TALY", signed int32-safe. | ||
| # ponytail: hardcoded; make it a flag/env var if a deploy ever needs to tune it. | ||
| LOCK_TIMEOUT_SECONDS = 900 | ||
|
|
||
|
|
||
| def _signed_int32(value): | ||
| return value - 2**32 if value >= 2**31 else value | ||
|
|
||
|
|
||
| MIGRATION_LOCK_KEY = _signed_int32(zlib.crc32(b'tally:migrations:default')) | ||
|
Comment on lines
+12
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win Duplicate The identical ♻️ Suggested consolidation# backend/utils/locks.py
def signed_int32(value: int) -> int:
return value - 2**32 if value >= 2**31 else valueThen import 🧰 Tools🪛 Ruff (0.15.20)[warning] 12-12: Missing return type annotation for private function (ANN202) 🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| class Command(BaseCommand): | ||
| help = 'Run Django migrations under a PostgreSQL advisory lock.' | ||
|
|
||
| def add_arguments(self, parser): | ||
| parser.add_argument( | ||
| '--noinput', | ||
| '--no-input', | ||
| action='store_false', | ||
| dest='interactive', | ||
| default=True, | ||
| help='Do not prompt the user for input.', | ||
| ) | ||
|
|
||
| def handle(self, *args, **options): | ||
| connection = connections['default'] | ||
| migrate_options = { | ||
| 'interactive': options['interactive'], | ||
| 'verbosity': options['verbosity'], | ||
| 'stdout': self.stdout, | ||
| 'stderr': self.stderr, | ||
| } | ||
|
|
||
| if connection.vendor != 'postgresql': | ||
| self.stdout.write( | ||
| f'Database backend is {connection.vendor}; running migrations without advisory lock.' | ||
| ) | ||
| call_command('migrate', **migrate_options) | ||
| return | ||
|
|
||
| # Postgres does the waiting: lock_timeout bounds the blocking | ||
| # pg_advisory_lock call, and the session-level lock releases itself | ||
| # when this command's connection closes. | ||
| connection.ensure_connection() | ||
| with connection.cursor() as cursor: | ||
| cursor.execute(f"SET lock_timeout = '{int(LOCK_TIMEOUT_SECONDS)}s'") | ||
| self.stdout.write('Waiting for PostgreSQL migration advisory lock...') | ||
| cursor.execute( | ||
| 'SELECT pg_advisory_lock(%s, %s)', | ||
| [MIGRATION_LOCK_NAMESPACE, MIGRATION_LOCK_KEY], | ||
| ) | ||
| cursor.execute('SET lock_timeout = 0') | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| self.stdout.write('Acquired PostgreSQL migration advisory lock.') | ||
| call_command('migrate', **migrate_options) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| from io import StringIO | ||
| from unittest.mock import patch | ||
|
|
||
| from django.test import SimpleTestCase | ||
|
|
||
| from utils.management.commands import migrate_with_lock | ||
| from utils.management.commands.migrate_with_lock import Command | ||
|
|
||
|
|
||
| class FakeCursor: | ||
| def __init__(self, connection): | ||
| self.connection = connection | ||
|
|
||
| def __enter__(self): | ||
| return self | ||
|
|
||
| def __exit__(self, exc_type, exc, traceback): | ||
| return False | ||
|
|
||
| def execute(self, sql, params=None): | ||
| self.connection.executed.append((sql, params)) | ||
|
|
||
|
|
||
| class FakeConnection: | ||
| def __init__(self, vendor='postgresql'): | ||
| self.vendor = vendor | ||
| self.executed = [] | ||
| self.ensure_connection_calls = 0 | ||
|
|
||
| def ensure_connection(self): | ||
| self.ensure_connection_calls += 1 | ||
|
|
||
| def cursor(self): | ||
| return FakeCursor(self) | ||
|
|
||
|
|
||
| class MigrateWithLockCommandTests(SimpleTestCase): | ||
| def run_command(self, connection): | ||
| stdout = StringIO() | ||
| command = Command(stdout=stdout, stderr=StringIO()) | ||
| with ( | ||
| patch.object(migrate_with_lock, 'connections', {'default': connection}), | ||
| patch.object(migrate_with_lock, 'call_command') as call_command, | ||
| ): | ||
| command.handle(interactive=False, verbosity=1) | ||
| return call_command, stdout.getvalue() | ||
|
|
||
| def test_non_postgres_runs_migrate_without_lock(self): | ||
| connection = FakeConnection(vendor='sqlite') | ||
|
|
||
| call_command, stdout = self.run_command(connection) | ||
|
|
||
| self.assertIn('without advisory lock', stdout) | ||
| self.assertEqual(connection.executed, []) | ||
| call_command.assert_called_once() | ||
|
|
||
| def test_postgres_takes_advisory_lock_with_bounded_wait(self): | ||
| connection = FakeConnection() | ||
|
|
||
| call_command, stdout = self.run_command(connection) | ||
|
|
||
| self.assertEqual(connection.ensure_connection_calls, 1) | ||
| self.assertIn('Acquired PostgreSQL migration advisory lock', stdout) | ||
| executed_sql = [sql for sql, _params in connection.executed] | ||
| self.assertEqual(executed_sql, [ | ||
| "SET lock_timeout = '900s'", | ||
| 'SELECT pg_advisory_lock(%s, %s)', | ||
| 'SET lock_timeout = 0', | ||
| ]) | ||
| self.assertEqual( | ||
| connection.executed[1][1], | ||
| [migrate_with_lock.MIGRATION_LOCK_NAMESPACE, migrate_with_lock.MIGRATION_LOCK_KEY], | ||
| ) | ||
| call_command.assert_called_once() |
Uh oh!
There was an error while loading. Please reload this page.