Is forking supposed to work for 3.0.0?

[jipiboily]

Hey,
it looks like forking is not working right now (on master). I did not look at it very deeply (i.e., I have no clue why it is not working well) but it does not work for me. The job runs, but it seems to be after deleting the job that it fails. Here is the error:

https://gist.github.com/jipiboily/1423f7b4437ec60018f0

We are using shared connection, but I have disabled it just to make sure it was not the root cause.

Any idea?

[ryandotsmith]

Yes. A forking worker is absolutely supposed to work on 3.0.0.

The fact that it is not is a bug.

[jipiboily]

By the way, I opened this, this is not part of the work I am doing right now, so it is not planned for me to fix this anytime soon.

[jipiboily]

When anyone is tackling this, it would be great (if not already the case) to have before_fork and after_fork callbacks.

[2468ben]

Hi, I was looking at this issue and wanted to double-check: no one else already started tackling it?
I wrote a test for a forked job, and when the forked process finishes it closes the parent connection. I haven't pinpointed exactly when it closes, but if I do, reconnecting "fixes" it. Would you instead want a generic pool (like in aea4433) that people could opt into?

[ryandotsmith]

@2468ben Do you think we could fix the issue without creating a connection pool? Is there a simpler fix?

Also, to my knowledge, there isn't any active development on this issue. It would be amazing if you were to take a crack at it.

[jipiboily]

I am not working on it and not planning anytime soon. I would love you forever if you could tackle this!:)

I think having before and after fork blocks.like Unicorn could make sense? Thoughts?

—
Sent from my phone

On Fri, May 2, 2014 at 9:29 AM, Ryan Smith [email protected]
wrote:

@2468ben Do you think we could fix the issue without creating a connection pool? Is there a simpler fix?

Also, to my knowledge, there isn't any active development on this issue. It would be amazing if you were to take a crack at it.

Reply to this email directly or view it on GitHub:
#207 (comment)

[2468ben]

Yeah the before/after hooks make total sense. I have a fix for the connection error that works but isn't ideal; I'll throw up a PR and explain what I found.

[Inviz]

I opened PR with alternative approach, that does not use default PG connection inside worker and throws exception if you try to do so.

[2468ben]

@Inviz awesome! I wish I saw that exit!(0) command a long time ago, that fixes everything.

Could the IO piping in the forked processes be enabled with a worker-level option? It's awesome to get the return values (especially during testing), but the worker.start method just runs the queue and ignores the return values, so it could skip the IO piping by default.

[Inviz]

#220 What do you think about this? It adds :asynchronous option that does not block the main process while worker is doing it's stuff. Instead of exception on re-using connection it just silently re-establishes it. So async workers report to the queue by themselves, unlike regular forked/sync workers

[Inviz]

Instead of output, asynchronous worker's #work() method returns pid, that you can wait for. It also supports callbacks that are executed in forked process, so you can do some manual IO piping if you feel like it (specs has examples)

[senny]

@jipiboily what is the status of this issue?

[jipiboily]

I didn't work on this. As far as I know, it still doesn't work. There is a PR here #216 but it wasn't reviewed or tested AFAIK.

[bronson]

Just curious: why does QueueClassic fork inside Worker#start? I don't quite see the use case.

Jobs forking while doing work make sense of course (for example, my jobs fork to render PDFs).

But forking to process the job queue? Seems like it's more common to run jobs in a different process entirely (rake qc:work) or in a thread (Rails5 action cable / puma, etc).

On top of that, if someone actually does want to fork to process jobs, maybe they would want to just do it themselves?

fork { QC.default_worker_class.new.work }