Closed tochev closed 3 years ago
thanks for this valuable input. Why don't you make it compatible to existing docker-compose files by adding a default value that is used, if no size is specified in the docker-compose file? if you e.g. add the parameter to the arg parse arguments with a default and keep the passing via the shell script as for other parameters - a missing value in the docker-compose won't matter at all and therefore the change should be backward compatible?!
do you think this makes sense and would you modify the pr before merging? cheers
Thanks for the consideration.
I will make the modifications you deem more appropriate.
I though about switching to --disable-dev-shm-usage
if the shm size is not enough (the docker default is only 64MB, in the current version it only issues a warning), but the problem is that this way people will continue to use it with the suboptimal configuration and it might be trickier to pinpoint exactly how the browser was run.
I'm not sure if I can check that this parameter has been passed to docker/docker-compose, and therefore I only check the filesystem size of /dev/shm.
So from my point of view the main choice options are:
BROWSER_DISABLE_DEV_SHM_USAGE=false
- backward incompatible (the current version)
BROWSER_DISABLE_DEV_SHM_USAGE=false
and /dev/shm < X MB
instead of only issuing a warningBROWSER_DISABLE_DEV_SHM_USAGE=true
- with additional documentation and docker examples without shm_size
BROWSER_DISABLE_DEV_SHM_USAGE=true/false/auto
and when it is auto (the default) determine whether to pass --disable-dev-shm-usage
based on the size (e.g., if over 500MB use /dev/shm/)BROWSER_DISABLE_DEV_SHM_USAGE=false
, but if /dev/shm < 512MB
(64 is currently the docker default which may be raised in the future) issue warning and use --disable-dev-shm-usage
B, C, and D are backwards compatible, with C and D using /dev/shm for new users/deploys.
If I have understood you correctly, you are proposing option D?
okay tried to figure out how that works.
I think best would be to go with something like that:
◦ there is an option to exit if BROWSER_DISABLE_DEV_SHM_USAGE=false and /dev/shm < X MB instead of only issuing a warning
so first let's say we want to exit if the size is not set correctly. Of course with a warning message why we exit. :-)
then regarding setting the size: could you add it to the docker-compose files (the main one, the build one and so on. in the build (development) docker-compose the shm-size parameter has to be under the build tag. in the other ones in the app / service tag directly. the child of the build will use it when building. the child of the service uses it on start.
and we will check if we can add the parameter to dockerhub so it is also build there with the shm-size set.
this way only users that use an older version may run into the issue and have to adopt their docker-compose files.
do you think this is acceptable?
Seems reasonable, tomorrow I will implement and test it (excluding the dockerhub thing).
The shm_size
is currently set in the docker examples.
I didn't know that you could put it under build in the development, thanks for letting me know.
In the build (.development) should it be present both under build and under the service?
the documentation does not clear that ... I guess if it was build with shm-size it is not necessary to set it on start again. But putting it into the file commented out with a note how to set it / change the built value, won't hurt :-)
I tested a bit and it seems that build/shm_size
affects only the shm size of the build container and does not affect the containers created from the image that has been built.
Or at least I was not able to do it locally (docker 17.09.1, docker-compose 1.27.4), I might be missing something.
I found this issue which states the same problem (the last verification is from Aug 2018).
Setting build/shm_size
, if I understand correctly ( https://docs.docker.com/compose/compose-file/#shm_size ) , is only for the build steps, which we do not need since no browser is started during the build.
The docker-compose.yml.development
configuration which I tried with was:
# docker-compose.yml.development
version: '3.5' # NOTE: bumped to 3.5 for build/shm_size
services:
bbb-streamer:
container_name: liveStreaming
env_file: .env
# FIXME: the below comment seems incorrect
# Uncomment below to override /dev/shm size built into the docker (see build/shm_size)
# shm_size: '1gb'
build:
shm_size: '2gb'
context: ./
Therefore, I have pushed only the "exiting on insufficient /dev/shm" and I have merged the changes from master (I have also changed what I believe is a stray *
in the changelog).
Not using /dev/shm resulted in disk trashing and high load under certain circumstances.
This is a backward incompatible change and the users will have to modify their docker-compose files to add
shm_size
entry.I'm open to making it backward compatible (change the default value) but I feel that for the use case of livestreaming it is safer to use /dev/shm .
The default value for /dev/shm size is based on #81 (2GB) and we have tested it during a conference with multiple running instances.
The circumstances under which we have observed disk trashing involved running several instances on the same host for a couple of hours.