Use flush_thread_count value for queued_chunks_limit_size when queued_chunks_limit_size is not specified #2173
Conversation
This file contains 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
…_chunks_limit_size is not specified Unlimited queued_chunks_limit_size causes unexpected resource consumption when the destination has a problem with smaller flush_interval. The default behaviour should be safe so use flush_thread_count by default. Signed-off-by: Masahiro Nakagawa <[email protected]>
cosmo0920
reviewed
Nov 9, 2018
| @@ -372,7 +372,7 @@ def write(metadata_and_data, format: nil, size: nil, enqueue: false) | |||
| end | |||
|
|
|||
| def queue_full? | |||
| @queued_chunks_limit_size && (synchronize { @queue.size } >= @queued_chunks_limit_size) | |||
| synchronize { @queue.size } >= @queued_chunks_limit_size | |||
There was a problem hiding this comment.
buffer test complains @queued_chunks_limit_sise is nil and not to be able to compare.
Could you check it?
https://travis-ci.org/fluent/fluentd/jobs/452714881#L547
| @@ -354,6 +354,10 @@ def configure(conf) | |||
| log.warn "'flush_interval' is ignored because default 'flush_mode' is not 'interval': '#{@flush_mode}'" | |||
| end | |||
| end | |||
|
|
|||
| if @buffer.queued_chunks_limit_size.nil? | |||
| @buffer.queued_chunks_limit_size = @buffer_config.flush_thread_count | |||
There was a problem hiding this comment.
@i.buffer.queue.size always seems to be assumed 1 in test cases.
Could you check it?
https://travis-ci.org/fluent/fluentd/jobs/452714881#L654
We have smaller queued_chunks_limit_size tests so set larger value, 100 in this patch, is enough for existing tests. Signed-off-by: Masahiro Nakagawa <[email protected]>
|
I forgot to change tests. Just push the fix. |
cosmo0920
approved these changes
Nov 9, 2018
repeatedly
deleted the
change-queued_chunks_limit_size-default-behaviour
branch
christianberg
added a commit
to zalando-incubator/kubernetes-on-aws
that referenced
this pull request
Mar 5, 2019
This increases the number of chunks that can be queued to be sent to S3. The [documentation][1] claims that this number is unlimited when not set, but the default was in fact [recently set][2] to `1`, which causes a backlog of chunks to build up when there is a larger number of log files. [1]: https://docs.fluentd.org/v1.0/articles/buffer-section#buffering-parameters [2]: fluent/fluentd#2173
christianberg
added a commit
to zalando-incubator/kubernetes-on-aws
that referenced
this pull request
Mar 5, 2019
This increases the number of chunks that can be queued to be sent to S3. The [documentation][1] claims that this number is unlimited when not set, but the default was in fact [recently set][2] to `1`, which causes a backlog of chunks to build up when there is a larger number of log files. [1]: https://docs.fluentd.org/v1.0/articles/buffer-section#buffering-parameters [2]: fluent/fluentd#2173 Signed-off-by: Christian Berg <[email protected]>
njuettner
pushed a commit
to zalando-incubator/kubernetes-on-aws
that referenced
this pull request
Mar 7, 2019
This increases the number of chunks that can be queued to be sent to S3. The [documentation][1] claims that this number is unlimited when not set, but the default was in fact [recently set][2] to `1`, which causes a backlog of chunks to build up when there is a larger number of log files. [1]: https://docs.fluentd.org/v1.0/articles/buffer-section#buffering-parameters [2]: fluent/fluentd#2173 Signed-off-by: Christian Berg <[email protected]> Signed-off-by: njuettner <[email protected]>
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.
Unlimited queued_chunks_limit_size causes unexpected resource consumption, e.g. lots of smaller files with file buffer, when
the destination has a problem with smaller flush_interval.
The default behaviour should be safe so use flush_thread_count by default.
Signed-off-by: Masahiro Nakagawa [email protected]