Fix performance issues caused by inefficient DB usage#617
Open
Fix performance issues caused by inefficient DB usage#617
Conversation
…issues GET /v1/resource/history GET /v1/project/<name> POST /v1/project/by_names were causing N+1 query issues
Coverage Report for CI Build 25213714944Coverage decreased (-0.009%) to 93.132%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce N+1 database access in the v1 sync API by batching history, project, workspace, and diff lookups instead of resolving them row-by-row.
Changes:
- Reworks file-history loading for
/v1/resource/historyand/v1/project?...&since=to fetch histories and diffs in bulk. - Refactors
/v1/project/by_namesto batch-resolve workspaces, projects, and project members. - Updates the public v1 schema/tests to stop exposing the
expirationfield in history responses.
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 |
|---|---|
server/mergin/tests/test_project_controller.py |
Updates history endpoint expectations to remove expiration. |
server/mergin/sync/workspace.py |
Adds get_by_names() to the global workspace handler. |
server/mergin/sync/public_api.yaml |
Removes expiration from documented v1 history payloads. |
server/mergin/sync/public_api_controller.py |
Batches history/diff/project/workspace loading in several public API endpoints. |
server/mergin/sync/models.py |
Changes FileHistory.diff to cached_property and adjusts FileHistory.changes() query loading. |
server/mergin/sync/interfaces.py |
Extends the workspace handler interface with get_by_names(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
739
to
+746
| FileHistory.query.join(ProjectFilePath) | ||
| .join(FileHistory.version) | ||
| .join(ProjectVersion.project) | ||
| .options( | ||
| contains_eager(FileHistory.version) | ||
| .load_only(ProjectVersion.name, ProjectVersion.project_id) | ||
| .contains_eager(ProjectVersion.project) | ||
| .load_only(Project.storage_params) |
Comment on lines
+575
to
+578
| for key, workspace, name in valid_projects: | ||
| result = found_map.get((workspace.id, name)) | ||
| if result: | ||
| results[key] = ProjectListSchema(context=ctx).dump(result) |
Comment on lines
1124
to
1127
| - added | ||
| - updated | ||
| - removed | ||
| expiration: | ||
| nullable: true | ||
| type: string | ||
| format: date-time | ||
| example: 2019-02-26T08:47:58.636074Z | ||
| UploadFileInfo: |
Comment on lines
+534
to
546
| workspaces_by_name = { | ||
| ws.name: ws for ws in current_app.ws_handler.get_by_names(unique_ws_names) | ||
| } | ||
|
|
||
| results = {} | ||
| for project in list_of_projects: | ||
| projects = projects_query(ProjectPermissions.Read, as_admin=False) | ||
| splitted = project.split("/") | ||
| if len(splitted) != 2: | ||
| results[project] = {"error": 404} | ||
| valid_projects = [] # list of (key, workspace, project_name) | ||
| for key in list_of_projects: | ||
| parts = key.split("/") | ||
| if len(parts) != 2: | ||
| results[key] = {"error": 404} | ||
| continue | ||
| ws = splitted[0] | ||
| name = splitted[1] | ||
| workspace = current_app.ws_handler.get_by_name(ws) | ||
| workspace = workspaces_by_name.get(parts[0]) | ||
| if not workspace: |
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.
Three endpoints were hammering the database with query counts that scaled with the number of files or projects in a request (N+1 problem).
GET /v1/resource/historyreduced from 6 + 4N queries to a fixed 7.GET /v1/project/<name>?since=reduced from 3 + M + N×M queries to a fixed 5 (where M = versioned files, N = versions in range):POST /v1/project/by_namesreduced from up to 100 + 2N queries to 3.