Skip to content

Conversation

@WilliamMcCumstie
Copy link
Contributor

@WilliamMcCumstie WilliamMcCumstie commented Aug 6, 2021

This PR switches the web server from puma to unicorn. Unicorn uses worker process to achieve concurrency as opposed to threads. Unicorn has the following features:

  • It is in a master-worker architecture,
  • The master maintains a "worker pool" and will automatically restart dead workers,
  • The master can pre-load the application, allowing the workers to leverage copy-on-write, and
  • Each worker handles a single request one at a time.

flight-job-script-api is an "unconventional" use case as it needs to run "flight-job" related code as a different user. This prevented flight-job being used as a shared library with puma. Instead, each unicorn worker now:

  • Authenticates the request when it receives it,
  • Switches to the relevant worker, and
  • Sets the after-reply hook to terminate itself.

This allows the worker to directly interact with the file-system as the correct user. As it has switched user, it can not be re-used for another request at this stage. This is a fairly uncommon use case for unicorn, so no inbuilt support is available. Instead the worker calls exit 0 in the rack.after_reply hook which then triggers the master to restart it.


This PR is focusing as a POC that unicorn can be successfully used as a replacement for puma. As such, it still uses a modified form of the fork-exec into flight-job.

There are some minor speed benefits when reading the results files, as this can be done directly by the worker. However the majority of the benefits will be realised by re-working flight-job into a shared library.


Misc:

  • At the time or writing, the new branch unicorn is equivalent to master. Further work is going to be required before unicorn can become part of the mainline release cycle.
  • The "double logging" issue has been fixed, and
  • Models are no longer initialised with the user as the can get it from ENV.

@WilliamMcCumstie WilliamMcCumstie changed the base branch from master to release/unicorn-beta August 6, 2021 12:01
@WilliamMcCumstie WilliamMcCumstie changed the base branch from release/unicorn-beta to unicorn August 6, 2021 12:12
This is a long standing issue from puma days.
Unicorn already logs, so the app doesn't have to
The worker needs to be shutdown after servicing the request
The request is now running as the correct user
# NOTE: This will trigger the unicorn worker terminator upon finishing the request
before do
# Only worry about authenticated requests
next unless role == :user
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On review this looks wrong, leaving unauthenticated requests as root leaves me uneasy.

Probably best to switch to nobody.

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.

2 participants