Apple patches 11-14

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Apple patches 11-14

Mike Abbott-4
These are the last patches for now, and they're small.  The base for  
each of them is dovecot-1.1.7 + Apple patches 9 and 10, again not  
because of a logical dependency but just because these patches change  
parts of the earlier patches.

Patch #11 adds a few dtrace providers to key points in the code.  We  
are still validating the correct placement and usefulness of these.  
One cool thing about dtrace is that when not in use the hooks  
literally are just a couple of no-op instructions.




Patch #12 increases many listen queue sizes to handle high loads  
better.  The new values are not necessarily the best possible values  
but they do allow more throughput.




Patch #13 makes dovecot adapt to dynamic host identity changes upon  
config reload.




Patch #14 covers three miscellaneous changes, to support more file  
descriptors and an "unlimited" number of mail processes, and a  
different default mail user.




That's all our patches for now!

patch11.txt (10K) Download Attachment
patch12.txt (3K) Download Attachment
patch13.txt (3K) Download Attachment
patch14.txt (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Apple patches 11-14

Timo Sirainen
On Tue, 2009-01-06 at 17:06 -0600, Mike Abbott wrote:
> Patch #11 adds a few dtrace providers to key points in the code.  We  
> are still validating the correct placement and usefulness of these.  
> One cool thing about dtrace is that when not in use the hooks  
> literally are just a couple of no-op instructions.

I don't see dtrace-dovecot.h included in it.

Anyway I think the current code is too invasive. For example:

#ifdef APPLE_OS_X_SERVER
        if (DOVECOT_OD_LOOKUP_START_ENABLED())
                DOVECOT_OD_LOOKUP_START(in_od_info, (char *) in_user_name);
#endif

Couldn't the DOVECOT_OD_LOOKUP_START() macro be changed so that the
whole code would become simply:

        DOVECOT_OD_LOOKUP_START(in_od_info, (char *) in_user_name);

And it would internally have the "if
(DOVECOT_OD_LOOKUP_START_ENABLED())" part. That also makes it simple to
get rid of the #ifdefs in .c files.

> Patch #12 increases many listen queue sizes to handle high loads  
> better.  The new values are not necessarily the best possible values  
> but they do allow more throughput.

I guess those can be changed. I've really no idea what the tradeoffs
here are. I suppose it uses some more memory, but is it relevant with
today's memory sizes? Could they be increased even higher than your
values?

> Patch #13 makes dovecot adapt to dynamic host identity changes upon  
> config reload.

Since it's not necessary to limit the domain name length, it could be
just i_strdup()ed. (I should have done this in the mbox code too.)

> Patch #14 covers three miscellaneous changes, to support more file  
> descriptors

I guess this is the same as running ulimit -n before starting dovecot?
Is it useful to have this code inside Dovecot?

> and an "unlimited" number of mail processes,

+ /* APPLE - treat as "unlimited" */
+ set->max_mail_processes = 1000000;

Hmm. Is it really a good idea to allow unlimited? Anyway -1U would be
"more" unlimited. :)

> and a different default mail user.

+ "uid=98", /* _dovecot's uid */
+ "gid=6", /* _dovecot's gid */

Why does it matter which uid/gid is used for dump-capability? Maybe it
could be just run as root.

