awslabs / aws-sdk-rust

AWS SDK for the Rust Programming Language
https://awslabs.github.io/aws-sdk-rust/
Apache License 2.0
2.91k stars 245 forks source link

Timestream's `ReloadEndpoint` is unclear #1124

Open simbleau opened 2 months ago

simbleau commented 2 months ago

Describe the issue

I've made good attempts to understand how Timestream's ReloadEndpoint works, which doesn't seem to documented well. It still doesn't make sense to me, unfortunately.

The docs.rs of ReloadEndpoint only says "Endpoint reloader."

Here's roughly our code:

    let config = aws_config::defaults(aws_config::BehaviorVersion::latest())
        .region(REGION)
        .load()
        .await;
    let (client, reload) = aws_sdk_timestreamwrite::Client::new(&config)
        .with_endpoint_discovery_enabled()
        .await
        .unwrap();

    // This task will terminate when the corresponding Client is dropped.
    tokio::task::spawn(reload.reload_task());

    ... work with the client here.

My questions are:

Links

https://github.com/awslabs/aws-sdk-rust/blob/29883b151225f7b27a22e4d343585d2dc8207521/sdk/timestreamwrite/src/client.rs#L18-L23

jdisanti commented 2 months ago

What function does the ReloadEndpoint have?

ReloadEndpoint enables you to decide whether you want to do your own endpoint refreshing, or if you want to have the SDK do it for you. It has the reload_once function if you want to do it yourself. Otherwise, you'll want to spawn reload_task.

Why do we need to call .with_endpoint_discovery_enabled()? It will not work without this.

I can't remember exactly why we made this opt-in instead of opt-out, but the general idea is that endpoint discovery makes service calls on your behalf to figure out what the endpoint is. So this is a bit more explicit that it's doing that.

It seems to work without me spawning the reload task - What does that task really do then?

I think .with_endpoint_discovery_enabled() will make it do an initial load, and then it needs to be refreshed later. Although, perhaps, it might just work for a while without refreshing. I don't think there's any documented expiration time on there. The SDK refreshes it every 60 seconds if using reload_task.

jdisanti commented 2 months ago

Leaving this open to track improving documentation.

rcoh commented 2 months ago

This can now be improved with the SRA I think. When the code was written, resolve endpoint was synchronous (hence the need for background refresh).

simbleau commented 2 months ago

Thanks John, appreciate the quick reply.

ReloadEndpoint enables you to decide whether you want to do your own endpoint refreshing, or if you want to have the SDK do it for you. It has the reload_once function if you want to do it yourself. Otherwise, you'll want to spawn reload_task.

I'm still not understanding what endpoint refreshing is and why it's necessary. Is the endpoint just the correct place to talk to Timestream?

I found https://docs.aws.amazon.com/general/latest/gr/timestream.html - I assume it is related. However in this documentation, it seems like the endpoint is constant. What does reloading the endpoint accomplish?

jdisanti commented 2 months ago

Endpoint discovery is a more general SDK feature. I'm not so familiar with Timestream, so I can't say one way or another if the endpoints change, but DynamoDB also uses endpoint discovery, and I think the endpoints can change there. Or at the very least, they're allowed to change, and thus, must be refreshed periodically.