Skip to content

Commit 348433f

Browse files
allquixoticclaude
andcommitted
perf(bedrock): pool BedrockRuntimeClient + memoize credentials across conversations
With many parallel Bedrock conversations, each AwsBedrockHandler created its own BedrockRuntimeClient (N connection pools) and re-read credentials from disk per handler (fromIni with ignoreCache). Now: - Cache clients by a stable, secret-free key (region | endpoint | credential identity: apiKey-hash / profile / accessKeyId(+sessionToken-hash) / default-chain). Handlers with identical auth+region+endpoint share one client (the AWS SDK client is concurrency-safe); any difference => own client. - Memoize the fromIni credential provider per profile (dropped ignoreCache, which forced a disk read on every conversation). - Everything else stays per-handler (model config, ARN/cross-region, service tier, 1M context, structured output, prompt router, per-request abort/timeout). - Added test-only __resetBedrockClientCache() for deterministic specs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 86039e8 commit 348433f

3 files changed

Lines changed: 117 additions & 8 deletions

File tree

src/api/providers/__tests__/bedrock-error-handling.spec.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,17 @@ vi.mock("@aws-sdk/client-bedrock-runtime", () => ({
2121
ConverseCommand: vi.fn(),
2222
}))
2323

24-
import { AwsBedrockHandler } from "../bedrock"
24+
import { AwsBedrockHandler, __resetBedrockClientCache } from "../bedrock"
2525
import { Anthropic } from "@anthropic-ai/sdk"
2626

