added explanation around BUFFER_QUEUE_LIMIT variable#10201
Conversation
openshift-ci-robot
added
the
size/M
There was a problem hiding this comment.
This is a terminology issue, so I might be wrong. But when I hear an "ElasticSearch node", I have an impression the ElasticSearch is running on a node, not in a container... In the Common Logging EFK stack, each component is containerized. In such case, how do you usually name it?
There was a problem hiding this comment.
Thanks @nhosoi for looking at this. I will update this to pod. Which is consistent with rest of our docs.
There was a problem hiding this comment.
This number_of_outputs is a bit tricky. :) Not just the 1, 2, 3, but there could be more combinations.
Single ElasticSearch for all -- 1 output
One ElasticSearch for ops and another for application logs -- 2 outputs
One ElasticSearch for all and forward them to other Fluentd -- 2 outputs
One ElasticSearch for ops and another for application logs plus forward one of the outputs to other Fluentd -- 3 outputs
One ElasticSearch for ops and another for application logs plus forward the both outputs to other Fluentd -- 4 outputs
Please note that I'm not suggesting to list up all of them, but I'd rather think 2 3 is not needed. Or do you have any good idea to explain this? Thanks!
|
@nhosoi I have updated the PR to address your comments. PTAL 🔢 |
|
@gaurav-nelson , thanks for the updates. |
gaurav-nelson
added
peer-review-needed
kalexand-rh
left a comment
kalexand-rh
left a comment
There was a problem hiding this comment.
Couple of suggestions to consider and a few nits. This sounds good!
There was a problem hiding this comment.
s/resource/resource,
s/will be/is
There was a problem hiding this comment.
I'd reformat the list entries: s/- If/if
I'd also remove the ending periods.
There was a problem hiding this comment.
Maybe "If application logs are sent to an ElasticSearch pod, ops logs are sent to
another ElasticSearch pod, and all logs are then forwarded to other Fluentd instances"
There was a problem hiding this comment.
Are these the default values? I'd say "By default, these parameters are assigned the following values:"
I'd move lines 89-95 to line 80, right after you present the formula.
There was a problem hiding this comment.
s/The value of buffer_queue_limit will be 32.//
s/FILE_BUFFER_LIMIT./FILE_BUFFER_LIMIT value or the number of outputs.
|
New changes are detected. LGTM label has been removed. |
|
@kalexand-rh I have made suggested changes. Except one, haven't changed the following: |
|
Sounds good to me! Thanks @gaurav-nelson! |
gaurav-nelson
added
peer-review-done
|
/cherrypick enterprise-3.7 |
|
/cherrypick enterprise-3.9 |
|
/cherrypick enterprise-3.10 |
|
@gaurav-nelson: new pull request created: #10798 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@gaurav-nelson: new pull request created: #10799 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@gaurav-nelson: new pull request created: #10800 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
5 participants
For https://bugzilla.redhat.com/show_bug.cgi?id=1588430