Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clarify the error message when log_iw_size is too small #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

clarify the error message when log_iw_size is too small #1

wants to merge 1 commit into from

Conversation

burr86
Copy link

@burr86 burr86 commented Jun 27, 2013

It turns out that this error is resolved only by setting log_iw_size
and max_connections. Make the error message reflect that, and provide
guidance as to the right settings to adjust.

It turns out that this error is resolved only by setting log_iw_size
and max_connections. Make the error message reflect that, and provide
guidance as to the right settings to adjust.
@algernon
Copy link
Contributor

This looks good to me, but I'd like an ACK from @bazsi, before I merge it.

@bazsi
Copy link
Collaborator

bazsi commented Jun 27, 2013

Hmm... it is log_fifo_size() that needs adjusting. Since we increased log_iw_size() automatically, it may happen
that the originally set log_fifo_size() is too small to hold log_iw_size() elements from all sources that feed that destination.

I intend to make window/fifo sizing automatic, but we are not there yet :( Defaults are usually ok, finetuning now requires understanding the syslog-ng flow-control behaviour (window on source side, fifo on destination side; fifo must be large enough to hold log_iw_size() elements from all its sources).

@bazsi
Copy link
Collaborator

bazsi commented Jun 27, 2013

all in all, the last sentence should not be removed, adjusting log_iw_size() can only make the warning go away, not the problem itself.

@burr86
Copy link
Author

burr86 commented Jun 27, 2013

I think I understand what you're saying, but it's confusing, because the section of the config that throws the warning (the sources) isn't the section of the config that you would adjust to fix the problem. Ultimately I'm hoping to make the warning offer clearer guidance to a resolution. I couldn't find any other way besides adjusting log_iw_size() ... are you saying that we should both adjust log_iw_size(), but beyond that, log_fifo_size() needs to change too? Can we check for both and warn against both?

@burr86
Copy link
Author

burr86 commented Jun 27, 2013

Alternately: maybe these are two different issues? Fixing the actual math/warning might be separate from proper window tuning?

@bazsi
Copy link
Collaborator

bazsi commented Jun 28, 2013

it would be much more to check if log_fifo_size is actually missized and report on that and suggest fixes.

once we have that we can probably continue with automatic sizing as well.

@burr86
Copy link
Author

burr86 commented Jun 28, 2013

I guess I don't understand how we can know that, at this point in the code -- we're just analyzing sources, here, not destinations also?

@bazsi
Copy link
Collaborator

bazsi commented Jun 28, 2013

that's exactly the difficulty. in order to provide the most relevant warning we would need to analyze both sides.

@abalage
Copy link

abalage commented Nov 17, 2016

Copy link

@PaulAnton PaulAnton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an explanation of how to solve it would be helpful. I had to do a couple of tests to figure things out.
"tcp windows size per connection=(log_iw_size)/(max-connections). And that a arbitary value for log_iw_size can be mentioned in the source definition where max-connections is mentioned Eg: 'max-connections(5000) log_iw_size(500000)' means a tcp window size per conection is 100 bytes. If it is equal to or more than 100 bytes there won't be any warnings"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants