[9.4](backport #7567) bulk: add configurable concurrency limit for ReadSecrets - #7628
Merged
Conversation
* bulk: add max_concurrent_secret_reads config option ReadSecrets makes direct HTTP calls to ES (bypassing the bulk dispatch cap) for each secret reference in an agent checkin. Under high concurrent checkin load this can produce unbounded concurrent ES connections and memory pressure. Add a MaxConcurrentSecretReads config field to ServerBulk, backed by a semaphore.Weighted in the Bulker struct. When set, ReadSecrets acquires a slot before proceeding and releases it on return, bounding the number of simultaneous secret reads fleet-server can perform. Default is 0 (no limit) to preserve existing behaviour. Fleet-controller can set this via server.bulk.max_concurrent_secret_reads in the project config secret. Related: https://github.com/elastic/ingest-dev/issues/8991 * bulk: wire max_concurrent_secret_reads through BulkOpt Add bulkOptT field, WithMaxConcurrentSecretReads BulkOpt constructor, zerolog logging, and BulkOptsFromCfg wiring for the new max_concurrent_secret_reads config option. Related: https://github.com/elastic/ingest-dev/issues/8991 * bulk: enforce concurrent ReadSecrets limit via semaphore ReadSecrets currently bypasses the bulk dispatch cap and makes unbounded concurrent direct HTTP calls to the ES Fleet secrets API. Under the high checkin concurrency seen in large serverless projects this causes both memory pressure and ES connection exhaustion. Add a readSecretsLimit *semaphore.Weighted to Bulker, initialized from the new max_concurrent_secret_reads config option (0 = no limit). ReadSecrets acquires one slot before performing secret reads and releases it on return, matching the pattern already used by apikeyLimit. Related: https://github.com/elastic/ingest-dev/issues/8991 * bulk: fix goimports: restore constant alignment, set default to 32 * bulk: set maxConcurrentSecretReads default in parseBulkOpts * config: default max_concurrent_secret_reads to 32 * bulk: remove spurious extra space in constant comment * bulk: reuse defaultAPIKeyMaxParallel for readSecretsLimit default * bulk: reuse defaultAPIKeyMaxParallel for readSecretsLimit default * bulk: always initialize readSecretsLimit, matching apikeyLimit pattern * bulk: acquire readSecretsLimit per secret call, matching apikeyLimit granularity * bulk: change maxConcurrentSecretReads to int, matching apikeyMaxParallel * config: use named constant and int type for MaxConcurrentSecretReads * bulk: add comment on readSecretsLimit acquire * bulk: trim acquire comment * changelog: add fragment for ReadSecrets concurrency limit * bulk: add unit tests for ReadSecrets concurrency limit * fix: reorder imports in engine.go and simplify changelog fragment Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * bulk: fix zero-value readSecretsLimit and add own default constant When maxConcurrentSecretReads is 0 (documented as "no limit"), NewBulker was creating a zero-capacity semaphore, causing all ReadSecrets calls to block until context cancellation instead of running without a limit. Fix by only initialising readSecretsLimit when the value is > 0, and nil-checking before Acquire/Release in ReadSecrets. Also introduce defaultMaxConcurrentSecretReads as its own constant (32) rather than reusing defaultAPIKeyMaxParallel, so the two limits can evolve independently. Drop the stale comment in config/input.go that referenced the apikey constant. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * bulk: remove max_concurrent_secret_reads from user-facing config apikeyMaxParallel is not user-configurable, so secret reads should follow the same pattern. Remove MaxConcurrentSecretReads from ServerBulk, drop the corresponding config constant, and remove the WithMaxConcurrentSecretReads wiring from parseBulkOptsFromConfig. The limit is now always defaultMaxConcurrentSecretReads (32). Update the changelog to drop the mention of a config option. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * bulk: add comment explaining nil readSecretsLimit for zero value Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test: add X-Elastic-Product header to blockingTransport responses go-elasticsearch performs a product check on the first request and requires the X-Elastic-Product: Elasticsearch response header. Without it, ReadSecrets calls that actually reach the transport fail with "the client noticed that the server is not Elasticsearch". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> (cherry picked from commit 5bef901)
8 tasks
ycombinator
approved these changes
Aug 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is the problem this PR solves?
ReadSecretsmakes direct HTTP calls to the ES Fleet secrets API (one per secret reference per agent checkin) without going through the bulk dispatch queue. This means it is not subject to themax_pending_bulk_dispatchescap introduced in #6751. Under high concurrent checkin load — as seen in large serverless projects — each checkin goroutine issues an independent ES connection, causing unbounded concurrent ES connections and additional memory pressure (goroutines, HTTP buffers, response allocations).This was observed as a contributing factor in an OOM incident for a large production serverless security project. PR #7416 added the
ReadSecretsper-checkin call ~24h before OOMs began.How does this PR solve the problem?
Adds a
semaphore.Weighted(readSecretsLimit) toBulker, capping concurrent in-flight secret reads at 32 (matchingapikeyLimit).ReadSecretsacquires a slot before each ES call and releases it immediately after — callers that arrive when all slots are taken block until one is free or their context is cancelled. SettingreadSecretsLimittonil(by passingWithMaxConcurrentSecretReads(0)) disables the cap entirely; the default is always 32.The limit is intentionally not exposed as a user-facing config option, consistent with how
apikeyMaxParallelis handled.Files changed
internal/pkg/bulk/engine.go: addreadSecretsLimit *semaphore.WeightedtoBulker; initialize inNewBulkerwhen limit > 0; nil-check acquire/release inReadSecrets; adddefaultMaxConcurrentSecretReads = 32internal/pkg/bulk/opt.go: addmaxConcurrentSecretReadsfield,WithMaxConcurrentSecretReadsBulkOpt, zerolog logginginternal/pkg/bulk/secret_limit_test.go: unit tests covering the concurrency limit, context cancellation while waiting, zero-value (no limit), and default capacityHow to test this PR locally
Design Checklist
Checklist
./changelog/fragmentsusing the changelog toolRelated issues
This is an automatic backport of pull request bulk: add configurable concurrency limit for ReadSecrets #7567 done by Mergify.