search: limit queries to 20s #854
Conversation
|
For example, https://hydra.nixos.org/search?query=chromium takes about 16s in total. Changing to pkgs.chromium takes 500+s. |
src/lib/Hydra/Controller/Root.pm
Outdated
| builds => $c->stash->{builds}, | ||
| buildsdrv => $c->stash->{buildsdrv} }; | ||
| $c->model('DB')->schema->txn_do(sub { | ||
| $c->model('DB')->schema->storage->dbh->prepare("SET statement_timeout = 20000;")->execute(); |
There was a problem hiding this comment.
What is the difference compared to SET LOCAL statement_timeout …? Does this have the possibility to escape the transaction?
There was a problem hiding this comment.
According to https://www.postgresql.org/docs/10/sql-set.html this would set the timeout for all other queries, even those outside of the transaction. We probably want SET LOCAL instead:
The effects of SET LOCAL last only till the end of the current transaction, whether committed or not. A special case is SET followed by SET LOCAL within a single transaction: the SET LOCAL value will be seen until the end of the transaction, but afterwards (if the transaction is committed) the SET value will take effect.
There was a problem hiding this comment.
Good catch.
If SET (or equivalently SET SESSION) is issued within a transaction that is later aborted, the effects of the SET command disappear when the transaction is rolled back. Once the surrounding transaction is committed, the effects will persist until the end of the session, unless overridden by another SET.
The effects of SET LOCAL last only till the end of the current transaction, whether committed or not. A special case is SET followed by SET LOCAL within a single transaction: the SET LOCAL value will be seen until the end of the transaction, but afterwards (if the transaction is committed) the SET value will take effect.
Changing to SET LOCAL.
There was a problem hiding this comment.
Fixed, tested locally that it still works. Thanks!
grahamc
force-pushed
the
limit-query-time
branch
from
b8a3b63
to
9f8c394
Compare
src/lib/Hydra/Controller/Root.pm
Outdated
| builds => $c->stash->{builds}, | ||
| buildsdrv => $c->stash->{buildsdrv} }; | ||
| $c->model('DB')->schema->txn_do(sub { | ||
| $c->model('DB')->schema->storage->dbh->prepare("SET LOCAL statement_timeout = 20000;")->execute(); |
There was a problem hiding this comment.
Why prepare->execute rather than just do?
There was a problem hiding this comment.
No reason than muddling through the DBIx docs made me just find other examples in Hydra 😅
Even 20s is really long, but it cuts off queries which are today running for 500+s.
grahamc
force-pushed
the
limit-query-time
branch
from
9f8c394
to
91e63fb
Compare
|
Maybe we should just remove the search feature if it's that expensive. Or limit it to things that can be queried efficiently. |
|
I have some in progress work which improves search from ~15s to ~1s, so
not sure we should ditch it already.
On January 31, 2021, GitHub ***@***.***> wrote:
Maybe we should just remove the search feature if it's that expensive.
Or limit it to things that can be queried efficiently.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#854 (comment)>, or
unsubscribe <https://github.com/notifications/unsubscribe-
auth/AAASXLHJMNC7UYQGDH7PKU3S4WMHLANCNFSM4W2OHJCA>.
|
Even 20s is really long, but it cuts off queries which are today
running for 500+s.