[Filebeat] Add timeout to GetObjectRequest for s3 input

[kaiyan-sheng]

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

[exekias]

I left a few comments, I understand you are still working on testing this one, right?

[jsoriano]

I see it is in progress, but I was taking a look and I have a couple of questions.

The new context inputCtx with its goroutine for cancelation doesn't smell well to me, why wasn't the previous p.context enough?

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.

Does it mean that for big objects that can take more than 2 minutes to download, the timeout is hit and then Filebeat retries with the same object? Does filebeat keep the last offset or something so it doesn't keep continuously retrying?

[kaiyan-sheng]

The new context inputCtx with its goroutine for cancelation doesn't smell well to me, why wasn't the previous p.context enough?

Yes, p.context is enough for this. I just made the change in this PR to used the existing p.context.

Does it mean that for big objects that can take more than 2 minutes to download, the timeout is hit and then Filebeat retries with the same object? Does filebeat keep the last offset or something so it doesn't keep continuously retrying?

@jsoriano I haven't seen any big objects request more than even 1 minute to download. I think the problem seeing in the issue is caused by resource leak from not having defer resp.Body.Close() in the code. The connection reset error happened when making GetObjectRequest API call which is one step before actual reading the log file. So if that failed, the SQS message goes back into the queue and the same S3 object will be retried with GetObjectRequest later after visibility timeout is done.

[jsoriano]

The connection reset error happened when making GetObjectRequest API call which is one step before actual reading the log file. So if that failed, the SQS message goes back into the queue and the same S3 object will be retried with GetObjectRequest later after visibility timeout is done.

Oh ok, so then this timeout and the retries don't affect the actual download of the log file, right? If that is the case then it LGTM.

Thanks for addressing all the changes!

[ycombinator]

@kaiyan-sheng Since the issue this fixes is labeled as a bug, should this fix be backported to 7.5 (in case there's a 7.5.3 before 7.6.0) as well? I see you did this for #15844.

[kaiyan-sheng]

@ycombinator Yes I agree, just in case if there's a 7.5.3. I will create the backport right now. Thank you!