Threaded reaper patch

[Andreas Ericsson]

On 08/17/2011 01:26 AM, Adam Augustine wrote:
> On Mon, Aug 15, 2011 at 7:25 AM, Andreas Ericsson<ae at op5.se>  wrote:
> 
>> On 08/09/2011 09:13 PM, Adam Augustine wrote:
>>>
>>> But in spite of that, it seems that moving the reaper code into a thread
>>> would be generically useful for Nagios. I know it has been discussed on
>> this
>>> list in the past.
>>>
>>
>> It would also cause a bunch of problems. What we're working on instead is
>> implementing worker processes which communicate with a master process via
>> a unix socket. One such process could act as a (mostly dormant) reaper for
>> the checkresult files in the spool directory.
>>
>>
> Ah, it seems the scope of the worker process socket effort is much larger
> than I had expected.

Indeed it is.

> Does this mean that modules that were initially NEBs
> can instead be implemented as wholly independent processes, communicating
> back over that socket (presumably more than just a unix domain socket, but
> also a network socket as well)?
> 

No. If that was the case Nagios core would be dependant on the latency
of a different process to allow eventbroker modules to be able to block
checks, notifications and whatnot.

> 
>>   >  If the Merlin reaper thread is wholly contained within the Merlin NEB
>> (as it
>>> appears to be) and is not in any way patching the Nagios core code, then
>> my
>>> question is, how is that working without conflicting with the main event
>>> loop reaper code?
>>
>> Mainly by making Nagios itself threadsafe all API's the broker module uses.
>> That's why Merlin needs Nagios 3.3.1 or one of the post-3.2.3 versions made
>> available through git.op5.org
>>
>>
> Ah, so there are modifications necessary to pre-3.3.1 versions of Nagios to
> override the reaping process.

You're not listening. There are several *different* reapers running at
the same time. The changes for Merlin's reaping has absolutely nothing
at all to do with Nagios' reaping of checkresults and external commands.
The changes in Nagios core for the benefit of Merlin was simply to make
macros obtainable from several threads at the same time, and then using
those new threadsafe macro fetching apis from other apis, making those
other apis safe to use from multiple threads as well.

> Nagios 3.3.1 now has real (and threadsafe)
> APIs for manipulating internal data structures, where before there weren't
> any. This makes perfect sense to me. The Merlin reaper thread uses the same
> API to update the in-memory data structures that the main event loop reaper
> code would, so no conflicts.
> 

Not really. Merlin uses manual but atomic updates of the internal objects.
Atomic operations are inherently threadsafe. The changes were solely so
that external merlin events triggering status updates, log messages and
other things that require macro resolution could happen in tandem with
Nagios' normal operations without either of them causing a NULL-read
segmentation violation.

> 
>>   >  My quick glance at the NEB callbacks for
>>> EVENT_CHECK_REAPER seems to indicate that there isn't any
>>> NEBERROR_CALLBACKOVERRIDE associated with it. So I am very curious how it
>> is
>>> being handled.
>>>
>>
>> You're talking about two different reapers. They don't interfere with
>> each other at all.
>>
> 
> I think I understand now, presuming that the Merlin reaper and the main
> Nagios event loop reaper are both using the new thread safe APIs.
> 

They are, more or less.

> But I am still a little confused. You mention above that implementing the
> reaper code as a Nagios thread would cause a lot of problems, but isn't that
> what the Merlin NEB module does?

Yes, it does, but Merlin doesn't load a bunch of modules that it has to
make sure are only called in a threadsafe manner.

> Are you encountering a lot of problems with
> that approach?

Not really. Merlin is running stable in production on well over 500 systems
and has been doing that for the past year or so.

> Or was it specifically the /moving/ the reaper into a thread
> that you thought was a bad idea?
> 

For Nagios, yes. It's a terribly bad idea to force all eventbrokers to
handle threadsafety, which they would have to do if different types of
events, or even the same type of events but with slightly different
parameters, can come from multiple threads to the same module at the
same time. Currently, modules are only ever called from a single
thread, so very simple modules needn't worry about threads at all.
That's a Good Thing(tm), and we should keep it that way to ensure the
entry barrier for module writers remains low.

> I certainly agree that socket communication provides a much cleaner
> separation, and would make things easier, and I am not advocating
> 

But it has its own bucket of problems. What do we do when the connection
dies or when the socket becomes full?

> But separate thread or separate process is really an implementation detail
> (an important one, admittedly, but still).
> 
> My base assumption is that, the single threaded nature of Nagios core is
> slowed significantly by the time spent in the reaping portion of the loop.
> Evidence supporting that assumption is the fact that we have a timeout
> associated with that portion of code. Assuming the default of 30 seconds is
> "sane" then the reaper could spend up to 30 seconds blocking checks from
> being executed, and significantly impacting check_latency.
> 

Huh? Why not just set the reaper frequency to something very low, or just
leave it out entirely? That would make Nagios try to reap as often as
possible.

> Anyway, for a larger number of checks (50K-100k), I would think a reaper
> implemented as a worker process (or thread, or whatever) would be very busy
> processing all the results coming into the checkresults files in the spool
> directory and updating the relevant in-memory data structures. But based on
> your statement above (the "mostly dormant" part), it would seem that I am
> wrong somewhere.
> 
> What am I missing?
> 

That we're moving away from the spool directory. Disks and (unnecessary)
directory traversals are the main culprits of useless waiting in Nagios
and the reason we currently can't handle 100k checks from a single process.
With separate worker processes we would avoid the directory traversal
completely, and IO-wait is moved from disk-based (disk subsystem kernel
calls will hang the caller while the kernel is working, even if the accessed
partition is a RAM-disk) to memory-based where all memory resides in user
space. Unless you're a programmer I doubt that answer means very much to
you, but that's the reason anyways.

> Thanks for your time in answer my questions. I have spent some time looking
> through the code and usually end up with more questions than answers on the
> internals of how Nagios is handling things.
> 

That's quite a normal reaction to organically grown code that wasn't
designed from the start to scale to hundreds of thousands of actions
per (insert-timeframe-of-choice) and where external interface simplicity
has been prioritized over performance.

-- 
Andreas Ericsson                   andreas.ericsson at op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.

------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system, 
user administration capabilities and model configuration. Take 
the hassle out of deploying and managing Subversion and the 
tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2