signature.asc (204 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Apple patches 11-14

Mike Abbott-4
> I don't see dtrace-dovecot.h included in it.

dtrace-dovecot.h is generated from dtrace-dovecot.d by the command  
"dtrace -h -s $(DTRACE_SOURCE)" in src/lib/Makefile.am.  See the  
section "BUILDING CODE CONTAINING USDT PROBES" in http://developer.apple.com/documentation/Darwin/Reference/ManPages/man1/dtrace.1.html 
  .  Sorry I should have mentioned that up front.

> Couldn't the DOVECOT_OD_LOOKUP_START() macro be changed

Unfortunately that's out of my control:  those macros are generated.  
But I suppose they could be replaced with an intermediate level of  
macros that are undefined when not wanted, and combine the enabled-
check and the probe-trigger when wanted, just to remove the in-line  
ifdefs.

>> Patch #12 increases many listen queue sizes
>
> I guess those can be changed. I've really no idea what the tradeoffs
> here are.

Fewer refused connections means happier users on good days, but longer  
wait times when something's wedged.  Too-large queue sizes can lead to  
livelock, according to http://www.hpl.hp.com/personal/Lucy_Cherkasova/papers/ieee-dtoc.ps 
  .  The trick is finding the right compromise.  The queue size of 8  
on the client-facing socket is certainly much too small.

>> Patch #14
> Is it useful to have this code inside Dovecot?

Useful for us....

> -1U would be "more" unlimited. :)


I think we tried that and something else broke.

> Why does it matter which uid/gid is used for dump-capability?

Not sure.
Reply | Threaded
Open this post in threaded view
|

Re: Apple patches 11-14

Asheesh Laroia
On Tue, 6 Jan 2009, Mike Abbott wrote:

>>> Patch #14
>> Is it useful to have this code inside Dovecot?
>
> Useful for us....

Why not just add "ulimit -n" to your init script that launches Dovecot?

-- Asheesh.

--
You look like a million dollars.  All green and wrinkled.

Reply | Threaded
Open this post in threaded view
|

Re: Apple patches 11-14

Timo Sirainen
In reply to this post by Mike Abbott-4
On Tue, 2009-01-06 at 19:18 -0600, Mike Abbott wrote:

> > I don't see dtrace-dovecot.h included in it.
>
> dtrace-dovecot.h is generated from dtrace-dovecot.d by the command  
> "dtrace -h -s $(DTRACE_SOURCE)" in src/lib/Makefile.am.  See the  
> section "BUILDING CODE CONTAINING USDT PROBES" in http://developer.apple.com/documentation/Darwin/Reference/ManPages/man1/dtrace.1.html 
>   .  Sorry I should have mentioned that up front.
>
> > Couldn't the DOVECOT_OD_LOOKUP_START() macro be changed
>
> Unfortunately that's out of my control:  those macros are generated.  
> But I suppose they could be replaced with an intermediate level of  
> macros that are undefined when not wanted, and combine the enabled-
> check and the probe-trigger when wanted, just to remove the in-line  
> ifdefs.
PostgreSQL is doing something like this:
http://archives.postgresql.org/pgsql-patches/2008-02/msg00159.php

For example:

        PG_TRACE2(lwlock__startwait, lockid, mode);

Would that be possible with Dovecot too?

signature.asc (204 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Apple patches 11-14

Timo Sirainen
In reply to this post by Mike Abbott-4
On Tue, 2009-01-06 at 17:06 -0600, Mike Abbott wrote:
> Patch #11 adds a few dtrace providers to key points in the code.  We  
> are still validating the correct placement and usefulness of these.  
> One cool thing about dtrace is that when not in use the hooks  
> literally are just a couple of no-op instructions.

I'll wait until each probe takes only one line of code.

> Patch #12 increases many listen queue sizes to handle high loads  
> better.  The new values are not necessarily the best possible values  
> but they do allow more throughput.

Applied to v1.1 and v1.2.

> Patch #13 makes dovecot adapt to dynamic host identity changes upon  
> config reload.

Applied to v1.2: http://hg.dovecot.org/dovecot-1.2/rev/829b6555392b
http://hg.dovecot.org/dovecot-1.2/rev/2609eca99495

Although the mbox change doesn't really do anything, because imap
processes never call hostpid_init() again. But sure it makes the code
cleaner. I made it a function so that the extra lookup overhead isn't
done unless it's actually necessary.

> Patch #14 covers three miscellaneous changes, to support more file  
> descriptors and an "unlimited" number of mail processes, and a  
> different default mail user.

I'll skip these at least for now.

signature.asc (204 bytes) Download Attachment