sni / mod_gearman

Distribute Naemon Host/Service Checks & Eventhandler with Gearman Queues. Host/Servicegroups affinity included.
http://www.mod-gearman.org
GNU General Public License v3.0
122 stars 42 forks source link

Addresses https://github.com/sni/mod_gearman/issues/154 #155

Closed infraweavers closed 4 years ago

infraweavers commented 4 years ago

I've not had a chance to compile and test this in the lab yet, however I can't really foresee any major problems with increasing this

sni commented 4 years ago

Hmm, the buffer size is used all over the place and increasing it this much might have performance impacts. How about using GM_MAX_OUTPUT instead when reading the performance data? It does not make sense to have two constants with the same value anyway.

sni commented 4 years ago

it seems like you just blindly replaced all GM_BUFFERSIZE whereas it would be sufficient to do this only if plugin output or performance data is involved, right?

infraweavers commented 4 years ago

it seems like you just blindly replaced all GM_BUFFERSIZE whereas it would be sufficient to do this only if plugin output or performance data is involved, right?

Sort of, because I've resized tempbuffer https://github.com/sni/mod_gearman/pull/155/files#diff-319540a31c7242a4955dc656458567aa7d06d75957558ce57aee657a00a60d95R76 I've updated all the places that are using tempbuffer to have the new maximum size.

If you'd prefer not to resize that buffer, I could create a new buffer for the uses here: https://github.com/sni/mod_gearman/pull/155/files#diff-319540a31c7242a4955dc656458567aa7d06d75957558ce57aee657a00a60d95R1566 and switch those to use that and leave the others alone?

sni commented 4 years ago

thinking about it... i forgot that the temp buffer is reused anyway, so it shouldn't make a huge difference. It would have been if the buffer would be recreated all the time, but thats not the case, so i guess its ok.

sni commented 4 years ago

thanks

infraweavers commented 4 years ago

Ahh thanks @sni Does this go straight into the OMD nightlies or do we need to PR a patch into https://github.com/ConSol/omd/tree/labs/packages/mod-gearman/patches for that?

sni commented 4 years ago

No, those mod-gearman nightlies will not automatically land in the omd nightly. So far we only do that for Thruk, LMD and the Mod-Gearman Go Worker. But there is no need for a pull request, i will put it there. Are you using OMD 3 or OMD 4?

infraweavers commented 4 years ago

Ah cool thanks, we're using OMD3

EDIT: No we're not, I was confused. We're using the OMD4 nightly!

infraweavers commented 4 years ago

@sni I can't see a commit in https://github.com/ConSol/omd/commits/labs putting the patch in, have you had a chance to do this yet?

sni commented 4 years ago

there is no commit, i injected the changes on the build machine. Todays packages contain your changes.

infraweavers commented 4 years ago

Ahh cool, we'll give it a runaround now then 👍

infraweavers commented 4 years ago

Yeah that's had the desired effect, we'll run it in our office environment for a week or so and report back any issues that crop up.

sni commented 4 years ago

great, let me know if there were any issues.

infraweavers commented 4 years ago

Quick update: been running since Tuesday; no issues so far. No discernable increase in memory footprint on naemon (we only run 4GiB RAM instances for this so it's obvious if there's a huge increase) or performance impacts so far.

infraweavers commented 3 years ago

Another quick update. Still no problems or issues with this change seen so far :)