[SVN] r893 - in /branches/cyrus23/cyrus-imapd-2.3-hmh/debian: changelog patches/08-clean_socket_closes.dpatch patches/12-fix_timeout_handling.dpatch patches/13-master_process_handling.dpatch
debian at incase.de
debian at incase.de
Sat Jul 17 14:49:47 UTC 2010
Author: hmh
Date: Sat Jul 17 16:49:47 2010
New Revision: 893
URL: https://mail.incase.de/viewcvs?rev=893&root=cyrus22&view=rev
Log:
Properly document patches 08, 12 and 13
Modified:
branches/cyrus23/cyrus-imapd-2.3-hmh/debian/changelog
branches/cyrus23/cyrus-imapd-2.3-hmh/debian/patches/08-clean_socket_closes.dpatch
branches/cyrus23/cyrus-imapd-2.3-hmh/debian/patches/12-fix_timeout_handling.dpatch
branches/cyrus23/cyrus-imapd-2.3-hmh/debian/patches/13-master_process_handling.dpatch
Modified: branches/cyrus23/cyrus-imapd-2.3-hmh/debian/changelog
URL: https://mail.incase.de/viewcvs/branches/cyrus23/cyrus-imapd-2.3-hmh/debian/changelog?rev=893&root=cyrus22&r1=892&r2=893&view=diff
==============================================================================
--- branches/cyrus23/cyrus-imapd-2.3-hmh/debian/changelog (original)
+++ branches/cyrus23/cyrus-imapd-2.3-hmh/debian/changelog Sat Jul 17 16:49:47 2010
@@ -52,6 +52,9 @@
* Patch 0025b-cve-2009-3235-extras: rediff, patch is now mostly
useless (and doesn't fix any security hole), needs to go upstream or
to be dropped
+ * Document patches 08, 12 and 13 which are forward ports of patches
+ originally written by me for Debian Cyrus 2.1 packages (and should
+ now all go upstream, almost 10 years later!)
* initscript: Required-Start/-Stop must refer to $remote_fs (lintian)
* initscript: Default-Stop should not refer to runlevel S (lintian)
* debian/rules: drop obsolete --with-tclsh option in configure
@@ -71,7 +74,7 @@
* Convert to use quilt, and update the patch headers to use clean paths
(Closes: #563303)
- -- Henrique de Moraes Holschuh <hmh at debian.org> Sat, 05 Jun 2010 19:45:25 -0300
+ -- Henrique de Moraes Holschuh <hmh at debian.org> Sat, 26 Jun 2010 20:33:43 -0300
cyrus-imapd-2.3 (2.3.14-2) UNRELEASED; urgency=medium
Modified: branches/cyrus23/cyrus-imapd-2.3-hmh/debian/patches/08-clean_socket_closes.dpatch
URL: https://mail.incase.de/viewcvs/branches/cyrus23/cyrus-imapd-2.3-hmh/debian/patches/08-clean_socket_closes.dpatch?rev=893&root=cyrus22&r1=892&r2=893&view=diff
==============================================================================
--- branches/cyrus23/cyrus-imapd-2.3-hmh/debian/patches/08-clean_socket_closes.dpatch (original)
+++ branches/cyrus23/cyrus-imapd-2.3-hmh/debian/patches/08-clean_socket_closes.dpatch Sat Jul 17 16:49:47 2010
@@ -1,10 +1,21 @@
-#! /bin/sh /usr/share/dpatch/dpatch-run
-## 08-clean_socket_closes.dpatch by Sven Mueller <debian at incase.de>
-##
-## All lines beginning with `## DP:' are a description of the patch.
-## DP: Cleanly close sockets
+From: Henrique de Moraes Holschuh <hmh at debian.org>
+Subject: Shutdown and close sockets cleanly
- at DPATCH@
+Origin: vendor, Debian Cyrus IMAPd 2.1.4-11 (2002-05-19)
+
+Cleanly shutdown and close sockets, this is supposed to allow for better
+TCP teardown on the remote end, and reduces CLOSE_WAIT time.
+
+This patch was written 8 years ago, it is possible that nowadays nothing
+will benefit from a shutdown() right before close(). The commit log
+from eight years ago mentions that SHUT_RD should be upgraded to
+SHUT_RDWR where possible, but only after verification that this is not
+going to cause problems (e.g. by discarding data still on flight to the
+remote).
+
+Also, it is possible that new daemons and utils in Cyrus 2.2 and 2.3 may
+need similar patches.
+
diff -urNad git~/imap/backend.c git/imap/backend.c
--- git~/imap/backend.c 2010-01-16 19:21:09.000000000 -0200
+++ git/imap/backend.c 2010-01-16 19:26:27.284091887 -0200
Modified: branches/cyrus23/cyrus-imapd-2.3-hmh/debian/patches/12-fix_timeout_handling.dpatch
URL: https://mail.incase.de/viewcvs/branches/cyrus23/cyrus-imapd-2.3-hmh/debian/patches/12-fix_timeout_handling.dpatch?rev=893&root=cyrus22&r1=892&r2=893&view=diff
==============================================================================
--- branches/cyrus23/cyrus-imapd-2.3-hmh/debian/patches/12-fix_timeout_handling.dpatch (original)
+++ branches/cyrus23/cyrus-imapd-2.3-hmh/debian/patches/12-fix_timeout_handling.dpatch Sat Jul 17 16:49:47 2010
@@ -1,10 +1,17 @@
-#! /bin/sh /usr/share/dpatch/dpatch-run
-## 12-fix_timeout_handling.dpatch by Sven Mueller <debian at incase.de>
-##
-## All lines beginning with `## DP:' are a description of the patch.
-## DP: fixes timeout handling
+From: Henrique de Moraes Holschuh <hmh at debian.org>
+Subject: Use blocking locks and SIGALRM to break hung locks
+Author: Henrique de Moraes Holschuh <hmh at debian.org>
- at DPATCH@
+Origin: vendor, Debian Cyrus IMAPd 2.1.15-5 (2003-07-26)
+
+RFC PATCH
+
+Change locking system to use blocking locks and signals to break out of
+locks.
+
+Is this patch correct? Will it cause issues with other uses of alarm()
+in Cyrus code?
+
diff -urNad git~/configure.in git/configure.in
--- git~/configure.in 2010-01-16 22:11:38.118932338 -0200
+++ git/configure.in 2010-01-16 22:11:38.287932356 -0200
Modified: branches/cyrus23/cyrus-imapd-2.3-hmh/debian/patches/13-master_process_handling.dpatch
URL: https://mail.incase.de/viewcvs/branches/cyrus23/cyrus-imapd-2.3-hmh/debian/patches/13-master_process_handling.dpatch?rev=893&root=cyrus22&r1=892&r2=893&view=diff
==============================================================================
--- branches/cyrus23/cyrus-imapd-2.3-hmh/debian/patches/13-master_process_handling.dpatch (original)
+++ branches/cyrus23/cyrus-imapd-2.3-hmh/debian/patches/13-master_process_handling.dpatch Sat Jul 17 16:49:47 2010
@@ -1,10 +1,91 @@
-#! /bin/sh /usr/share/dpatch/dpatch-run
-## 13-master_process_handling.dpatch by Sven Mueller <debian at incase.de>
-##
-## All lines beginning with `## DP:' are a description of the patch.
-## DP: Fixes process (child) handling in master process
-
- at DPATCH@
+From: Henrique de Moraes Holschuh <hmh at debian.org>
+Subject: Fixes process (child) handling in Cyrus master
+
+Origin: vendor, Debian Cyrus IMAPd 2.1.16-7 (2004-08-07)
+
+* Allows Cyrus master to process all pending child messages once per
+ loop, which fixes a DoS situation if there is too much message churn
+ in a slower box. If the pending messages never get processed, child
+ accounting goes bad and eventually master stops spawning the service
+ or handling connections.
+
+ It seems that the problem this patch fixes has also been reported in
+ Cyrus IMAP 2.2.12 by Earl Shannon, and Jules Agee actually tried to
+ get the fix from Debian integrated upstream.
+
+ Jules Agee described the problem quite well in a private message:
+
+ "The problem occurs when the child message queue is very backed up.
+ A child process dies with several messages in the queue. reap_child
+ marks it's state in the child table as dead and decrements the
+ number of ready_workers. Then the janitor process removes that
+ child's entry from the child table. When master finally gets
+ around to processing all the messages in the child message queue,
+ it has no idea where the message came from because the child's
+ entry has already been removed from the ctable. So it creates a
+ new ctable entry, marking the child process state as unknown,
+ assuming the message is from a child process that's still alive.
+ When master finishes processing the messages from that long-dead
+ child, it again (an inaccurately) decrements the count of
+ ready_workers.
+
+ The easiest solution in my opinion is to make sure master stays
+ caught up with messages from its children by processing all child
+ messages on each loop. After all, it's the duty of a parent."
+
+ Kenneth Murchison reported that they couldn't reproduce it in CMU.
+ However, it is clearly something that depends on pathological loads
+ and kernel behaviour to trigger.
+
+ This fix is a trade off: we risk increasing latency to hand off
+ connections/spawn new services (because we're in a loop processing
+ child messages). If the messages never stop coming, or come in
+ extremely large bursts, master will backlog connections. If syslog()
+ or memory allocation decides to introduce large latencies, the cost
+ of processing messages can get too high and master will backlog
+ connections. Note: this *CAN* happen, and in fact it did happen to
+ Jules until he reduced the amount of data going to syslog.
+
+ Also, even with this patch, master behaviour will remain sane only if
+ we don't clean the child table BEFORE we processes any messages it
+ could have sent that are still in-flight (this *can* happen), as
+ described by Jules. This is probably a bug in the decisions we take
+ when we promote something from SERVICE_STATE_UNKNOWN to some other
+ state. We *could* account children when they are migrated from
+ SERVICE_STATE_UNKNOWN to some other state, so that we can do normal
+ accounting afterwards... but we need to do a full failure mode
+ analysis to know if there isn't another failure mode where this
+ causes trouble...
+
+ IMO, there must be a better way to do all this process accounting
+ and child messaging that doesn't require such a complicated
+ state machine. I wonder how postfix does it?
+
+ Reported-analysed-and-tested-by: Jules Agee <julesa at pcf.com>.
+
+* Allows tunning of the amount of time Cyrus will remember dead
+ children ("janitor mourning time"), which can be useful when messages
+ are being delayed by extreme system load (use -J to increase the
+ default time of two seconds to something large enough to get the
+ messages), or there is way too much child churn to wait that long (use
+ -J to decrease mourning time to 1s or to 0s).
+
+ Setting it to 0 means we free the child PID as soon as possible. If
+ the kernel decides to hang around the messages and we process the
+ SIGCHLD *and* do a janitor sweep before the messages arrive, you get
+ the process accounting bug described above.
+
+ We should fix master to not need this at all, but that is not trivial.
+
+ This option was added to help tune master on systems which are hitting
+ the kind of crap described by Jules Agee.
+
+* Cleans up various data fields for reconfig.
+
+* Fixes various error messages and MAXFD handling.
+
+NOTE: patch is missing manpage fixes to describe -J
+
diff -urNad git~/lib/util.c git/lib/util.c
--- git~/lib/util.c 2010-01-16 19:21:09.000000000 -0200
+++ git/lib/util.c 2010-01-16 19:28:14.288091659 -0200
More information about the Pkg-Cyrus-imapd-Debian-devel
mailing list