-
Notifications
You must be signed in to change notification settings - Fork 63
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
Extremely slow test on ci runner #98
Comments
Could you paste the exact version numbers of ecto_sql and myxql you tried? |
ecto 3.3.1 |
what's your myxql version? |
0.3.1 |
@nulian do they run faster if you use named prepared statements ? We're investigating a similar issue with 0.3.2 where tests are 4-5x slower with unnamed prepare. |
It does seem to fix test performance removing the unnamed statement flag. |
Retested them. The myxql without unnamed option performance I would be overall satisfied with. Mariaex test run: 31.5s |
I encountered the same problem, but it was solved by setting
However, I didn't know why this option isn't needed in mariaex. |
Thank you @melpon! That change got our test execution time back down to where it should be. |
Hello @josevalim @wojtekmach. Maybe not related to the original problem (let me know if this should go to the separate issue) but I've noticed performance slow-down after 7a5363a. That commit leads to the case when:
Will create 3 prepared statements. After first query - first prepared statement will be created and executed - as expected. After the second query the first prepared statement will be closed and second will be created. And after the third query - second prepared statement will be closed and new one created again. Obviously that may significantly hurt perfomance. Especially if an application often executes insert/update for the same model. Tests of my application are executed almost two times slower after update of myxql. That commit was introduced in context of I am not really familar with postgex code, but it seems it does not do such things https://github.com/elixir-ecto/postgrex/blob/master/lib/postgrex/protocol.ex#L3363 |
@0xAX the commit above should only reprepare the query if they are different. I would expect the three Repo.insert above to be the exact same statement. Can you check if that's the case or not for you?
MySQL unfortunately has a limit on the number of prepared statements, which means we need to be more aggressive to purge them. |
@josevalim Yes, I checked and I see (in wireshark) that prepared statement is closed everytime after new such insert |
re-checked just for the case: I've set pool_size of my Repo configuration to
and I see following queries were sent to my db: is it expected behaviour?
Thanks for explanation. But isn't it too agressive? |
Oh to be clear, it shouldn't be closing if the query (i.e. the SQL) is the same. So my question is if the statements that are logged are the same or if they have small differences between them. |
That is exactly the problem. They are different. First and third do not contain placeholder for
and the second one:
I've found a ticket that describes that |
I see. So it is nothing to change here. The issue is that we are using the same name for different queries. If you change this line: https://github.com/elixir-ecto/ecto_sql/blob/master/lib/ecto/adapters/myxql.ex#L259 To be:
It should introduce another degree of caching. Another option is to emit all fields and have the value be |
Nevermind, I believe we can't pass DEFAULT as parameter. :( |
@josevalim Yes, thank you for this, I saw cache_statement in ecto_sql but didn't think about touch it. So I will prepare PR for ecto_sql if you don't have any objections. But before I have some additional question/suggestions about
|
Agreed on both notes. Thanks for the convo and I am looking forward to your PR! |
Weird I had that same delay issue trigger on my linux dev environment after installing updates. Which I could solve with the nodelay: true. |
Closing this. It is stale and it may even have been fixed in Ecto (it has been a while, so I don’t fully remember). |
I'm trying to upgrade ecto and to myxql but for some reason on the ci runner the tests are extremely slow like 10x slower then normal.
Running test locally they perform fine with the ecto upgrade.
All the queries have around 40 ms delay like below. So originally I thought it's maybe because of the CI server not having the unix socket and it takes myxql some time to fallback. But even when setting protocol in test config for ecto on :tcp it still has the around 40 ms delay on everything.
QUERY OK source="applications" db=39.9ms
QUERY OK db=0.4ms queue=40.3ms
It's a kubernetes gitlab ci runner with mysql running a connected service.
With like the following ecto config
The text was updated successfully, but these errors were encountered: