readthedocs / ext-theme

Read the Docs drop in replacement site templates
2 stars 2 forks source link

Organizations: security log page doesn't load #209

Closed agjohnson closed 11 months ago

agjohnson commented 1 year ago

I think this is specific to our organization, because of the number of log lines. But this UI does load on the production instance.

The URL I am having trouble with is: https://beta.readthedocs.com/organizations/read-the-docs/security-log/

This produces a 502 response, timing out as the beta instance tries to load the page.

agjohnson commented 11 months ago

I thought maybe the pagination wasn't being used correctly, but I am indeed able to use pagination locally. What is odd is this is a view that there are no additional view logic added for. It is using the same filter instance in production, so I'm not quite sure what would be different.

This is difficult to debug without debug toolbar, new relic, or sentry. @humitos do you remember why these aren't set up? I tried to enable debug toolbar really quickly and was not able to see it on the site.

agjohnson commented 11 months ago

I have been very unsuccessful at tracking this down.

I upgraded the RTD QA organization to a free pro plan to get security logs there and all the profiling looks fine :confused:

I added a slice to the main template to limit the number of objects being paginated (100k objects), still nothing notable in profiling.

So somewhere between 100k and 1M+ objects paginated, this display breaks :shrug:

Edit: it seems 500k is right on the edge of functional. I got a profile back that looks bad, but I'll leave off here for today. I don't see anything obvious yet, other than obviously the query performance falls off steeply at 500k objects. My only guess is that there is some memory/etc issue on this smaller instance. But that points to a larger problem with the query able to consume this much memory.

image

humitos commented 11 months ago

This is difficult to debug without debug toolbar, new relic, or sentry. @humitos do you remember why these aren't set up? I tried to enable debug toolbar really quickly and was not able to see it on the site.

Django Debug Toolbar is not enabled on production due to security reasons. It's only enabled on admin instance. However, New Relic and Sentry should be enabled. New Relic example for web-ext-theme-* instances: https://onenr.io/0qwL67kLzj5

agjohnson commented 11 months ago

Ah yes, thanks for the pointer on New Relic. The instance filter was the missing part here, I was not finding that. This gives me at least something to work on, it does look like the template render step is where all the time is spent. It's either dj-pagination or something in the templates.

agjohnson commented 11 months ago

Found it! Apparently if not objects_list does something heinous in the background. I'm guessing it evaluates everything in the list for truthiness?

This conditional was intentional, to support iteration over both querysets and arrays, but changing this to if not objects_list.exists solved the problem on this view. I'll have to think how to solve this more universally.

humitos commented 11 months ago

Apparently if not objects_list does something heinous in the background. I'm guessing it evaluates everything in the list for truthiness?

Yes, if you do if queryset the underlying SQL query is executed. If you only want to know if there is a result, you have to call .exists() as you figured it out.

In the "When QuerySets are evaluated" from https://docs.djangoproject.com/en/5.0/ref/models/querysets/, it says:

bool(). Testing a QuerySet in a boolean context, such as using bool(), or, and or an if statement, will cause the query to be executed. If there is at least one result, the QuerySet is True, otherwise False. For example: Note: If you only want to determine if at least one result exists (and don’t need the actual objects), it’s more efficient to use exists().