[FreeNX-kNX] nxserver low performance when querying sessions (patch included)

Mon Jul 27 14:40:15 UTC 2009

On our site we have noticed very poor performance of nxserver when there
are more than a few users.

The problem is that querying the current sessions takes a long time. For
instance, on our 50 user production site, running nxserver --list would
take 5.3 seconds of real time.

Where the problem really got bad was when attempting to shadow. In this
mode, while presenting the list of available sessions to shadow,
nxclient overloads the server with repeated list requests without
waiting for a response. If the user leaves the session list on-screen,
nxserver gets further and further behind, until it could take several
minutes to respond once you chose a session to shadow, besides causing
high server load by invoking a couple of thousand processes per
second. This makes it nearly impossible to shadow any sessions, since
waiting just a few seconds to select the session to be shadowed causes
enough delay that nxclient times out.

The included patch introduces a few optimisations and replaces the
getparam function with a much faster one implemented in bash, giving a
5x speed increase. To further alleviate the problem with nxclient
DOS'ing the server while selecting a session to shadow, it caches the
list of suspended sessions and regenerates it only every 30 seconds,
instead of every time nxclient issues a list command. This reduces the
server load while initiating a shadow session by about 50 times.

Note that this patch also includes changed behaviour to show suspended
and running sessions in the shadowable session list. This is a local
change we use; to get the standard behaviour (where only running
sessions are listed) just change "Suspended|Running" to "Running" on the
4th to last line of the patch.

This patch requires bash version 3, hence it may not be suitable for
mainline. However, it increases performance and reduces server load
dramatically, so for sites with a recent version of bash, it is highly
recommended.

--8<---------------cut here---------------start------------->8---

--- nxserver.1	2009-07-27 05:20:56.117777651 +1200
+++ nxserver	2009-07-27 05:20:59.077771541 +1200
@@ -26,9 +26,9 @@
 
 # Reread boot command line; echo last parameter's argument or return false.
 getparam(){
-stringinstring "&$1=" "$CMDLINE" || return 1
-echo "$CMDLINE" |  tr "&" "\n" | egrep "^"$1"=" | awk -F= '{ VAL=$2 } END { print VAL }'
-return 0
+    [[ "&$CMDLINE" =~ ".*&$1=([^&]*)" ]]
+    echo ${BASH_REMATCH[1]}
+    [ "$BASH_REMATCH" != "" ]
 }
 
 ############### PACKAGE log.bm #######################
@@ -219,7 +219,7 @@
 
 session_get_cmdline()
 {
-	echo "a=b" | cat - $1 | tr '\n' '&'
+	echo "a=b&$(tr '\n' '&' <"$1")"
 }
 
 # session_get <uniqueid>
@@ -264,7 +264,19 @@
 
 # List all sessions of a user
 
-session_list_user_suspended()
+# Cache the session list output for up to 30 seconds because nxclient
+# repeatedly queries it very fast, which creates performance problems.
+session_list_user_suspended() {
+	args="$1 $2 $3 $4"
+	if [[ "$args" != "$SESSION_LIST_CACHE_ARGS" || $(($(date +%s) - ${SESSION_LIST_CACHE_TIME:-0})) -gt 30 ]] ; then
+	  SESSION_LIST_CACHE_ARGS="$args"
+	  SESSION_LIST_CACHE_TIME=$(date +%s)
+	  SESSION_LIST_CACHE_DATA=$(_session_list_user_suspended "$@")
+	fi
+	echo "$SESSION_LIST_CACHE_DATA"
+}
+
+_session_list_user_suspended()
 {
 	SESSION_COUNT=0
 	SESSION_COUNT_USER=0
@@ -285,10 +297,9 @@
 	do
 		[ -f $i ] || break
 		let SESSION_COUNT=$SESSION_COUNT+1
-		if egrep -q "^userName=$1$" $i && egrep -q "^status=$2$" $i #&& grep -q "screeninfo=$3" $i
+		CMDLINE=$(session_get_cmdline $i)
+		if [[ ( "$1" == ".*" || "$(getparam userName)" == "$1" ) && $(getparam status) =~ "$2" ]]
 		then
-			CMDLINE=$(session_get_cmdline $i)
-			
 			if [ "$4" = "shadow" ]
 			then
 				if [ "$(getparam userName)" != "$USER" ]
@@ -302,15 +313,14 @@
 				fi
 			fi
 	
-			depth=$(getparam screeninfo | cut -d "x" -f3 | cut -d "+" -f1 )
-			[ "$depth" = "32" ] && depth=24
-			geom=$(getparam screeninfo | cut -d "x" -f1,2) 
-			render=$(getparam screeninfo | cut -d "+" -f2 )
-			available="N/A"
-			udepth=$(echo $3 | cut -d "x" -f3 | cut -d "+" -f1 )
-			[ "$udepth" = "32" ] && udepth=24
-			urender=$(echo $3 | cut -d "+" -f2 )
-
+			[[ "$(getparam screeninfo)" =~ '^([0-9]*x[0-9]*)x([0-9]*)\+?([^+]*)' ]]
+			geom=${BASH_REMATCH[1]}
+			depth=${BASH_REMATCH[2]}
+			render=${BASH_REMATCH[3]}
+			[[ "$3" =~ '^([0-9]*x[0-9]*)x([0-9]*)\+?([^+]*)' ]]
+			udepth=${BASH_REMATCH[2]}
+			urender=${BASH_REMATCH[3]}
+			
 			mode="D"
 			[ "$(getparam sessionRootlessMode)" = "1" ] && mode="-"
 			
@@ -341,7 +351,7 @@
 				printf "%-7s %-16s %32s %8s %5s %-14s %-11s %s\n" "$(getparam display)" "$(getparam type)" "$(getparam sessionId)" "$options" "$depth" "$geom" "$available" "$(getparam sessionName)" >> $TMPFILE
 			fi
 		fi
-		egrep -q "^userName=$1$" $i && let SESSION_COUNT_USER=$SESSION_COUNT_USER+1
+		[ "$1" = ".*" -o "$(getparam userName)" = "$1" ] && let SESSION_COUNT_USER=$SESSION_COUNT_USER+1
 	done
 	
 	if [ "$4" = "vnc" -a "$ENABLE_DESKTOP_SHARING" = "1" ]
@@ -1625,12 +1635,12 @@
 				session_list_user_suspended "$USER" 'Suspended' "$(getparam geometry)" "$(getparam type)"
 			elif [ "$status" = "suspended,running" -o "$status" = "suspended" ] # since 1.5.0
 			then
-				status=$(echo $status | sed 's/,/$|^status=/g; s/suspended/Suspended/g; s/running/Running/g')
+				status=$(echo $status | sed 's/,/|/g; s/suspended/Suspended/g; s/running/Running/g')
 				[ "$ENABLE_SHOW_RUNNING_SESSIONS" = "0" ] && status="Suspended"
 				session_list_user_suspended "$USER" "$status" "$(getparam geometry)" "$(getparam type)"
 			elif [ "$(getparam type)" = "shadow" ]
 			then
-				session_list_user_suspended ".*" "Running" "" "shadow"
+				session_list_user_suspended ".*" "Suspended|Running" "" "shadow"
 			else
 				session_list_user "$USER" | log_tee
 			fi
--8<---------------cut here---------------end--------------->8---


-- 
Mario Becroft <mb at gem.win.co.nz>

