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 support for JVB_MAX_MEM var #608

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

Add support for JVB_MAX_MEM var #608

wants to merge 5 commits into from

Conversation

goacid
Copy link
Contributor

@goacid goacid commented May 22, 2020

Fix to update over .env file the amount of memories of the videobridge java process.

@alexanderankin
Copy link

shouldnt this set the VIDEOBRIDGE_MAX_MEMORY instead, per the variable used in this file:

/usr/share/jitsi-videobridge# cat jvb.sh 
#!/bin/bash

if [[ "$1" == "--help"  || $# -lt 1 ]]; then
    echo -e "Usage:"
    echo -e "$0 [OPTIONS], where options can be:"
    echo -e "\t--secret=SECRET\t sets the shared secret used to authenticate to the XMPP server"
    echo -e "\t--domain=DOMAIN\t sets the XMPP domain (default: none)"
    echo -e "\t--min-port=MP\t sets the min port used for media (default: 10001)"
    echo -e "\t--max-port=MP\t sets the max port used for media (default: 20000)"
    echo -e "\t--host=HOST\t sets the hostname of the XMPP server (default: domain, if domain is set, \"localhost\" otherwise)"
    echo -e "\t--port=PORT\t sets the port of the XMPP server (default: 5275)"
    echo -e "\t--subdomain=SUBDOMAIN\t sets the sub-domain used to bind JVB XMPP component (default: jitsi-videobridge)"
    echo -e "\t--apis=APIS where APIS is a comma separated list of APIs to enable. Currently supported APIs are 'xmpp' and 'rest'. The default is 'xmpp'."
    echo
    exit 1
fi

SCRIPT_DIR="$(dirname "$(readlink -f "$0")")"

mainClass="org.jitsi.videobridge.Main"
cp=$(JARS=($SCRIPT_DIR/jitsi-videobridge*.jar $SCRIPT_DIR/lib/*.jar); IFS=:; echo "${JARS[*]}")
libs="$SCRIPT_DIR/lib/native/linux-64"
logging_config="$SCRIPT_DIR/lib/logging.properties"
videobridge_rc="$SCRIPT_DIR/lib/videobridge.rc"

# if there is a logging config file in lib folder use it (running from source)
if [ -f $logging_config ]; then
    LOGGING_CONFIG_PARAM="-Djava.util.logging.config.file=$logging_config"
fi

if [ -f $videobridge_rc  ]; then
        source $videobridge_rc
fi

if [ -z "$VIDEOBRIDGE_MAX_MEMORY" ]; then VIDEOBRIDGE_MAX_MEMORY=3072m; fi

LD_LIBRARY_PATH=$libs exec java -Xmx$VIDEOBRIDGE_MAX_MEMORY $VIDEOBRIDGE_DEBUG_OPTIONS -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=/tmp -Djava.library.path=$libs $LOGGING_CONFIG_PARAM $JAVA_SYS_PROPS -cp $cp $mainClass $@

@goacid
Copy link
Contributor Author

goacid commented May 25, 2020

Yes this is also possible, but this variable is not yet retrieve over the env variable set in .env file.

@alexanderankin
Copy link

that is right, i forgot that i had just edited my docker-compose file as well

jvb/rootfs/etc/cont-init.d/10-config Outdated Show resolved Hide resolved
@@ -221,6 +221,9 @@ JVB_TCP_MAPPED_PORT=4443
# See https://github.com/jitsi/jitsi-videobridge/blob/master/doc/rest.md for more information
#JVB_ENABLE_APIS=rest,colibri

# Memory allocated to the JVM
# JVB_JVM_MAX_MEM=3072m
Copy link
Member

Choose a reason for hiding this comment

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

This won't work unless you declare it on docker-compose.yml

@goacid goacid requested a review from saghul August 3, 2020 08:11
@dofl
Copy link

dofl commented Sep 20, 2020

Would be nice if this can be commited, I need to be able to set this variable

@goacid
Copy link
Contributor Author

goacid commented Oct 1, 2020

Would be nice if this can be commited, I need to be able to set this variable

cherry-pick will help

@vetorialnet
Copy link

Hi,
there is a typo on 10-config (\n and /libs/viedobridge.rc)

diff --git a/jvb/rootfs/etc/cont-init.d/10-config b/jvb/rootfs/etc/cont-init.d/10-config
index 3cab1fb..5532ed5 100644
--- a/jvb/rootfs/etc/cont-init.d/10-config
+++ b/jvb/rootfs/etc/cont-init.d/10-config
@@ -20,7 +20,11 @@ if [[ ! -f /config/logging.properties ]]; then
fi

if [[ ! -z $JVB_JVM_MAX_MEM ]]; then

  • echo -n "# The amount of memory that the java process can consume.\n# Use the format that is in use for java's -Xmx switch# The default is 3072m\n# Only effective on linux, 64 bit \nVIDEOBRIDGE_MAX_MEMORY=$JVB_JVM_MAX_MEM" > /usr/share/jitsi-videobridge/libs/viedobridge.rc
  • echo "# The amount of memory that the java process can consume." > /usr/share/jitsi-videobridge/lib/videobridge.rc
  • echo "# Use the format that is in use for java's -Xmx switch" >> /usr/share/jitsi-videobridge/lib/videobridge.rc
  • echo "# The default is 3072m" >> /usr/share/jitsi-videobridge/lib/videobridge.rc
  • echo "# Only effective on linux, 64 bit" >> /usr/share/jitsi-videobridge/lib/videobridge.rc
  • echo "VIDEOBRIDGE_MAX_MEMORY=$JVB_JVM_MAX_MEM" >> /usr/share/jitsi-videobridge/lib/videobridge.rc
    fi

chown -R jvb:jitsi /config

@goacid
Copy link
Contributor Author

goacid commented Nov 3, 2020

@saghul : any improvement to perform ? or this PR is ok ?

@saghul
Copy link
Member

saghul commented Nov 3, 2020

Hum, can't we set it in the run file, as a generic Java prop?

@goacid
Copy link
Contributor Author

goacid commented Nov 5, 2020

Hum, can't we set it in the run file, as a generic Java prop?

I will ask a friend for that, unfortunately not a java dev

@johncadengo
Copy link

Is this a PR that's still relevant?

@saghul
Copy link
Member

saghul commented Apr 3, 2023

It no longer applies cleanly, so I'd say it needs a rebase at least.

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

6 participants