On Sat, Feb 13, 2010 at 10:00:52AM -0600, Gary Mills wrote: > On Sat, Feb 13, 2010 at 09:09:05PM +1100, Bron Gondwana wrote: > > On Fri, Feb 12, 2010 at 09:45:02AM -0600, Gary Mills wrote: > > > I'm willing to add a `keepalive' option to Cyrus master along with the > > > setsockopt() system call to enable that setting. This option could be > > > added to the cyrus.conf file for any services that could benefit from > > > it. Would this be a reasonable addition to Cyrus? > > > > How does this look? > > Wow, you beat me to it! You even covered all of the settings.
:) > > +{ "tcp_keepalive", 0, SWITCH } > > +/* Enable keepalive on TCP connections */ > > + > > +{ "tcp_keepalive_cnt", 0, INT } > > +/* Number of TCP keepalive probes to send before declaring the > > + connection dead (0 == system default) */ > > + > > +{ "tcp_keepalive_idle", 0, INT } > > +/* Number of seconds a connection must be idle before keepalive > > + probes are sent (0 == system default) */ > > + > > +{ "tcp_keepalive_intvl", 0, INT } > > +/* Number of seconds between keepalive probes (0 == system default) */ > > > > A switch to enable keepalive, plus options to edit each of the > > tunables. The full patch is attached - not tested except for > > a compile yet. > > I can't comment on imap/sync_client.c because I don't use that > technique. In master.c, I would have put my changes into > spawn_service(), just before master exec'ed the daemon. However, > putting them where the connection was first accepted should be fine > too. I considered that - but I wanted it to be after: cyrus_init(alt_config, service, 0); So that you could control options per service. > One thing to watch is that only SO_KEEPALIVE is standard. The other > three symbols: TCP_KEEPCNT, TCP_KEEPIDLE, and TCP_KEEPINTVL only exist > in some operating systems. They have global settings but don't have > per-socket options. For these, the setsockopt() function calls need > to be conditional on the symbols. Hmm - yeah, OK. I'll protect them with #ifdefs. > For which Cyrus version is your patch intended. I'm still running > cyrus-imapd-2.3.8 . It's roughly against 2.3-tail (I have a couple more patches in between that build debian packages and add a .gitignore file, but otherwise it's 2.3-tail). I'm pretty sure it will backport to 2.3.8 though, that area of the code hasn't been changed in a while. Bron. that far OK. ---- Cyrus Home Page: http://cyrusimap.web.cmu.edu/ Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html