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

Missing index for username and identity_zone on users table (Postgres) #1713

Closed
torsten-sap opened this issue Nov 4, 2021 · 9 comments · Fixed by #1750
Closed

Missing index for username and identity_zone on users table (Postgres) #1713

torsten-sap opened this issue Nov 4, 2021 · 9 comments · Fixed by #1750
Assignees
Labels
accepted Accepted the issue performance
Milestone

Comments

@torsten-sap
Copy link
Contributor

What version of UAA are you running?

75.9.0

How are you deploying the UAA?

I am deploying the UAA

  • locally only using gradlew

What did you do?

Executed a SQL manually because we had a postgres issue in bigger landscapes

Example:

select count(*) from users where ((LOWER(username) = LOWER('marissa') and LOWER(identity_zone_id) = LOWER('uaa')))

You may execute explain analyze.

What did you expect to see? What goal are you trying to achieve with the UAA?

Expected is an indexed scan in the query plan to ensure that we run with the index.

What did you see instead?

Sequenced scan is done because we have no lower index on idz and origin.

Reason found in git log

Second query that way adapted in the following commit: dca40d9

@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/180190790

The labels on this github issue will be updated when the story is started.

@strehle
Copy link
Member

strehle commented Nov 4, 2021

@torsten-sap since this seemed to be related to dca40d9
The issue #1663 solved one, now additional one, but is there still one issue missing ? because there were 3 SQL changed to lower....?

@strehle strehle added this to the 75.10.0 milestone Nov 4, 2021
@torsten-sap
Copy link
Contributor Author

Yes, we should also check the third one. In the best case changing the order of the recently created index might also solve the others.

@strehle strehle removed this from the 75.10.0 milestone Nov 11, 2021
@strehle
Copy link
Member

strehle commented Nov 11, 2021

@torsten-sap I verified with a colleague in a larger DB that existing index from
issue #1713 and fix
https://github.com/cloudfoundry/uaa/pull/1663/files

solves also this.
If you have other findings , please re-open

@torsten-sap
Copy link
Contributor Author

Finding from productive CF UAA showed that there are situations where the existing index is not hit. The proposed index improved the situation significantly. Thus, reopening the issue.

@torsten-sap torsten-sap reopened this Dec 16, 2021
@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/180664577

The labels on this github issue will be updated when the story is started.

@torsten-sap
Copy link
Contributor Author

Affected queries:
select count(*) from users where ((LOWER(username) = LOWER($1) AND LOWER(identity_zone_id) = LOWER($2)))

select id,version,created,lastModified,username,email,givenName,familyName,active,phoneNumber,verified,origin,external_id,identity_zone_id,salt,passwd_lastmodified,last_logon_success_time,previous_logon_success_time from users where ((LOWER(username) = LOWER($1) AND LOWER(identity_zone_id) = LOWER($2))) ORDER BY userName ASC limit ? offset ?

@torsten-sap
Copy link
Contributor Author

Command for index creation:
create index concurrently users_username_idz_tmp on users(LOWER(username), LOWER(identity_zone_id))

@strehle strehle added this to the 75.13.0 milestone Dec 16, 2021
@strehle
Copy link
Member

strehle commented Dec 16, 2021

@torsten-sap thanks, so can you please create PR for that

@strehle strehle self-assigned this Dec 16, 2021
torsten-sap added a commit that referenced this issue Dec 17, 2021
strehle pushed a commit that referenced this issue Dec 20, 2021
* Fix issue #1713

* Update V4_101_1639764160__Add_LowerIndex_To_Users_Wo_Origin.sql
@strehle strehle linked a pull request Dec 20, 2021 that will close this issue
@strehle strehle moved this from Inbox to Pending Merge | Prioritized in Foundational Infrastructure Working Group Dec 20, 2021
@strehle strehle added the accepted Accepted the issue label Dec 20, 2021
@strehle strehle closed this as completed Dec 20, 2021
Repository owner moved this from Pending Merge | Prioritized to Done in Foundational Infrastructure Working Group Dec 20, 2021
@cf-gitbot cf-gitbot removed the accepted Accepted the issue label Dec 20, 2021
@cf-gitbot cf-gitbot added delivered accepted Accepted the issue and removed delivered labels Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Accepted the issue performance
Projects
Development

Successfully merging a pull request may close this issue.

3 participants