fix: optimize org stats count queries and fix branching bug#3531
fix: optimize org stats count queries and fix branching bug#3531
Conversation
…keys (#3527) Add partial index on key(project_id, name, namespace_id) WHERE deleted_at IS NULL to speed up org-wide COUNT(DISTINCT) queries. Fix bug where keys on orphan branches were counted even when project.useBranching=false. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis pull request fixes slow organization stats count queries and incorrect branch key counting when branching is disabled. Changes include adding a database index for the count queries, updating the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
backend/app/src/test/kotlin/io/tolgee/service/OrganizationStatsServiceTest.kt (1)
84-105: These two new tests are useful but currently duplicate existing org-total assertions.Consider consolidating or parameterizing to keep intent explicit while reducing maintenance repetition.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/src/test/kotlin/io/tolgee/service/OrganizationStatsServiceTest.kt` around lines 84 - 105, The two new tests (`getKeyCount excludes branch keys when project has branching disabled` and `getTranslationCount excludes branch translations when project has branching disabled`) duplicate existing organization-wide assertions; instead, change them to assert only the specific project-level behavior (call organizationStatsService.getKeyCount and getTranslationCount for the specific no-branching project id or use a helper to fetch per-project counts) or convert them into a parameterized test that takes project id and expected counts, and remove the duplicated org-total assertions using testData.organization.id so the intent (exclude branch items for a no-branching project) remains explicit without repeating org-wide sums.backend/data/src/main/resources/db/changelog/schema.xml (1)
5178-5188: Consider validating overlap with the existing broad index before keeping both.The new partial index is great for active-key counts, but the older non-partial index on the same key order still exists (Line 5081-Line 5090). Keeping both can increase write overhead; worth confirming with
EXPLAIN/index usage stats whether the older one is still needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/data/src/main/resources/db/changelog/schema.xml` around lines 5178 - 5188, You added a partial index key_project_name_ns_not_deleted in changeSet id 1741855200000-1 but an older non-partial index on the same column order (project_id, name, namespace_id) still exists; validate whether both are needed by running EXPLAIN for representative queries and checking index usage (pg_stat_user_indexes / pg_stat_all_indexes) and index definitions, then either remove the old broad index (via a new rollback/drop changeSet) or adjust this changeSet to drop the older index conditionally to avoid write overhead; reference the new index name key_project_name_ns_not_deleted and the existing non-partial index on public.key (project_id, name, namespace_id) when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@backend/app/src/test/kotlin/io/tolgee/service/OrganizationStatsServiceTest.kt`:
- Around line 84-105: The two new tests (`getKeyCount excludes branch keys when
project has branching disabled` and `getTranslationCount excludes branch
translations when project has branching disabled`) duplicate existing
organization-wide assertions; instead, change them to assert only the specific
project-level behavior (call organizationStatsService.getKeyCount and
getTranslationCount for the specific no-branching project id or use a helper to
fetch per-project counts) or convert them into a parameterized test that takes
project id and expected counts, and remove the duplicated org-total assertions
using testData.organization.id so the intent (exclude branch items for a
no-branching project) remains explicit without repeating org-wide sums.
In `@backend/data/src/main/resources/db/changelog/schema.xml`:
- Around line 5178-5188: You added a partial index
key_project_name_ns_not_deleted in changeSet id 1741855200000-1 but an older
non-partial index on the same column order (project_id, name, namespace_id)
still exists; validate whether both are needed by running EXPLAIN for
representative queries and checking index usage (pg_stat_user_indexes /
pg_stat_all_indexes) and index definitions, then either remove the old broad
index (via a new rollback/drop changeSet) or adjust this changeSet to drop the
older index conditionally to avoid write overhead; reference the new index name
key_project_name_ns_not_deleted and the existing non-partial index on public.key
(project_id, name, namespace_id) when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5496a483-6969-46da-990b-d24750a9b327
📒 Files selected for processing (4)
backend/app/src/test/kotlin/io/tolgee/service/OrganizationStatsServiceTest.ktbackend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/OrganizationStatsTestData.ktbackend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationStatsService.ktbackend/data/src/main/resources/db/changelog/schema.xml
…iltering Replace EXISTS subqueries with LEFT JOIN on branch table. This excludes keys on deleted branches and correctly counts default-branch keys when project.useBranching=false. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ount PostgreSQL kept driving the translation count query from the language table, causing 11M+ nested loop iterations. Use materialized CTEs to force the join order: org keys first, then translations, then language filter. Also fix branching filter to count default-branch keys when useBranching=false, and exclude keys on deleted branches. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| <changeSet author="dkrizan" id="1741855200000-1" runInTransaction="false"> | ||
| <sql> | ||
| create index concurrently if not exists key_project_name_ns_not_deleted | ||
| on public.key (project_id, name, namespace_id) |
There was a problem hiding this comment.
it looks like the same index, but not partial, already exists. It was added in the changeset 1771846650000-1. So it's not clear why the new one is required here.
Maybe, for the usage query it must be for 2 fields: project_id and deleted_at? Not sure, just asking
There was a problem hiding this comment.
It improved the performance by 25%, I think due to where deleted_at is null .. but I will dive deeper and find a better way.
There was a problem hiding this comment.
hm, got it. It's interesting why the existing index isn't taken then... Maybe because of more fields in it 🤔
There was a problem hiding this comment.
I think the reason Postgres does not choose it is the condition WHERE deleted_at IS NOT NULL, in this case, the opposite is needed.
There was a problem hiding this comment.
Looks like this is the best solution + the refactor of SQLs.
2 participants
Summary
key_project_name_ns_not_deletedonkey(project_id, name, namespace_id) WHERE deleted_at IS NULLto speed up org-wideCOUNT(DISTINCT)queries inOrganizationStatsServicegetKeyCount()andgetTranslationCount()counted keys on orphan branches even whenproject.useBranching = falseCloses #3527
Test plan
OrganizationStatsServiceTest— all 6 tests pass including new branching exclusion tests🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Bug Fixes
Chores