Thoughts on a plugin architecture about pitchfork HOT 5 CLOSED

other features like our "memory limiter" could be developed on the same set of rails without us having to fork or monkey patch the code base.

Have you seen: #92

I'm not convinced a plugin system is needed for that, but also perhaps I don't quite understand what you have in mind?

Also the soft timeout is a bit inadequate right now, I need to make some changes to it.

from pitchfork.

Yes we're aware of after_request_complete but just as with the timeout implementation it wouldn't work because we wanted to kill a worker during the request if it went above a certain threshold, as you have with the timeout thread.

Maybe you can add like a around_request event which we can yield to perform the request and spawn a thread that monitors the memory usage while the request is running and kills it (like timeout does) if it goes above a threshold.

Does that make sense?

EDIT: Meant to say around_request 🤦 not after_request

from pitchfork.

we wanted to kill a worker during the request if it went above a certain threshold,

I don't understand why you'd want to do that. But if you really want to do that, you don't need anything special from Pitchfork, you can just spawn that thread from after_worker_fork. So I don't see any need for a plugin system here.

from pitchfork.

Sorry @casperisfine not sure how after_worker_fork helps.

request_env = process_client(client, worker, prepare_timeout(worker))
@after_request_complete&.call(self, worker, request_env)
worker.increment_requests_count

What we need is a callback before process_client is called that allows us to begin the memory thread monitor, just as you're doing with timeout via prepare_timeout(worker)

Maybe something simple like before_request like:

@before_request&.call(self, worker)
request_env = process_client(client, worker, prepare_timeout(worker))
@after_request_complete&.call(self, worker, request_env)
worker.increment_requests_count

Will you accept a PR for this?

from pitchfork.

My point is you don't need to start that memory monitor thread on each request. Actually it would be really bad for performance.

You can just do something like:

after_worker_fork do
  loop do
    if memory_exhausted?
     exit! 42
    end
   sleep 1
  end
end

But again, I don't see why you wouldn't just do that on after_request_complete so you let the request terminate first.

from pitchfork.