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

[Roles Page] page load is very slow #15854

Closed
3 tasks done
aseembansal-gogo opened this issue Jul 23, 2021 · 28 comments · Fixed by #27233
Closed
3 tasks done

[Roles Page] page load is very slow #15854

aseembansal-gogo opened this issue Jul 23, 2021 · 28 comments · Fixed by #27233
Labels
#bug Bug report

Comments

@aseembansal-gogo
Copy link

aseembansal-gogo commented Jul 23, 2021

On superset 1.1.0 roles list page loading is slow. It takes approx. 16 seconds to load. From roles listing page if I click edit to edit a role then it takes 18 to 20seconds for the edit page to open up. This makes working with roles very difficult.

Viewing user list takes just 2s in comparison.

Expected results

Roles related pages to load in <5seconds.

Actual results

Roles related pages load in >15seconds.

Screenshots

See developer tools
image

How to reproduce the bug

  1. Go to Roles list page
  2. Edit a role

Environment

(please complete the following information):

  • superset version: superset version 1.1.0
  • python version: python --version Python 3.7.9
  • node.js version: node -v NA. docker images does not have node

Checklist

Make sure to follow these steps before submitting your issue - thank you!

  • I have checked the superset logs for python stacktraces and included it here as text if there are any.
  • I did not upgrade to latest release but I searched latest release notes and there isn't anything related to slowness
  • I have checked the issue tracker for the same issue and I haven't found one similar.
@aseembansal-gogo aseembansal-gogo added the #bug Bug report label Jul 23, 2021
@aseembansal-gogo
Copy link
Author

The UI works. It is just slow. The DB is not slow as I am able to query the same tables (ab_role etc.) very fast via sql lab. There must be something in the code itself which is causing this slowness when the page is being loaded.

@stale
Copy link

stale bot commented Apr 30, 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. For admin, please label this issue .pinned to prevent stale bot from closing the issue.

@stale stale bot added the inactive Inactive for >= 30 days label Apr 30, 2022
@grumbler
Copy link

I have the same with the Superset 1.4.2

@stale stale bot removed the inactive Inactive for >= 30 days label Aug 16, 2022
@hungpv29
Copy link

hungpv29 commented Oct 18, 2022

I have the same with the Superset 1.5.1. When go to List Roles Page, it calls Postgres ab_view_menu select 44900 times

image

Query:

SELECT ab_view_menu.name AS ab_view_menu_name FROM ab_view_menu JOIN ab_permission_view ON ab_view_menu.id = ab_permission_view.view_menu_id JOIN ab_permission ON ab_permission.id = ab_permission_view.permission_id JOIN ab_permission_view_role ON ab_permission_view.id = ab_permission_view_role.permission_view_id JOIN ab_role ON ab_role.id = ab_permission_view_role.role_id JOIN ab_user_role ON ab_role.id = ab_user_role.role_id JOIN ab_user ON ab_user.id = ab_user_role.user_id WHERE ab_user.id = %(id_1)s AND ab_permission.name = %(name_1)s

@circobit
Copy link

circobit commented Jun 2, 2023

Hey, folks! Have anyone found a workaround to this? I'm facing the same with version 2.0.1 and 2.1.0.

@ohappykust
Copy link

ohappykust commented Jun 29, 2023

Hi!
Let me bring some clarity. This problem does not come from Superset, but from the Flask AppBuilder library, on which our favorite product is based.
I created just recently there a new issue about this. With the team we are now excavating this problem, because we get up to 5-10 minutes of page load 🤯

dpgaspar/Flask-AppBuilder#2073

@tirkarthi
Copy link
Contributor

There was similar issue in Airflow where roles and users page was slow apache/airflow#31340. The fix was to remove certain columns from filter with foreign key reference that causes queries to increase. In Airflow case created_by, changed_by were present in filter widget referencing to users and large number of users increased the the time it takes to load all users. Relevant flask-appbuilder issues

dpgaspar/Flask-AppBuilder#796
dpgaspar/Flask-AppBuilder#875

@tirkarthi
Copy link
Contributor

If filtering by permission and users is not needed then please try adding RoleModelView.search_exclude_columns = ["permissions", "users"] in below code that should help with page load

RoleModelView.list_columns = ["name"]
RoleModelView.edit_columns = ["name", "permissions", "user"]
RoleModelView.related_views = []

@ohappykust
Copy link

ohappykust commented Jul 10, 2023

@tirkarthi this solution is already in the Superset codebase and it doesn't work in the same way that people's solutions don't with modifying or cutting out the search.
In the query function, in the SQLAInterface class, where the query itself occurs, there are no external link attachments, which, presumably, at the time the template is rendered, causes a large number of queries.

https://github.com/dpgaspar/Flask-AppBuilder/blob/7f194cfd28099688e0a398ca7b1317aafa804c92/flask_appbuilder/models/sqla/interface.py#L458-L507

@Paulo456
Copy link

search_exclude_columns helps to fast load but permissions - absence
problem solves with adding to find_roles_permission_view_menus and exist_permission_on_roles
this option:
.options(contains_eager(self.permissionview_model.view_menu))
then count query to check menu_view will be lower

bigger problem in representation:
class PermissionView(Model):
....
return str(self.permission).replace("_", " ") + " on " + str(self.view_menu)

