-
Notifications
You must be signed in to change notification settings - Fork 407
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
Adaptive Postgres HA Support #2920
Conversation
9fbc961
to
42c74af
Compare
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.
Looks good to me. I only have a few nit comments.
def get_backend(parsed_dsn: urllib.parse.ParseResult) -> Optional[HABackend]: | ||
backend, _, sub_scheme = parsed_dsn.scheme.partition("+") | ||
if backend == "stolon": | ||
from . import stolon |
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.
Any reason for a dynamic import?
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.
This was to break a circular import ha.base
<-> ha.base.stolon
self.server = server | ||
|
||
def mark_as_system_db(self): | ||
assert defines.EDGEDB_SYSTEM_DB in self.dbname |
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.
Why does this check use in
and not ==
?
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.
It's related to multi-tenancy: #2158
* Also fixed a bug that pg message is not finished * Add comments, refine logging
Depends on #2580, implementation of #2887
This PR supports a "adaptive" Postgres HA cluster which doesn't actively notify EdgeDB of failovers.