In the case of groups and permissions there's probably only a few of each, so fetching all of them is probably fine. But depending on your data -- say you're fetching comments written by a subset of users, you can tweak the above to use IN filtering, something like this Python-ish code:
users = select('SELECT id, name FROM users WHERE id IN $1', user_ids)
comments = select('SELECT user_id, text FROM comments WHERE user_id IN $1', user_ids)
comments_by_user_id = defaultdict(list)
for c in comments:
comments_by_user_id[c.user_id].append(c)
for u in users:
u.comments = comments_by_user_id[u.id]
For development, we had a ?queries=1 query parameter you could add to the URL to show the number of SQL queries and their total time at the bottom of the page. Very helpful when trying to optimize this stuff. "Why is this page doing 350 queries totalling 5 seconds? Oops, I must have an N+1 query issue!"
users = User.objects.all()
for u in user:
print(u.name, len(u.comments))
users = select('SELECT id, name FROM users WHERE id IN $1', user_ids)
for u in user:
num_comments = get('SELECT COUNT(*) FROM comments WHERE user_id = $1', u.id)
print(u.name, num_comments)
The other thing I don't like about (most) ORMs is they fetch all the columns by default, even if the code only uses one or two of them. I know most ORMs provide a way to explicitly specify the columns you want, but the easy/simple default is to fetch them all.
I get the value ORMs provide: save a lot of boilerplate and give you nice classes with methods for your tables. I wonder if there's a middle ground where you couldn't do obviously bad things with the ORM without explicitly opting into them. Or even just a heuristic mode for development where it yelled loudly if it detected what looked like an N+1 issue or other query inside a loop.
https://github.com/django-query-profiler/django-query-profil... has a neat option for detecting likely N+1 queries. I usually use the Django Debug Toolbar for this.
Django's ".only()" method lets you specify just the columns you want to retrieve - with the downside that any additional property access can trigger another SQL query. I thought I'd seen code somewhere that can turn those into errors but I'm failing to dig it up again now.
I've used the assertNumQueries() assertion in tests to guard against future changes that accidentally increase the number of queries being made without me intending that.
The points you raise are valid, but there are various levels of mitigations for them. Always room for improvement though!
users = User.objects.all()
for u in user:
print(u.name, len(u.comments))
Django ORM has a succinct way of doing the "SELECT COUNT(*)" pattern:
users = User.objects.all()
for u in user:
print(u.name, u.comments.count())
users = User.objects.annotate(n_comments=Count("comments"))
for u in user:
print(u.name, u.n_comments)And this is how you end up with the problems the parent is describing. During testing and when you setup the system you always have a small dataset so it appears to work fine. But when it’s real work the system collapses.
> SELECT user_id, COUNT(*) FROM comments WHERE user_id IN $1 GROUP BY user_id
Fwiw, ActiveRecord with the Bullet gem does exactly that. I'd guess there's an equivalent for Django.
SqlAlchemy has this as part of the ORM, it should really be part of Django IMO.
It's one of those things that in the long run would have been much more time effective to write, but the debugging never quite took long enough each time to make me take the time.
I'm so glad Symfony (PHP framework) has a built-in profiler and analysis tooling there...
[0]: https://stackoverflow.com/questions/97197/what-is-the-n1-sel...
For SQL you can also do a stored procedure. Sometimes that works well if you are good at your DBMS's procedure language and the schema is good.
With either technique you are still pulling all the data you need from the DB but with multiple queries instead of a stored procedure you are usually pulling more data than you need with each query and then dropping any rows or fields you’re not interested in. Together with multiple calls over the network to the DB server and (often) multiple SQL connection setups this is much worse for performance on both the web and database servers
Lots of people seem to not realize that db roundtrips are expensive, and should be avoided whenever possible.
One of the best illustrations of this I've found is in Transaction Processing book by Jim Gray and Andreas Reuters where they illustrate the relative cost of getting data from CPU vs CPU cache vs RAM vs cross host query.
for each user in (get_freeipa_users | grep_attribute uid):
email = (get_freeipa_users | client_side_find user | grep_attribute email)
last_change = (get_freeipa_users | client_side_find user | grep_attribute krblastpwdchange)
expiration = (get_freeipa_users | client_side_find user | grep_attribute krbpasswordexpiration)
# Some slightly incorrect date math...
send_email
- it's memory efficient
- it's atomic
- it's faster
Also doesn't LDAP support filtering in query?
It turned out that the dashboard had been built on top of Wordpress. The way that it checked if the user had permission to access the dashboard was to query all users, join the meta table which held the permission as a serialized object, run a full text search to check which users had permission to access this page, and return the list of all users with permission to access the page. Then, it checked if the current user was in that list.
I switched it to only check permissions for the current user, and the page loaded instantaneously.
Everything else are network hops and what I call "distributed clutter", including authorizing via a third party like Auth0 multiple times for machine-to-machine token (because "zero trust"!), multiple parameter store calls, hitting a dcache, if interacting with a serverless function, cold starts, API gateway latency, etc...
So for the meat of a 20-40 ms call, we get about a 400ms-2s backend response time.
Then if you are loading a front end SPA with javascript...fugetaboutit it
But DevOps will say "but my managed services and infinite scalability!"
In that case, was there a reason joins couldn't be used? As it still seems pretty wasteful (and less performant) to load all of this data in memory and post-process; whereas a well-indexed database could possibly do it faster and with less-memory usage.
The mistake of course was not thinking about why this approach is faster in a database query and that it doesn't work that way when you already need to get all the data out of LDAP to do anything with it.
We had a system management backend at my last company. Loading the users list was unbearably slow; 10+ seconds on a warm cache. Not too terrible, except that most user management tasks required a page reload, so it was just wildly infuriating.
Eventually I took a look at the code for the page, which queried LDAP for user data and the database for permissions data. It did:
I'm no algorithm genius, but I'm pretty sure O(n^2+n^3) is not an efficient one.I replaced it with
Suffice to say, it was a lot more responsive.Also worth noting was that fetching the user list required shelling out to a command (a python script) which shelled out to a command (ldapsearch), and the whole system was a nightmare. There were also dozens of pages where almost no processing was done in the view, but a bunch of objects with lazy-loaded properties were passed into the template and always used, so when benchmarking you'd get 0.01 seconds for the entire function and then 233 seconds for "return render(...)' because for every single row in the database (dozens or hundreds) the template would access a property that would trigger another SQL call to the backend, rather than just doing one giant "SELECT ALL THE THINGS" and hammering it out that way.
Note that we also weren't using Django's foreign keys support, so we couldn't even tell Django to "fetch everything non-lazily" because it had no idea.
If that app were written right it could have run on a Raspberry Pi 2, but instead there was no amount of cores that could have sped it up.