refactor(router): separate DB pool configs for master, accounts, and global tenant schemas#13335
Open
sai-harsha-vardhan wants to merge 8 commits into
Open
refactor(router): separate DB pool configs for master, accounts, and global tenant schemas#13335sai-harsha-vardhan wants to merge 8 commits into
sai-harsha-vardhan wants to merge 8 commits into
Conversation
…and global tenant schemas Previously master_database was reused to build the pools for the accounts schema and the global tenant schema too, coupling their pool sizing together even though they carry very different traffic patterns. Adds dedicated accounts_database and global_database config blocks (write side) while replica_database remains shared across all three read paths.
|
Review changes with Changed Files
|
…l site instead of inside get_store get_store previously re-derived "is this the global tenant" via a tenant_id comparison against config.multitenancy.global_tenant, even though every caller already knows statically which case it's in (the global store is built from exactly one call site, separate from the per-tenant maps). get_store and get_store_interface now just take the already-resolved master/accounts Database configs as parameters.
Contributor
|
[blocking] Please make the new |
get_store_interface now takes master_config/accounts_config as explicit params, pushing it to 8 arguments and over clippy's default threshold. Follows the same allow already used 280+ times elsewhere in this crate.
sai-harsha-vardhan
added
the
S-test-ready
github-actions
Bot
removed
the
S-test-ready
…ical schema/config pools AppState::with_storage built a full Store (4 bb8 pools each) twice per tenant -- once via get_store_interface_map for the stores map, once via get_accounts_store_interface_map for the accounts_store map -- even though both calls are for the same tenant and each only ever reaches half of the pools it built. Merged into a single get_store_interface_maps that builds the store once per tenant and derives both StorageInterface and AccountsStorageInterface from that one instance. Also, ReplicaStore::new/Store::new always opened a separate pool for the accounts schema even when it targets the exact same schema and connection config as the main pool (always true for the global tenant, and also true for any regular tenant configured with matching schema/accounts_schema and matching database config). They now reuse the existing bb8::Pool (Arc-backed, no new connections) in that case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
master_database's pool config, coupling their connection-pool sizing to regular per-tenant payment traffic even though they have very different access patterns.accounts_databaseandglobal_databaseconfig blocks for the write/master side of those two pools, sized independently going forward.replica_databasestays shared across all three read paths (tenant schema, accounts schema, global schema), since reads don't need the same per-role isolation.get_store()now picksglobal_databasewhen building the global tenant's store, andmaster_database/accounts_databaseotherwise.config.example.toml,development.toml,docker_compose.toml,config/deployments/env_specific.toml,loadtest/config/development.toml) with the new sections, defaulted to mirrormaster_database's values so existing deployments aren't broken until operators choose to tune them.Test plan
cargo check -p router -p storage_impl— cleancargo clippy -p router -p storage_impl --no-deps— clean, no new warnings🤖 Generated with Claude Code