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.

Sign up for GitHub

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

Jump to bottom

Open connections through rntbd #28470

Merged
merged 22 commits into from
May 18, 2022
Merged

Open connections through rntbd #28470

merged 22 commits into from
May 18, 2022

Conversation

xinlian12
Copy link
Member

@xinlian12 xinlian12 commented Apr 22, 2022 •
edited
Loading

Task: #28267

Description
Currently we have the api on container openConnectionsAndInitCaches , which leverage dummy query on all feed ranges to open connection , with retry capability to increase the chances of touching max replica

Problem with current approach

  • It requires unnecessary query plan request to gateway
  • No guarantee to touch all secondary replicas
  • Do not open connection to primary
  • Expensive in latency and RU

Changes:

  1. Instead of using dummy query, using the Rntbd context negotiation to finish the connection establishment
  2. Getting partition key range addresses with batches

High level workflow:
image

Major change in Rntbd layer:
Normal request flow:
image

Open request flow:
image

BenchmarkTests:Same region, Document Count 100,000, RU configured 100,000, 8 CPU core
P95 Latency:
image

P99 Latency:
image

P999 Latency:
image

Throughput:
image

@ghost ghost added the Cosmos label Apr 22, 2022
@azure-sdk
Copy link
Collaborator

azure-sdk commented Apr 22, 2022 •
edited
Loading

API change check for com.azure:azure-cosmos

API changes have been detected in com.azure:azure-cosmos. You can review API changes here

API changes

-         @Beta public Mono<Void> openConnectionsAndInitCaches() 
+         @Deprecated @Beta public Mono<Void> openConnectionsAndInitCaches() 
+         @Beta public Mono<String> openConnectionsAndInitializeCaches() 
-         @Beta public void openConnectionsAndInitCaches() 
+         @Deprecated @Beta public void openConnectionsAndInitCaches() 
+         @Beta public String openConnectionsAndInitializeCaches() 
-             V4_28_0;
+             V4_28_0,
+             V4_29_0;

@xinlian12 xinlian12 changed the title OpenAsyncThroughRntbd Open connections through rntbd Apr 22, 2022
@xinlian12 xinlian12 marked this pull request as ready for review April 22, 2022 16:54
@milismsft
Copy link
Contributor

awesome charts, wonder if we can capture all these in a "Doc" folder. I remember we had one, can't find it anymore. @kushagraThapar ?

@xinlian12
Copy link
Member Author

awesome charts, wonder if we can capture all these in a "Doc" folder. I remember we had one, can't find it anymore. @kushagraThapar ?
yea, we have a docs folder in our repo now.
And if the PR got approved, I will add the flow chart under the docs as well

xinlian12
xinlian12 commented May 5, 2022
View reviewed changes
...rc/test/java/com/azure/cosmos/implementation/directconnectivity/GatewayAddressCacheTest.java Outdated
null, null);
null,
null,
ConnectionPolicy.getDefaultPolicy(),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: indent

FabianMeiswinkel
FabianMeiswinkel approved these changes May 11, 2022
View reviewed changes
Copy link
Member

@FabianMeiswinkel FabianMeiswinkel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Annie - very clean, good PR description - looks very good.

@azure-sdk
Copy link
Collaborator

API change check

APIView has identified API level changes in this PR and created following API reviews.

azure-cosmos

@xinlian12
Copy link
Member Author

/azp run java - cosmos - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@xinlian12
Copy link
Member Author

/azp run java - cosmos - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@xinlian12
Copy link
Member Author

xinlian12 commented May 18, 2022 •
edited
Loading

Tested locally:
splitQueryContinuationToken
queryDocumentsWithMultiOrder
createItem_withBulkAndThroughputControl
orderbyContinuationOnUndefinedAndNull

@xinlian12
Copy link
Member Author

/check-enforcer evaluate

@xinlian12
Copy link
Member Author

/check-enforcer override

@xinlian12 xinlian12 merged commit ba054b7 into Azure:main May 18, 2022
This was referenced May 18, 2022
Merged
Merged
anushkasingh16 pushed a commit to anushkasingh16/azure-sdk-for-java that referenced this pull request Jun 6, 2022
@xinlian12 @anushkasingh16
Open connections through rntbd (Azure#28470)
308d7d7 
* openAsync through rntbd

Co-authored-by: annie-mac <annie-mac@annie-macs-MacBook-Pro.local>
Co-authored-by: annie-mac <annie-mac@XBX-2505-B09E.redmond.corp.microsoft.com>
Co-authored-by: annie-mac <annie-mac@yindeng2019.fareast.corp.microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@j82w j82w j82w left review comments

@FabianMeiswinkel FabianMeiswinkel FabianMeiswinkel approved these changes

@kushagraThapar kushagraThapar Awaiting requested review from kushagraThapar kushagraThapar is a code owner

@kirankumarkolli kirankumarkolli Awaiting requested review from kirankumarkolli kirankumarkolli is a code owner

@mbhaskar mbhaskar Awaiting requested review from mbhaskar

@simplynaveen20 simplynaveen20 Awaiting requested review from simplynaveen20

@milismsft milismsft Awaiting requested review from milismsft milismsft is a code owner

@aayush3011 aayush3011 Awaiting requested review from aayush3011 aayush3011 is a code owner

Assignees
No one assigned
Labels
Cosmos
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@xinlian12 @azure-sdk @milismsft @j82w @FabianMeiswinkel