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

Update partitionKey path definitions for cosmosDatabase containers. #240

Merged
merged 8 commits into from
Aug 28, 2023

Conversation

crickman
Copy link
Contributor

@crickman crickman commented Aug 23, 2023

Motivation and Context

Customer issue reported in #238 related to partition scheme in cosmosdb containers.

Description

Update partition-key path definition for ChatMessage, ChatPartitipant, and MemorySource.

Existing deployments will continue to work in their current mode.

CosmosDB containers must be removed and redeployed to realize the peformance update related to the partition scheme.

Contribution Checklist

@github-actions github-actions bot added webapi Pull requests that update .net code PR: ready for review labels Aug 23, 2023
@dluc
Copy link
Contributor

dluc commented Aug 23, 2023

is there any table that should be partitioned by user?

@crickman
Copy link
Contributor Author

TryFindByIdAsync

is there any table that should be partitioned by user?

No sir...this model is "chat-centric"

@dluc
Copy link
Contributor

dluc commented Aug 23, 2023

Another perf detail that came up is the consistency level. The tables are setup for "strong consistency" which could be relaxed to reduce latency.

@crickman crickman marked this pull request as ready for review August 23, 2023 17:45
@crickman crickman self-assigned this Aug 23, 2023
@crickman crickman removed PR: in progress preview Not even close to completed labels Aug 23, 2023
dluc
dluc previously approved these changes Aug 24, 2023
@crickman crickman changed the title Update patitionKey path definitions for cosmosDatabase containers. Update partitionKey path definitions for cosmosDatabase containers. Aug 24, 2023
alliscode
alliscode previously approved these changes Aug 28, 2023
scripts/deploy/main.bicep Outdated Show resolved Hide resolved
@crickman crickman dismissed stale reviews from alliscode and dluc via 8034b9e August 28, 2023 20:44
@crickman crickman added this pull request to the merge queue Aug 28, 2023
Merged via the queue into main with commit 679bd07 Aug 28, 2023
7 checks passed
@crickman crickman deleted the fix-cosmosdb-partitioning branch August 28, 2023 20:51
teamleader-dev pushed a commit to vlink-group/chat-copilot that referenced this pull request Oct 7, 2024
…icrosoft#240)

### Motivation and Context
Customer issue reported in
microsoft#238 related to
partition scheme in cosmosdb containers.

### Description
Update partition-key path definition for `ChatMessage`,
`ChatPartitipant`, and `MemorySource`.

Existing deployments will continue to work in their current mode.

CosmosDB containers must be removed and redeployed to realize the
peformance update related to the partition scheme.

### Contribution Checklist
- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [Contribution
Guidelines](https://github.com/microsoft/copilot-chat/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/copilot-chat/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployment Issues related to deploying Chat-Copilot external dependency issue Issues related to external dependencies (e.g. Azure) performance webapi Pull requests that update .net code
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

CosmosDb partitioning strategy causing high cost/low perf, max quota/too many requests
4 participants