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

Add eventlet support and gunicorn config vals #951

Merged
merged 1 commit into from Oct 19, 2020

Conversation

raymondjacobson
Copy link
Member

@raymondjacobson raymondjacobson commented Oct 19, 2020

Trello Card Link

https://trello.com/c/FJa1ydwW/1619-template-gunicorn-config

Description

Templates the gunicorn config for discovery provider with env vars.
Corresponding k8s change: https://github.com/AudiusProject/audius-k8s/pull/135

Services

  • Discovery Provider

Does it touch a critical flow like Discovery indexing, Creator Node track upload, Creator Node gateway, or Creator Node file system?

Delete an option.

  • ✅ Nope

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide repro instructions & any configuration.
Include log analysis if applicable.

  1. Update local compose script to run prod-server instead of dev-server and ensured updating .env would change running gunicorn config
  2. Pushed branch up as a hotfix & tested the same way on staging (deploying using both audius-k8s at master and audius-k8s at the branch)

With new audius-k8s

[2020-10-19 19:19:41 +0000] [1] [DEBUG] Current configuration:
  config: None
  bind: [':5000']
  backlog: 2048
  workers: 16
  worker_class: eventlet
  threads: 1
  worker_connections: 1000

with old audius-k8s

[2020-10-19 19:20:35 +0000] [1] [DEBUG] Current configuration:
  config: None
  bind: [':5000']
  backlog: 2048
  workers: 2
  worker_class: sync
  threads: 8
  worker_connections: 1000
  max_requests: 0
  max_requests_jitter: 0


# Use specified number of workers if present
if [[ -z "${audius_gunicorn_workers}" ]]
Copy link
Contributor

Choose a reason for hiding this comment

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

is there an associated config update that is missing?

i also think these should be audius_discprov_gunicorn_workers if we follow the format for other configs

Copy link
Contributor

Choose a reason for hiding this comment

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

oh because these are environment configs and not used by the service this doesn't really matter?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah -- the .ini is for python, this is before python's even running

Copy link
Member Author

Choose a reason for hiding this comment

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

config is all updated on the k8s side

Copy link
Member Author

Choose a reason for hiding this comment

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

there's a world in which we want this locally too, but we never run gunicorn locally as it stands, so it doesn't make a diff

Copy link
Contributor

@dmanjunath dmanjunath left a comment

Choose a reason for hiding this comment

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

This looks good to me

Copy link
Contributor

@hareeshnagaraj hareeshnagaraj left a comment

Choose a reason for hiding this comment

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

looks good to me!

@raymondjacobson raymondjacobson merged commit ad755b3 into master Oct 19, 2020
@raymondjacobson raymondjacobson deleted the hotfix-allow-gunicorn-config branch October 19, 2020 20:26
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