nsqd: instrument aggregate message bytes on topics

[andyxning]

This PR adds message size per topic for HTTP protocol. In order to avoid more CPU usage to compute the length for messages, this PR leverage the HTTP header Content-Length for message length. As for MPUB, this includes the meta bytes involved in encoding multiple messages.

[andyxning]

/cc @mreiferson @ploxiln

[ploxiln]

I would use the word "bytes" instead of "size".

There's also PUB() and MPUB() in nsqd/protocol_v2.go which is called for publishing by tcp-protocol clients. But I think the best place to add to the total is in Topic.put() and I think it's OK to do len(m.Body) to get the size of the message.

EDIT: eh probably better to do it in Topic.PutMessage() and Topic.PutMessages() similar to how messageCount is handled

[andyxning]

Yep. I have think about this also. But considering that in order not to put more burden to nsqd, i choose to use the existing HTTP header 'Content-Length' which is computed by distributed clients.

But, we can not gather the TCP-based producer requests.

[andyxning]

I think it's OK to do len(m.Body) to get the size of the message.

We may have some messages with more than 1M bytes. This may trigger a serious CPU burden.

[andyxning]

I will make a perf test on different message size and qps. Will feed that back later.

[ploxiln]

len(m.Body) is not like libc strlen(), Body is a slice which stores its length explicitly

[ploxiln]

but if you're really worried about efficiency, you can add-up the lengths in in PutMessages() and only do a single atomic add :)

[andyxning]

@ploxiln

but if you're really worried about efficiency, you can add-up the lengths in in PutMessages() and only do a single atomic add :)

Good suggestion.

[ploxiln]

I think this feature was a good idea - thanks

[andyxning]

@ploxiln PTAL.

[ploxiln]

LGTM but I'll wait for @mreiferson to have a quick look

[andyxning]

Seems that the cpu usage does not increase along with message size. https://docs.google.com/spreadsheets/d/1WsThzaqXjQY4-_cP1KUJVrXR61XYD-eeJdONEeqm2U8/edit#gid=1042961327.

[mreiferson]

LGTM

[ploxiln]

one last tiny thing - the commit title is no longer accurate, this is no longer specific to the http interface

[andyxning]

@ploxiln It seems that @mreiferson has changed the PR title. PTAL. It should be ready to be merged.

[ploxiln]

We usually don't use the "squash and merge" option on github, which is what gives the opportunity to change the commit title and description, and defaults to the PR title. We usually "create a merge commit" so the commit in the PR is completely un-changed when introduced to the master branch.

But it's not a big deal IMHO I'll just do it :)

[ploxiln]

actually "squash and merge" button is disabled because matt really doesn't like it

[ploxiln]

let me clarify: the commit title is "add topic message size metric for http protocol"

[andyxning]

let me clarify: the commit title is "add topic message size metric for http protocol"

Wait 3 minutes.

[andyxning]

@ploxiln PTAL. The Ci should be green.