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

Hide temporary projects #3232

Merged
merged 3 commits into from
May 6, 2024
Merged

Conversation

FrostyX
Copy link
Member

@FrostyX FrostyX commented Apr 22, 2024

Fix #3136
Fix #3087

@@ -70,6 +71,11 @@ def list_projects_by_group(group_name, page=1):
pinned = [pin.copr for pin in PinnedCoprsLogic.get_by_group_id(group.id)] if page == 1 else []
query = CoprsLogic.get_multiple_by_group_id(group.id)
query = CoprsLogic.filter_without_ids(query, [copr.id for copr in pinned])
query = query.order_by(
sqlalchemy.nullsfirst(models.Copr.delete_after),
Copy link
Member

Choose a reason for hiding this comment

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

do we have index on the delete_after field?

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't. I tried to add it, and alternatively, I tried to add an index on a combination of both delete_after and id. Neither of them didn't seem to bring any performance improvement. For the record, the code was:

def upgrade():
    op.create_index('copr_delete_after_id', 'copr', ['delete_after', 'id'],
                    unique=False)

and

def upgrade():
    op.create_index('copr_delete_after_id', 'copr', ['delete_after'],
                    unique=False)

Possibly an error on my side? If not, do you want to add the index anyway?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Looking at EXPLAIN ANALYZE:

 Sort  (cost=148.80..148.81 rows=3 width=873) (actual time=3.615..3.631 rows=159 loops=1)
   Sort Key: copr.delete_after NULLS FIRST, copr.id DESC
   Sort Method: quicksort  Memory: 196kB

I suppose that once an index is applied, we should see something like Index Scan instead of the quicksort? I tried everything I could think of and it doesn't change.

Anyway it takes:

 Planning Time: 0.582 ms
 Execution Time: 2.163 ms

Copy link
Member

Choose a reason for hiding this comment

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

Can you C&P the query here?

Copy link
Member Author

Choose a reason for hiding this comment

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

explain analyze SELECT copr.id, copr_private.copr_id, "user".id AS id_1, user_private.user_id, "user".username, "user".proven, "user".admin, "user".openid_groups, user_private.mail, user_private.timezone, user_private.api_login, user_private.api_token, user_private.api_token_expiration, copr.name, copr.homepage, copr.contact, copr.repos, copr.created_on, copr.description, copr.instructions, copr.deleted, copr.playground, copr.auto_createrepo, copr.user_id AS user_id_1, copr.group_id, copr.forked_from_id, copr.build_enable_net, copr.unlisted_on_hp, copr.latest_indexed_data_update, copr.persistent, copr.auto_prune, copr.isolation, copr.bootstrap, copr.follow_fedora_branching, copr.scm_repo_url, copr.scm_api_type, copr.delete_after, copr.multilib, copr.module_hotfixes, copr.runtime_dependencies, copr.fedora_review, copr.appstream, copr.packit_forge_projects_allowed, copr.repo_priority, copr_private.webhook_secret, copr_private.scm_api_auth_json  FROM copr LEFT OUTER JOIN copr_private ON copr.id = copr_private.copr_id JOIN ("user" LEFT OUTER JOIN user_private ON "user".id = user_private.user_id) ON "user".id = copr.user_id LEFT OUTER JOIN "group" ON "group".id = copr.group_id  WHERE copr.deleted IS false AND "user".username = 'frostyx' AND copr.group_id IS NULL ORDER BY copr.delete_after NULLS FIRST, copr.id DESC;

Copy link
Member

Choose a reason for hiding this comment

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

I haven't found a good index either. Seems like we work with a relatively small number of rows (coprs owned by a single user), yet the query is a relatively complicated set of joins. The production DB performs very well, too. +1

@praiskup
Copy link
Member

Good job, not sure about the delete_after index, otherwise LGTM

Copy link
Member

@praiskup praiskup left a comment

Choose a reason for hiding this comment

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

LGTM

@praiskup praiskup merged commit 5adf057 into fedora-copr:main May 6, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants