Cache repo revs in order to avoid read-after-write for up-to-date responses #2784
Conversation
| @@ -548,6 +548,15 @@ export class SeedClient< | |||
| return result.data | |||
| } | |||
|
|
|||
| pdsAgent(did: string) { | |||
| export class RepoRevCacheRedis implements SimpleStore<string, string> { | ||
| /** | ||
| * @param redis - Redis client | ||
| * @param maxAge - Maximum age of a cached revision in milliseconds |
There was a problem hiding this comment.
just noting that this is called maxAge here but px in the constructor
|
Overall looking good!
|
| @@ -187,7 +189,8 @@ export class AccountManager | |||
| } | |||
|
|
|||
| async updateRepoRoot(did: string, cid: CID, rev: string) { | |||
| return repo.updateRoot(this.db, did, cid, rev) | |||
| await repo.updateRoot(this.db, did, cid, rev) | |||
| await this.repoRevCache?.set(did, rev) | |||
There was a problem hiding this comment.
may be good to fail open on this rather than returning an error to the user
There was a problem hiding this comment.
oh rip saw you handle these errors in the simple store 😛
|
|
||
| // If the response's "atproto-repo-rev" header matches the current repo rev, | ||
| // we can skip the munge step and return the response as-is. | ||
| const repoRev = await ctx.repoRevCache?.get(requester) |
There was a problem hiding this comment.
similarly, it'd be great to fail open here, maybe with a quick timeout on the redis request
packages/pds/src/config/env.ts
Outdated
| // Repo revision cache | ||
| repoRevCacheMaxAge: envInt('PDS_REPO_REV_CACHE_MAX_AGE'), // Seconds (use 0 to disable) |
There was a problem hiding this comment.
To make the PDS config more uniform I recommend we switch this to be _MAX_TTL and to accept a value in milliseconds.
|
Dropping since it didn't produce the expected perf improvements. |
No description provided.