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 Perseus database type #827

Merged
merged 3 commits into from
Jan 10, 2023
Merged

Add Perseus database type #827

merged 3 commits into from
Jan 10, 2023

Conversation

gabrieljackson
Copy link
Contributor

This change includes the basic functionality needed to deploy the Perseus database proxy service and to use it for new installations.

High Level Perseus Architecture:
Screenshot 2023-01-09 at 10 53 28 PM

Reviewer Notes:

  • Although this PR refactors some of the shared code with PGBouncer, there is a lot more we can do. Out of a desire to not break anything some code was duplicated for now with some minor tweaks which can be improved later.
  • Some PGBouncer provisioning happens as part of Perseus database setup. This is intended to allow for migrations to and from PGBouncer in the future. It's easier to do this stuff proactively instead of needing to do extensive updates later.
  • Sorry, I tried to keep this change smaller, but this is all needed for basic functionality.
Add Perseus database type

@gabrieljackson gabrieljackson added 2: Dev Review Requires review by a developer kind/feature Categorizes issue or PR as related to a new feature. labels Jan 10, 2023
@mm-cloud-bot mm-cloud-bot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jan 10, 2023
This change includes the basic functionality needed to deploy the
Perseus database proxy service and to use it for new installations.
Copy link
Contributor

@fmartingr fmartingr left a comment

Choose a reason for hiding this comment

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

Following logic was a bit tricky at first but this looks good to me, my only worries are with naming (commented out below) on parts that I did not fully understand or found confusing. Some tests for the critical methods would also be nice.

Although this PR refactors some of the shared code with PGBouncer, there is a lot more we can do. Out of a desire to not break anything some code was duplicated for now with some minor tweaks which can be improved later.

Can you put comments on the ones you already duplicated so we can easily track those?

internal/tools/aws/database_perseus.go Outdated Show resolved Hide resolved
internal/tools/aws/database_perseus.go Outdated Show resolved Hide resolved
internal/tools/aws/database_perseus.go Outdated Show resolved Hide resolved
internal/tools/aws/database_perseus.go Show resolved Hide resolved
cmd/cloud/server_flag.go Outdated Show resolved Hide resolved
internal/tools/aws/database_perseus.go Outdated Show resolved Hide resolved
internal/tools/aws/database_perseus.go Show resolved Hide resolved
@gabrieljackson gabrieljackson merged commit 1b7b897 into master Jan 10, 2023
@gabrieljackson gabrieljackson deleted the perseus branch January 10, 2023 17:24
@gabrieljackson gabrieljackson added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a developer labels Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request kind/feature Categorizes issue or PR as related to a new feature. release-note Denotes a PR that will be considered when it comes time to generate release notes.
Projects
None yet
4 participants