NSCA bottleneck / NSCA Timestamp

[Andreas Ericsson]

Thomas Guyot-Sionnest wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Thomas Guyot-Sionnest wrote:
>> * Or did you have another idea to do this? Tabs are currently allowed in
>> the last field for service checks so the number of separators isn't an
>> option, even when adding two new fields. Perhaps the position of the
>> return code + two new fields, but then a service named "0" trough "3"
>> would mess the thing up.
> 
> Oh, I overlooked a bit too much the protocol (I thought the data was
> sent as-it but I believe I actually got confused reading client code
> thinking it was server-side code).
> 
> Solution #1:
> Since the server only read sizeof(receive_packet) bytes, we should
> normally be able to add things to the struct without breaking anything
> (older server will just ignore it).
> 
> Solution #2:
> Each packet sent have a version number, so we can create new packet
> formats with different version (older servers will discard non-matching
> version). I'm not an expert in network protocols but I guess we should
> do something like this:
> 
> typedef struct data_packet_struct{
>         int16_t   packet_version;
>         u_int32_t crc32_value;
>         u_int32_t timestamp;
>         int16_t   return_code;
>         uint16_t  count; /* bytes of data */
>         char      payload[MAX_PACKET_SIZE];
>         }data_packet;
> 
> typedef struct nsca_message_v3_struct{
>         int16_t   return_code;
>         char      host_name[MAX_HOSTNAME_LENGTH];
>         char      svc_description[MAX_DESCRIPTION_LENGTH];
>         char      plugin_output[MAX_PLUGINOUTPUT_LENGTH];
>         }nsca_message_v3;
> 

This is bad. Look up modbus for how to write header + body part
messages which allows for near-unlimited protocol extensions.

With this way of doing things, you're limiting yourself to a
definitive size of each host/service. It makes for (a little)
easier coding, but it's a useless limitation to put on people.

A header should (basically) consist of something like this:
struct proto_header {
	u_int32_t proto_version;
	u_int32_t pkt_type;
	u_int32_t body_len;
	char pad[DESIRED_HEADER_SIZE - (sizeof(u_int32_t) * 3);
};

where DESIRED_HEADER_SIZE is usually 32 or 64, for alignment purposes.
It's usually desirable to have protocol_version be 32 bits wide as
well, also for alignment reasons (no modern architecture aligns on
2 byte boundaries).

Then you just invent packets as you go along and can handle them all
in a single switch() statement (to decide which function *really*
should take care of them) or, if performance is really critical,
you do it with a list such as this:

---%<---%<---%<---
#define MAX_PKT_TYPE 3
int (*handle_pkt[MAX_PKT_TYPE_HANDLED])(void *buf, size_t len) = {
	handle_pkt0,
	handle_pkt1,
	handle_pkt2,
};

if (hdr.pkt_type > MAX_PKT_TYPE)
	unsupported_packet();

handle_pkt[hdr.pkt_type](packet_body, len);
---%<---%<---%<---

which will incur exactly one branch and one memory lookup per
packet. Note that this is really a micro-optimization though.
For anything but embedded systems, the switch() statement will
almost certainly be a better choice, providing better readability
at the expense of binary footprint size and memory usage.

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

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/