feat: read replica db for findByCid #1212
Conversation
samika98
force-pushed
the
feature/ws2-3238-create-a-separate-read-replica-to-improve-cas-performace
branch
from
802d040
to
b82f094
Compare
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #1212 +/- ##
===========================================
- Coverage 78.44% 78.20% -0.25%
===========================================
Files 47 48 +1
Lines 1935 1964 +29
Branches 309 314 +5
===========================================
+ Hits 1518 1536 +18
- Misses 417 428 +11 ☔ View full report in Codecov by Sentry. |
|
| if (!request) { | ||
| throw new RequestDoesNotExistError(cid) | ||
| request = await this.requestRepository.findByCid(cid) | ||
| logger.debug(`Request not found in replica db for ${cid}, fetching from main_db`) |
There was a problem hiding this comment.
Nitpick: probably should go before line 61
There was a problem hiding this comment.
Thanks for catching this!
| throw new RequestDoesNotExistError(cid) | ||
| request = await this.requestRepository.findByCid(cid) | ||
| logger.debug(`Request not found in replica db for ${cid}, fetching from main_db`) | ||
| Metrics.count(METRIC_NAMES.REPLICA_DB_REQUEST_NOT_FOUND, 1) |
There was a problem hiding this comment.
Do we want to move this above line 61 (if line 61 errors, the metric won't be counted)
| @@ -69,6 +69,38 @@ export async function createDbConnection(dbConfig: Db = config.db): Promise<Knex | |||
| return connection | |||
| } | |||
|
|
|||
| export async function createReplicaDbConnection( | |||
There was a problem hiding this comment.
instead of having a separate function can we reuse the one above , I like the explicit key value pairs
There was a problem hiding this comment.
Also does this also need to do migrations (in case of future)
There was a problem hiding this comment.
I like having two separate functions. So that if in the future we want to disable a read replica, it would be easier to add that as a config, and add a check in app.ts. I like having a function do just one thing.
No we don't need any migrations, rds takes care of that
samika98
deleted the
feature/ws2-3238-create-a-separate-read-replica-to-improve-cas-performace
branch
[Read Replica] - #[https://linear.app/3boxlabs/issue/WS2-3238/create-a-separate-read-replica-to-improve-cas-performace]
Description
We noticed Knex connection pools getting exhausted during high load after some time. In order to scale CAS we have to introduce a separate read replica to handle the read calls. This will distribute the load between the replicas giving us better throughput
This PR has env vars defined to access the CAS-API : https://github.com/3box/ceramic-infra/pull/798/files#diff-7a1b7f1deef12acc800844bd1061eecf14fc08ded251a2363f32325a3210fcea
How Has This Been Tested?
TODO : Saw decreased latency monitored via data dog for read queries. Earlier read queries would take ~3-4 minutes under high load. We observe time in order of milli seconds for reads with this change
References:
Update ts for typed-inject compatibility : microsoft/TypeScript#37400. For some reason this bug was still present on ts 5.0.4 but not on the latest version which is 5.4.5
https://www.npmjs.com/package/typed-inject