Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cosmos Client Builder overrides direct connection config idleConnectionTimeout with gateway connection config #12971

Closed
kushagraThapar opened this issue Jul 9, 2020 · 7 comments
Assignees
Labels
cosmos:v4-item Indicates this feature will be shipped as part of V4 release train Cosmos
Milestone

Comments

@kushagraThapar
Copy link
Member

This is a bug which popped up in this ICM: https://portal.microsofticm.com/imp/v3/incidents/details/194516399/home

In the below code ->

if (this.directConnectionConfig != null) {

    this.connectionPolicy = new ConnectionPolicy(directConnectionConfig);

    //  Check if the user passed additional gateway connection configuration

    if (this.gatewayConnectionConfig != null) {

        this.connectionPolicy.setMaxConnectionPoolSize(this.gatewayConnectionConfig.getMaxConnectionPoolSize());

        this.connectionPolicy.setRequestTimeout(this.gatewayConnectionConfig.getRequestTimeout());


       //  gateway config overrides direct config right here
        this.connectionPolicy.setIdleConnectionTimeout(this.gatewayConnectionConfig.getIdleConnectionTimeout());

    }

If both directConnectionConfig and gatewayConnectionConfig are present, then idleConnectionTimeout will always be overriden by gatewayConnectionConfig

@ghost ghost added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jul 9, 2020
@kushagraThapar kushagraThapar self-assigned this Jul 9, 2020
@kushagraThapar kushagraThapar added Cosmos cosmos:v4-item Indicates this feature will be shipped as part of V4 release train labels Jul 9, 2020
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jul 9, 2020
@kushagraThapar kushagraThapar added this to the [2020] August milestone Jul 9, 2020
@allenhumphreys
Copy link

Excellent! Thanks @kushagraThapar

@kushagraThapar
Copy link
Member Author

This is done and will be released in 4.2.0 release tentatively next week.

@allenhumphreys
Copy link

@kushagraThapar It seems that the requestTimeout suffers the same fate.

@kushagraThapar
Copy link
Member Author

kushagraThapar commented Jul 29, 2020

@allenhumphreys Not really, as requestTimeout is not public API and it is hardcoded internally at 5 second for both direct and gateway connection configs.

@allenhumphreys
Copy link

I see. I didn't realize it wasn't public. Spring properties binding has no problem setting the values 😬. So public or private, I have set the value to 2 seconds. Is that inadvisable?

@kushagraThapar
Copy link
Member Author

Hahaha, not really, I think that should be fine - although you are already aware of the fact that requestTimeout only indicates the I/O timeout - so I think 2 seconds should be fine. I don't see a problem with that.

@moderakh - do you see any issue with Allen setting the requestTimeout to 2 seconds ?

@moderakh
Copy link
Contributor

our P99 latency for items of size 1KB is < 10ms however for some corner case scenario (query payload being huge, etc), the server side contract is to respond in 5 seconds, this is the reason the SDK default is 5 seconds.

I don't think this will be hit on the normal path though. You can set it to 2 seconds provided that you are aware of the above.

openapi-sdkautomation bot referenced this issue in AzureSDKAutomation/azure-sdk-for-java Feb 21, 2021
Testing automations on build pipeline (#12971)

* blank file

* Add dummy text
openapi-sdkautomation bot referenced this issue in AzureSDKAutomation/azure-sdk-for-java Feb 22, 2021
Testing automations on build pipeline (#12971)

* blank file

* Add dummy text
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cosmos:v4-item Indicates this feature will be shipped as part of V4 release train Cosmos
Projects
None yet
Development

No branches or pull requests

3 participants