-
Notifications
You must be signed in to change notification settings - Fork 24
feat(core): support horizontally scaling platform postgres #2957
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
base: main
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @jakedoublev, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the platform's PostgreSQL capabilities by introducing horizontal scaling for read operations and robust high availability for the primary database. It enables the system to handle increased read loads more efficiently through read replicas with intelligent load balancing and automatic health checking. Furthermore, it provides automatic failover mechanisms for the primary database, ensuring greater resilience against outages. These changes are designed to be backward compatible and opt-in, allowing for a smooth transition and flexible deployment configurations. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Database expands, Reads flow free, no longer bound, High availability. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request significantly enhances the database layer by introducing PostgreSQL horizontal scaling and high availability features. Key changes include support for read replicas with round-robin load balancing and circuit breaker health checking using sony/gobreaker, multi-host primary failover via pgx's target_session_attrs=primary, and context-based primary routing (WithForcePrimary) to ensure read-after-write consistency. The docker-compose.yaml and opentdf-example.yaml files have been updated with new configurations and commented-out scenarios for local development and testing, alongside production-ready PostgreSQL replication settings. Comprehensive integration tests using Testcontainers have been added to validate these new capabilities, covering circuit breaker behavior, multi-host failover, and read replica functionality. The db.Client interface has been extended to manage replica connections and integrate circuit breakers, with Query and QueryRow methods now intelligently routing reads to healthy replicas or falling back to the primary. A critical review comment highlighted a flaw in executeQueryRow's circuit breaker integration, noting that it only checks the circuit state but doesn't wrap the execution, potentially leading to uncounted failures. Another comment pointed out an inconsistent and less secure pg_hba.conf setting in the docker-compose.yaml for the secondary primary, which should be aligned with the main primary's more restrictive configuration. Additionally, the Exec method's signature and behavior changed, no longer erroring on zero rows affected, which is a breaking API change requiring clear communication to consumers.
X-Test Failure Reportopentdf |
X-Test Failure Reportopentdf |
X-Test Failure Reportopentdf |
X-Test Failure Reportopentdf |
X-Test Failure Reportopentdf |
X-Test Failure Reportopentdf |
X-Test Failure Reportopentdf |
X-Test Failure Reportopentdf |
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
|
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
|
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
|
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
|
X-Test Failure Reportopentdf |
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
|
X-Test Failure Reportopentdf |
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
|
X-Test Failure Reportopentdf |
…ocker ocmpose and config
…in for docker ocmpose and config" This reverts commit cb7efa4.
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
|
X-Test Failure Reportopentdf |
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
|
X-Test Failure Reportopentdf |
…ed configs and docker composes
|
|
||
| # Start PostgreSQL with query logging enabled | ||
| exec postgres \ | ||
| -c log_statement=all \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not log all queries in any postgres servers for production, though useful for debugging.
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
|
Proposed Changes
Checklist
Testing Instructions