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

Improve how forking is handled #82

Merged
merged 3 commits into from Apr 6, 2021

Conversation

johnnyshields
Copy link
Collaborator

@johnnyshields johnnyshields commented Apr 2, 2021

This is a follow-up to #81.

I've updated the before_fork and after_fork callbacks. Here's the nuts and bolts:

  • before_fork is called by DelayedJob only on the parent process, which doesn't ever execute jobs. Since this doesn't need to talk to the DB, we should ensure it's Mongoid connections are closed.
  • after_fork is called on child processes as they are forked from the parent. I've updated the logic to obtain fresh connections. Technically speaking this is not necessary as a Mongoid 5+, because the Mongo Ruby Driver checks if pid to see if it has been forked. However, since we know we are forking it's good practice to do this immediately rather than waiting for Mongo Driver to do it.

In addition, this PR make some Job class method private which should not be exposed publicly.

I've added specs to increase coverage. I've also removed the YARDstick gem which verifies documentation. Bye-bye.

@coveralls
Copy link

Coverage Status

Coverage decreased (-24.1%) to 64.474% when pulling 49802ef on johnnyshields:forking-improvements into db87465 on collectiveidea:master.

@coveralls
Copy link

coveralls commented Apr 2, 2021

Coverage Status

Coverage increased (+2.2%) to 90.789% when pulling c4f10f6 on johnnyshields:forking-improvements into db87465 on collectiveidea:master.

@dblock
Copy link
Collaborator

dblock commented Apr 2, 2021

Pretty scary may need tests and go :green: ;)

@johnnyshields
Copy link
Collaborator Author

OK specs now fixed. Please merge.

@dblock dblock merged commit 2cfd640 into collectiveidea:master Apr 6, 2021
@johnnyshields
Copy link
Collaborator Author

Thanks for merging. I'm running this in prod with no issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants