sneako / finch

Elixir HTTP client, focused on performance
MIT License
1.23k stars 114 forks source link

Discard not ready connections from HTTP/2 pool #227

Open Goose97 opened 1 year ago

Goose97 commented 1 year ago

HTTP2 connections will unregister themselves from the pool once enter connected_read_only and disconnected states, and register themselves once they reconnect.

Fixes #216

sneako commented 1 year ago

Thanks for taking this on! I am curious, did you run any kind of stress testing on this? I would like to see how the error rate and throughput compare to what we have in main.

Goose97 commented 1 year ago

Oh, I haven't. Do we have any kind of stress testing/benchmark framework already in the repo? If not, do you have any suggestions about tools/frameworks?

sneako commented 1 year ago

Sorry for the delay, but I finally got around to testing this out and found just a minimal performance regression but a total elimination of errors that are present on finch:main, so I think we should proceed with this change. I fixed up a couple things, but I want to merge #228 first, and then deal with any merge conflicts in this pr.

hkrutzer commented 11 months ago

@sneako is this the last issue for a new release?

nallwhy commented 6 months ago

I made a application using OpenAI API via Finch HTTP2 pool. And I got so much (Finch.Error) disconnected errors. Is this issue related to the problem?

sneako commented 6 months ago

I made a application using OpenAI API via Finch HTTP2 pool.

And I got so much (Finch.Error) disconnected errors.

Is this issue related to the problem?

It could be. I would be very interested to know if this PR helps with the issue you are seeing. I have been struggling to find time to test this properly myself.