[PM-39197] - improve vault sync performance#7829
Conversation
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the Code Review DetailsNo blocking findings.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7829 +/- ##
==========================================
+ Coverage 65.67% 65.70% +0.03%
==========================================
Files 2195 2209 +14
Lines 97542 97716 +174
Branches 8807 8815 +8
==========================================
+ Hits 64062 64209 +147
- Misses 31259 31288 +29
+ Partials 2221 2219 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
r-tome
left a comment
There was a problem hiding this comment.
This is really great! The query changes don't seem to change the existing query logic.
Non-blocker: I'd love to see some tests for GetManyByUserIdAsync added to https://github.com/bitwarden/server/blob/main/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/CollectionRepositoryTests.cs



🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-39197
📔 Objective
Reduce vault sync latency for users with very large direct collection assignments.
This PR optimizes the SQL Server collection-access query path used by sync by updating
UserCollectionDetailsto avoid the expensive join/OR shape that was causing high latency at scale.Outcome from local verification on the same seeded dataset:
/sync?excludeDomains=truearound 21s/sync?excludeDomains=truearound 426ms🔧 Performance approach
The sync bottleneck was isolated to collection access expansion in SQL Server (
Collection_ReadByUserIdviaUserCollectionDetails), not API serialization or cipher loading.To improve performance,
UserCollectionDetailswas reshaped from a broad left-join + OR filter pattern into two explicit branches:CollectionUser)GroupUser+CollectionGroup) with exclusion of directly-assigned rowsThis reduces unnecessary row expansion and gives SQL Server a cleaner execution path for large collection cardinalities.
✅ Measured impact (same dataset)
/sync?excludeDomains=true≈ 21s/sync?excludeDomains=true≈ 426ms📸 Screenshots
Not applicable (no UI changes).