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

Cherry-pick #15590 to 7.6: [Filebeat] Add timeout to GetObjectRequest for s3 input #15899

Merged
merged 1 commit into from
Jan 28, 2020
Merged

Cherry-pick #15590 to 7.6: [Filebeat] Add timeout to GetObjectRequest for s3 input #15899

merged 1 commit into from
Jan 28, 2020

Conversation

kaiyan-sheng
Copy link
Contributor

Cherry-pick of PR #15590 to 7.6 branch. Original message:

Problem we see when using s3 input:
When using s3 input to read logs from S3 bucket, after a while with high amount of logs read: connection reset by peer error showed up. This error is triggered by reader.ReadString function, then processorKeepAlive found it's taking too long to run processMessage, which is longer than half of the set visibility timeout. So changeVisibilityTimeout function keep getting called repeatedly.

This PR is to add timeout into GetObjectRequest API call by using context pattern to implement timeout logic that will cancel the request if it takes too long. This way, after the default timeout 2 minute is hit, this specific S3 object will be skipped, SQS message will return back to the queue later. So Filebeat can try to read it again later.

I decided to add a config option called context_timeout for s3 input because based on your visibility_timeout value, context_timeout can be as large as half of the visibility_timeout. This will allow users to modify both timeout values when using s3 input or filebeat aws module with larger s3 objects or smaller network bandwidth.

closes #15502

* Add timeout to GetObjectRequest which will cancel the request if it takes too long
* Close resp.Body from S3 GetObject API to prevent resource leak
* Change aws_api_timeout to api_timeout

(cherry picked from commit 86c3e63)
Copy link
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

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

LGTM.

@kaiyan-sheng kaiyan-sheng merged commit 6d0836c into elastic:7.6 Jan 28, 2020
@kaiyan-sheng kaiyan-sheng deleted the backport_15590_7.6 branch January 28, 2020 17:51
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