PLATFORM-9062 | Allow Cargo to use replica databases #19
Conversation
This file contains 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
mszabo-wikia
force-pushed
the
PLATFORM-9062-bp
branch
2 times, most recently
from
5f918be
to
0384d8c
Compare
The Cargo extension currently uses a single database connection to access and manage its database tables, managed entirely by CargoUtils without going through the usual MW DBAL layers. This ipso facto precludes the use of a proper primary - replica setup, as this connection must allow database writes. We want to do just that, so that we can offload read queries to replicas where possible and avoid putting undue load on the primary DB in an active-active world. So, first make it possible for Cargo to utilize replicas at all by factoring out its DB connection management logic into a new class that can be optionally configured to use the MW DBAL to obtain DB connections while keeping backwards compatibility with the existing setup. With this done, the main source of primary DB access in Cargo - the cargo_query parser function, which often gets run in case of parser cache misses - can actually be switched to use a replica, as it does not really need to read from the primary. However, one may imagine a case where it is necessary for a query to lead the latest and greatest data from the primary, e.g. on a page that stores some data via cargo_store then queries some of that data via cargo_query further down the line. Sidestep this by leveraging MW's hasOrMadeRecentPrimaryChanges() helper to always return a primary DB connection irrespective of the caller, if writes have been detected on the configured Cargo LB. This code has not yet been actually tested - I am submitting this to gather opinions and suggestions. Change-Id: I5ed6661f46be257d1ea6b194aaccbbc5b02c406a
t-tomalak
force-pushed
the
PLATFORM-9062-bp
branch
from
0384d8c
to
0667936
Compare
…argoAttach, CargoDeclare and CargoPageValues Some of the actions are still using PRIMARY DB connection during the Read requests, and therefore we should replace them with use of the proper REPLICA DB. All of those cases are simple and shouldn't yield any errors when PRIMARY DB is not used to query the results. Also, mark CargoRecreateTablesAPI as writing API to minimise noise from transaction profile.
mszabo-wikia
force-pushed
the
PLATFORM-9062-bp
branch
2 times, most recently
from
4ce06bb
to
e86f124
Compare
mszabo-wikia
force-pushed
the
PLATFORM-9062-bp
branch
from
e86f124
to
2eca71e
Compare
t-tomalak
approved these changes
Feb 29, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The Cargo extension currently uses a single database connection to access and manage its database tables, managed entirely by CargoUtils without going through the usual MW DBAL layers. This ipso facto precludes the use of a proper primary - replica setup, as this connection must allow database writes. We want to do just that, so that we can offload read queries to replicas where possible and avoid putting undue load on the primary DB in an active-active world. So, first make it possible for Cargo to utilize replicas at all by factoring out its DB connection management logic into a new class that can be optionally configured to use the MW DBAL to obtain DB connections while keeping backwards compatibility with the existing setup.
With this done, the main source of primary DB access in Cargo - the cargo_query parser function, which often gets run in case of parser cache misses - can actually be switched to use a replica, as it does not really need to read from the primary. However, one may imagine a case where it is necessary for a query to lead the latest and greatest data from the primary, e.g. on a page that stores some data via cargo_store then queries some of that data via cargo_query further down the line. Sidestep this by leveraging MW's hasOrMadeRecentPrimaryChanges() helper to always return a primary DB connection irrespective of the caller, if writes have been detected on the configured Cargo LB.
Change-Id: I5ed6661f46be257d1ea6b194aaccbbc5b02c406a