Skip to content
This repository has been archived by the owner on Jul 7, 2022. It is now read-only.

Temporary fix scale docker compose #45

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

machow
Copy link
Contributor

@machow machow commented Mar 16, 2020

@mdbecker this should allow deploying multiple streamlit containers (I couldn't tell via streamlit docs and code, whether their tornado implementation uses subprocesses. Guessing not, based on their emphasis on load balancers).

Once merged, I'll go ahead and deploy on the linode box (and then hopefully the devops team will swoop in and save us 😅)

I don't set up load balancers that often so any eyes on this would be helpful!

@machow machow changed the title Fix scale docker compose Temporary fix scale docker compose Mar 16, 2020
@themightychris
Copy link
Contributor

themightychris commented Mar 17, 2020

@machow I've never used this scaling ability of docker-compose, but I'm struggling to see how this could actually make multiple app instances available to nginx. If the port is constant, than either docker-compose would need to be round-robining the IP set on app somehow, or nginx would need to have some kind of capability to make its own DNS query and get back multiple IPs on a single A record -- but I doubt either of those setups are in place here (I could be wrong though)

I don't know that it's worth us diving too deep into docker-compose-based scaling strategies though if this isn't something we already know will work, k8s deploy should be ready soon and we know it will handle managing multiple app instances

@themightychris
Copy link
Contributor

Hmm nvm sounds like docker-compose does round-robin the DNS and people talk about that working behind nginx, I'd expect nginx to be caching DNS on things it's proxy_passing to... ¯_(ツ)_/¯

@machow
Copy link
Contributor Author

machow commented Mar 17, 2020

I'm surprised docker even implemented a feature like this, but it turned out weirdly useful in this case.

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

Successfully merging this pull request may close these issues.

2 participants