Skip to content

Commit

Permalink
Add request.db_replica property connecting to a DB replica
Browse files Browse the repository at this point in the history
This commit only adds the setup necessary and a /_status check for the
new property.

Follow up commits will start using the new property in endpoints that
potentially make long queries.
  • Loading branch information
marcospri committed Apr 5, 2024
1 parent d4d6cc3 commit 8007c25
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 0 deletions.
3 changes: 3 additions & 0 deletions h/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ def configure(environ=None, settings=None): # pylint: disable=too-many-statemen
settings_manager.set("mail.host", "MAIL_HOST")
settings_manager.set("mail.port", "MAIL_PORT", type_=int)
settings_manager.set("sqlalchemy.url", "DATABASE_URL", required=True)
settings_manager.set(
"sqlalchemy.replica.url", "REPLICA_DATABASE_URL", required=False
)

# Configuration for Pyramid
settings_manager.set("secret_key", "SECRET_KEY", type_=_to_utf8, required=True)
Expand Down
20 changes: 20 additions & 0 deletions h/db/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,25 @@ def close_the_sqlalchemy_session(_request):
return session


def _replica_session(request): # pragma: no cover
engine = create_engine(request.registry.settings["sqlalchemy.replica.url"])
session = Session(bind=engine)

# While this is superflux when using a real replica it guarantees that usage of request.db_replica
# in the codebase never expects to be able to write to the DB, useful on the dev and tests environments.
session.execute(text("SET SESSION CHARACTERISTICS AS TRANSACTION READ ONLY;"))

@request.add_finished_callback
def close_the_sqlalchemy_session(_request):
# Close any unclosed DB connections.
# It's okay to call `session.close()` even if the session does not need to
# be closed, so just call it so that there's no chance
# of leaking any unclosed DB connections.
session.close()

return session


def _maybe_create_default_organization(engine, authority): # pragma: no cover
from h.services.organization import OrganizationService

Expand Down Expand Up @@ -144,3 +163,4 @@ def includeme(config): # pragma: no cover
# that view functions need only refer to `request.db` in order to retrieve
# the current database session.
config.add_request_method(_session, name="db", reify=True)
config.add_request_method(_replica_session, name="db_replica", reify=True)
7 changes: 7 additions & 0 deletions h/views/status.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ def status(request):
log.exception(err)
raise HTTPInternalServerError("Database connection failed") from err

if "replica" in request.params:
try:
request.db_replica.execute(text("SELECT 1"))
except Exception as err:
log.exception(err)
raise HTTPInternalServerError("Replica database connection failed") from err

if "sentry" in request.params:
capture_message("Test message from h's status view")

Expand Down
15 changes: 15 additions & 0 deletions tests/unit/h/views/status_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,27 @@ def test_it_sends_test_messages_to_sentry(self, pyramid_request, capture_message

capture_message.assert_called_once_with("Test message from h's status view")

def test_it_access_the_replica(self, pyramid_request, db_replica):
pyramid_request.params["replica"] = ""
db_replica.execute.side_effect = Exception("explode!")

with pytest.raises(HTTPInternalServerError) as exc:
status(pyramid_request)

assert "Replica database connection failed" in str(exc.value)

@pytest.fixture
def db(self, pyramid_request):
db = mock.Mock()
pyramid_request.db = db
return db

@pytest.fixture
def db_replica(self, pyramid_request):
db = mock.Mock()
pyramid_request.db_replica = db
return db


@pytest.fixture(autouse=True)
def capture_message(patch):
Expand Down
1 change: 1 addition & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ setenv =
dev: WEB_CONCURRENCY = {env:WEB_CONCURRENCY:2}
tests: COVERAGE_FILE = {env:COVERAGE_FILE:.coverage.{envname}}
dev: DATABASE_URL = {env:DATABASE_URL:postgresql://postgres@localhost/postgres}
dev: REPLICA_DATABASE_URL = {env:DATABASE_URL:postgresql://postgres@localhost/postgres}
tests: DATABASE_URL = {env:DATABASE_URL:postgresql://postgres@localhost/h_tests}
functests: DATABASE_URL = {env:DATABASE_URL:postgresql://postgres@localhost/h_functests}
OBJC_DISABLE_INITIALIZE_FORK_SAFETY = YES
Expand Down

0 comments on commit 8007c25

Please sign in to comment.