queue_classic icon indicating copy to clipboard operation
queue_classic copied to clipboard

Is forking supposed to work for 3.0.0?

Open jipiboily opened this issue 10 years ago • 14 comments

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?

jipiboily avatar Mar 26 '14 20:03 jipiboily

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

The fact that it is not is a bug.

ryandotsmith avatar Mar 26 '14 20:03 ryandotsmith

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 avatar Mar 26 '14 21:03 jipiboily

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

jipiboily avatar Apr 09 '14 16:04 jipiboily

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 https://github.com/ryandotsmith/queue_classic/commit/aea44335861af319ab56a751cc26b87feded5787) that people could opt into?

2468ben avatar May 01 '14 02:05 2468ben

@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.

ryandotsmith avatar May 02 '14 13:05 ryandotsmith

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: https://github.com/ryandotsmith/queue_classic/issues/207#issuecomment-42031916

jipiboily avatar May 02 '14 16:05 jipiboily

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.

2468ben avatar May 02 '14 19:05 2468ben

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

Inviz avatar May 14 '14 00:05 Inviz

@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.

2468ben avatar May 14 '14 01:05 2468ben

https://github.com/ryandotsmith/queue_classic/pull/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 avatar May 14 '14 05:05 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)

Inviz avatar May 14 '14 05:05 Inviz

@jipiboily what is the status of this issue?

senny avatar Mar 04 '15 16:03 senny

I didn't work on this. As far as I know, it still doesn't work. There is a PR here https://github.com/QueueClassic/queue_classic/pull/216 but it wasn't reviewed or tested AFAIK.

jipiboily avatar Mar 04 '15 21:03 jipiboily

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 }

bronson avatar Feb 05 '16 04:02 bronson