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

[Idea wanted] Add centralised link list in the network management [long loading time] #2898

Closed
wants to merge 1 commit into from

Conversation

MizukiTemma
Copy link
Member

@MizukiTemma MizukiTemma commented Jul 10, 2024

Short description

This is the (probably) easiest and laziest implementation of centralised link list and the classical problem exists here too: the loading time is unacceptably long.

This PR is not intended as final proposal and opened to demonstrate the problem and to ask for ideas/suggestion how to solve it.

  • Do you see any possibility to improve loading time with this implementation?
  • If we structure the centalised link checker in another way (different from region link list), what do you imagine?

See this cool insctuction for how to test with realistically large data.

Proposed changes

  • Reuse the template and view of region link list

Side effects

  • Extremely long loading time

Resolved issues

Fixes: #1443


Pull Request Review Guidelines

Copy link

codeclimate bot commented Jul 10, 2024

Code Climate has analyzed commit b76811b and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 55.5% (50% is the threshold).

This pull request will bring the total coverage in the repository to 83.1% (0.0% change).

View more on Code Climate.

@david-venhoff david-venhoff self-requested a review July 22, 2024 13:37
@david-venhoff
Copy link
Member

david-venhoff commented Jul 22, 2024

I have done some optimizing and pushed everything to a branch: feature/global_link_list_performance
Originally, the global link list did not load at all for me, and with this branch it takes about 7 seconds.
Feel free to merge if it makes sense to you :)

The actual database queries only take about 500ms, so there is still a lot of potential for optimization, but I think that would require a bit more code restructuring. The lowest hanging fruit it probably removing the hot python loop in filter_urls. Maybe it would be possible to use something like https://docs.djangoproject.com/en/5.0/ref/models/expressions/#window-functions for partitioning everything in a query, i will take a look at the the next days.


The main problem with the current approach was that the python loop in filter_urls performed a database query for every single of those ~100k urls, which was way too slow. Moving everything into a single query causes the page to load for me with production data. Also calling prefetch is not required when loading all links, and removing that also gave a nice performance boost.

@MizukiTemma
Copy link
Member Author

As succeeded by #2971

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.

Show broken links in network management
2 participants