feat(x2a): add sorting by project status#2635
feat(x2a): add sorting by project status#2635mareklibra wants to merge 1 commit intoredhat-developer:mainfrom
Conversation
|
Important This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior. Changed Packages
|
Review Summary by QodoImplement sorting by project status with in-memory pagination
WalkthroughsDescription• Implement in-memory sorting by project status field • Sort projects by semantic status state order with secondary sort by module summary percentages • Apply pagination after in-memory sort to handle computed field sorting • Add comprehensive test coverage for status sorting with various scenarios Diagramflowchart LR
A["Query with sort=status"] --> B["Skip DB pagination"]
B --> C["Fetch all matching projects"]
C --> D["Enrich projects with status"]
D --> E["Sort in-memory by state order"]
E --> F["Secondary sort by module summary"]
F --> G["Apply pagination"]
G --> H["Return paginated results"]
File Changes1. workspaces/x2a/plugins/x2a-backend/src/services/X2ADatabaseService/index.ts
|
Code Review by Qodo
1. Unbounded status-sort enrichment
|
| const sortByComputedField = isNonDbSortField(query.sort); | ||
|
|
||
| const result = await this.#projectOps.listProjects(query, options, { | ||
| skipPagination: sortByComputedField, | ||
| }); | ||
|
|
||
| if ( | ||
| sortByComputedField && | ||
| result.totalCount > IN_MEMORY_SORT_WARN_THRESHOLD | ||
| ) { | ||
| // If this proves to be a performance bottleneck, let's consider either removing the sort by status or having materialized DB column (works with Postgres only). | ||
| this.#logger.warn( | ||
| `In-memory sort by "${query.sort}" is loading ${result.totalCount} projects.`, | ||
| ); | ||
| } | ||
|
|
||
| this.#logger.info( | ||
| `this.#projectOps.listProjects finished, adding migration plans to projects`, | ||
| ); | ||
| await Promise.all(result.projects.map(p => this.enrichProject(p))); | ||
|
|
||
| if (sortByComputedField) { | ||
| this.sortAndPaginateInMemory(result, query); | ||
| } |
There was a problem hiding this comment.
1. Unbounded status-sort enrichment 🐞 Bug ➹ Performance
When sort=status is requested, the backend disables DB pagination and then concurrently enriches every matching project via Promise.all, which triggers multiple DB queries per project/module. This can overwhelm the DB connection pool and significantly degrade or crash the service under larger datasets or repeated requests.
Agent Prompt
## Issue description
`GET /projects?sort=status` disables DB pagination and then enriches *every* matching project concurrently. Enrichment performs additional DB queries (jobs + modules, plus multiple job lookups per module), so this becomes an unbounded fan-out that can exhaust the DB connection pool / CPU.
## Issue Context
- `skipPagination` is set when sorting by a computed field (currently `status`), which makes the DB query return all matching rows.
- The service then does `Promise.all(result.projects.map(p => this.enrichProject(p)))` without any concurrency limit.
- `enrichProject()` calls `listModules()` and `listJobs()`, and `listModules()` itself does multiple `Promise.all` calls over modules.
## Fix Focus Areas
- workspaces/x2a/plugins/x2a-backend/src/services/X2ADatabaseService/index.ts[125-156]
- workspaces/x2a/plugins/x2a-backend/src/services/X2ADatabaseService/index.ts[67-90]
- workspaces/x2a/plugins/x2a-backend/src/services/X2ADatabaseService/index.ts[310-343]
- workspaces/x2a/plugins/x2a-backend/src/services/X2ADatabaseService/projectOperations.ts[110-137]
## Suggested fix
1. Add a concurrency limiter for project enrichment when `skipPagination` is enabled (e.g., `p-limit` / custom queue) so you only enrich N projects at a time.
2. Consider enforcing a hard cap / config flag for computed-field sorting (e.g., reject or require additional filtering when `totalCount` exceeds a safe threshold), since the current threshold only logs a warning.
3. (Optional but impactful) Reduce query fan-out by batching: fetch init jobs for all projectIds in one query and/or compute module summaries with fewer queries instead of per-module `Promise.all` calls.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
This would be a complex change in the listProjects service, let's optimize when it proves to be needed.
There was a problem hiding this comment.
Introducing maxConcurrency() function to address this issue at least from the stability perspective
2297d7e to
4d0e1d8
Compare
Signed-off-by: Marek Libra <marek.libra@gmail.com>
4d0e1d8 to
796f9f4
Compare
|
|
What is clear is that I made a big mistake not forcing DDD from the beginning, each change is getting painfully with hacks here and there(max_concurrency). We should start thinking on move to a DDD structure, mostly when we have events from agents and users, and it's going to get harder and harder. We should start thinking on services(real ones), infraestructure(current services), doing aggregate roots, and keep things more individual/modularized. We should create an epic and a RFC for getting into DDD |



The Status field on the projects' list page can be newly sorted.
First by name, then by modules summary.
Since this field is calculated, the sorting can not be done in the DB and so such sorting can become expensive.
If it proves to be a bottleneck in large setups, it will be a subject either to remove or find an optimization like materialized DB columns.