2727
describe("AwsBedrockHandler Error Handling", () => {
2828
let handler: AwsBedrockHandler
2929

3030
beforeEach(() => {
3131
vi.clearAllMocks()
32+
// Reset the process-wide BedrockRuntimeClient pool so each test starts with a fresh
33+
// (mocked) client rather than a client cached from a previous test.
34+
__resetBedrockClientCache()
3235
handler = new AwsBedrockHandler({
3336
apiModelId: "anthropic.claude-3-5-sonnet-20241022-v2:0",
3437
awsAccessKey: "test-access-key",

src/api/providers/__tests__/bedrock.spec.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ vi.mock("@aws-sdk/client-bedrock-runtime", () => {
2525
}
2626
})
2727

28-
import { AwsBedrockHandler } from "../bedrock"
28+
import { AwsBedrockHandler, __resetBedrockClientCache } from "../bedrock"
2929
import { ConverseStreamCommand, BedrockRuntimeClient, ConverseCommand } from "@aws-sdk/client-bedrock-runtime"
3030
import {
3131
BEDROCK_1M_CONTEXT_DEFAULT_MODEL_IDS,
@@ -47,6 +47,11 @@ describe("AwsBedrockHandler", () => {
4747
// Clear all mocks before each test
4848
vi.clearAllMocks()
4949

50+
// Reset the process-wide BedrockRuntimeClient pool so each test's handler
51+
// construction actually instantiates a fresh (mocked) client, keeping the
52+
// `new BedrockRuntimeClient` call-args assertions valid.
53+
__resetBedrockClientCache()
54+
5055
handler = new AwsBedrockHandler({
5156
apiModelId: "anthropic.claude-3-5-sonnet-20241022-v2:0",
5257
awsAccessKey: "test-access-key",

src/api/providers/bedrock.ts

Lines changed: 107 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
} from "@aws-sdk/client-bedrock-runtime"
1414
import OpenAI from "openai"
1515
import { fromIni } from "@aws-sdk/credential-providers"
16+
import { createHash } from "node:crypto"
1617
import { Anthropic } from "@anthropic-ai/sdk"
1718

1819
import {
@@ -240,6 +241,95 @@ export type UsageType = {
240241
cacheWriteInputTokenCount?: number
241242
}
242243

244+
/************************************************************************************
245+
*
246+
* CLIENT / CREDENTIAL POOLING
247+
*
248+
*************************************************************************************/
249+
250+
// This fork runs many conversations in parallel, and each conversation builds its own
251+
// AwsBedrockHandler. Constructing a fresh BedrockRuntimeClient per handler means N TCP
252+
// connection pools plus N credential reads for N parallel conversations. The AWS SDK v3
253+
// BedrockRuntimeClient is safe for concurrent use, so handlers whose client configuration
254+
// is identical (same region, endpoint, and credential identity) share one client.
255+
256+
// Process-wide cache of BedrockRuntimeClient instances, keyed by a stable identity string
257+
// derived from the config's identity-bearing fields (NOT function references).
258+
const bedrockClientCache = new Map<string, BedrockRuntimeClient>()
259+
260+
// Process-wide memoization of fromIni() credential providers, keyed by profile. fromIni
261+
// builds a provider that reads ~/.aws/config|credentials from disk; rebuilding it per
262+
// handler repeats that work for every conversation. Memoizing by profile keeps correctness
263+
// (different profiles never share a provider) while avoiding redundant provider construction.
264+
const bedrockIniCredentialsCache = new Map<string, ReturnType<typeof fromIni>>()
265+
266+
/**
267+
* Return a fromIni credential provider for the given profile, reusing a previously built
268+
* provider for the same profile when available. Note: we intentionally do NOT pass
269+
* ignoreCache here — the previous per-handler code set ignoreCache:true to defeat the SDK's
270+
* in-provider cache, but that forced a fresh disk read for every handler. Memoizing the
271+
* provider gives the same correctness (different profiles get different providers) while the
272+
* SDK's own credential caching avoids re-reading the profile on every request.
273+
*/
274+
function getMemoizedIniCredentials(profile: string): ReturnType<typeof fromIni> {
275+
let provider = bedrockIniCredentialsCache.get(profile)
276+
if (!provider) {
277+
provider = fromIni({ profile })
278+
bedrockIniCredentialsCache.set(profile, provider)
279+
}
280+
return provider
281+
}
282+
283+
/**
284+
* Derive a stable, secret-free string that captures the CREDENTIAL IDENTITY of a client
285+
* config. Two configs that would authenticate identically produce the same string; any
286+
* difference (different profile, different access key, different api key, default chain)
287+
* produces a different string. We never key on the credential function reference.
288+
*/
289+
function bedrockCredentialIdentity(options: ProviderSettings): string {
290+
if (options.awsUseApiKey && options.awsApiKey) {
291+
// Bearer token auth. Hash the token so the secret never lives in the cache key, but
292+
// distinct tokens still yield distinct clients.
293+
const hash = createHash("sha256").update(options.awsApiKey).digest("hex").slice(0, 16)
294+
return `apiKey:${hash}`
295+
}
296+
if (options.awsUseProfile && options.awsProfile) {
297+
return `profile:${options.awsProfile}`
298+
}
299+
if (options.awsAccessKey && options.awsSecretKey) {
300+
// Access key id is non-secret and uniquely identifies the principal; the session token
301+
// (when present) distinguishes otherwise-identical static credentials. We never include
302+
// the secret access key.
303+
const session = options.awsSessionToken
304+
? createHash("sha256").update(options.awsSessionToken).digest("hex").slice(0, 16)
305+
: ""
306+
return `akid:${options.awsAccessKey}${session ? `:st:${session}` : ""}`
307+
}
308+
return "default-chain"
309+
}
310+
311+
/**
312+
* Compute the full, stable cache key for a BedrockRuntimeClient. Captures everything that
313+
* distinguishes one client from another: region, the resolved endpoint (only when actually
314+
* applied to the config), and the credential identity.
315+
*/
316+
function bedrockClientCacheKey(clientConfig: BedrockRuntimeClientConfig, options: ProviderSettings): string {
317+
const region = clientConfig.region ?? ""
318+
const endpoint = typeof clientConfig.endpoint === "string" ? clientConfig.endpoint : ""
319+
return [`region:${region}`, `endpoint:${endpoint}`, `cred:${bedrockCredentialIdentity(options)}`].join("|")
320+
}
321+
322+
/**
323+
* Test-only escape hatch: clear the process-wide client and credential caches. Production
324+
* code never calls this; the bedrock specs call it in beforeEach so that each test's handler
325+
* construction actually instantiates a fresh (mocked) BedrockRuntimeClient and the
326+
* `new BedrockRuntimeClient` call-count / call-args assertions remain valid.
327+
*/
328+
export function __resetBedrockClientCache(): void {
329+
bedrockClientCache.clear()
330+
bedrockIniCredentialsCache.clear()
331+
}
332+
243333
/************************************************************************************
244334
*
245335
* PROVIDER
@@ -318,11 +408,10 @@ export class AwsBedrockHandler extends BaseProvider implements SingleCompletionH
318408
requestTimeout: 0,
319409
}
320410
} else if (this.options.awsUseProfile && this.options.awsProfile) {
321-
// Use profile-based credentials if enabled and profile is set
322-
clientConfig.credentials = fromIni({
323-
profile: this.options.awsProfile,
324-
ignoreCache: true,
325-
})
411+
// Use profile-based credentials if enabled and profile is set. The fromIni provider
412+
// is memoized per profile so parallel conversations on the same profile don't each
413+
// rebuild a provider that reads ~/.aws config from disk.
414+
clientConfig.credentials = getMemoizedIniCredentials(this.options.awsProfile)
326415
} else if (this.options.awsAccessKey && this.options.awsSecretKey) {
327416
// Use direct credentials if provided
328417
clientConfig.credentials = {
@@ -332,7 +421,19 @@ export class AwsBedrockHandler extends BaseProvider implements SingleCompletionH
332421
}
333422
}
334423

335-
this.client = new BedrockRuntimeClient(clientConfig)
424+
// Pool/reuse BedrockRuntimeClient instances across handlers with an identical client
425+
// configuration (same region, endpoint, and credential identity). The client is safe
426+
// for concurrent use, so sharing one avoids N TCP connection pools across N parallel
427+
// conversations. Everything else on this handler stays per-handler; only the underlying
428+
// client instance is shared.
429+
const cacheKey = bedrockClientCacheKey(clientConfig, this.options)
430+
const cachedClient = bedrockClientCache.get(cacheKey)
431+
if (cachedClient) {
432+
this.client = cachedClient
433+
} else {
434+
this.client = new BedrockRuntimeClient(clientConfig)
435+
bedrockClientCache.set(cacheKey, this.client)
436+
}
336437
}
337438

338439
override async *createMessage(

0 commit comments

Comments
 (0)