From faa8cd99dd5992986d14cda16de37d6df99b6744 Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Sat, 31 Oct 2026 12:10:39 -0700 Subject: [PATCH] fix(kv): per-holder share of the KV handle budget (#4433) * fix(permissions): chunk the exact-match subtree delete The holder-scoped subtree delete names every matched grant, so a large subtree could exceed an engine's limit on bound parameters. It now deletes 500 grants per statement. * fix(kv): give each holder a share of a namespace's handle budget One holder could spend a namespace's whole handle budget and starve the others. Each holder now gets at most 3,750 of the namespace's 15,000 units per 10 seconds, and the owner's own calls through a handle spend none. The holder's budget is checked before the grant lookups, so throttled retries cost none; the namespace's is only told to callers the grant admits. Multi-unit charges use a new counted claim that keeps two counters per key instead of a sorted-set member per unit. --- src/backend/core/http/middleware/rateLimit.js | 130 +++++++++++++++ .../core/http/middleware/rateLimit.test.js | 105 ++++++++++++ .../KVStoreDriver.handles.integration.test.ts | 152 +++++++++++++++--- src/backend/drivers/kv/KVStoreDriver.ts | 118 +++++++++----- src/backend/drivers/kv/limits.ts | 19 ++- .../stores/permission/PermissionStore.test.ts | 47 ++++++ .../stores/permission/PermissionStore.ts | 30 ++-- src/docs/src/rate-limits-and-quotas.md | 2 +- 8 files changed, 526 insertions(+), 77 deletions(-) diff --git a/src/backend/core/http/middleware/rateLimit.js b/src/backend/core/http/middleware/rateLimit.js index 0f94ede22..ee8d0681d 100644 --- a/src/backend/core/http/middleware/rateLimit.js +++ b/src/backend/core/http/middleware/rateLimit.js @@ -84,6 +84,10 @@ export function sweepMemoryWindows() { ) memoryWindows.delete(k); } + for (const [k, entry] of memoryCounters) { + if (entry.index < Math.floor(now / entry.windowMs) - 1) + memoryCounters.delete(k); + } } { @@ -155,6 +159,97 @@ async function peekMemory(key, limit, windowMs) { return timestamps.length < limit; } +// -- Counted claims -------------------------------------------------- +// +// `claim` keeps a member per unit, which is fine for a few units and not for +// thousands. The counted form keeps two counters per key, the current fixed +// window and the one before it, and slides by weighting the previous one by +// how much of it the window still covers. It can be off by a little at window +// edges, and its buckets are separate from the other forms'. + +/** The sliding-window estimate from the two fixed windows it overlaps. */ +const slidingCount = (previous, current, now, windowMs) => + previous * (1 - (now % windowMs) / windowMs) + current; + +/** How many of `units` a counted claim that left `used` spent may keep. */ +const countedAdmits = (used, limit, units) => + units - Math.min(units, Math.max(0, Math.ceil(used - limit))); + +/** Key → `{ index, current, previous, windowMs }`, swept with the windows. */ +const memoryCounters = new Map(); + +async function claimCountMemory(key, limit, windowMs, units) { + const now = Date.now(); + const index = Math.floor(now / windowMs); + let entry = memoryCounters.get(key); + if (!entry) { + if (memoryCounters.size >= MEMORY_MAX_KEYS) { + memoryCounters.delete(memoryCounters.keys().next().value); + } + entry = { index, current: 0, previous: 0, windowMs }; + memoryCounters.set(key, entry); + } else if (entry.index !== index) { + entry.previous = entry.index === index - 1 ? entry.current : 0; + entry.current = 0; + entry.index = index; + } + entry.windowMs = windowMs; + const used = slidingCount(entry.previous, entry.current, now, windowMs); + const admitted = countedAdmits(used + units, limit, units); + entry.current += admitted; + return admitted; +} + +async function claimCountRedis(redis, key, limit, windowMs, units) { + const now = Date.now(); + const index = Math.floor(now / windowMs); + // The hash tag keeps both windows in one slot, so one MULTI reads both. + const current = `ratec:{${key}}:${index}`; + const results = await redis + .multi() + .incrby(current, units) + .pexpire(current, 2 * windowMs) + .get(`ratec:{${key}}:${index - 1}`) + .exec(); + const used = slidingCount( + Number(multiResult(results, 2) ?? 0), + Number(multiResult(results, 0)), + now, + windowMs, + ); + const admitted = countedAdmits(used, limit, units); + if (admitted < units) await redis.decrby(current, units - admitted); + return admitted; +} + +async function claimCountKv(kv, key, limit, windowMs, units) { + const now = Date.now(); + const index = Math.floor(now / windowMs); + const current = `ratec:${key}:${index}`; + const [counted, previous] = await Promise.all([ + kv.incr({ + key: current, + pathAndAmountMap: { '': units }, + expireAt: Math.ceil((now + 2 * windowMs) / 1000), + }), + kv.get({ key: `ratec:${key}:${index - 1}` }), + ]); + const used = slidingCount( + Number(previous.res ?? 0), + Number(counted.res), + now, + windowMs, + ); + const admitted = countedAdmits(used, limit, units); + if (admitted < units) { + await kv.decr({ + key: current, + pathAndAmountMap: { '': units - admitted }, + }); + } + return admitted; +} + // -- Redis backend --------------------------------------------------- // ioredis MULTI/EXEC reports per-command failures inside the exec() result @@ -475,6 +570,10 @@ function instrumentBackendPair(name, pair) { withSpan('rate_limit.claim', attrs, () => pair.claim(key, limit, windowMs, units), ), + count: (key, limit, windowMs, units) => + withSpan('rate_limit.count', attrs, () => + pair.count(key, limit, windowMs, units), + ), acquire: (key, limit) => withSpan('rate_limit.acquire', attrs, () => pair.acquire(key, limit), @@ -486,6 +585,7 @@ const memoryBackendPair = instrumentBackendPair('memory', { rate: checkMemory, peek: peekMemory, claim: claimMemory, + count: claimCountMemory, acquire: acquireMemoryConcurrent, }); @@ -536,6 +636,8 @@ export function configureRateLimit({ peekRedis(redis, key, limit, windowMs), claim: (key, limit, windowMs, units) => claimRedis(redis, key, limit, windowMs, units), + count: (key, limit, windowMs, units) => + claimCountRedis(redis, key, limit, windowMs, units), acquire: (key, limit) => acquireRedisConcurrent(redis, key, limit), }); } @@ -545,6 +647,8 @@ export function configureRateLimit({ peek: (key, limit) => peekKv(kv, key, limit), claim: (key, limit, windowMs, units) => claimKv(kv, key, limit, windowMs, units), + count: (key, limit, windowMs, units) => + claimCountKv(kv, key, limit, windowMs, units), acquire: (key, limit) => acquireKvConcurrent(kv, key, limit), }); } @@ -874,6 +978,32 @@ export async function claimRateLimit(key, limit, windowMs, units, backend) { } } +/** + * `claimRateLimit` for budgets measured in many units per call: a claim costs + * the same however many units it spends (see "Counted claims" above). Its + * buckets are its own, so a key is only ever claimed one way. Fails open, + * admitting every unit. + */ +export async function claimCountedRateLimit( + key, + limit, + windowMs, + units, + backend, +) { + if (!(units > 0)) return 0; + const bk = resolveBackend(backend); + try { + return await bk.count(key, limit, windowMs, units); + } catch (err) { + console.error( + '[rate-limit] imperative counted claim failed, failing open:', + err, + ); + return units; + } +} + /** * Charge a route-shaped rate-limit spec from inside a handler. * diff --git a/src/backend/core/http/middleware/rateLimit.test.js b/src/backend/core/http/middleware/rateLimit.test.js index c71b50ad6..39f594aef 100644 --- a/src/backend/core/http/middleware/rateLimit.test.js +++ b/src/backend/core/http/middleware/rateLimit.test.js @@ -37,6 +37,7 @@ import { acquireDriverConcurrent, checkDriverRateLimit, checkRateLimit, + claimCountedRateLimit, claimRateLimit, concurrencyGate, consumeRouteRateLimit, @@ -1487,6 +1488,13 @@ describe('rate + concurrent limiting — kv backend', () => { expect(await claimRateLimit(key, 5, 60_000, 1)).toBe(0); }); + it('claims counted units, admitting only what fits', async () => { + const key = uniqueScope(); + expect(await claimCountedRateLimit(key, 5, 60_000, 3)).toBe(3); + expect(await claimCountedRateLimit(key, 5, 60_000, 3)).toBe(2); + expect(await claimCountedRateLimit(key, 5, 60_000, 1)).toBe(0); + }); + it('claims against a full bucket as full while expired rows are unswept', async () => { const key = uniqueScope(); await seedUnswept(`rate:${key}:`, 3); @@ -2010,6 +2018,103 @@ describe('acquireConcurrent (imperative)', () => { }); }); +describe('claimCountedRateLimit', () => { + const uniqueKey = () => + `counted-${Math.random().toString(36).slice(2, 10)}`; + const WINDOW = 10_000; + /** The start of a fixed window, so nothing before it carries in. */ + const windowStart = (n) => vi.setSystemTime(n * WINDOW); + + afterEach(() => { + vi.useRealTimers(); + }); + + describe.each([ + ['memory', () => configureRateLimit()], + [ + 'redis', + () => + configureRateLimit({ + default: 'redis', + redis: new RedisMock(), + }), + ], + ])('%s backend', (_name, configure) => { + beforeEach(() => { + configure(); + vi.useFakeTimers({ toFake: ['Date'] }); + windowStart(1_000); + }); + + it('admits up to the limit and spends only what fits', async () => { + const key = uniqueKey(); + expect(await claimCountedRateLimit(key, 5, WINDOW, 3)).toBe(3); + expect(await claimCountedRateLimit(key, 5, WINDOW, 3)).toBe(2); + expect(await claimCountedRateLimit(key, 5, WINDOW, 1)).toBe(0); + }); + + it('spends thousands of units in one claim', async () => { + const key = uniqueKey(); + expect( + await claimCountedRateLimit(key, 15_000, WINDOW, 6_000), + ).toBe(6_000); + expect( + await claimCountedRateLimit(key, 15_000, WINDOW, 10_000), + ).toBe(9_000); + }); + + it('carries the previous window by how much of it is still inside', async () => { + const key = uniqueKey(); + expect(await claimCountedRateLimit(key, 100, WINDOW, 100)).toBe( + 100, + ); + // A quarter into the next window, 75 of those still count. + vi.setSystemTime(1_001 * WINDOW + WINDOW / 4); + expect(await claimCountedRateLimit(key, 100, WINDOW, 50)).toBe(25); + // Two windows on, nothing carries. + windowStart(1_003); + expect(await claimCountedRateLimit(key, 100, WINDOW, 100)).toBe( + 100, + ); + }); + + it('keeps its buckets apart from claimRateLimit', async () => { + const key = uniqueKey(); + expect(await claimCountedRateLimit(key, 2, WINDOW, 2)).toBe(2); + expect(await claimRateLimit(key, 2, WINDOW, 2)).toBe(2); + }); + }); + + it('claims nothing for zero units', async () => { + const key = uniqueKey(); + expect(await claimCountedRateLimit(key, 1, WINDOW, 0)).toBe(0); + expect(await claimCountedRateLimit(key, 1, WINDOW, 1)).toBe(1); + }); + + it('fails open, admitting every unit, when the backend throws', async () => { + const redis = new RedisMock(); + redis.multi = () => { + throw new Error('redis down'); + }; + configureRateLimit({ default: 'redis', redis }); + const spy = vi.spyOn(console, 'error').mockImplementation(() => {}); + expect(await claimCountedRateLimit(uniqueKey(), 1, WINDOW, 4)).toBe(4); + spy.mockRestore(); + }); + + it('drops a memory counter once its windows have passed', async () => { + configureRateLimit(); + vi.useFakeTimers({ toFake: ['Date'] }); + windowStart(2_000); + const key = uniqueKey(); + expect(await claimCountedRateLimit(key, 1, WINDOW, 1)).toBe(1); + windowStart(2_002); + sweepMemoryWindows(); + windowStart(2_000); + expect(await claimCountedRateLimit(key, 1, WINDOW, 1)).toBe(1); + }); +}); + describe('claimRateLimit', () => { const uniqueKey = () => `claim-${Math.random().toString(36).slice(2, 10)}`; diff --git a/src/backend/drivers/kv/KVStoreDriver.handles.integration.test.ts b/src/backend/drivers/kv/KVStoreDriver.handles.integration.test.ts index 5456f97d1..3e05a1c85 100644 --- a/src/backend/drivers/kv/KVStoreDriver.handles.integration.test.ts +++ b/src/backend/drivers/kv/KVStoreDriver.handles.integration.test.ts @@ -45,7 +45,7 @@ import { kvSharePermission, type KvShareAccess, } from '../../services/kvShare/kvShares.js'; -import { claimRateLimit } from '../../core/http/middleware/rateLimit.js'; +import { claimCountedRateLimit } from '../../core/http/middleware/rateLimit.js'; import { kvNamespace } from '../../stores/systemKv/SystemKVStore.js'; import { createTestUser, @@ -246,9 +246,35 @@ afterAll(async () => { await env?.shutdown(); }); -/** The redis key of the budget every handle call into a namespace spends. */ -const budgetKey = (user: TestUser, appUid = GLOBAL) => - `rate:kv:handles:${kvNamespace(user.uuid, appUid)}`; +/** + * The handle budget calls into `owner`'s namespace spend from: every holder's, + * or with `holder`, that holder's own. + */ +const budgetKey = (owner: TestUser, holder?: TestUser, appUid = GLOBAL) => { + const namespace = `kv:handles:${kvNamespace(owner.uuid, appUid)}`; + return holder ? `${namespace}:holder:${holder.id}` : namespace; +}; + +/** A budget's counters for this window and the one before. */ +const budgetCounters = (key: string) => { + const index = Math.floor(Date.now() / KV_HANDLE_NAMESPACE_BUDGET.window); + return [index - 1, index].map((at) => `ratec:{${key}}:${at}`); +}; + +/** Units spent from a budget since it was last cleared. */ +const spent = async (key: string) => { + let total = 0; + for (const counter of budgetCounters(key)) + total += Number(await env.server.clients.redis.get(counter)); + return total; +}; + +const clearBudget = (key: string) => + env.server.clients.redis.del(...budgetCounters(key)); + +/** Spend all of a budget, as though other calls had. */ +const exhaust = (key: string, limit: number) => + claimCountedRateLimit(key, limit, KV_HANDLE_NAMESPACE_BUDGET.window, limit); // Every mint spends the 60/min budget, and this file makes more than that. beforeEach(async () => { @@ -257,8 +283,9 @@ beforeEach(async () => { `rate:${EVENTS_KV_HANDLE_LIMIT.scope}:${user.id}`, ); await env.server.clients.redis.del(`rate:events:subscribe:${user.id}`); - await env.server.clients.redis.del(budgetKey(user)); + await clearBudget(budgetKey(owner, user)); } + await clearBudget(budgetKey(owner)); }); describe('a call through a handle', () => { @@ -1420,9 +1447,8 @@ describe('what a call costs', () => { }); }); -describe('the budget every holder of a namespace shares', () => { - const spent = async () => - Number(await env.server.clients.redis.zcard(budgetKey(owner))); +describe('the handle budget of a namespace', () => { + const { limit, perHolder } = KV_HANDLE_NAMESPACE_BUDGET; it('is spent by every holder together, and refuses them all once it is gone', async () => { const prefix = region('budget'); @@ -1434,15 +1460,10 @@ describe('the budget every holder of a namespace shares', () => { expect(await guestGet(forStranger.handle, 'k', stranger.actor)).toBe( 'v', ); - expect(await spent()).toBe(2); + expect(await spent(budgetKey(owner))).toBe(2); + expect(await spent(budgetKey(owner, guest))).toBe(1); - const { limit, window } = KV_HANDLE_NAMESPACE_BUDGET; - await claimRateLimit( - budgetKey(owner).slice('rate:'.length), - limit, - window, - limit, - ); + await exhaust(budgetKey(owner), limit); for (const call of [ () => guestGet(forGuest.handle, 'k'), () => guestGet(forStranger.handle, 'k', stranger.actor), @@ -1451,9 +1472,10 @@ describe('the budget every holder of a namespace shares', () => { statusCode: 429, legacyCode: 'too_many_requests', }); - // The owner's own calls spend none of it. + // The owner's own calls spend none of it, with a handle or without. expect(await ownerGet(`${prefix}k`)).toBe('v'); - // Nor does another owner's namespace. + expect(await guestGet(forGuest.handle, 'k', owner.actor)).toBe('v'); + // Another owner's namespace has its own. const { handle: elsewhere } = await shares().mintKvHandle( guest.actor, { @@ -1465,22 +1487,102 @@ describe('the budget every holder of a namespace shares', () => { ); expect(await guestGet(elsewhere, 'k', owner.actor)).toBeNull(); - await env.server.clients.redis.del(budgetKey(owner)); + await clearBudget(budgetKey(owner)); expect(await guestGet(forGuest.handle, 'k')).toBe('v'); }); + it('keeps one holder to its share, while the others get through', async () => { + const prefix = region('fair'); + await ownerSet(`${prefix}k`, 'v'); + const forGuest = await share(prefix, 'read'); + const forStranger = await share(prefix, 'read', { holder: stranger }); + expect(perHolder).toBeLessThan(limit); + + await exhaust(budgetKey(owner, guest), perHolder); + await expect(guestGet(forGuest.handle, 'k')).rejects.toMatchObject({ + statusCode: 429, + legacyCode: 'too_many_requests', + }); + expect(await guestGet(forStranger.handle, 'k', stranger.actor)).toBe( + 'v', + ); + // The throttled holder's refusals spent nothing of the namespace's. + expect(await spent(budgetKey(owner))).toBe(1); + }); + + it('spends nothing when the owner calls through a handle', async () => { + const prefix = region('own'); + await ownerSet(`${prefix}k`, 'v'); + const { handle } = await share(prefix, 'read'); + await exhaust(budgetKey(owner, owner), perHolder); + + for (let i = 0; i < 3; i++) + expect(await guestGet(handle, 'k', owner.actor)).toBe('v'); + expect(await spent(budgetKey(owner))).toBe(0); + await exhaust(budgetKey(owner), limit); + expect(await guestGet(handle, 'k', owner.actor)).toBe('v'); + }); + + it('refuses a throttled holder before looking at its grant', async () => { + const { handle } = await share(region('early'), 'read'); + await exhaust(budgetKey(owner, guest), perHolder); + const checks = vi.spyOn(env.server.services.permission, 'check'); + try { + await expect(guestGet(handle, 'k')).rejects.toMatchObject({ + statusCode: 429, + }); + expect(checks).not.toHaveBeenCalled(); + } finally { + checks.mockRestore(); + } + }); + + it('is only heard by a caller the grant admits', async () => { + const { handle } = await share(region('heard'), 'watch'); + await exhaust(budgetKey(owner), limit); + // Below the level: refused for that, not for the budget. + await expect(guestGet(handle, 'k')).rejects.toMatchObject({ + legacyCode: 'kv_handle_access_denied', + }); + }); + + it('tells another app of the holder the handle is not there, however spent', async () => { + const app = await makeApp(); + const otherApp = await makeApp(); + await authorizeApp(guest, app); + await authorizeApp(guest, otherApp); + const { handle } = await share(region('apps'), 'read', { + appUid: app.uid, + }); + const holderBudget = budgetKey(owner, guest, app.uid); + await clearBudget(holderBudget); + await exhaust(holderBudget, perHolder); + + await expect( + guestGet(handle, 'k', appActorFor(guest, otherApp)), + ).rejects.toMatchObject({ legacyCode: 'kv_handle_not_found' }); + await expect( + guestGet(handle, 'k', appActorFor(guest, app)), + ).rejects.toMatchObject({ statusCode: 429 }); + await clearBudget(holderBudget); + }); + it('charges a call by the storage it reads and writes', async () => { const prefix = region('cost'); const { handle } = await share(prefix, 'write'); const big = 'x'.repeat(100 * 1024); for (const key of ['a', 'b', 'c', 'd']) await ownerSet(`${prefix}big:${key}`, big); - expect(await spent()).toBe(0); + const both = async () => [ + await spent(budgetKey(owner)), + await spent(budgetKey(owner, guest)), + ]; + expect(await both()).toEqual([0, 0]); await guestSet(handle, 'small', 'x'); - await vi.waitFor(async () => expect(await spent()).toBe(3)); + await vi.waitFor(async () => expect(await both()).toEqual([3, 3])); expect(await guestGet(handle, 'small')).toBe('x'); - await vi.waitFor(async () => expect(await spent()).toBe(4)); + await vi.waitFor(async () => expect(await both()).toEqual([4, 4])); const page = (await as(guest.actor, (d) => d.list({ as: 'entries', pattern: 'big:*', ...via(handle) }), @@ -1488,8 +1590,10 @@ describe('the budget every holder of a namespace shares', () => { expect(page).toHaveLength(4); // About 400 KB read, at a unit per 8 KB. await vi.waitFor(async () => { - expect(await spent()).toBeGreaterThanOrEqual(4 + 50); - expect(await spent()).toBeLessThan(4 + 60); + for (const units of await both()) { + expect(units).toBeGreaterThanOrEqual(4 + 50); + expect(units).toBeLessThan(4 + 60); + } }); }); }); diff --git a/src/backend/drivers/kv/KVStoreDriver.ts b/src/backend/drivers/kv/KVStoreDriver.ts index 45ceec5b0..76de6f9a3 100644 --- a/src/backend/drivers/kv/KVStoreDriver.ts +++ b/src/backend/drivers/kv/KVStoreDriver.ts @@ -19,16 +19,13 @@ import { HttpError } from '../../core/http/HttpError.js'; import { Context } from '../../core/context.js'; -import { - checkRateLimit, - claimRateLimit, -} from '../../core/http/middleware/rateLimit.js'; +import { claimCountedRateLimit } from '../../core/http/middleware/rateLimit.js'; import { DEFAULT_FREE_SUBSCRIPTION, DEFAULT_TEMP_SUBSCRIPTION, } from '../../services/metering/consts.js'; import { PuterDriver } from '../types.js'; -import type { Actor } from '../../core/actor.js'; +import { type Actor, isAccountContext, isAppActor } from '../../core/actor.js'; import type { DriverConcurrentConfig, DriverRateLimitConfig } from '../meta.js'; import { APP_DATA_KV_METHOD_OPS, @@ -100,7 +97,20 @@ type KvCallArgs = { * Store options plus, for a call through a share handle, the handle and its * region's prefix: keys go in under it and come back out relative to it. */ -type DriverKvOpts = KVOpts & { handle?: string; handlePrefix?: string }; +type DriverKvOpts = KVOpts & { + handle?: string; + handlePrefix?: string; + /** Set when the call spends from a namespace's handle budget. */ + handleBudget?: HandleBudget; +}; + +/** The budgets a holder's call through a handle spends from. */ +interface HandleBudget { + /** Every holder of a share into the namespace together. */ + namespace: string; + /** This holder alone, within the namespace. */ + holder: string; +} type KvListResult = unknown[] | { items: unknown[]; cursor?: string; total?: number }; @@ -171,15 +181,42 @@ const handlePatternInvalid = (): HttpError => { legacyCode: 'kv_handle_key_invalid' }, ); -/** Calls through every share into the namespace used up its budget. */ +/** The holder's budget in the namespace, or the namespace's, is used up. */ const handleBudgetSpent = (): HttpError => new HttpError(429, 'Too many requests.', { legacyCode: 'too_many_requests', }); -/** The budget every handle call into `namespace` spends from. */ -const handleBudgetKey = (namespace: string): string => - `kv:handles:${namespace}`; +/** The budgets a holder's calls into `namespace` spend from. */ +const handleBudget = ( + namespace: string, + holderUserId: number, +): HandleBudget => ({ + namespace: `kv:handles:${namespace}`, + holder: `kv:handles:${namespace}:holder:${holderUserId}`, +}); + +/** Spend `units` from a budget, and say whether any of them fit. */ +const spendHandleBudget = async ( + key: string, + limit: number, + units: number, +): Promise => + (await claimCountedRateLimit( + key, + limit, + KV_HANDLE_NAMESPACE_BUDGET.window, + units, + )) > 0; + +/** + * Whether a share's grant can answer for this actor at all: the holder's own + * account, or the app whose data is shared. Any other caller is refused as if + * the handle weren't there. + */ +const onShareSurface = (actor: Actor, appUid: string): boolean => + isAccountContext(actor) || + (isAppActor(actor) && actor.effectiveApp?.uid === appUid); const invalidCursor = (): HttpError => new HttpError(400, 'invalid kv: cursor', { legacyCode: 'bad_request' }); @@ -454,6 +491,22 @@ export class KVStoreDriver extends PuterDriver { if (userId !== row.granteeUserId && userId !== row.ownerUserId) throw handleNotFound(); + // The holder's own budget goes before the grant lookups, so throttled + // retries cost none. It only counts the holder's own calls, so it says + // nothing about the namespace. The owner spends no handle budget. + const { limit, perHolder } = KV_HANDLE_NAMESPACE_BUDGET; + const budget = + userId === row.ownerUserId + ? undefined + : handleBudget(kvNamespace(ownerUuid, row.appUid), userId); + const holderFirst = + budget !== undefined && onShareSurface(actor, row.appUid); + if ( + holderFirst && + !(await spendHandleBudget(budget.holder, perHolder, 1)) + ) + throw handleBudgetSpent(); + // Side by side, so the app lookup costs no round trip of its own. const [appGone, allowed] = await Promise.all([ row.appUid === KV_GLOBAL_APP_KEY @@ -485,16 +538,17 @@ export class KVStoreDriver extends PuterDriver { Buffer.byteLength(row.keyPrefix, 'utf8') > KV_SHARE_PREFIX_MAX_BYTES ) throw handleAccessDenied(); - // One unit now; `#meter` spends the rest of what the call consumed. - const { limit, window } = KV_HANDLE_NAMESPACE_BUDGET; - if ( - !(await checkRateLimit( - handleBudgetKey(kvNamespace(ownerUuid, row.appUid)), - limit, - window, - )) - ) - throw handleBudgetSpent(); + // The namespace's budget tells how busy other holders are, so only a + // caller the grant admits may hear it. `#meter` spends the rest. + if (budget) { + if ( + !holderFirst && + !(await spendHandleBudget(budget.holder, perHolder, 1)) + ) + throw handleBudgetSpent(); + if (!(await spendHandleBudget(budget.namespace, limit, 1))) + throw handleBudgetSpent(); + } return { actor, @@ -502,6 +556,7 @@ export class KVStoreDriver extends PuterDriver { namespaceAppUuid: row.appUid, handle, handlePrefix: row.keyPrefix, + ...(budget ? { handleBudget: budget } : {}), }; } @@ -702,28 +757,19 @@ export class KVStoreDriver extends PuterDriver { } /** - * Bill the caller for what the call consumed. Through a handle, also spend - * it from the owner namespace's budget, past the unit admission took. + * Bill the caller for what the call consumed. A holder's call through a + * handle also spends it from its budgets, past the unit admission took. */ #meter(opts: DriverKvOpts, usage: KVUsage): void { - if (opts.handle !== undefined) { + const budget = opts.handleBudget; + if (budget) { const units = Math.ceil( usage.read + usage.write * KV_HANDLE_WRITE_UNIT_WEIGHT, ); if (units > 1) { - const { limit, window } = KV_HANDLE_NAMESPACE_BUDGET; - // More than the limit would only add entries to a full bucket. - void claimRateLimit( - handleBudgetKey( - kvNamespace( - opts.namespaceUser!.uuid, - opts.namespaceAppUuid!, - ), - ), - limit, - window, - Math.min(units - 1, limit), - ); + const { limit, perHolder } = KV_HANDLE_NAMESPACE_BUDGET; + void spendHandleBudget(budget.holder, perHolder, units - 1); + void spendHandleBudget(budget.namespace, limit, units - 1); } } const actor = opts.actor; diff --git a/src/backend/drivers/kv/limits.ts b/src/backend/drivers/kv/limits.ts index 8e1a159f9..49cbf5dd7 100644 --- a/src/backend/drivers/kv/limits.ts +++ b/src/backend/drivers/kv/limits.ts @@ -34,15 +34,20 @@ export const KV_MAX_LIST_LIMIT = 1000; /** * Storage capacity calls through share handles may spend in one owner's - * namespace per window, every holder together. A namespace's keys share one - * partition key, so this keeps holders to about a fifth of what that key can - * serve and leaves the rest to the owner's own app. + * namespace per window: `limit` for every holder together, and `perHolder` for + * any one of them, so no holder can starve the others. The owner's own calls + * through a handle spend none. Sized to leave about half of what one namespace + * can serve to the owner's own app. */ -export const KV_HANDLE_NAMESPACE_BUDGET = { limit: 6_000, window: 10_000 }; +export const KV_HANDLE_NAMESPACE_BUDGET = { + limit: 15_000, + perHolder: 3_750, + window: 10_000, +}; /** - * What a call spends from that budget: its read units, plus this many per write - * unit, since a partition serves about that many reads per write. Every call - * spends at least one. + * What a call spends from those budgets: its read units, plus this many per + * write unit, since a write costs the store about three times what a read does. + * Every call spends at least one. */ export const KV_HANDLE_WRITE_UNIT_WEIGHT = 3; diff --git a/src/backend/stores/permission/PermissionStore.test.ts b/src/backend/stores/permission/PermissionStore.test.ts index 106e22d0d..942a7c8d2 100644 --- a/src/backend/stores/permission/PermissionStore.test.ts +++ b/src/backend/stores/permission/PermissionStore.test.ts @@ -29,6 +29,7 @@ import { CACHE_TOMBSTONE_SUFFIX } from '../types.ts'; import { FLAT_PERM_SCAN_CONCURRENCY, U2U_CACHE_MAX_ROWS, + U2U_EXACT_DELETE_CHUNK, } from './PermissionStore.ts'; describe('PermissionStore', () => { @@ -1855,6 +1856,52 @@ describe('PermissionStore', () => { ).toEqual(live); }); + it('withdraws a subtree larger than one delete names, in chunks', async () => { + const issuer = await makeUser(); + const holder = await makeUser(); + const root = 'kv-share-read:owner:app:big'; + const perms = Array.from( + { length: U2U_EXACT_DELETE_CHUNK + 1 }, + (_, i) => `${root}:k${i}`, + ); + for (let i = 0; i < perms.length; i += 200) { + const rows = perms.slice(i, i + 200); + await server.clients.db.write( + 'INSERT INTO `user_to_user_permissions` (`holder_user_id`, `issuer_user_id`, `permission`, `extra`) VALUES ' + + rows.map(() => '(?, ?, ?, ?)').join(', '), + rows.flatMap((perm) => [holder.id, issuer.id, perm, '{}']), + ); + } + + const writes = vi.spyOn(server.clients.db, 'write'); + let removed: string[]; + let deletes: unknown[][]; + try { + removed = await store.deleteUserUserPermSubtreeForHolder( + holder.id, + issuer.id, + root, + ); + deletes = writes.mock.calls.filter(([sql]) => + String(sql).startsWith( + 'DELETE FROM `user_to_user_permissions`', + ), + ); + } finally { + writes.mockRestore(); + } + + expect(removed.sort()).toEqual([...perms].sort()); + expect(deletes).toHaveLength(2); + expect( + await store.findUserUserPermsByIssuer( + holder.id, + issuer.id, + perms, + ), + ).toEqual([]); + }); + it('reports whether the delete matched a row', async () => { const issuer = await makeUser(); const holder = await makeUser(); diff --git a/src/backend/stores/permission/PermissionStore.ts b/src/backend/stores/permission/PermissionStore.ts index bf10052e2..070144c26 100644 --- a/src/backend/stores/permission/PermissionStore.ts +++ b/src/backend/stores/permission/PermissionStore.ts @@ -55,6 +55,12 @@ const AUDIT_PAGE_SIZE = 50; const MAX_AUDIT_PAGE_SIZE = 200; /** Flat-entry prefix scans one subtree revoke keeps in flight. */ export const FLAT_PERM_SCAN_CONCURRENCY = 4; + +/** + * Grants one exact-match delete names, so a large subtree stays well under + * every engine's limit on bound parameters. + */ +export const U2U_EXACT_DELETE_CHUNK = 500; /** Flat entries one page of a deleted holder's purge lists, and the pages. */ const HELD_FLAT_PURGE_PAGE = 500; const HELD_FLAT_PURGE_MAX_PAGES = 200; @@ -746,15 +752,21 @@ export class PermissionStore extends PuterStore { .map((row) => String(row.permission)) .filter((held) => roots.some((root) => inSubtree(held, root))); if (removed.length > 0) { - const { scope, params: exact } = this.#issuerHolderScope( - holderUserId, - issuerUserId, - removed, - ); - await this.clients.db.write( - `DELETE FROM \`user_to_user_permissions\` WHERE ${scope}`, - exact, - ); + for ( + let start = 0; + start < removed.length; + start += U2U_EXACT_DELETE_CHUNK + ) { + const { scope, params: exact } = this.#issuerHolderScope( + holderUserId, + issuerUserId, + removed.slice(start, start + U2U_EXACT_DELETE_CHUNK), + ); + await this.clients.db.write( + `DELETE FROM \`user_to_user_permissions\` WHERE ${scope}`, + exact, + ); + } await this.tombstoneCacheKeys([this.#u2uCacheKey(holderUserId)]); } diff --git a/src/docs/src/rate-limits-and-quotas.md b/src/docs/src/rate-limits-and-quotas.md index 6ad548bbc..4910e8bd8 100644 --- a/src/docs/src/rate-limits-and-quotas.md +++ b/src/docs/src/rate-limits-and-quotas.md @@ -252,7 +252,7 @@ Before the provider runs, the balance must cover every page the call can be bill | Concurrent calls | 30 | 15 | 8 | | Concurrent `list` | 5 | 3 | 2 | -Calls through [share handles](/KV/share/) into one user's data for one app also share one budget across every holder of those shares. The budget is 6,000 units per 10 seconds on every plan. The budget counts storage use, so large entries and big listings use more of it. Reading counts about 1 unit for every 8 KB read, with each entry rounded up to 4 KB. Writing counts 3 units for every 1 KB of each entry written. Every call counts at least 1 unit. So a `get()` of a small entry counts 1 unit, a `set()` of a value up to 1 KB counts 3, and a `list()` page that reads 1 MB counts 128. Over the budget, calls through those shares fail with `too_many_requests` until older calls leave the 10-second window. Calls the owner makes without a handle don't count toward it. +Calls through [share handles](/KV/share/) into one user's data for one app also share a budget across every holder of those shares. All the holders together can spend 15,000 units per 10 seconds, and one holder can spend at most 3,750 of them, so one busy holder can't use up the budget for the others. These numbers are the same on every plan. The budget counts storage use, so large entries and big listings use more of it. Reading counts about 1 unit for every 8 KB read, with each entry rounded up to 4 KB. Writing counts 3 units for every 1 KB of each entry written. Every call counts at least 1 unit. So a `get()` of a small entry counts 1 unit, a `set()` of a value up to 1 KB counts 3, and a `list()` page that reads 1 MB counts 128. Over the holder's share or the total, calls through those shares fail with `too_many_requests` until older calls leave the 10-second window. The owner's own calls don't count, with or without a handle. Sizes are the same for every account: