Skip to content
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

Preferences caching #2792

Draft
wants to merge 3 commits into
base: msieben/read-after-write-caching
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 0 additions & 48 deletions packages/pds/src/account-manager/repo-rev-cache-redis.ts

This file was deleted.

10 changes: 10 additions & 0 deletions packages/pds/src/api/app/bsky/actor/getPreferences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@ export default function (server: Server, ctx: AppContext) {
auth: ctx.authVerifier.accessStandard(),
handler: async ({ auth }) => {
const requester = auth.credentials.did

const cachedPrefs = await ctx.preferencesCache?.get(requester)
if (cachedPrefs) {
const preferences = JSON.parse(cachedPrefs)
return {
encoding: 'application/json',
body: { preferences },
}
}
Comment on lines +11 to +18
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still want to apply the scope here.


const preferences = await ctx.actorStore.read(requester, (store) =>
store.pref.getPreferences('app.bsky', auth.credentials.scope),
)
Expand Down
6 changes: 6 additions & 0 deletions packages/pds/src/api/app/bsky/actor/putPreferences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,19 @@ export default function (server: Server, ctx: AppContext) {
throw new InvalidRequestError('Preference is missing a $type')
}
}

await ctx.actorStore.transact(requester, async (actorTxn) => {
await actorTxn.pref.putPreferences(
checkedPreferences,
'app.bsky',
auth.credentials.scope,
)
})

await ctx.preferencesCache?.set(
requester,
JSON.stringify(checkedPreferences),
)
Comment on lines +30 to +33
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this could cache different contents depending on the user's scope. Maybe it would be more straightforward to control the cache contents entirely on the read path as a read-through cache, and turn this into an invalidation.

},
})
}
51 changes: 51 additions & 0 deletions packages/pds/src/caching/redis-simple-store.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { SimpleStore } from '@atproto-labs/simple-store'
import Redis from 'ioredis'

import { redisLogger } from '../logger'

