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

Bulk API congestion control #1074

Merged
merged 40 commits into from
Feb 25, 2020
Merged

Bulk API congestion control #1074

merged 40 commits into from
Feb 25, 2020

Conversation

rakkuma
Copy link
Contributor

@rakkuma rakkuma commented Dec 3, 2019 •
edited by abhijitpai
Loading

Congestion control for bulk API

Description

This PR modify the partition key range limiter using few metrics such as throttle count, time taken, documents inserted count.

Logic is equivalent to bulk congestion control with few implementation difference.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Performance improvement.

Collection RU: 300k - 120 seconds

  • Without congestion control: 4.3 million
  • With Congestion control: 4.8 million

Collection RU: 1 Million - 60 seconds

  • Without congestion control: 2.9 million (with 5 as min value in limiter)
  • With Congestion control: 3.5 million

Collection RU: 1 Million - 300 seconds

  • Without congestion control: 12 million (with 5 as min value in limiter)
  • With Congestion control: 16 million

Comparison

Congestion Control RU Documents Time (Sec) RateLimited
True 3K 10K 14 45K
False 3K 10K 14 40K
True 30K 100K 17 16K
False 30K 100K 17 14K
True 50K 200K 21 19.5K
False 50K 200K 21 16.5K
True 100K 1M 60 47K
False 100K 1M 60 25K
True 1M(No Indexing) 1M 16 0
False 1M(No Indexing) 1M 60 0
True 1M(No Indexing) 5M 77 0
False 1M(No Indexing) 5M 350 0

This PR closes #690

@rakkuma
Bulk congestion control changes with perf optimizations
8f0e180 
Signed-off-by: Rakesh Kumar <rakkuma@microsoft.com>
@rakkuma
Resource stream null exception - To be reverted later
f4ea89e 
Signed-off-by: Rakesh Kumar <rakkuma@microsoft.com>
@rakkuma
Fix
fa63dc0 
Signed-off-by: Rakesh Kumar <rakkuma@microsoft.com>
@rakkuma
Merging with master
51c2134 
Signed-off-by: Rakesh Kumar <rakkuma@microsoft.com>
@rakkuma
fix
7a675ca 
Signed-off-by: Rakesh Kumar <rakkuma@microsoft.com>
@rakkuma
fix
b42a8ee 
Signed-off-by: Rakesh Kumar <rakkuma@microsoft.com>
@rakkuma rakkuma changed the title Bulk congestion control changes with perf optimizations Bulk (Batch) API congestion control Dec 10, 2019
@rakkuma rakkuma marked this pull request as ready for review December 10, 2019 15:14
ealsur
ealsur reviewed Dec 10, 2019
View reviewed changes
Microsoft.Azure.Cosmos.Samples/Usage/BulkSupport/Program.cs Outdated Show resolved Hide resolved
Microsoft.Azure.Cosmos.Samples/Usage/BulkSupport/Program.cs Outdated Show resolved Hide resolved
Microsoft.Azure.Cosmos.Samples/Usage/BulkSupport/Program.cs Outdated Show resolved Hide resolved
Microsoft.Azure.Cosmos.Samples/Usage/BulkSupport/Program.cs Outdated Show resolved Hide resolved
Microsoft.Azure.Cosmos.Samples/Usage/BulkSupport/Program.cs Show resolved Hide resolved
Microsoft.Azure.Cosmos/src/Batch/ItemBatchOperationContext.cs Outdated Show resolved Hide resolved
Microsoft.Azure.Cosmos/src/Batch/BatchAsyncContainerExecutor.cs Outdated Show resolved Hide resolved
Microsoft.Azure.Cosmos/src/Batch/BatchAsyncStreamer.cs Outdated Show resolved Hide resolved
Microsoft.Azure.Cosmos/src/Batch/BatchAsyncContainerExecutor.cs Outdated Show resolved Hide resolved
Microsoft.Azure.Cosmos/src/Batch/BatchAsyncContainerExecutor.cs Outdated Show resolved Hide resolved
@rakkuma
Code review fix
2cd6b00 
Signed-off-by: Rakesh Kumar <rakkuma@microsoft.com>
@rakkuma
Fixing program.cs
c3cd1cb 
Signed-off-by: Rakesh Kumar <rakkuma@microsoft.com>
ealsur
ealsur previously approved these changes Dec 12, 2019
View reviewed changes
Copy link
Member

@ealsur ealsur left a comment

Choose a reason for hiding this comment

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

LGTM on the SDK changes, I still think that the sample is more of a perf project that might be better in the Tools folder along with the other Benchmark project though.

@rakkuma
correcting .json
8ef1e99 
Signed-off-by: Rakesh Kumar <rakkuma@microsoft.com>
@rakkuma
fix
0b96a30 
Signed-off-by: Rakesh Kumar <rakkuma@microsoft.com>
@ealsur ealsur mentioned this pull request Dec 13, 2019
Open
@rakkuma
Code review fix
d74df2e 
Signed-off-by: Rakesh Kumar <rakkuma@microsoft.com>
@rakkuma
Null check removing
0bf11cb 
Signed-off-by: Rakesh Kumar <rakkuma@microsoft.com>
ealsur
ealsur previously approved these changes Dec 18, 2019
View reviewed changes
ealsur
ealsur previously approved these changes Feb 12, 2020
View reviewed changes
@ealsur ealsur mentioned this pull request Feb 14, 2020
Closed
@rakkuma
Reverting change for not exposing congestion control knob
21dd69f 
Signed-off-by: Rakesh Kumar <rakkuma@microsoft.com>
@kirankumarkolli
Copy link
Member

@ealsur, @j82w please take a look.

ealsur
ealsur previously approved these changes Feb 18, 2020
View reviewed changes
@rakkuma
Code review fix
3bad759 
Signed-off-by: Rakesh Kumar <rakkuma@microsoft.com>
@rakkuma
Minor fix
0f038fc 
Signed-off-by: Rakesh Kumar <rakkuma@microsoft.com>
@rakkuma rakkuma dismissed stale reviews from ealsur and kirankumarkolli via 0f038fc February 21, 2020 06:51
@kirankumarkolli kirankumarkolli merged commit 48e9549 into master Feb 25, 2020
@kirankumarkolli kirankumarkolli deleted the users/rakkuma/batch-api-congestion-control branch February 25, 2020 19:39
@j82w
Copy link
Contributor

j82w commented Feb 25, 2020

@rakkuma can you please create a PR to update the changelog.

@rakkuma
Copy link
Contributor Author

rakkuma commented Feb 26, 2020

@j82w will do. Thanks.

@ealsur ealsur mentioned this pull request Mar 5, 2020
Closed
@j82w j82w mentioned this pull request Mar 13, 2020
Closed
@j82w j82w mentioned this pull request Apr 3, 2020
Closed
@ealsur ealsur mentioned this pull request Jun 16, 2020
Closed
@j82w j82w mentioned this pull request Jul 19, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abhijitpai abhijitpai abhijitpai left review comments

@chetanmsft chetanmsft chetanmsft left review comments

@j82w j82w j82w left review comments

@bartelink bartelink bartelink left review comments

@ealsur ealsur ealsur approved these changes

@kirankumarkolli kirankumarkolli kirankumarkolli left review comments

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

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

Successfully merging this pull request may close these issues.

Bulk Stream > Rate limiting
7 participants
@rakkuma @kirankumarkolli @abhijitpai @ealsur @j82w @bartelink @chetanmsft