ISSUE-326 Expose request timeout configuration for retrieving datafile

[GordonSo]

Summary

• Offer the ability to customise request timeout in ConfigManager.

To have a more predictable duration on experiment variant retrieval duration.

Test plan

Same mimic'd test from blocking_timeout

Fixes #326

[GordonSo]

The travis CI build is only recognising branch against the optimizely org (which I have no write access to); if somebody with great power push it into a branch under the optimizely and open a PR, it will unblock the pipeline/ tests.

NB: cliche might it sound, but the tests are passing on my machine 😆

[jaeopt]

@GordonSo Thanks for your contribution. We'll take a look at it.
Can you sign our CLA if not done yet: https://github.com/optimizely/swift-sdk/blob/master/CONTRIBUTING.md

[Mat001]

Please check that all appropriate instances of REQUEST_TIMEOUT have been replaced with DEAFAULT_REQUEST_TIMEOUT. I thunk they are but pls double check yourself.

[Mat001]

Need to update the docstring (comment) mentioning request_timeout.

[GordonSo]

Thanks for the comments, REQUEST_TIMEOUT renamed was done via refactoring into DEAFAULT_REQUEST_TIMEOUT and I have double-checked for any reminding presence with "find all".

docstring is now updated.

[GordonSo]

@GordonSo Thanks for your contribution. We'll take a look at it.
Can you sign our CLA if not done yet: https://github.com/optimizely/swift-sdk/blob/master/CONTRIBUTING.md

Cheers, I have signed.

[GordonSo]

Please let me know if there is anything else 🙏

[Mat001]

Hey @GordonSo
Looking into this.
We expect you want to set the request timeout for initial datafile fetch. But do you care about the request time for background polling? The initial fetch is more time sensitive, the background polling is managed by itself pretty much after the initial fetch.

your suggested change would impact request timeout across all the polling not just the initial fetch, so just wanted to clarify what is your need/intent.

[GordonSo]

Hey @GordonSo
Looking into this.
We expect you want to set the request timeout for initial datafile fetch. But do you care about the request time for background polling? The initial fetch is more time sensitive, the background polling is managed by itself pretty much after the initial fetch.

your suggested change would impact request timeout across all the polling not just the initial fetch, so just wanted to clarify what is your need/intent.

Hey @Mat001 ,

Thanks for the clarifying question and ensuring that my PR is for the right purpose.

Our system is built with lambdas with a short timeout (currently 6 seconds), they will create a Redis connection to lock some resources, process the triggering events [publish some more events] and then release the lock. As we recently started to integrate with Optimizely to run A/B tests, and we would like to ensure that the handling of events will not be delayed from some sporadic bad network connections between our host the Optimizely API server. So we are trying to expose the timeout settings, to prevent the number of race condition/ eventual consistency.

Please check my understanding about the library: the "blocking_timeout" setting will already serve the purpose of limiting the timeout on client creation (where it retrieves the "meta" experiment/variant information). The client is then used to create the 'user_context' which allow us to get "decisions" (variant) about an experiment. Our concern is that the "decision" will hold up the process by using the 'default of 10 seconds (from the "request_timeout" setting that we found when navigating into the client configs).

You are correct that background polling is not so important to us, in fact, in the context of lambdas, and this is interesting because it happens under a separate thread whilst the main (lambda) thread may be terminated when the function finishes, and then the container can be reused again (along with the previously created client) by another invocation... but this is a separate question that we are seeking advice from our Optimilzely TAM.

Sorry for the wordy reply, I hope this clarifies our intent; and will appreciate your knowledge sharing and recommendations.

[jaeopt]

@GordonSo Thanks for clarification.
I think what you want is to set "blocking_timeout" to your tighter limit.

The Optimizely SDK won't hold CPU long (~msecs) once datafile fetch is completed. When network connection fails, if you set "blocking_timeout" less than the default "request_timeout", then decide API will return fallback on "blocking_timeout". Datafile fetching will continue in a thread, and dropped at the request timeout or update config successfully.

[GordonSo]

@GordonSo Thanks for clarification.
I think what you want is to set "blocking_timeout" to your tighter limit.

The Optimizely SDK won't hold CPU long (~msecs) once datafile fetch is completed. When network connection fails, if you set "blocking_timeout" less than the default "request_timeout", then decide API will return fallback on "blocking_timeout". Datafile fetching will continue in a thread, and dropped at the request timeout or update config successfully.

Thank you! We shall go with that existing parameter under load test and will revisit if we counter any problems.