from this point we have big amount of queries to two tables ab_view_menu and ab_permission

@Paulo456
Copy link

Paulo456 commented Jul 11, 2023

in log sqlalchemy looks good if u change model PermissionView to this:

- permission = relationship("Permission")
+ permission = relationship("Permission", lazy="joined")
view_menu_id = Column(Integer, ForeignKey("ab_view_menu.id"))
- view_menu = relationship("ViewMenu")
+ view_menu = relationship("ViewMenu", lazy="joined")

@Paulo456
Copy link

Paulo456 commented Aug 18, 2023

if u have 50k+ virtual datasets - to edit very slow in frontend superset.
to change this - check only phis datasets for access and parse virt dataset for real tables, like sqllab.

u need to update security manager superset for check real tables and set sql updates (example):


UPDATE public.ab_view_menu SET "sql"=false where 1=1;

UPDATE public.ab_view_menu
SET "sql" = true
from tables t
WHERE t.perm = public.ab_view_menu."name" 
and (t."sql" is not null or t."sql" != '');

to update python code in FAB model:

class PermissionView(Model):
    __tablename__ = "ab_permission_view"
    __table_args__ = (UniqueConstraint("permission_id", "view_menu_id"),)
    id = Column(Integer, Sequence("ab_permission_view_id_seq"), primary_key=True)
    permission_id = Column(Integer, ForeignKey("ab_permission.id"))
    permission = relationship("Permission", lazy="joined")
    view_menu_id = Column(Integer, ForeignKey("ab_view_menu.id"))
    +view_menu = relationship(
        ViewMenu,
        primaryjoin="""and_(
                ViewMenu.id==PermissionView.view_menu_id,
                or_(ViewMenu.sql==False, ViewMenu.sql == None )
                )""",
        innerjoin=True,
        lazy="joined",
    )


class ViewMenu(Model):
    __tablename__ = "ab_view_menu"
    id = Column(Integer, Sequence("ab_view_menu_id_seq"), primary_key=True)
    name = Column(String(250), unique=True, nullable=False)
   + sql = Column(Boolean)```


@Paulo456
Copy link

Paulo456 commented Aug 29, 2023

also add to set_perm

            +if hasattr(target, 'sql') and target.sql:
             +   return

            if not pv and permission and view_menu:

@ambarishp
Copy link

I am facing this issue in version 3.1.0. The Role edit request taking more that minute. Any fix or work around available

@Paulo456
Copy link

Paulo456 commented Feb 16, 2024

You can set this to FAB security model

- permission = relationship("Permission")
+ permission = relationship("Permission", lazy="joined")

@ohappykust
Copy link

Yup)

File: flask_appbuilder/security/sqla/models.py
Line 76

permission = relationship("Permission", lazy="joined")
view_menu = relationship("ViewMenu", lazy="joined")

@ambarishp
Copy link

Thanks @Paulo456 and @happykust, It worked.

@rusackas
Copy link
Member

@happykust @ambarishp I'm a little lost in all this now 😅 Is there something that needs to be done to close the loop here (adding documentation, a PR to fix things, etc.?)

@Paulo456
Copy link

@rusackas I think the problem is that the superset is not designed for the number of dataset entities greater than 50 thousand. More precisely, the problem is not in the superset itself, but in the FAB model. The current solution is just a workaround.

@ohappykust
Copy link

@rusackas The problem is solely in the Flask-AppBuilder library, in the file described above. Its author apparently does not want to fix this; there have already been a lot of discussions about this. In our company, we forked FAB, fixed it and are enjoying life.

@rusackas
Copy link
Member

CC @dpgaspar

@dpgaspar
Copy link
Member

@happykust I'm sorry to ear that, can you point to any current/past issue/PR on flask-appbuilder or open a new one please, so we can visit this.

@ohappykust
Copy link

@ohappykust
Copy link

@dpgaspar I've been adding an issue for quite some time now. I switched it to closed status because of the solution found for me. Will you have a chance to fix this? Or are there any pitfalls when using the solution written above?

@ohappykust
Copy link

@dpgaspar Thank you very much for the fixes!

@dpgaspar
Copy link
Member

dpgaspar commented Feb 23, 2024

@dpgaspar Thank you very much for the fixes!

np, FAB 4.4.1 is released with the fix, bumping Superset also.

@pashtet04
Copy link

Which version of Superset has this fix?

The issue is still in 3.1.1.
And I couldn't find where to fix it manually in the container image because there is nothing in src except the migrations directory

superset@superset-1661616798-779bfb6d69-vxsqw:/app$ grep -ri 'permission = relationship' .
./superset/migrations/versions/2020-09-24_12-04_3fbbc6e8d654_fix_data_access_permissions_for_virtual_.py:    permission = relationship("Permission")
./superset/migrations/versions/2022-05-18_16-07_e786798587de_delete_none_permissions.py:    permission = relationship("Permission")
./superset/migrations/shared/security_converge.py:    permission = relationship("Permission")

@ohappykust
Copy link

ohappykust commented Jun 25, 2024

@pashtet04

Yep, Superset 3.1.1 doesn't have this fixes because of use FAB version 4.3.11, not 4.4.1.
You should update your Superset instance to 3.1.2 version or manually upgrade your FAB to 4.4.1 version in your environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
#bug Bug report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

12 participants