From b14e19a938802f822748d0fd1baccf5ad34e9cf5 Mon Sep 17 00:00:00 2001 From: chris48s Date: Tue, 28 Jan 2025 15:13:42 +0000 Subject: [PATCH 1/5] make DB router code easier to understand --- polling_stations/db_routers.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/polling_stations/db_routers.py b/polling_stations/db_routers.py index d48be011a3..c0735d22d3 100644 --- a/polling_stations/db_routers.py +++ b/polling_stations/db_routers.py @@ -3,20 +3,27 @@ from django.conf import settings from django.db import DEFAULT_DB_ALIAS +PRIMARY = settings.PRINCIPAL_DB_NAME +REPLICA = DEFAULT_DB_ALIAS + class ReplicationRouter(object): def db_for_read(self, model, **hints): + if os.environ.get("CIRCLECI"): + return DEFAULT_DB_ALIAS + if model._meta.label in ( "file_uploads.Upload", "file_uploads.File", - ) and not os.environ.get("CIRCLECI"): - return settings.PRINCIPAL_DB_NAME - return DEFAULT_DB_ALIAS + ): + return PRIMARY + return REPLICA def db_for_write(self, model, **hints): if os.environ.get("CIRCLECI"): return DEFAULT_DB_ALIAS - return settings.PRINCIPAL_DB_NAME + + return PRIMARY def allow_relation(self, obj1, obj2, **hints): return True @@ -26,6 +33,6 @@ def allow_migrate(self, db, app_label, model_name=None, **hints): def get_principal_db_name(): - if settings.PRINCIPAL_DB_NAME in settings.DATABASES: - return settings.PRINCIPAL_DB_NAME - return DEFAULT_DB_ALIAS + if PRIMARY in settings.DATABASES: + return PRIMARY + return REPLICA From f8c429b1b2b230b41d59e72bd255a8bd7daa6150 Mon Sep 17 00:00:00 2001 From: chris48s Date: Tue, 28 Jan 2025 14:35:10 +0000 Subject: [PATCH 2/5] only serve from replice when there is a request in scope --- polling_stations/db_routers.py | 8 +++++++- polling_stations/settings/base.py | 2 ++ requirements/base.in | 1 + requirements/base.txt | 7 +++++++ requirements/constraints.txt | 5 +++++ 5 files changed, 22 insertions(+), 1 deletion(-) diff --git a/polling_stations/db_routers.py b/polling_stations/db_routers.py index c0735d22d3..3adad6dd30 100644 --- a/polling_stations/db_routers.py +++ b/polling_stations/db_routers.py @@ -2,6 +2,8 @@ from django.conf import settings from django.db import DEFAULT_DB_ALIAS +from django_middleware_global_request import get_request + PRIMARY = settings.PRINCIPAL_DB_NAME REPLICA = DEFAULT_DB_ALIAS @@ -17,7 +19,11 @@ def db_for_read(self, model, **hints): "file_uploads.File", ): return PRIMARY - return REPLICA + + if get_request(): + return REPLICA + + return PRIMARY def db_for_write(self, model, **hints): if os.environ.get("CIRCLECI"): diff --git a/polling_stations/settings/base.py b/polling_stations/settings/base.py index 537ade11c1..621cbe6311 100644 --- a/polling_stations/settings/base.py +++ b/polling_stations/settings/base.py @@ -124,6 +124,7 @@ def get_ec2_ip(): MIDDLEWARE = ( "whitenoise.middleware.WhiteNoiseMiddleware", "corsheaders.middleware.CorsMiddleware", + "django_middleware_global_request.middleware.GlobalRequestMiddleware", "django.contrib.sessions.middleware.SessionMiddleware", "django.middleware.common.CommonMiddleware", "django.middleware.locale.LocaleMiddleware", @@ -197,6 +198,7 @@ def get_ec2_ip(): "dc_design_system", "dc_utils", "drf_spectacular", + "django_middleware_global_request", ) PROJECT_APPS = ( diff --git a/requirements/base.in b/requirements/base.in index 5d9b21cec7..a57a33ebd1 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -11,6 +11,7 @@ django-dotenv django-extensions django-filter django-localflavor +django-middleware-global-request django-sesame djangorestframework djangorestframework-csv diff --git a/requirements/base.txt b/requirements/base.txt index 33d982b0c3..a013615342 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -198,6 +198,7 @@ django==4.2.13 \ # django-extensions # django-filter # django-localflavor + # django-middleware-global-request # django-sesame # djangorestframework # drf-spectacular @@ -233,6 +234,12 @@ django-localflavor==4.0 \ # -c requirements/constraints.txt # -r requirements/base.in # dc-django-utils +django-middleware-global-request==0.3.5 \ + --hash=sha256:37c79c9cd80e73751ae2821696e20af13ba7628a0915ff1818d124e0e72980b1 \ + --hash=sha256:7beb2c8ca9106aac641692cda2bac290bd7083a315446a12db06fa91b73903d6 + # via + # -c requirements/constraints.txt + # -r requirements/base.in django-pipeline==3.0.0 \ --hash=sha256:49a8bee298668100bb6e8a2144dff8c607baa5297820a2503793c38693f34103 \ --hash=sha256:e9e08b084ef3ebf599795510519a8d44f2240b487782bebf4a8fcaf6302c31d1 diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 6813c0b579..152c5ac9a0 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -381,6 +381,7 @@ django==4.2.13 \ # django-extensions # django-filter # django-localflavor + # django-middleware-global-request # django-sesame # djangorestframework # drf-spectacular @@ -415,6 +416,10 @@ django-localflavor==4.0 \ # via # -r requirements/base.in # dc-django-utils +django-middleware-global-request==0.3.5 \ + --hash=sha256:37c79c9cd80e73751ae2821696e20af13ba7628a0915ff1818d124e0e72980b1 \ + --hash=sha256:7beb2c8ca9106aac641692cda2bac290bd7083a315446a12db06fa91b73903d6 + # via -r requirements/base.in django-pipeline==3.0.0 \ --hash=sha256:49a8bee298668100bb6e8a2144dff8c607baa5297820a2503793c38693f34103 \ --hash=sha256:e9e08b084ef3ebf599795510519a8d44f2240b487782bebf4a8fcaf6302c31d1 From 66317994bbfab101a588a862ff183d12378dcc6c Mon Sep 17 00:00:00 2001 From: chris48s Date: Tue, 28 Jan 2025 14:35:36 +0000 Subject: [PATCH 3/5] explain with comments --- polling_stations/db_routers.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/polling_stations/db_routers.py b/polling_stations/db_routers.py index 3adad6dd30..2fe9bcc5fe 100644 --- a/polling_stations/db_routers.py +++ b/polling_stations/db_routers.py @@ -11,18 +11,27 @@ class ReplicationRouter(object): def db_for_read(self, model, **hints): + # In CI, there is only one DB connection if os.environ.get("CIRCLECI"): return DEFAULT_DB_ALIAS + # We don't need to scale requests touching these models + # but we do want to prevent race conditions + # when reading a record we just wrote if model._meta.label in ( "file_uploads.Upload", "file_uploads.File", ): return PRIMARY + # We only care about trying to scale + # by serving traffic from a replica + # when we are serving HTTP traffic if get_request(): return REPLICA + # in all other cases (e.g: management commands, shell) + # perform reads from the primary to prevent race conditions return PRIMARY def db_for_write(self, model, **hints): From 873cc0195ba22f3e1f8f05fe8647d2b741b7a45f Mon Sep 17 00:00:00 2001 From: chris48s Date: Tue, 28 Jan 2025 15:11:21 +0000 Subject: [PATCH 4/5] always read from primary in the admin --- polling_stations/db_routers.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/polling_stations/db_routers.py b/polling_stations/db_routers.py index 2fe9bcc5fe..1a9eaba5dc 100644 --- a/polling_stations/db_routers.py +++ b/polling_stations/db_routers.py @@ -27,7 +27,10 @@ def db_for_read(self, model, **hints): # We only care about trying to scale # by serving traffic from a replica # when we are serving HTTP traffic - if get_request(): + request = get_request() + if request: + if request.path.startswith("/admin"): + return PRIMARY return REPLICA # in all other cases (e.g: management commands, shell) From e42255853a98ac44a7ad1861c8b038c799422d95 Mon Sep 17 00:00:00 2001 From: chris48s Date: Wed, 12 Feb 2025 14:08:24 +0000 Subject: [PATCH 5/5] use consistent terminology --- polling_stations/db_routers.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/polling_stations/db_routers.py b/polling_stations/db_routers.py index 1a9eaba5dc..33174652df 100644 --- a/polling_stations/db_routers.py +++ b/polling_stations/db_routers.py @@ -5,7 +5,7 @@ from django_middleware_global_request import get_request -PRIMARY = settings.PRINCIPAL_DB_NAME +PRINCIPAL = settings.PRINCIPAL_DB_NAME REPLICA = DEFAULT_DB_ALIAS @@ -22,7 +22,7 @@ def db_for_read(self, model, **hints): "file_uploads.Upload", "file_uploads.File", ): - return PRIMARY + return PRINCIPAL # We only care about trying to scale # by serving traffic from a replica @@ -30,18 +30,18 @@ def db_for_read(self, model, **hints): request = get_request() if request: if request.path.startswith("/admin"): - return PRIMARY + return PRINCIPAL return REPLICA # in all other cases (e.g: management commands, shell) - # perform reads from the primary to prevent race conditions - return PRIMARY + # perform reads from the principal to prevent race conditions + return PRINCIPAL def db_for_write(self, model, **hints): if os.environ.get("CIRCLECI"): return DEFAULT_DB_ALIAS - return PRIMARY + return PRINCIPAL def allow_relation(self, obj1, obj2, **hints): return True @@ -51,6 +51,6 @@ def allow_migrate(self, db, app_label, model_name=None, **hints): def get_principal_db_name(): - if PRIMARY in settings.DATABASES: - return PRIMARY + if PRINCIPAL in settings.DATABASES: + return PRINCIPAL return REPLICA