Drop new logs once the queue holds maxQueueSize of them - #32
Merged
Merged
Conversation
The tests use the new maxQueueSize option, so this commit does not compile yet. With only the property added, all 8 logs written while a batch is stuck are queued and delivered, nothing is reported, and maxQueueSize="0" is accepted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The drain queued without limit, so an application whose endpoint is down grew until it was killed. New target option maxQueueSize (100000 by default) bounds the queue like the Java client does: a full queue drops new logs and reports the first one in NLog's internal log, again for the next overflow once the drain has taken logs off the queue. The length is an Interlocked counter, since ConcurrentQueue.Count walks the segments. maxQueueSize below 1 is a configuration error. Drain keeps its constructor and gets an overload that takes the limit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PetrHeinz
marked this pull request as ready for review
September 30, 2026 15:37
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Drainqueues logs without limit. While the endpoint is down, a busy application keeps growing (the red-team run measured about 600 MB RSS at 300k queued logs) until it is killed, and then everything queued is lost anyway.This bounds the queue the way our Java client does with
maxQueueSize:maxQueueSize, 100000 by default. Once the queue holds that many logs, new logs are dropped; nothing already queued is removed.Interlockedcounter kept on enqueue and dequeue, notConcurrentQueue.Count, which walks the segments.maxQueueSizebelow 1 is reported asNLogConfigurationException. The check sits at the top ofInitializeTargetrather than next to the checks of Validate sourceToken and endpoint, accept a bare ingesting host #22 and Reject maxBatchSize and flushPeriodMilliseconds below 1 and negative retries #28, so that this PR merges with them without conflicts.Drainis public API: its constructor keeps its signature, and an overload takesmaxQueueSize. The existing constructor uses the same default of 100000.This changes behaviour for existing users: the queue used to be unlimited, now it holds at most 100000 logs by default. An application that logs faster than the endpoint takes its logs for long enough now loses the newest logs instead of growing until it runs out of memory.
The first commit adds the tests and does not compile, since they use the new option. With only the property added (checked locally), the 8 logs written while a batch waits for its retry are all queued and delivered, nothing reaches the internal log, and
maxQueueSize="0"is accepted. The second commit bounds the queue.The tests restore NLog's internal log settings in
Disposewith the same lines #20 adds, so the two merge cleanly in either order.🤖 Generated with Claude Code