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

Logstash client window size enhancement #598

Merged
merged 1 commit into from
Dec 30, 2015

Conversation

urso
Copy link

@urso urso commented Dec 29, 2015

  • fix an error with window-sizes of 1 never increasing.
  • Have window size converge towards batch size requested to be published. If
    maximum batch size ever published is less then maximum allowed window size.
  • Always shrink window size on error. If connection was lost due to logstash
    closing it by internal timeouts (circuite breaker) we want to decrease
    the window size on next try.

@@ -71,6 +72,16 @@ func newLumberjackClient(
}
}

func (l *lumberjackClient) Connect(timeout time.Duration) error {
Copy link
Member

Choose a reason for hiding this comment

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

Trying to follow how this change is related to the other changes? Is this Method used? Same for close.

Copy link
Author

Choose a reason for hiding this comment

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

Connect and Close are used by output/modes module. Just adding some debug here to better see how window size changes and reconnects/errors happen (as these are related). We re-connect on failure + we adapt window sizes on error.

Copy link
Member

Choose a reason for hiding this comment

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

👍

@ruflin
Copy link
Member

ruflin commented Dec 30, 2015

  • Could you update the CHANGELOG?
  • Is it possible to add a simple test to confirm this "fix"?
  • Should we pick this into 1.1?

- Have window size converge towards batch size requested to be published.  If
  maximum batch size ever published is less then maximum allowed window size.

- Always shrink window size on error. If connection was lost due to logstash
  closing it by internal timeouts (circuite breaker) we want to decrease
  the window size on next try.

- Add unit tests for logstash client window sizing behavior

- Add changelog entry
@urso urso force-pushed the enh/logstash-output-win-size branch from bc6065b to b9ac89c Compare December 30, 2015 13:38
ruflin added a commit that referenced this pull request Dec 30, 2015
Logstash client window size enhancement
@ruflin ruflin merged commit f4b25bb into elastic:master Dec 30, 2015
@urso urso deleted the enh/logstash-output-win-size branch January 11, 2016 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants