Open adriantam opened 1 year ago
Are you setting timeouts in the context that's passed to DAX SDK? If you are explicitly passing in a context, it should look like below if you want to configure 50 ms timeout:
ctxTimeout, cancel := context.WithTimeout(context.Background(), time.Millisecond*50)
client.GetItemWithContext(ctxTimeout, in)
When context is passed to the SDK, we will honour the timeout that's set in the context and will not consider the RequestTimeout
field value which was set during client initialisation.
That's why in the below code, the request timeout is overridden if context is provided - https://github.com/aws/aws-dax-go/blob/264dfde337bc592b0ada5a8c587f14f610cd922a/dax/service.go#L163
I executed couple of tests to validate this behaviour and it works as expected:
I set the RequestTimeout
configuration as 10 ms and while calling GetItemWithContext
I provided context.Background()
(infinite timeout)
I set the RequestTimeout
configuration as 5 seconds and while calling GetItemWithContext
, passed in context with 50 ms as timeout - context.WithTimeout(context.Background(), time.Millisecond*50)
Are you setting timeouts in the context that's passed to DAX SDK? If you are explicitly passing in a context, it should look like below if you want to configure 50 ms timeout:
ctxTimeout, cancel := context.WithTimeout(context.Background(), time.Millisecond*50) client.GetItemWithContext(ctxTimeout, in)
When context is passed to the SDK, we will honour the timeout that's set in the context and will not consider the
RequestTimeout
field value which was set during client initialisation.That's why in the below code, the request timeout is overridden if context is provided -
https://github.com/aws/aws-dax-go/blob/264dfde337bc592b0ada5a8c587f14f610cd922a/dax/service.go#L163
I executed couple of tests to validate this behaviour and it works as expected:
I set the
RequestTimeout
configuration as 10 ms and while callingGetItemWithContext
I providedcontext.Background()
(infinite timeout)
- There were no timeouts and we would see high latencies (note: simulated high server-side latencies by manually adding sleep on server-side)
![]()
I set the
RequestTimeout
configuration as 5 seconds and while callingGetItemWithContext
, passed in context with 50 ms as timeout -context.WithTimeout(context.Background(), time.Millisecond*50)
- I saw that client-side latencies were capped at 50 ms
![]()
We did not set the timeout in the context that we passed into the QueryWithContext request.
When we call the API, we do something like
cfg := dax.DefaultConfig()
cfg.RequestTimeout = 75 * time.Millisecond
client, err := dax.New(cfg)
...
ctx := context.Background()
// some code to add in our own context info
output, err := client.QueryWithContext(ctx, input, opts)
There was nothing in the documentation to suggest that Config's RequestTimeout (i.e., https://github.com/aws/aws-dax-go/blob/264dfde337bc592b0ada5a8c587f14f610cd922a/dax/service.go#L49) will only be used if there is no context provided.
Why have the RequestTimeout
config if the context dominates, especially if context.Background()
overrides the top-level config setting?
Perhaps I may suggest that RequestTimeout
is just dropped altogether in favor of the more idiomatic Go style of providing solely the context with timeout.
Thank you for the feedback! We'll update the documentation in the next release to call out that the top-level configs are not honoured if the context is set.
Perhaps I may suggest that RequestTimeout is just dropped altogether in favor of the more idiomatic Go style of providing solely the context with timeout.
That's a good suggestion but unfortunately if we make that change now, the change will be backwards incompatible.
That's a good suggestion but unfortunately if we make that change now, the change will be backwards incompatible.
@anudeeb looking forward to github.com/aws/aws-dax-go/v2
😄 !
We have set the RequestTimeout to 75ms in our DAX client. However, when TCP connection is re-establshed, we still see some DAX request in the 6s range (which based on our discussion with AWS team are caused by connection being re-established).
Looking at the Go SDK, it appears that the issue is due to SDK not respecting the request timeout if it is called with context.
See https://github.com/aws/aws-dax-go/blob/264dfde337bc592b0ada5a8c587f14f610cd922a/dax/service.go#L163
Please note that we use QueryWithContext as per https://pkg.go.dev/github.com/zabawaba99/aws-dax-go/dax#Dax.QueryWithContext.