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

Improve server listing performance #602

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Improve server listing performance #602

wants to merge 2 commits into from

Conversation

databus23
Copy link
Member

@databus23 databus23 commented Aug 9, 2021

Instead of listing all servers of a project we use a name filter with the regex ^(kks-?)$KLUSTERNAME-$POOLNAME for the name attribute.
In addition we also process the pages individually when filtering through the returned servers.
This should avoid listing thousands of servers from s4 projects on a constant basis.

CAREFUL: This needs extra special testing and careful review. If the listing is somehow filtering out valid kubernikus nodes launchctl will spawn new servers until the quota is exceeded.

var filteredNodes []Node

err := servers.List(c.ComputeClient, servers.ListOpts{}).EachPage(func(page pagination.Page) (bool, error) {
err := servers.List(c.ComputeClient, servers.ListOpts{Name: "^(kks-)?" + k.Spec.Name + "-" + pool.Name + "-"}).EachPage(func(page pagination.Page) (bool, error) {
Copy link
Member Author

@databus23 databus23 Aug 9, 2021

Choose a reason for hiding this comment

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

@joker-at-work Is using a regex for listing servers by name safe and won't break in the future in your opinion? Seems like its depended on the database used for nova which unsettles me a bit.

Choose a reason for hiding this comment

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

All I can say is that we don't plan to change the DB under Nova once again.

FYI this is the function translating the string into the query: https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L1943-L1973 Looks pretty DB-dependent. It basically just uses a sanitized version of whatever you provide.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok thanks.

@databus23 databus23 changed the title Improve server listing Improve server listing performance Aug 9, 2021
Instead of listing all servers of a project we use a name filter the uses the regex `^(kks?)-$KLUSTERNAME-$POOLNAME` for the `name` attribute.
In addition we also process the pages individually when filtering through the returned servers.
@stale
Copy link

stale bot commented Jan 9, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jan 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants