-
Notifications
You must be signed in to change notification settings - Fork 25
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
Explorer tools performance #305
Comments
I don't know if it's a red herring, but I'm still getting 17s from the database on tmpfs (disk latency seems important...), after changing: diff --git a/cosima_cookbook/querying.py b/cosima_cookbook/querying.py
index 8ff2e47..05cf3c1 100644
--- a/cosima_cookbook/querying.py
+++ b/cosima_cookbook/querying.py
@@ -210,7 +210,7 @@ def get_variables(
q = q.outerjoin(subq, subq.c.ncvar_id == NCVar.id)
q = q.order_by(NCFile.frequency, CFVariable.name, NCFile.time_start, NCFile.ncfile)
- q = q.group_by(CFVariable, NCFile.frequency)
+ q = q.group_by(CFVariable.id, NCFile.frequency)
if experiment is not None:
q = q.group_by(subq.c.value) Before the change was about 30s:
I think any latency beyond that is down to serving the database over lustre, which is unavoidable... |
Nice detective work, and a good idea to take the disk latency out of the equation. I don't know why it used to be so much faster. It was on the OOD, so perhaps the IO was being saturated. I know that it has always been slower on the cloud infrastructure. Now I am intrigued by the option of loading the DB into memory to improve performance. A couple of quick tests on
Second was fast because it was still in cache. Third was after ~30min break, so similar to first. Sounds like that code change is worth making if it halves the time. |
The time it takes to load the Database Explorer seems to have increased quite markedly recently.
Previous timings when the explorer was retooled because it was taking a minute to start up suggests this is a recent, and significant, slow down
#274 (comment)
TL;DR it used to take 17s
The text was updated successfully, but these errors were encountered: