Repository navigation
Raise default ingest consumer concurrency to 8 loops per process #SUPERLOG - #530
Closed
superlog-app[bot] wants to merge 1 commit into
Closed
superlog-app[bot] wants to merge 1 commit into
superlog-app[bot] wants to merge 1 commit into
Conversation
…ERLOG Delivery-Id: c4d698a9bcf07fd4d191240de5021ec52640e8ffaaab5b1ba958addc6d12ccfc Delivery-Base: main
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
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.
Summary
The ingest queue
message.age_mshistogram averaged 212 s (~3.5 min) during the Oct 9 incident, a 3.16× spike over the 67 s baseline. Delivery duration rose only 5%, ruling out ClickHouse slowness — the extra latency was pure queue-wait time caused by the consumer not draining fast enough.Root cause
Each consume loop follows a blocking batch pattern: it calls
ReceiveMessage(MaxNumberOfMessages=10), awaitsPromise.all(processMessage × 10), then deletes and loops. This means throughput per loop is bounded bybatchSize / per-batch-insert-time = 10 / 2.5 s = 4 msg/s. With the previous default of 4 concurrent loops per process, one process caps at 16 msg/s, and any task disruption (rolling deploy, OOM kill) reduces capacity proportionally with no headroom.The
ClickHouseIngestWritercoalesces all concurrentinsert()calls within abatchLingerMswindow into one ClickHouse INSERT, so raising the loop count from 4 to 8 doesn't increase the number of ClickHouse inserts — it doubles how many SQS messages are coalesced into each insert, doubling per-task throughput (80 messages / 2.5 s = 32 msg/s per process).Fix
DEFAULT_CONSUMER_CONCURRENCYfrom4to8..env.exampleto match.INGEST_QUEUE_CONSUMER_CONCURRENCYis not set (the existing bounds test at≥ 32 → 32is unchanged).Deployment-side environments that set
INGEST_QUEUE_CONSUMER_CONCURRENCYexplicitly are unaffected by this default change.Was this PR helpful? Leave feedback — goes straight to the Superlog team.
Summary by cubic
Raises the default ingest consumer concurrency from 4 to 8 SQS receive loops per process so the queue drains fast enough under load. During the Oct 9 incident, ingest queue message age averaged 3.5 minutes (3.16× the 67 s baseline) while delivery duration rose only 5%, showing the bottleneck was queue-wait time, not ClickHouse.
ClickHouseIngestWriteralready coalesces concurrent inserts within abatchLingerMswindow, so doubling the loops doubles the messages coalesced per insert rather than adding ClickHouse insert overhead. Deployments that setINGEST_QUEUE_CONSUMER_CONCURRENCYexplicitly keep their current value; the change also updates.env.exampleand adds a test asserting the new default.Written for commit 4a8ecb0. Summary will update on new commits.