Possible memory leak?

@jakeonfire

Imagine a vanilla rails app, where controller actions might look like:

class SomeController < ApplicationController
  def update
    response = request_handler.do_post("https://microsoft.com/update_thing", params[:post_body])
    render json: response.body
  end

  def request_handler
    @_request_handler ||= RequestHandler.new
  end
end

and RequestHandler something like:

class RequestHandler
  def do_post(url, body)
    task = Async do
      conn.post(url) do |req|
        req.body = body
      end
    end
    response = task.wait
  end

  def conn
    Faraday.new do |c|
      c.adapter :async_http
    end
  end
end

the app is using falcon host or falcon virtual with the rack configuration, a preload of the rails environment, and endpoint Async::HTTP::Endpoint.parse("http://0.0.0.0:3000"). pretty simple.

Originally posted by @jakeonfire in #32 (comment)

I believe you need to invoke #close on conn after you are done with it... Are you able to check if this solves your problem? I will also take a look on my end if I have time.

i thought about that but it doesn't seem closeable. faraday should close the adapter automatically after every request afaik.

NoMethodError: undefined method `close' for #<Faraday::Connection...

It should be this one:

https://github.com/lostisland/faraday/blob/main/lib/faraday/connection.rb#L122-L127

ah i'm looking at the wrong source. let me try with faraday 1.x.

#close works. canarying the change in production...

would it make sense to restrict async-http-faraday to faraday >= 1.0? might help others to put a call to close in the README instructions, as well. something like:

task = Async do
	response = conn.get("/index")
ensure
	conn.close
end

I recommend trying the

begin
  connection = ...
  connection.get ...
ensure
  connection.close
end

model to start with (as it seems you are). However, you won't get persistent connections this way.

So, after you tried that, use the thread-local gem: https://github.com/socketry/thread-local?tab=readme-ov-file to share an instance:

connection = MyConnection.instance
connection.get ...

Using the persistent connection will improve throughput and latency.

getting a different issue now

despite the new issue, memory usage is stable, so closing the connection seems to have been the ticket 🥳

Faraday's myriad instantiation-time configurations and state makes sharing a connection difficult. what if different requests from the same thread need different middleware?

i guess i could create a separate thread-local class for each configuration, or a single thread-local with a hash of different options => different faraday connections...

Yeah, I understand your issue. For the sake of persistent connections, maybe we want a shared internal pool of HTTP connections. Most of the request/response processing won't actually impact the underlying connection pool...

yeah that sounds about right. i may take a look.

you can close this issue 🙂

@ioquatix

Using the persistent connection will improve throughput and latency.

@ioquatix do you think using a process-wide connection pool instead of a thread-local one, via https://github.com/mperham/connection_pool (or https://github.com/socketry/async-pool which i think does the same thing?), might be more efficient in some ways?

i noticed async-http uses its own connection pool: https://github.com/socketry/async-http/blob/main/lib/async/http/client.rb#L39

No, you should avoid doing additional connection pooling.

The reason is, there are life-cycle issues at play.

Using thread-local for your connection object is the best but... I'll try to default to the shared connection pool provided by async-http. Please give me a day to check it and come back to you.

thought you might like to know: with falcon and async-http-faraday (and persistent faraday connections) our servers are about 50% more efficient. using a bit more CPU and slightly more memory, but able to handle (at least) 2x the traffic with half the latency.

@jakeonfire

@jakeonfire is it okay to use your testimonial on my website?

@ioquatix

@ioquatix sure!

Navigation Menu

Repository navigation

Possible memory leak? #35

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Possible memory leak? #35

Description

Activity

ioquatix commented on Feb 7, 2024

jakeonfire commented on Feb 7, 2024

ioquatix commented on Feb 7, 2024

jakeonfire commented on Feb 7, 2024

jakeonfire commented on Feb 7, 2024

ioquatix commented on Feb 7, 2024

jakeonfire commented on Feb 7, 2024

jakeonfire commented on Feb 7, 2024

jakeonfire commented on Feb 7, 2024

ioquatix commented on Feb 8, 2024

jakeonfire commented on Feb 8, 2024

jakeonfire commented on Feb 8, 2024

jakeonfire commented on Feb 8, 2024

ioquatix commented on Feb 8, 2024

jakeonfire commented on Feb 10, 2024

ioquatix commented on Feb 13, 2024

jakeonfire commented on Feb 15, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions