Skip to content
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

[db] Improve performance of DBUserStorageResource.update(...) #3151

Merged
merged 2 commits into from
Feb 15, 2021

Conversation

geropl
Copy link
Member

@geropl geropl commented Feb 8, 2021

This does improve the performance of update(...) on UserStorageResourceDBImpl by:

  • using the char(36) for userId instead of varchar(255)
  • use INSERT INTO ... ON DUPLICATE KEY UPDATE instead of the update logic in the client

Note: The type change will trigger a index re-build and should be initiated offline.

export class UserStorageUserIdChar1612781029090 implements MigrationInterface {

public async up(queryRunner: QueryRunner): Promise<any> {
await queryRunner.query("ALTER TABLE d_b_user_storage_resource MODIFY userId char(36);");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it a problem?
changing userId varchar(255) to fixed char(36) sound like there is some risk.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it a problem?

Yes, it is for the lookup performance. The DB cannot assume that the 219 characters that are always blank. Ideally they all end up in one leaf of the Red-Black-tree but one never know. Also, it blows up the memory used up by the index.

changing userId varchar(255) to fixed char(36) sound like there is some risk.

There is no risk to break something as we never used anything else than UUIDs (36 characters), and we're maintaining that limit with the current id change.
During deployment however this might be a problem as it might take some time because it requires re-creating indexes.

@AlexTugarev
Copy link
Member

Just tested. Works fine.

@geropl geropl changed the title [db] Improve performance of DBUserStorageResource.update(...) [do-not-merge][db] Improve performance of DBUserStorageResource.update(...) Feb 8, 2021
@geropl geropl force-pushed the gpl/fix-update-res branch from c0f5b95 to c05795b Compare February 15, 2021 08:10
Copy link
Member

@AlexTugarev AlexTugarev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@geropl geropl changed the title [do-not-merge][db] Improve performance of DBUserStorageResource.update(...) [db] Improve performance of DBUserStorageResource.update(...) Feb 15, 2021
@geropl geropl added this to the February 2021 milestone Feb 15, 2021
@geropl geropl merged commit d819a6b into master Feb 15, 2021
@geropl geropl deleted the gpl/fix-update-res branch February 15, 2021 15:39
pavan-tri pushed a commit to trilogy-group/gitpod that referenced this pull request Apr 28, 2021
…-io#3151)

* [db] UserStorageResource: make userId char(36) instead of varchar(255)

* [db] Use INSERT INTO ... ON DUPLICATE KEY UPDATE for user storage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants