-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[dashboard][server] Make Project overview page load faster by pre-fetching and caching Git provider data (branch details) #7610
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
3f07860
to
9b4074d
Compare
This comment has been minimized.
This comment has been minimized.
95ff25f
to
40a6848
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
40a6848
to
4bf716d
Compare
… and caching Git provider data (branches)
4bf716d
to
feabf72
Compare
export class ProjectInfo1642497869312 implements MigrationInterface { | ||
|
||
public async up(queryRunner: QueryRunner): Promise<void> { | ||
await queryRunner.query("CREATE TABLE IF NOT EXISTS `d_b_project_info` ( `projectId` char(36) NOT NULL, `overview` longtext NOT NULL, `deleted` tinyint(4) NOT NULL DEFAULT '0', `_lastModified` timestamp(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6), PRIMARY KEY (`projectId`), KEY `ind_dbsync` (`_lastModified`)) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: I'm using longtext
for overview
instead of just text
because the (enormous) Project Overview of the GitLab repository (my stress test) wouldn't fit into a simple text
.
We could also look into trimming unnecessary information from Project Overviews, but I'd leave this as a potential future optimization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should look into this issue. I assume it's because of commit message length.
Hi @AlexTugarev 👋 if you like, could you please take a look at this change when you have time? 👀 It basically changes the behavior of
to:
I think this is a good trade-off to make the Branches page load very fast. ⚡ |
@jankeromnes, I'm sorry have missed this PR. Happy to look into it and test ride the next thing. |
await projectInfoRepo.save({ | ||
projectId, | ||
overview, | ||
creationTime: new Date().toISOString(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not using for now, but let's keep it. we can add more optimization based on it.
/lgtm Changes look good! Thanks! |
LGTM label has been added. Git tree hash: 6da2feca088f465eb87d1f38d2a8ad6bd0780d35
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AlexTugarev Associated issue: #7536 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
Make Project Branches page load faster by pre-fetching and caching GitHub/GitLab/Bitbucket data (branch details).
Related Issue(s)
Fixes #7536
How to test
Release Notes
Documentation
/uncc