-
Notifications
You must be signed in to change notification settings - Fork 30
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
Switch HTTP client to Finch #60
base: master
Are you sure you want to change the base?
Conversation
@hkrutzer Hi! Thanks for contribution, I'll look ASAP and give feedback |
@hkrutzer Should I do some extra actions? I've tried this PR on my project and got error on startup:
|
That pool should be started from |
Do you have production project with pillar library? Could you check with update, how does it work? In my case I've added as dependency and started tests, 10 tests failed, and by some reason some tests stucked. So |
@hkrutzer Could you make Finch client as additional? (via config mechanism https://github.com/balance-platform/pillar#http-adapters ) It could fix problem with backward compatibility |
I can take a look @sofakingworld but I'm not sure it will be possible. In my PR, Poolboy is replaced with Finch. I am not sure it can be done in a way where only other adapters use Poolboy and it is not used with Finch. Which backward compatibility do you mean? |
To start operation, now you need to start Pillar.Application. For this reason, it is necessary that the Finch client be optional and configurable via config |
87d3426
to
e30960d
Compare
As discussed in #39, this will allow for the addition of streaming queries. If this PR is accepted I will add those as well.
Some async functions no longer make sense to include so I have deprecated them.
Benchmarking with Benchee shows significant improvement, likely at least partially due to Finch using keepalive.
Benchmarks
All benchmarks were performed on the following configuration:
Except the ones where I set
parallel: 4
. Clickhouse was running on the same machine. To get more reliable measurements the benchmark should probably performed on different machines.With Tesla
With parallel: 4 in Benchee
With Finch (this PR)
With parallel: 4 in Benchee
As indicated by these benchmarks, performance can be improved 20 times by using Finch connection pools. Complexity is also reduced because Poolboy is no longer necessary.
Let me know what you think and if you need any changes made to this PR.