-
Notifications
You must be signed in to change notification settings - Fork 391
feat(aggregation-mode): support multiple dbs #2214
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: staging
Are you sure you want to change the base?
Conversation
430d4db to
5340365
Compare
| /// Clones share health state (the atomics) and the underlying pools, so all clones observe and influence | ||
| /// the same “preferred node” ordering decisions. | ||
| #[derive(Debug, Clone)] | ||
| pub struct DbOrchestartor { |
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 should be called DbOrchestrator
| // until the delay reaches `max_delay_seconds`, after which it stays at that max. | ||
| // see reference: https://en.wikipedia.org/wiki/Exponential_backoff | ||
| // and here: https://docs.aws.amazon.com/prescriptive-guidance/latest/cloud-design-patterns/retry-backoff.html | ||
| fn next_backoff_delay(&self, current_delay: Duration) -> Duration { |
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.
Maybe we can move this function to aggregation_mode/db/src/retry.rs, as it's related to the retry functionality.
| Ok(value) => return Ok(value), | ||
| Err(RetryError::Permanent(err)) => return Err(err), | ||
| Err(RetryError::Transient(err)) => { | ||
| if attempts >= self.retry_config.max_delay_seconds { |
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.
I don't understand this comparison between the number of attempts and the max delay in seconds
| return Err(err); | ||
| } | ||
|
|
||
| tracing::warn!(attempt = attempts, delay_milis = delay.as_millis(), error = ?err, "retrying after backoff"); |
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.
| tracing::warn!(attempt = attempts, delay_milis = delay.as_millis(), error = ?err, "retrying after backoff"); | |
| tracing::warn!(attempt = attempts, delay_millis = delay.as_millis(), error = ?err, "retrying after backoff"); |
| fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { | ||
| match self { | ||
| RetryError::Transient(e) => write!(f, "{e}"), | ||
| RetryError::Permanent(e) => write!(f, "{e}"), |
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 could be simplified, as both arms have the same behavior.
| #[derive(Clone, Debug)] | ||
| pub struct Db { | ||
| pool: Pool<Postgres>, | ||
| orchestartor: DbOrchestartor, |
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.
Same typo here
Description
Adds support for multiple databases and retry logic to the following components:
To enable this, a new module/struct called
DbOrchestratorwas introduced. It is responsible for handling retry logic and selecting the appropriate database. The DbOrchestrator exposes two methods:This design reflects the architecture where there is a single primary database for writes, while all other databases are read-only.
How to test
Type of change
Please delete options that are not relevant.
Checklist
testnet, everything else tostaging