export class RedisSimpleStore implements SimpleStore<string, string> {
/**
* @param redis - Redis client
* @param maxTTL - Maximum age of a cached revision in milliseconds
*/
constructor(
protected readonly redis: Redis,
protected readonly maxTTL: number,
protected readonly storeName: string,
) {
// Redis expects the expiration time in milliseconds
if (!Number.isFinite(this.maxTTL) || this.maxTTL <= 0) {
throw new TypeError('maxAge must be a positive number')
}
}

protected key(did: string): string {
return `${this.storeName}:${did}`
}

async get(did: string): Promise<string | undefined> {
try {
const value = await this.redis.get(this.key(did))
return value || undefined
} catch (err) {
redisLogger.error({ err, did }, `error getting ${this.storeName}`)
return undefined
}
}

async set(did: string, value: string): Promise<void> {
try {
await this.redis.set(this.key(did), value, 'PX', this.maxTTL)
} catch (err) {
redisLogger.error({ err, did, value }, `error setting ${this.storeName}`)
}
}

async del(did: string): Promise<void> {
try {
await this.redis.del(this.key(did))
} catch (err) {
redisLogger.error({ err, did }, `error deleting ${this.storeName}`)
}
}
}
17 changes: 9 additions & 8 deletions packages/pds/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,10 +235,10 @@ export const envToCfg = (env: ServerEnvironment): ServerConfig => {

const crawlersCfg: ServerConfig['crawlers'] = env.crawlers ?? []

const repoRevCacheCfg: ServerConfig['repoRevCache'] =
env.repoRevCacheMaxTTL != null && env.repoRevCacheMaxTTL > 0
? { maxTTL: env.repoRevCacheMaxTTL }
: null
const cachingCfg: ServerConfig['caching'] = {
repoRevMaxTTL: env.repoRevCacheMaxTTL,
preferencesMaxTTL: env.preferencesCacheMaxTTL,
}

const fetchCfg: ServerConfig['fetch'] = {
disableSsrfProtection: env.fetchDisableSsrfProtection ?? false,
Expand Down Expand Up @@ -306,7 +306,7 @@ export const envToCfg = (env: ServerEnvironment): ServerConfig => {
redis: redisCfg,
rateLimits: rateLimitsCfg,
crawlers: crawlersCfg,
repoRevCache: repoRevCacheCfg,
caching: cachingCfg,
fetch: fetchCfg,
oauth: oauthCfg,
}
Expand All @@ -329,7 +329,7 @@ export type ServerConfig = {
redis: RedisScratchConfig | null
rateLimits: RateLimitsConfig
crawlers: string[]
repoRevCache: RepoRevCacheConfig | null
caching: CachingConfig
fetch: FetchConfig
oauth: OAuthConfig
}
Expand Down Expand Up @@ -398,8 +398,9 @@ export type EntrywayConfig = {
plcRotationKey: string
}

export type RepoRevCacheConfig = {
maxTTL: number
export type CachingConfig = {
repoRevMaxTTL?: number
preferencesMaxTTL?: number
}

export type FetchConfig = {
Expand Down
6 changes: 4 additions & 2 deletions packages/pds/src/config/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,9 @@ export const readEnv = (): ServerEnvironment => {
// fetch
fetchDisableSsrfProtection: envBool('PDS_DISABLE_SSRF_PROTECTION'),

// Repo revision cache
// caching
repoRevCacheMaxTTL: envInt('PDS_REPO_REV_CACHE_MAX_TTL'), // milliseconds (use 0 to disable)
preferencesCacheMaxTTL: envInt('PDS_REPO_REV_CACHE_MAX_TTL'), // milliseconds (use 0 to disable)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
preferencesCacheMaxTTL: envInt('PDS_REPO_REV_CACHE_MAX_TTL'), // milliseconds (use 0 to disable)
preferencesCacheMaxTTL: envInt('PDS_PREFS_CACHE_MAX_TTL'), // milliseconds (use 0 to disable)

}
}

Expand Down Expand Up @@ -239,6 +240,7 @@ export type ServerEnvironment = {
// fetch
fetchDisableSsrfProtection?: boolean

// Repo revision cache
// Caching
repoRevCacheMaxTTL?: number
preferencesCacheMaxTTL?: number
}
24 changes: 21 additions & 3 deletions packages/pds/src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import { DiskBlobStore } from './disk-blobstore'
import { getRedisClient } from './redis'
import { ActorStore } from './actor-store'
import { LocalViewer, LocalViewerCreator } from './read-after-write/viewer'
import { RepoRevCacheRedis } from './account-manager/repo-rev-cache-redis'
import { RedisSimpleStore } from './caching/redis-simple-store'

export type AppContextOptions = {
actorStore: ActorStore
Expand All @@ -56,6 +56,7 @@ export type AppContextOptions = {
backgroundQueue: BackgroundQueue
redisScratch?: Redis
repoRevCache?: SimpleStore<string, string>
preferencesCache?: SimpleStore<string, string>
ratelimitCreator?: RateLimiterCreator
crawlers: Crawlers
appViewAgent?: AtpAgent
Expand Down Expand Up @@ -83,6 +84,7 @@ export class AppContext {
public backgroundQueue: BackgroundQueue
public redisScratch?: Redis
public repoRevCache?: SimpleStore<string, string>
public preferencesCache?: SimpleStore<string, string>
public ratelimitCreator?: RateLimiterCreator
public crawlers: Crawlers
public appViewAgent: AtpAgent | undefined
Expand All @@ -109,6 +111,7 @@ export class AppContext {
this.backgroundQueue = opts.backgroundQueue
this.redisScratch = opts.redisScratch
this.repoRevCache = opts.repoRevCache
this.preferencesCache = opts.preferencesCache
this.ratelimitCreator = opts.ratelimitCreator
this.crawlers = opts.crawlers
this.appViewAgent = opts.appViewAgent
Expand Down Expand Up @@ -231,8 +234,22 @@ export class AppContext {
: null

const repoRevCache =
redisScratch && cfg.repoRevCache
? new RepoRevCacheRedis(redisScratch, cfg.repoRevCache.maxTTL)
redisScratch && cfg.caching.repoRevMaxTTL
? new RedisSimpleStore(
redisScratch,
cfg.caching.repoRevMaxTTL,
'latestRev',
)
: // Note: Single instance PDS that have no redis could use a memory cache
undefined

const preferencesCache =
redisScratch && cfg.caching.preferencesMaxTTL
? new RedisSimpleStore(
redisScratch,
cfg.caching.preferencesMaxTTL,
'preferences',
)
: // Note: Single instance PDS that have no redis could use a memory cache
undefined

Expand Down Expand Up @@ -340,6 +357,7 @@ export class AppContext {
backgroundQueue,
redisScratch,
repoRevCache,
preferencesCache,
ratelimitCreator,
crawlers,
appViewAgent,
Expand Down