perf: skip segment leaf expansion in merge suggestions (CM-1230)#4232
Open
skwowet wants to merge 5 commits into
Open
perf: skip segment leaf expansion in merge suggestions (CM-1230)#4232skwowet wants to merge 5 commits into
skwowet wants to merge 5 commits into
Conversation
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to eliminate Postgres timeouts on POST /membersToMerge and POST /organizationsToMerge (especially with countOnly: true) by avoiding expansion of a selected segment into large leaf-subproject ID lists, and instead relying on rollup rows stored at each segment hierarchy level.
Changes:
- Frontend: stop expanding project groups into leaf subproject IDs when requesting merge suggestions; send only the selected project group ID.
- Backend: remove repository-level
getSegmentSubprojects(...)expansion in merge suggestion queries and useSequelizeRepository.getSegmentIds(options)directly. - Data quality UI: simplify the
segmentscomputed arrays to no longer include leaf subproject IDs.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/modules/organization/services/organization.api.service.ts | Sends only the selected project group ID for organization merge suggestions. |
| frontend/src/modules/organization/organization-service.js | Same change for org merge suggestions in the legacy JS service. |
| frontend/src/modules/data-quality/components/organization/data-quality-organization-merge-suggestions.vue | Drops leaf-subproject expansion from the segments payload. |
| frontend/src/modules/data-quality/components/member/data-quality-member-merge-suggestions.vue | Drops leaf-subproject expansion from the segments payload. |
| backend/src/database/repositories/organizationRepository.ts | Removes repository-level leaf expansion for org merge suggestions. |
| backend/src/database/repositories/memberRepository.ts | Removes repository-level leaf expansion for member merge suggestions. |
Comments suppressed due to low confidence (1)
frontend/src/modules/organization/services/organization.api.service.ts:41
segmentscan become[undefined]whenselectedProjectGroupisn’t set yet. That will still serialize into the request body and can lead to the backend resolving zero segments (or behaving unexpectedly), whereas other methods in this file intentionally sendnullwhen there is no selection (seegetSelectedProjectGroupId()).
Consider aligning behavior here by sending null (or an empty array) when no project group is selected.
static async fetchMergeSuggestions(limit: number = 20, offset: number = 0, query: any = {}) {
const lsSegmentsStore = useLfSegmentsStore();
const { selectedProjectGroup } = storeToRefs(lsSegmentsStore);
const segments = [selectedProjectGroup.value?.id];
const data = {
limit,
offset,
segments,
...query,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
joanagmaia
previously approved these changes
Jun 18, 2026
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes query timeouts on
POST /membersToMergeandPOST /organizationsToMergewhencountOnly: trueis called with a large segment scope.Both endpoints were expanding the selected segment into all leaf subproject IDs and filtering
memberSegmentsAgg/organizationSegmentsAggwith a largesegmentId IN (...)list. On large groups this caused Postgres to pick expensive scans over hundreds of millions of rows, triggering read timeouts.Both tables already maintain rollup rows at every hierarchy level (group / project / subproject). Using the selected segment ID directly lets the planner do index-only lookups per merge row instead.
The frontend for organizations was also sending every leaf ID on each request — it now sends the project group ID, matching the existing member behaviour.
Changes
memberRepository.ts— removegetSegmentSubprojectsexpansion infindMembersWithMergeSuggestions; use request segment IDs directlyorganizationRepository.ts— same fix infindOrganizationsWithMergeSuggestionsorganization-service.js/organization.api.service.ts— send[projectGroupId]instead of all leaf IDsNo SQL shape changes, no new indexes.
Query plan impact (EXPLAIN ANALYZE on prod)
memberSegmentsAgg; >15s timeoutLIMIT 20LIMIT 20segmentsjoin; ~6.8sBefore:
After:
Removes one extra DB round-trip per request (subproject expansion query). Rollup rows refresh on a short interval so counts may differ slightly from a live leaf-union — consistent with how other segment-scoped reads work.