Re: [PATCH] squid init v3: Update-suggestions for (3.5.xx)-initscript

[Michael Tremer <michael.tremer@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 4620 bytes --]

Hi,

there are a ton of patches now on this list and I have really no idea where I
should start commenting on those. I will start with this one. But please try to
put everything into one patch(set) the next time and then wait for feedback.
There should not be too many drafts here because it gets messy and I do not know
what I actually reviewed already.

On Sun, 2016-05-15 at 18:43 +0200, Matthias Fischer wrote:
> These changes are mainly meant for upcoming squid 3.5.xx,
> but should still work with 3.4.xx:
> 
> - Raised 'while-loop'-time for stopping squid to 360 seconds,
>   even bigger caches should get a chance. ;-)

Same question. Why 360 seconds? That is as arbitrary as the former timeout.

> - Changed the 'while-loop' from using 'statusproc' to 'squid -k check',
>   since 'statusproc' didn't find the still running '(squid-1)'-process.
>   Thus, 'squid' hadn't enough time to close the index. This led to a
>   damaged 'swap.state'-file and "Rebuilding storage in /var/log/cache"
>   always ended with "(dirty log)".
> 
> - Process detection for leftover '(squid-1)'-process uses 'pgrep'.
> 
> - Cosmetic changes to some 'boot_mesg' lines. Added a few.
> 
> - Changed the 'flush'-command to delete the entire '/var/log/cache'-structure,
>   it will automatically be rebuild during the next start.
> 
> Best,
> Matthias
> 
> Signed-off-by: Matthias Fischer <matthias.fischer(a)ipfire.org>
> ---
>  src/initscripts/init.d/squid | 43 ++++++++++++++++++++++++++++---------------
>  1 file changed, 28 insertions(+), 15 deletions(-)
> 
> diff --git a/src/initscripts/init.d/squid b/src/initscripts/init.d/squid
> index abed90a..47bf182 100644
> --- a/src/initscripts/init.d/squid
> +++ b/src/initscripts/init.d/squid
> @@ -94,8 +94,9 @@ case "$1" in
>  	stop)
>  			iptables -t nat -F SQUID
>  		if [ -e /var/run/squid.pid ]; then
> -			boot_mesg "Stopping Squid Proxy Server..."
> -			squid -k shutdown >/dev/null 2>&1
> +			boot_mesg -n "Shutting down Squid Proxy Server...\n"
> +			boot_mesg "(this may take up to a few minutes)."
> +			/usr/sbin/squid -k shutdown >/dev/null 2>&1
>  			evaluate_retval
>  

Why is the comment changed to "Shutting down..."? All the other processes show
"Stopping..." as it was before.

>  			# Stop squidGuard, updxlrator, squidclamav
> @@ -108,22 +109,35 @@ case "$1" in
>  			# Wait until all redirectors have been stopped.
>  			wait
>  
> -			# If squid is still running, wait up to 30 seconds
> -			# before we go on to kill it.
> -			counter=30
> -
> -			while [ ${counter} -gt 0 ]; do
> -				statusproc /usr/sbin/squid >/dev/null &&
> break;
> +			# If some squid processes are still running, wait up
> to 360 seconds
> +			# before we go on to kill all and delete entire cache
> structure.
> +			n=0
> +			while [ /usr/sbin/squid -k check > /dev/null 2>&1 ]
> && [ $n -lt 360 ]; do
>  				sleep 1
> -				counter=$(( ${counter} - 1))
> +				n=$(( ${n} + 1 ))
>  			done

There is no square brackets needed for executing "squid -k check". Again, please
do not use absolute path names.

>  
> -			# Kill squid service, if still running.
> -			killproc /usr/sbin/squid >/dev/null
> +			# If (squid-1) is still running after 360 seconds,
> +			# kill all squid processes and delete damaged cache
> structure.
> +			if ( pgrep -fl "(squid-1)" > /dev/null 2>&1 ); then
> +				killproc /usr/sbin/squid >/dev/null
> +				rm -rf /var/log/cache/*
> +				boot_mesg -n "You should not be reading this
> warning.\n"
> +				boot_mesg -n "Some squid-processes had to be
> killed after 360 seconds,\n"
> +				boot_mesg -n "so index file and cache
> contents had to be deleted.\n"
> +				boot_mesg -n "Therefore, the complete cache
> structure will be rebuild\n"
> +				boot_mesg "during next start."
> +				echo_warning
> +			else
> +				boot_mesg "All squid processes exited
> normally."
> +				echo_ok
> +				boot_mesg ""
> +			fi
> +		fi

I do not see why it is suddenly necessary to completely destroy the cache. As
far as I know the index was broken, but not the actual data.

Why was this changed?

>  
> -			# Trash remain pid file from squid.
> +			# Delete remaining pid file from squid if it still
> exists.
>  			rm -rf /var/run/squid.pid
> -		fi
> +
>  		;;
>  
>  	restart)
> @@ -143,8 +157,7 @@ case "$1" in
>  
>  	flush)
>  		$0 stop
> -		echo > /var/log/cache/swap.state
> -		chown squid.squid /var/log/cache/swap.state
> +		rm -rf /var/log/cache/*
>  		sleep 1
>  		$0 start
>  		;;

-Michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]