fix: precompute project group merge suggestion counts (CM-1230)#4250
Open
skwowet wants to merge 4 commits into
Open
fix: precompute project group merge suggestion counts (CM-1230)#4250skwowet wants to merge 4 commits into
skwowet wants to merge 4 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>
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a precomputed, per–project-group cache of merge-suggestion counts to avoid expensive COUNT(*) queries timing out on large project groups, while keeping filtered counts and list pagination as live queries. It adds a cron refresher plus immediate count adjustments when users merge or dismiss suggestions.
Changes:
- Adds
segmentMergeSuggestionCountstable + DAL helpers to compute, upsert, fetch, and decrement counts per project-group segment. - Adds a cron job to refresh cached counts every 4 hours (only writing when counts change).
- Updates member/org merge-suggestion repositories and merge/no-merge flows to use cached totals for unfiltered single-project-group counts and decrement counts on user actions.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| services/libs/data-access-layer/src/segments/index.ts | Adds DAL functions for project-group segment enumeration, count calculation, cached count upsert/get, common-segment lookup, and decrement helpers. |
| services/libs/common_services/src/services/common.member.service.ts | Decrements cached member merge-suggestion counts after a successful member merge. |
| services/apps/cron_service/src/jobs/refreshSegmentMergeSuggestionCounts.job.ts | New cron job that refreshes cached counts on a 4h schedule. |
| backend/src/services/organizationService.ts | Decrements cached org merge-suggestion counts after org merge and org no-merge actions. |
| backend/src/services/memberService.ts | Decrements cached member merge-suggestion counts after member no-merge actions. |
| backend/src/database/repositories/organizationRepository.ts | Uses cached totals for unfiltered single-project-group counts; adjusts mergeActions filtering in list/count SQL. |
| backend/src/database/repositories/memberRepository.ts | Uses cached totals for unfiltered single-project-group counts; adjusts mergeActions filtering in list/count SQL. |
| backend/src/database/migrations/V1782128525__segment-merge-suggestion-counts.sql | Adds the segmentMergeSuggestionCounts table schema. |
Comments suppressed due to low confidence (1)
backend/src/database/repositories/organizationRepository.ts:820
- Using a mergeActions LEFT JOIN +
(ma.id IS NULL OR ma.state = ERROR)can multiply rows and can incorrectly include a suggestion if there are mergeActions recorded in both directions with different states (the ERROR row still passes). A NOT EXISTS predicate avoids join-multiplication and correctly excludes suggestions when any non-ERROR mergeAction exists.
LEFT JOIN "mergeActions" ma
ON ma.type = :mergeActionType
AND (
(ma."primaryId" = otm."organizationId" AND ma."secondaryId" = otm."toMergeId")
OR (ma."primaryId" = otm."toMergeId" AND ma."secondaryId" = otm."organizationId")
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+593
to
+621
| ` | ||
| SELECT COUNT(*) AS count | ||
| FROM "memberToMerge" mtm | ||
| LEFT JOIN "mergeActions" ma | ||
| ON ma.type = $(mergeActionType) | ||
| AND ( | ||
| (ma."primaryId" = mtm."memberId" AND ma."secondaryId" = mtm."toMergeId") | ||
| OR (ma."primaryId" = mtm."toMergeId" AND ma."secondaryId" = mtm."memberId") | ||
| ) | ||
| WHERE EXISTS ( | ||
| SELECT 1 | ||
| FROM "memberSegmentsAgg" ms | ||
| WHERE ms."memberId" = mtm."memberId" | ||
| AND ms."segmentId" = $(segmentId) | ||
| ) | ||
| AND EXISTS ( | ||
| SELECT 1 | ||
| FROM "memberSegmentsAgg" ms2 | ||
| WHERE ms2."memberId" = mtm."toMergeId" | ||
| AND ms2."segmentId" = $(segmentId) | ||
| ) | ||
| AND (ma.id IS NULL OR ma.state = $(mergeActionState)) | ||
| `, | ||
| { | ||
| segmentId, | ||
| mergeActionType: MergeActionType.MEMBER, | ||
| mergeActionState: MergeActionState.ERROR, | ||
| }, | ||
| ) |
Comment on lines
+631
to
+659
| ` | ||
| SELECT COUNT(*) AS count | ||
| FROM "organizationToMerge" otm | ||
| LEFT JOIN "mergeActions" ma | ||
| ON ma.type = $(mergeActionType) | ||
| AND ( | ||
| (ma."primaryId" = otm."organizationId" AND ma."secondaryId" = otm."toMergeId") | ||
| OR (ma."primaryId" = otm."toMergeId" AND ma."secondaryId" = otm."organizationId") | ||
| ) | ||
| WHERE EXISTS ( | ||
| SELECT 1 | ||
| FROM "organizationSegmentsAgg" os1 | ||
| WHERE os1."organizationId" = otm."organizationId" | ||
| AND os1."segmentId" = $(segmentId) | ||
| ) | ||
| AND EXISTS ( | ||
| SELECT 1 | ||
| FROM "organizationSegmentsAgg" os2 | ||
| WHERE os2."organizationId" = otm."toMergeId" | ||
| AND os2."segmentId" = $(segmentId) | ||
| ) | ||
| AND (ma.id IS NULL OR ma.state = $(mergeActionState)) | ||
| `, | ||
| { | ||
| segmentId, | ||
| mergeActionType: MergeActionType.ORG, | ||
| mergeActionState: MergeActionState.ERROR, | ||
| }, | ||
| ) |
Comment on lines
273
to
295
| const totalCount = await options.database.sequelize.query( | ||
| ` | ||
| SELECT | ||
| COUNT(*) AS count | ||
| FROM "memberToMerge" mtm | ||
| ${membersJoin} | ||
| LEFT JOIN "mergeActions" ma | ||
| ON ma.type = :mergeActionType | ||
| AND ( | ||
| (ma."primaryId" = mtm."memberId" AND ma."secondaryId" = mtm."toMergeId") | ||
| OR (ma."primaryId" = mtm."toMergeId" AND ma."secondaryId" = mtm."memberId") | ||
| ) | ||
| WHERE EXISTS ( | ||
| SELECT 1 FROM "memberSegmentsAgg" ms | ||
| WHERE ms."memberId" = mtm."memberId" AND ms."segmentId" IN (:segmentIds) | ||
| ) | ||
| AND EXISTS ( | ||
| SELECT 1 FROM "memberSegmentsAgg" ms2 | ||
| WHERE ms2."memberId" = mtm."toMergeId" AND ms2."segmentId" IN (:segmentIds) | ||
| ) | ||
| AND (ma.id IS NULL OR ma.state = :mergeActionState) | ||
| ${memberFilter} | ||
| ${similarityFilter} |
Comment on lines
419
to
437
| @@ -395,6 +433,7 @@ | |||
| SELECT 1 FROM "memberSegmentsAgg" ms2 | |||
| WHERE ms2."memberId" = mtm."toMergeId" AND ms2."segmentId" IN (:segmentIds) | |||
| ) | |||
| AND (ma.id IS NULL OR ma.state = :mergeActionState) | |||
| AND mtm.similarity IS NOT NULL | |||
Comment on lines
+382
to
+389
| if (segmentIds.length === 1 && !hasCountFilters) { | ||
| const counts = await getSegmentMergeSuggestionCounts( | ||
| SequelizeRepository.getQueryExecutor(options), | ||
| segmentIds[0], | ||
| ) | ||
|
|
||
| return counts?.memberMergeSuggestionsCount ?? 0 | ||
| } |
Comment on lines
+905
to
+911
| if (segmentIds.length === 1 && !hasCountFilters) { | ||
| const counts = await getSegmentMergeSuggestionCounts( | ||
| SequelizeRepository.getQueryExecutor(options), | ||
| segmentIds[0], | ||
| ) | ||
| return counts?.organizationMergeSuggestionsCount ?? 0 | ||
| } |
Comment on lines
+447
to
+452
| const projectGroupSegmentIds = await getMembersCommonProjectGroupSegmentIds(this.qx, [ | ||
| originalId, | ||
| toMergeId, | ||
| ]) | ||
|
|
||
| await decrementMemberMergeSuggestionCounts(this.qx, projectGroupSegmentIds) |
Comment on lines
+785
to
+792
| const qx = SequelizeRepository.getQueryExecutor(this.options) | ||
| const projectGroupSegmentIds = await getMembersCommonProjectGroupSegmentIds(qx, [ | ||
| memberOneId, | ||
| memberTwoId, | ||
| ]) | ||
|
|
||
| await decrementMemberMergeSuggestionCounts(qx, projectGroupSegmentIds) | ||
|
|
Comment on lines
+729
to
+735
| const projectGroupSegmentIds = await getOrganizationsCommonProjectGroupSegmentIds(qx, [ | ||
| originalId, | ||
| toMergeId, | ||
| ]) | ||
|
|
||
| await decrementOrganizationMergeSuggestionCounts(qx, projectGroupSegmentIds) | ||
|
|
Comment on lines
+847
to
+853
| const qx = SequelizeRepository.getQueryExecutor(this.options) | ||
| const projectGroupSegmentIds = await getOrganizationsCommonProjectGroupSegmentIds(qx, [ | ||
| organizationId, | ||
| noMergeId, | ||
| ]) | ||
|
|
||
| await decrementOrganizationMergeSuggestionCounts(qx, projectGroupSegmentIds) |
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
CM-1230 addressed
membersToMerge/organizationsToMergetimeouts by scoping merge suggestions against rollup rows inmemberSegmentsAgg/organizationSegmentsAgg— matching the selected project-group segment directly instead of expanding to leaf subprojects and hitting the agg tables with hugesegmentId IN (...)lists.That fixed the worst leaf-expansion cases, but unfiltered COUNT requests were still slow for large project groups: even with a single rollup segment ID, Postgres still scanned all merge rows and ran expensive EXISTS checks against very large agg buckets, which could exceed API timeouts.
This PR adds precomputed per–project-group merge suggestion counts (same idea as other agg-backed reads: expensive work offline, cheap read online). Unfiltered badge/list counts are served from
segmentMergeSuggestionCounts; filtered counts still use the live query. Counts are refreshed on a schedule and adjusted immediately when users merge or dismiss suggestions.How it works now
segmentMergeSuggestionCounts(skips write when values are unchanged).memberToMerge/organizationToMerge+ agg EXISTS (unchanged behavior, correct totals for filtered views).Changes
segmentMergeSuggestionCountstable (segmentId, member/org counts,updatedAt).segments/index.ts): fetch project groups; calculate/upsert/get counts; common project-group segment lookup; decrement helpers.refresh-segment-merge-suggestion-countsjob (batched parallelism, fail-fast on segment errors).getTotalCount()— precomputed vs live; aligned member/org list + COUNT SQL (mergeActions, consistent EXISTS); simplified org list query (removed CTE / hash dedup).