fix: unify statistics indexes with TTL expiration and add migration #38572
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
WalkthroughThis change consolidates database indexes on the Statistics collection by removing a redundant descending index on Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🦋 Changeset detectedLatest commit: d57f8cb The changes in this PR will be included in the next version bump. This PR includes changesets to release 40 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/meteor/server/startup/migrations/v336.ts`:
- Around line 9-13: Replace the empty catch in the v336.ts migration around
Statistics.col.dropIndex('createdAt_-1') with error handling that only swallows
MongoDB "index not found" errors (code 27) and re-throws all other errors; i.e.,
catch the thrown error (e) and if e?.code !== 27 then throw e so
authentication/network or other failures surface, matching the pattern used in
v323.ts.
🧹 Nitpick comments (1)
packages/models/src/models/Statistics.ts (1)
12-14: TTL retention period is hardcoded to 365 days.The linked issue (CORE-1785) mentions a configurable retention period, but this value is a compile-time constant. If making it configurable is planned for a follow-up, this is fine for now. Otherwise, consider reading from a server setting so admins can tune retention without a code change.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/dry-gifts-report.mdapps/meteor/server/startup/migrations/index.tsapps/meteor/server/startup/migrations/v336.tspackages/models/src/models/Statistics.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/server/startup/migrations/v336.tsapps/meteor/server/startup/migrations/index.tspackages/models/src/models/Statistics.ts
🧬 Code graph analysis (1)
apps/meteor/server/startup/migrations/v336.ts (2)
apps/meteor/server/lib/migrations.ts (1)
addMigration(99-107)packages/models/src/index.ts (1)
Statistics(197-197)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 🔨 Test Storybook / Test Storybook
- GitHub Check: 📦 Meteor Build (coverage)
🔇 Additional comments (3)
apps/meteor/server/startup/migrations/index.ts (1)
44-44: LGTM!Migration v336 is correctly appended to the sequence.
.changeset/dry-gifts-report.md (1)
1-6: LGTM!Changeset accurately describes the migration and TTL index consolidation.
apps/meteor/server/startup/migrations/v336.ts (1)
5-15: No additional index drops are needed. The migration correctly removes the old descendingcreatedAtindex. The git history shows the previousmodelIndexes()definition only contained a simple index on{ createdAt: -1 }with no compound variants or TTL configuration, so no other conflicting indexes require removal.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| try { | ||
| await Statistics.col.dropIndex('createdAt_-1'); | ||
| } catch { | ||
| // Index might not exist on fresh installations | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how other migrations in this repo handle dropIndex errors
rg -n -A5 'dropIndex' --glob '**/migrations/*.ts' | head -100Repository: RocketChat/Rocket.Chat
Length of output: 2852
🏁 Script executed:
#!/bin/bash
# Also check the specific file and its context
cat -n apps/meteor/server/startup/migrations/v336.tsRepository: RocketChat/Rocket.Chat
Length of output: 518
🌐 Web query:
MongoDB error code 27 IndexNotFound official documentation
💡 Result:
MongoDB error code 27 = IndexNotFound in the official MongoDB Server documentation under Reference → Error Codes. (mongodb.com)
Citations:
Catch block swallows all errors, not just "index not found".
The empty catch will silently ignore any error (e.g., authentication failures, network issues), not just the expected IndexNotFound. The codebase already establishes the proper pattern in v323.ts: check for MongoDB error code 27 and re-throw unexpected errors. Match that pattern here.
Proposed fix
try {
await Statistics.col.dropIndex('createdAt_-1');
- } catch {
- // Index might not exist on fresh installations
+ } catch (e: any) {
+ if (e?.code !== 27 && e?.codeName !== 'IndexNotFound') {
+ throw e;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| await Statistics.col.dropIndex('createdAt_-1'); | |
| } catch { | |
| // Index might not exist on fresh installations | |
| } | |
| try { | |
| await Statistics.col.dropIndex('createdAt_-1'); | |
| } catch (e: any) { | |
| if (e?.code !== 27 && e?.codeName !== 'IndexNotFound') { | |
| throw e; | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@apps/meteor/server/startup/migrations/v336.ts` around lines 9 - 13, Replace
the empty catch in the v336.ts migration around
Statistics.col.dropIndex('createdAt_-1') with error handling that only swallows
MongoDB "index not found" errors (code 27) and re-throws all other errors; i.e.,
catch the thrown error (e) and if e?.code !== 27 then throw e so
authentication/network or other failures surface, matching the pattern used in
v323.ts.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## fix/statistics-ttl #38572 +/- ##
======================================================
- Coverage 70.44% 70.31% -0.13%
======================================================
Files 3162 3162
Lines 110650 110657 +7
Branches 19862 19863 +1
======================================================
- Hits 77943 77810 -133
- Misses 30681 30809 +128
- Partials 2026 2038 +12
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
📦 Docker Image Size Report➡️ Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 17:34", "11/27 22:32", "11/28 19:05", "12/01 23:01", "12/02 21:57", "12/03 21:00", "12/04 18:17", "12/05 21:56", "12/08 20:15", "12/09 22:17", "12/10 23:26", "12/11 21:56", "12/12 22:45", "12/13 01:34", "12/15 22:31", "12/16 22:18", "12/17 21:04", "12/18 23:12", "12/19 23:27", "12/20 21:03", "12/22 18:54", "12/23 16:16", "12/24 19:38", "12/25 17:51", "12/26 13:18", "12/29 19:01", "12/30 20:52", "02/10 02:26 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.00]
line "omnichannel-transcript-service" [0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.00]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "queue-worker-service" [0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.00]
line "rocketchat" [0.35, 0.35, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.00]
Statistics (last 30 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Tue, 10 Feb 2026 02:26:49 GMT |
1 participant
Proposed changes (including videos or screenshots)
This PR unifies the existing statistics indexes across MongoDB collections and introduces a migration to ensure the
rocketchat_statisticscollection uses a consistent TTL index for automatic expiration of old documents.This complements the PR #38568 that adds a TTL index for automatic cleanup of the
rocketchat_statisticscollection.Issue(s)
Steps to test or reproduce
Further comments
Can only merge on a major version since it contains a migration.
Summary by CodeRabbit