public inbox for development@lists.ipfire.org
 help / color / mirror / Atom feed
From: Michael Tremer <michael.tremer@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: [PATCH] squid init v2: Update-suggestions for (3.5.xx)-initscript
Date: Sun, 15 May 2016 11:37:16 +0100	[thread overview]
Message-ID: <1463308636.18591.272.camel@ipfire.org> (raw)
In-Reply-To: <1463248376-1471-1-git-send-email-matthias.fischer@ipfire.org>

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

Hi,

thanks for working on this.

I still have a few questions and remarks:

On Sat, 2016-05-14 at 19:52 +0200, Matthias Fischer wrote:
> These changes are mainly meant for upcoming squid 3.5.xx:
> 
> - Raised 'while-loop'-time for stopping squid to 120 seconds.

Is there any objective for this value or is this just an estimate?

I assume what you are trying to do here is to find a timeout that is high enough
that even slow storage will have enough time to write back the cache from
memory.

I have seen systems where closing and reopening the cache takes 15-20 minutes.

> - 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)".
> 
> - Added some 'boot_mesg' lines in stop-routine.

That debugging output should not be shown to the user. Just show [ OK ] or
[ FAIL ] at the end of the line.

> - Changed the 'flush'-command to delete the entire 'swap.state'-file - it
>   will be automatically rebuild during the next start.

I get that it is intentional to delete this file when it is damaged. It is
however the journal of the cache index and when deleted will require squid to
rebuild the entire cache from the all files in the cache. That will certainly
take a long time (especially on those systems that are likely to lose their
swap.state because of slow storage).

We should *always* avoid this when ever we can.

Generally I take this as a workaround for now since squid does not notice when
the swap.state file was damaged and continues working with corrupt data. Would
you like to file a bug report and see if the squid developers can find a way to
automatically detect when squid is reading a damaged file and discard it then? I
guess that would be the safest solution.

> 
> Best,
> Matthias
> 
> Signed-off-by: Matthias Fischer <matthias.fischer(a)ipfire.org>
> ---
>  src/initscripts/init.d/squid | 29 +++++++++++++++++------------
>  1 file changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/src/initscripts/init.d/squid b/src/initscripts/init.d/squid
> index abed90a..5d9521b 100644
> --- a/src/initscripts/init.d/squid
> +++ b/src/initscripts/init.d/squid
> @@ -108,22 +108,28 @@ case "$1" in
>  			# Wait until all redirectors have been stopped.
>  			wait
>  
> -			# If squid is still running, wait up to 30 seconds
> +			# If squid is still running, wait up to 120 seconds
>  			# before we go on to kill it.
> -			counter=30
> -
> -			while [ ${counter} -gt 0 ]; do
> -				statusproc /usr/sbin/squid >/dev/null &&
> break;
> +			n=0
> +			while /usr/sbin/squid -k check && [ $n -lt 120 ]; do
>  				sleep 1
> -				counter=$(( ${counter} - 1))
> +				n=$(( ${n} + 1 ))
>  			done

So the status output with the dots is gone now. Can we add a notice to the
status line that says something like "Shutting down squid (this may take up to a
few minutes).

>  
> -			# Kill squid service, if still running.
> -			killproc /usr/sbin/squid >/dev/null
> +			# If (squid-1) is still running, kill all squid
> processes
> +			# and delete damaged '/var/log/cache/swap.state'.
> +			if ( ps ax | grep "(squid-1)$" ); then

Can we use pgrep here?

> +				killproc /usr/sbin/squid >/dev/null
> +				/bin/rm -f /var/log/cache/swap.state

Do not use the full path. "rm" should be enough.

> +				boot_mesg "(squid-1) was killed after 120
> seconds."
> +			else
> +				boot_mesg "(squid-1) exited normally,
> shutdown OK."
> +			fi
> +		fi
>  
> -			# 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 +149,7 @@ case "$1" in
>  
>  	flush)
>  		$0 stop
> -		echo > /var/log/cache/swap.state
> -		chown squid.squid /var/log/cache/swap.state
> +		/bin/rm -f /var/log/cache/swap.state

See above for "rm".

I am also wondering what this was supposed to do. These lines are executed when
you click the button to clear the cache on the web user interface. However, the
cache is not really cleared.

The cache is rebuilt, but not cleared. Shouldn't we remove all data in the
directory and reinitialize everything with squid -z?

Here is some documentation about that:

  http://wiki.squid-cache.org/SquidFaq/ClearingTheCache

Just removing the swap.state file still leaves all the data on disk and my
assumption when clearing the cache is that I get rid of the data in it. Either
to clear up space, because it was corrupted, etc...

>  		sleep 1
>  		$0 start
>  		;;

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

  reply	other threads:[~2016-05-15 10:37 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-14 17:52 Matthias Fischer
2016-05-15 10:37 ` Michael Tremer [this message]
2016-05-15 14:57   ` Matthias Fischer
2016-05-16 15:15     ` Michael Tremer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1463308636.18591.272.camel@ipfire.org \
    --to=michael.tremer@ipfire.org \
    --cc=development@lists.ipfire.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox