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

Add support for RDS DB with read replica #15

Merged
merged 19 commits into from
Nov 27, 2024
Merged

Conversation

RealOrangeOne
Copy link
Contributor

@RealOrangeOne RealOrangeOne commented Oct 31, 2024

What is the context of this PR?

IAM authentication

In production, rather than a username and password, authentication to the database will be done using IAM. This means there are no credentials to leak or rotate.

When a connection is opened, a signed token is generated and used for authentication. This token is only valid for 15 minutes. The complexities of this are handled by an external library.

Connecting the application to IAM itself is done by the cluster, hence no additional configuration.

Multiple database connections

In production, multiple database connections will be used: one for writes and one for reads. The write connection points to a single database server, whilst reads are load balanced between a set of servers ("read replicas").

It's up to the application to decide whether the write or read connections is used, which is what the database router handles. Django internally identifies whether a query is a "read" or a "write", and uses the router to identify the connection. Most of the time, there's no need for a developer to worry about this.

Specifically:

  • Writes should always be handled by the write server (read replica connections are read-only)
  • Reads should usually be handled by the read replicas
  • If there is an open transaction, reads should be handled by the write connection. This ensures that whilst a transaction is open, the application gets a consistent view of state (since data held in the transaction can't be seen by the read connection).

Note: AWS is responsible for balancing traffic across the replicas - this is not the responsibility of the application.

For consistency, local development also uses multiple connections handled by the same router, however these point to the same server, and there's no restriction that the read connection only handles reads.

How to review

This shouldn't make a difference to local development.

Relevant Django documentation: https://docs.djangoproject.com/en/5.1/topics/db/multi-db/

Follow-up Actions

There are discussions happening about being able to handle a completely read-only connection. "read replica" functionality is separate.

@RealOrangeOne RealOrangeOne force-pushed the cms194-rds-support branch 3 times, most recently from 5d15a6a to b019cfa Compare November 1, 2024 13:59
@RealOrangeOne RealOrangeOne marked this pull request as ready for review November 1, 2024 15:06
@RealOrangeOne RealOrangeOne requested a review from a team as a code owner November 1, 2024 15:06
@RealOrangeOne
Copy link
Contributor Author

Putting this PR on hold whilst the pytest issues are resolved in a separate PR

AdamHawtin

This comment was marked as resolved.

@RealOrangeOne

This comment was marked as resolved.

@RealOrangeOne RealOrangeOne marked this pull request as ready for review November 20, 2024 12:14
@RealOrangeOne RealOrangeOne changed the title Add support for RDS DB with read replica CMS214: Add support for RDS DB with read replica Nov 22, 2024
@MebinAbraham MebinAbraham changed the title CMS214: Add support for RDS DB with read replica Add support for RDS DB with read replica Nov 22, 2024
Copy link
Contributor

@AdamHawtin AdamHawtin left a comment

Choose a reason for hiding this comment

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

Looks good to me, and I was also able to get the functional tests working rebased off this branch with a small update 👍.

@sanjeevz3009 sanjeevz3009 self-requested a review November 22, 2024 22:03
Copy link
Contributor

@MebinAbraham MebinAbraham left a comment

Choose a reason for hiding this comment

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

Code looks fine, unable to test for 'real' atm.

Copy link
Contributor

@sanjeevz3009 sanjeevz3009 left a comment

Choose a reason for hiding this comment

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

Reviewed =) Have no comments. Some of the code here overlapped with some things I was looking into to come up with the solution for the BDD UI test suite issues, particularly the code in test_db_router.py tests interestingly.

Copy link
Contributor

@sanjeevz3009 sanjeevz3009 left a comment

Choose a reason for hiding this comment

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

LGTM!

@RealOrangeOne RealOrangeOne merged commit b001906 into main Nov 27, 2024
7 checks passed
@RealOrangeOne RealOrangeOne deleted the cms194-rds-support branch November 27, 2024 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants