refactor: optimize MongoDB queries in findMentionedMessages and findStarredMessages with Promise.all#39607
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 |
🦋 Changeset detectedLatest commit: 2383357 The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughReplaces sequential awaits for independent room and user lookups with concurrent Promise.all calls across message and chat thread endpoints; adds a changeset entry and removes a stray TODO comment. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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 |
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".changeset/perf-thread-endpoints-promise-all.md">
<violation number="1" location=".changeset/perf-thread-endpoints-promise-all.md:5">
P2: Changeset description is inconsistent with the PR’s actual scope, so release notes would report the wrong endpoints being optimized.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| '@rocket.chat/meteor': patch | ||
| --- | ||
|
|
||
| Improve performance of thread-related chat endpoints by replacing sequential `await` calls with concurrent `Promise.all` for independent `Users` and `Rooms` database lookups. Affected endpoints: `chat.getThreadsList`, `chat.syncThreadsList`, `chat.getThreadMessages`, `chat.syncThreadMessages`. |
There was a problem hiding this comment.
P2: Changeset description is inconsistent with the PR’s actual scope, so release notes would report the wrong endpoints being optimized.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .changeset/perf-thread-endpoints-promise-all.md, line 5:
<comment>Changeset description is inconsistent with the PR’s actual scope, so release notes would report the wrong endpoints being optimized.</comment>
<file context>
@@ -0,0 +1,5 @@
+'@rocket.chat/meteor': patch
+---
+
+Improve performance of thread-related chat endpoints by replacing sequential `await` calls with concurrent `Promise.all` for independent `Users` and `Rooms` database lookups. Affected endpoints: `chat.getThreadsList`, `chat.syncThreadsList`, `chat.getThreadMessages`, `chat.syncThreadMessages`.
</file context>
Proposed changes
Optimizes MongoDB query performance in two Activity Hub-related service functions by replacing sequential database lookups with concurrent
Promise.allcalls.Affected Functions
File:
apps/meteor/app/api/server/lib/messages.tsfindMentionedMessages(lines 7–44)findStarredMessages(lines 46–83)The Problem
Both functions were performing two independent MongoDB queries sequentially, even though neither result depends on the other:
Performance Impact
For a typical deployment with 50ms DB latency:
Issue(s)
Related to Activity Hub project (#39577). Closes #39606.
This optimizes the backend service functions used by:
GET /v1/chat.getMentionedMessagesGET /v1/chat.getStarredMessagesThese endpoints power the Mentions and Starred Messages tabs in the Activity Hub UI.
Steps to test or reproduce
Performance improvement can be verified via:
Further comments
Promise.alloptimization applied in fix: replace sequential awaits with Promise.all in thread endpoints #39580 for thread endpointsSummary by CodeRabbit