Re: Init script slowness
On Tue, Oct 4, 2011 at 10:28 AM, Henrique de Moraes Holschuh h...@debian.org wrote: On Tue, 04 Oct 2011, Ondřej Surý wrote: I agree on fixing SIGQUIT behaviour. Does anyone have an easy way to test whether sigquit is doing the right thing on open but idle connections of the various protocols? After all, if does not have an open transaction _now_, it should get immediately closed by a sigquit... Checking in proc, my user has 5 imapd processes right now. I attached strace to them one at a time, then issued a sigquit. Each time the process exited immediately. The logs show something like: Oct 4 10:36:50 thetis cyrus/imaps[12021]: fetching user_deny.db entry for 'stevenkurylo' Oct 4 10:37:55 thetis cyrus/imaps[12021]: auditlog: traffic sessionid=cyrus-12021-1317741678-1 bytes_in=16484 bytes_out=80756 Oct 4 10:37:55 thetis cyrus/master[1325]: process 12021 exited, status 75 Oct 4 10:37:55 thetis cyrus/master[1325]: service imaps pid 12021 in BUSY state: terminated abnormally I haven't had an issue with shut down taking the full 30 seconds. Seeing a strace and the log files from a pid on a machine exhibiting this behavoir would be interesting. ___ Pkg-Cyrus-imapd-Debian-devel mailing list Pkg-Cyrus-imapd-Debian-devel@lists.alioth.debian.org http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-cyrus-imapd-debian-devel
Bug#629603: Acknowledgement (cyrus-common-2.4: Fails to upgrade to 2.4.9~beta1-1)
I've attached two patches.
The first once changes grep to exclude the *_path options in
lib/imapoptions. This ensures cyrus-db-types.txt is generated correctly.
Next, cyrus-common-2.4.postinst was only setting RET if upgrade-db
failed. So RET was null in the case statement, triggering the failure
message when upgrade-db actually succeeded.
Thank you.
--- rules 2011-06-06 13:20:46.0 -0700
+++ rules-new 2011-06-08 08:15:19.127986268 -0700
@@ -109,7 +109,7 @@
# store database configuration for possible automatic
# upgrading later
echo DBENGINE $(DBENGINE) debian/cyrus-db-types.txt
- grep _db lib/imapoptions \
+ grep '_db' lib/imapoptions \
| cut -d, -f1-2 | sed -e 's/{ //;s/_db, / /;s/$$//' \
| sed -e 's/^tls.* /TLS /;s/^subs.* /SUBS /;s/^seen.* /SEEN /;s/^pts.* /PTS /;s/^mbox.* /MBOX /'\
| awk '{printf(%s %s\n,toupper($$1),$$2);}' \
--- cyrus-common-2.4.postinst 2011-06-06 13:20:46.0 -0700
+++ cyrus-common-2.4.postinst-new 2011-06-08 08:59:30.159982191 -0700
@@ -29,7 +29,8 @@
case $1 in
configure)
if [ -f /usr/lib/cyrus/cyrus-db-types.active ]; then
- /usr/lib/cyrus/bin/upgrade-db || RET=$?
+ /usr/lib/cyrus/bin/upgrade-db
+ RET=$?
case $RET in
0) ;;
2)
___
Pkg-Cyrus-imapd-Debian-devel mailing list
Pkg-Cyrus-imapd-Debian-devel@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-cyrus-imapd-debian-devel
Bug#629609: cyrus-imapd-2.4: proc and lock directories should be cleaned and put on tmpfs
Package: cyrus-imapd-2.4
Version: 2.4.9~beta1-1
Severity: normal
Tags: patch
Per the documentation for 2.4, the locks should be cleaned out. I think
cleaning out proc
at the same time would be a good idea, in case a file is left after a crash.
As well in the init script I:
* Silenced try-restart
* Improved SYNCSHUTDOWN check
They should both be on tmpfs for better performance too. So I've changed the
default config file to /run/cyrus/
Thank you for considering my patches.
-- System Information:
Debian Release: wheezy/sid
APT prefers unstable
APT policy: (500, 'unstable')
Architecture: amd64 (x86_64)
Kernel: Linux 2.6.38-2-amd64 (SMP w/1 CPU core)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Versions of packages cyrus-imapd-2.4 depends on:
ii cyrus-c 2.4.9~beta1-1Cyrus mail system - common files
ii libc6 2.13-5 Embedded GNU C Library: Shared lib
ii libcome 1.41.12-4common error description library
ii libdb5. 5.1.25-10Berkeley v5.1 Database Libraries [
ii libsasl 2.1.24~rc1.dfsg1+cvs2011-05-23-3 Cyrus SASL - authentication abstra
ii libssl1 1.0.0d-2 SSL shared libraries
ii libwrap 7.6.q-19 Wietse Venema's TCP wrappers libra
ii zlib1g 1:1.2.3.4.dfsg-3 compression library - runtime
cyrus-imapd-2.4 recommends no packages.
cyrus-imapd-2.4 suggests no packages.
-- no debconf information
--- imapd.conf 2011-06-07 19:15:00.523999297 -0700
+++ imapd.conf-new 2011-06-07 19:20:46.251998286 -0700
@@ -4,6 +4,10 @@
# Configuration directory
configdirectory: /var/lib/cyrus
+# Directories for proc and lock files
+proc_path: /run/cyrus/proc
+mboxname_lockpath: /run/cyrus/lock
+
# Which partition to use for default mailboxes
defaultpartition: default
partition-default: /var/spool/cyrus/mail
--- cyrus-common.cyrus-imapd.init 2011-06-07 19:05:14.155998569 -0700
+++ cyrus-common.cyrus-imapd.init-new 2011-06-07 19:24:30.451999607 -0700
@@ -34,9 +34,6 @@
PIDFILE=/var/run/${NAME}.pid
DESC=Cyrus IMAPd
-SYNC_CLIENT=/usr/lib/cyrus/bin/sync_client
-SYNCSHUTDOWN=$([ -r /etc/imapd.conf ] (grep sync_shutdown_file
/etc/imapd.conf|awk '{print $2}'))
-
# Check if Cyrus is installed (vs. removed but not purged)
test -x $DAEMON || exit 0
@@ -56,6 +53,20 @@
[ x${MASTERCONF} != x ] OPTIONS=-M ${MASTERCONF} ${OPTIONS}
[ x${LISTENQUEUE} != x ] OPTIONS=-l ${LISTENQUEUE} ${OPTIONS}
+if [ -n $CONF ]; then
+ [ -r $CONF ] || ( echo Could not read config file $CONF; exit
1)
+else
+ CONF=/etc/imapd.conf
+fi
+
+SYNC_CLIENT=/usr/lib/cyrus/bin/sync_client
+SYNCSHUTDOWN=$(gawk '/^sync_shutdown_file:[[:blank:]]/ { print $2 }' $CONF)
+CONFIGDIR=$(gawk '/^configdirectory:[[:blank:]]/ {print $2}' $CONF)
+LOCK_DIR=$(gawk '/^mboxname_lockpath:[[:blank:]]/ {print $2}' $CONF)
+PROC_DIR=$(gawk '/^proc_path:[[:blank:]]/ {print $2}' $CONF)
+[ -z $LOCK_DIR ] LOCK_DIR=$CONFIGDIR/lock
+[ -z $PROC_DIR ] PROC_DIR=$CONFIGDIR/proc
+
# Load the VERBOSE setting and other rcS variables
. /lib/init/vars.sh
@@ -132,6 +143,11 @@
# 0 if daemon has been started
# 1 if daemon was already running
# 2 if daemon could not be started
+
+# Clean stale entries
+find $LOCK_DIR -mindepth 1 -depth -size 0 # -delete
+find $PROC_DIR -mindepth 1 -depth -name '[0-9]*' # -delete
+
start-stop-daemon --start --quiet --pidfile $PIDFILE --exec $DAEMON --test
/dev/null \
|| return 1
start-stop-daemon --start --quiet --pidfile $PIDFILE --exec $DAEMON -- \
@@ -262,7 +278,7 @@
status_of_proc $DAEMON $NAME exit 0 || exit $?
;;
try-restart)
- pidofproc $DAEMON exec $0 restart
+ pidofproc $DAEMON /dev/null exec $0 restart
;;
*)
echo Usage: $0
{start|stop|status|restart|reload|force-reload|try-restart} 12
___
Pkg-Cyrus-imapd-Debian-devel mailing list
Pkg-Cyrus-imapd-Debian-devel@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-cyrus-imapd-debian-devel
Bug#627339: init script should use QUIT instead of TERM
On 11-05-20 06:45 AM, Ondřej Surý wrote:
Hi Steven,
could you try attached init.d script?
I made some minor corrections.
* Silenced pidofproc in do_start and try-restart
* Fix pidofproc return code handling in do_stop (otherwise
/etc/init.d/cyrus-imapd stop always returned failed)
I'll have to so more testing of the sync_client. One thought is there
is may be race condition with cyrmaster (or a user) starting a new
sync_client process after sync_stop has run, but before the shutdown is
finished.
Perhaps the touch $SYNCSHUTDOWN should be done regardless and rm -f
$SYNCSHUTDOWN should be done after cyrmaster has stopped. I would
assume if $SYNCSHUTDOWN exists, then no new sync_client will start.
--- cyrus-common.cyrus-imapd.init 2011-05-20 09:48:51.351117475 -0700
+++ /etc/init.d/cyrus-imapd 2011-05-20 09:29:33.831127422 -0700
@@ -138,7 +138,7 @@
|| return 2
# cyrmaster is not running
-pidofproc $DAEMON || return 2
+pidofproc $DAEMON /dev/null || return 2
}
sync_stop () {
@@ -176,11 +176,21 @@
[ $RETVAL = 2 ] return 2
# cyrmaster is still running
-pidofproc $DAEMON || return 2
-
-# Many daemons don't delete their pidfiles when they exit.
-rm -f $PIDFILE
-return $RETVAL
+# pidofproc will return:
+# 0 is running
+# 1 is $DAEMON isn't running
+# 3 is $DAEMON isn't running
+# 4 for other error
+pidofproc $DAEMON
+RETVAL=$?
+[ $RETVAL = 0 ] return 2
+if [ $RETVAL = 1 ] || [ $RETVAL = 3 ]; then
+ # Many daemons don't delete their pidfiles when they exit.
+ rm -f $PIDFILE
+ return 0
+else
+ return 2
+fi
}
#
@@ -259,7 +269,7 @@
status_of_proc $DAEMON $NAME exit 0 || exit $?
;;
try-restart)
- pidofproc $DAEMON exec $0 restart
+ pidofproc $DAEMON /dev/null exec $0 restart
;;
*)
echo Usage: $0 {start|stop|status|restart|reload|force-reload|try-restart} 12
___
Pkg-Cyrus-imapd-Debian-devel mailing list
Pkg-Cyrus-imapd-Debian-devel@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-cyrus-imapd-debian-devel
Bug#627339: init script should use QUIT instead of TERM
Package: cyrus-common-2.4
Version: 2.4.8-7
install-configure.html says we should use QUIT first:
Since a clean shutdown may never finish if a child process is stuck
for some reason the recommended approach is to send a SIGQUIT then loop
on the master process sending a signal 0 every second until either the
master process has gone away or a suitable time has expired (maybe 10
seconds). You can then send a SIGTERM if the process still exists.
As well imapd.conf says sync_shutdown_file should be used if we're doing
replication.
I've attached a first draft of a patch. Please let me know what you
think. Thank you.
diff -r c5fdbe96f08d debian/cyrus-common.cyrus-imapd.init
--- a/debian/cyrus-common.cyrus-imapd.init Thu May 19 08:49:41 2011 -0700
+++ b/debian/cyrus-common.cyrus-imapd.init Thu May 19 10:23:09 2011 -0700
@@ -128,4 +128,29 @@
}
+sync_stop () {
+ if [ -e /usr/lib/cyrus/bin/sync_client ]; then
+ # Check if the sync file is set
+ SYNCSHUTDOWN=`grep sync_shutdown_file /etc/imapd.conf|awk '{print $2}'`
+ if [ x${SYNCSHUTDOWN} = x ]; then
+ # Do we want to throw an error if this isn't configured?
+ # Maybe we should add a default sync_shutdown_file to imapd.conf
+ # on installation?
+ return 0
+ else
+ if ! touch $SYNCSHUTDOWN ; then
+echo Failed to create sync_shutdown_file: $SYNCSHUTDOWN
+exit 1
+ else
+# Give sync_client time to exit.
+sleep 1
+rm -f $SYNCSHUTDOWN
+return 0
+ fi
+ fi
+ fi
+ return 0
+
+}
+
case $1 in
start)
@@ -175,22 +200,39 @@
stop)
echo -n Stopping $DESC:
- if start-stop-daemon --stop --quiet --pidfile /var/run/$NAME.pid \
- --name ${NAME} --quiet --startas $DAEMON /dev/null 21 ; then
- echo $NAME.
+ sync_stop
+ set +e
+ # Send STOP
+ start-stop-daemon --stop --signal 3 --quiet --pidfile /var/run/$NAME.pid \
+ --name ${NAME} --quiet --startas $DAEMON /dev/null 21
+ set -e
+ # process running?
+ if check_status; then
+ i=0
+ # Wait up to 10 seconds for cyrus to exit
+ until [ $i -gt 10 ]; do
+ if ! check_status; then
+break
+ fi
+ echo -n .
+ sleep 1
+ i=$(( $i + 1 ))
+ done
+ # If we're still running, send TERM
+ if check_status; then
+ set +e
+ start-stop-daemon --stop --quiet --pidfile /var/run/$NAME.pid \
+ --name ${NAME} --quiet --startas $DAEMON /dev/null 21
+ set -e
+ fi
+ fi
+
+ if check_status; then
+ # Yes, report failure.
+ echo (failed).
+ exit 1
+ else
+ echo $NAME
rm -f ${PIDFILE}
exit 0
- else
- # process running?
- if check_status; then
- # Yes, report failure.
- echo (failed).
- exit 1
- else
- # No, return as if stopped a running process
- # successfully.
- echo .
- rm -f ${PIDFILE}
- exit 0
- fi
fi
;;
___
Pkg-Cyrus-imapd-Debian-devel mailing list
Pkg-Cyrus-imapd-Debian-devel@lists.alioth.debian.org
http://lists.alioth.debian.org/mailman/listinfo/pkg-cyrus-imapd-debian-devel
Bug#627202: sync_client fatal error seen_db.c: 127: *seendbptr == NULL
Package: cyrus-replication-2.4
Version: 2.4.8-7
I'm testing cyrus 2.4.8 to upgrade our cyrus deployment. On various
mailboxes sync_client exits with the error:
Fatal error: Internal error: assertion failed: seen_db.c: 127:
*seendbptr == NULL
I can reproduce this error consistently by doing a test sync of my own
mailbox sync_client -v -o -u stevenkurylo
This is fixed upstream and I can confirm is resolves my problem:
http://git.cyrusimap.org/cyrus-imapd/commit/?id=5de9eee60d947243a4b4b2f4eccc63cff2771b30
http://git.cyrusimap.org/cyrus-imapd/commit/?id=9dacbfcd6ee077a850570508d0d97f30bfe96640
I've included a patch to the debian/patches directory for this fix.
Thank you.
diff -r 28ed9e107ab6 cyrus-imapd-sync_client-1
--- /dev/null Thu Jan 01 00:00:00 1970 +
+++ b/cyrus-imapd-sync_client-1 Wed May 18 09:26:29 2011 -0700
@@ -0,0 +1,19 @@
+From 9dacbfcd6ee077a850570508d0d97f30bfe96640 Mon Sep 17 00:00:00 2001
+From: Bron Gondwana br...@opera.com
+Date: Mon, 04 Apr 2011 04:42:42 +
+Subject: seen: and another site!
+
+---
+diff --git a/imap/sync_client.c b/imap/sync_client.c
+index a7fc12d..7aeee59 100644
+--- a/imap/sync_client.c
b/imap/sync_client.c
+@@ -1884,7 +1884,7 @@ static int do_user_seen(const char *user, struct sync_seen_list *replica_seen)
+ {
+ int r;
+ struct sync_seen *mseen, *rseen;
+-struct seen *seendb;
++struct seen *seendb = NULL;
+ struct sync_seen_list *list;
+
+ /* silently ignore errors */
diff -r 28ed9e107ab6 cyrus-imapd-sync_client-2
--- /dev/null Thu Jan 01 00:00:00 1970 +
+++ b/cyrus-imapd-sync_client-2 Wed May 18 09:26:29 2011 -0700
@@ -0,0 +1,33 @@
+From 5de9eee60d947243a4b4b2f4eccc63cff2771b30 Mon Sep 17 00:00:00 2001
+From: Bron Gondwana br...@opera.com
+Date: Thu, 31 Mar 2011 05:03:10 +
+Subject: seen: fix seen_db related crash
+
+---
+diff --git a/imap/sync_client.c b/imap/sync_client.c
+index d7e632c..a7fc12d 100644
+--- a/imap/sync_client.c
b/imap/sync_client.c
+@@ -1434,7 +1434,7 @@ static int update_seen_work(const char *user, const char *uniqueid,
+ static int do_seen(char *user, char *uniqueid)
+ {
+ int r = 0;
+-struct seen *seendb;
++struct seen *seendb = NULL;
+ struct seendata sd;
+
+ if (verbose)
+@@ -1448,12 +1448,8 @@ static int do_seen(char *user, char *uniqueid)
+ if (r) return 0;
+
+ r = seen_read(seendb, uniqueid, sd);
+-if (r) {
+- seen_close(seendb);
+- return 0;
+-}
+
+-r = update_seen_work(user, uniqueid, sd);
++if (!r) r = update_seen_work(user, uniqueid, sd);
+
+ seen_close(seendb);
+ seen_freedata(sd);
diff -r 28ed9e107ab6 series
--- a/series Wed May 18 09:25:19 2011 -0700
+++ b/series Wed May 18 09:26:29 2011 -0700
@@ -29,4 +29,6 @@
cyrus-imapd-2.4.2-902-accept-invalid-from-header.patch
cyrus-imapd-2.4.2-903-normalize-authorization-id.patch
+cyrus-imapd-sync_client-1
+cyrus-imapd-sync_client-2
85-perl-imap-croak-FTBFS-fix.patch
86-fix_PATH_MAX_on_hurd.patch
___
Pkg-Cyrus-imapd-Debian-devel mailing list
Pkg-Cyrus-imapd-Debian-devel@lists.alioth.debian.org
http://lists.alioth.debian.org/mailman/listinfo/pkg-cyrus-imapd-debian-devel
