Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is forking supposed to work for 3.0.0? #207

Open
jipiboily opened this issue Mar 26, 2014 · 14 comments
Open

Is forking supposed to work for 3.0.0? #207

jipiboily opened this issue Mar 26, 2014 · 14 comments
Assignees
Labels
Milestone

Comments

@jipiboily
Copy link
Contributor

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
Copy link
Contributor

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

The fact that it is not is a bug.

@jipiboily
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@2468ben
Copy link

2468ben commented May 1, 2014

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
Copy link
Contributor

@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
Copy link
Contributor Author

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 notifications@github.com
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
Copy link

2468ben commented May 2, 2014

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
Copy link
Contributor

Inviz commented May 14, 2014

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
Copy link

2468ben commented May 14, 2014

@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
Copy link
Contributor

Inviz commented May 14, 2014

#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
Copy link
Contributor

Inviz commented May 14, 2014

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
Copy link
Contributor

senny commented Mar 4, 2015

@jipiboily what is the status of this issue?

@jipiboily
Copy link
Contributor Author

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
Copy link
Contributor

bronson commented Feb 5, 2016

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 }

@ukd1 ukd1 added this to the 4.0.0 milestone Jul 23, 2019
@ukd1 ukd1 added the bug label Jul 23, 2019
@ukd1 ukd1 self-assigned this Jul 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants