aws_s3 source: async receive_messages does not return after poll_secs

[AvihaiSam]

A note for the community

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment

Problem

when using aws_s3 source, the sqs client's receive_messages fn sometimes hangs with no return value.
this causes vector to stop polling from sqs (loop still waiting for a future to end before requesting again)

Configuration

sources:
  s3:
    auth:
      assume_role: ${S3_RO_ROLE}
      region: ${S3_REGION}
    region: ${S3_REGION}
    sqs:
      queue_url: ${SQS_QUEUE_URL}
    type: aws_s3

Version

0.27.0 -> 0.37.0

Debug Output

No response

Example Data

No response

Additional Context

I have made a fork with a PR that sets a timeout on the receive_messages fn, It's probably not the best way to handle it but it's working.
It's possible that updating the SDK might help, I wasn't able to update it though.

References

https://discord.com/channels/742820443487993987/1019686524742271038/threads/1214559445963505734

[jszwedko]

Thanks @AvihaiSam . This seems like it may potentially point to an issue in the AWS SDK. The receive_messages call should wait up to poll_secs (default of 15) to return if there are no messages available. It may be worth trying to reproduce with just the Rust AWS SDK and, if you can reproduce it, file a bug upstream: https://github.com/awslabs/aws-sdk-rust

[andjhop]

Hi @jszwedko. My understanding is that poll_secs only configures the WaitTimeSeconds parameter of the SQS RequestMessage honoured API side. We don't appear to have a way to configure a client request timeout, so
connectivity could be lost at the network level, the client never sees a RST or FIN and stalls. We've seen this a number of times making updates to our CNI.

[AvihaiSam]

I agree with @andjhop
@jszwedko i have opened a ticket at the sdk repo, they have suggested to set an operation_attempt_timeout on client's timeout_config, what do you think of that? It sounds like they implemented that on all types of clients, so it can be a generic config for all AWS sources/sinks for vector, optionally set to null if timeout not wanted.

[jszwedko]

I agree with @andjhop @jszwedko i have opened a ticket at the sdk repo, they have suggested to set an operation_attempt_timeout on client's timeout_config, what do you think of that? It sounds like they implemented that on all types of clients, so it can be a generic config for all AWS sources/sinks for vector, optionally set to null if timeout not wanted.

Good find! I think adding that attempt timeout would make sense to have the client timeout if it receives no response. I agree it could be a generic setting supported by all AWS components. I think it could be configured around here:

vector/src/aws/mod.rs

Lines 198 to 207 in 1b87cce

	let mut config_builder = SdkConfig::builder()
	.http_client(connector)
	.sleep_impl(Arc::new(TokioSleep::new()))
	.identity_cache(auth.credentials_cache().await?)
	.credentials_provider(
	auth.credentials_provider(region.clone(), proxy, tls_options)
	.await?,
	)
	.region(region.clone())
	.retry_config(retry_config.clone());

[jszwedko]

Actually, I see we already have a config option for some components that could maybe be used here: https://vector.dev/docs/reference/configuration/sinks/aws_s3/#request.timeout_secs. We maybe just need to expose that on AWS sources too.

[AvihaiSam]

@jszwedko I will try that, but it states that the default value is 60s, which if was true - i wouldn't have found this issue in the first place... maybe it doesn't apply for any aws-sdk requests...

[AvihaiSam]

well... I get

Configuration error. error=unknown field `request`

when I try to add that:

sources:
  s3:
    ...
    request:
      timeout_secs: 15

so I guess it's not really working...

[andjhop]

I don't think @jszwedko was suggesting that this should already work, rather that this configuration could perhaps be made available for AWS sources as it already is for sinks. The problem I see is that the request configuration already maps closely to Tower middleware which, if I understand correctly, isn't relevant to the aws_s3 source.

[jszwedko]

Right, yeah, sources don't have the same request options that sinks do which, as @andjhop points out, are closely tied to Tower. I think we could just add request.timeout_secs for sources that make requests though (like aws_s3).

[AvihaiSam]

oooh.... I thought I was looking at the source aws_s3...
well, I guess this might work...

[andjhop]

That sounds good, although it seems reasonable that a user might want finer grain control over the settings the SDK client already allows for; here we have the ability to configure the connect timeout, read timeout, operation and operation attempt timeouts. Also considering #15518 which already exposed some of these settings for IMDS requests, introducing imds.connect_timeout_seconds and imds.read_timeout_seconds.

[jszwedko]

Agreed, some of those settings (operation and operation attempt timeouts) are AWS specific so it could make sense to expose them under AWS-specific options like request.aws.operation_timeout. Connect and read timeouts are general enough that we could consider supporting them on any component that makes HTTP requests assuming hyper, the HTTP crate we use, supports them. We could start with just supporting them on AWS components though.