filterfish / smith2

A complete rewrite of Smith
12 stars 7 forks source link

Defang file_descriptors so the default doesn't make things worse #7

Closed mpalmer closed 9 years ago

mpalmer commented 9 years ago

The file_descriptors configuration variable causes EM.set_descriptor_table_size to be called, which (at best) merely sets the NOFILE rlimit. Since the soft limit is usually set to the hard limit already, the hard limit can only be raised by root (or someone with CAP_SYS_RESOURCE -- and does anyone use capabilities?), and anyone running agents as root needs their head examined, the most likely outcome of setting file_descriptors is that NOFILE will either be unchanged (if file_descriptors is greater than or equal to NOFILE), or else it will be lowered (if file_descriptors is less than NOFILE).

All of this would just be a funny little anecdote about why you shouldn't use configuration parameters whose purpose you don't understand, except that the default for file_descriptors is 1024. This means that on any system which has deliberately increased the NOFILE limit, agents won't get the benefit of this change with a default smith configuration.

I'd strongly recommend making the default for this parameter be nil, and not call EM.set_descriptor_table_size if the value for the parameter is nil. Personally, I'd just remove the config parameter entirely, since only crazy people running agents as root (and the three people on earth who use capabilities) are likely to get any benefit from it.

filterfish commented 9 years ago

Given how little I apparently understand I'm eternally grateful that you have deemed it worthy to tell me of my failings. So I have bowed down to your infinite knowledge and removed as you suggested.