search

UnownHash / Golbat

The Unlicense
36 stars 19 forks source link

Add DB call count stats #190

Closed comstud closed 10 months ago

comstud commented 10 months ago

I was trying to track down some IO spikes to my DB filesystem. Strangely nothing really turned up in slow query log. So I thought maybe there was a spam of requests happening for some strange reason. So, to check or eliminate golbat, I added these stats. (It turns out this eliminated Golbat and made me look elsewhere for the stupid thing I was doing.)

This might be interesting for others, also, so here it is. And timings might also be useful to add at some point to catch degradation.

comstud commented 10 months ago

I'm open to different values for the query label. I tried to name them for the function they were performing.

jfberry commented 10 months ago

This is quite nice; not sure there is any way to do it but the usual bug that you see is when updates are caused by the same thing too often - eg, update because of a single field flapping. This could be done during the update, but we optimised away the generic reflection comparison because it was too slow (it was the hottest part of the code) leading to this being a pain to actually implement as it would be lots of copy/pasta or generated code

comstud commented 10 months ago

Was that ever committed to git? I can go back and see what it looked like.

jfberry commented 10 months ago

It was, but it was just a call to cmp.Diff rather than our hand rolled comparison. there is still a commented out call of the verbose version for debugging so you can watch for flapping but some way (even in a debug mode) of forcing the change field to Prometheus to be able to see them at helicopter view would probably mean these get caught earlier

comstud commented 10 months ago

Yeah, I see. hasChanges*() could be re-worked to note the fields that are different vs just returning a bool. But you lose the short-circuiting in the current 'if' statements if you want a full list of fields changing.

Well, I'll keep this in mind. I think that's for another PR, if there's a reasonable solution.

This one is ready to be reviewed/merged as I've been running it for the past 24h.

lenisko commented 10 months ago

Hmm... Good idea actually!

As extension, you can also collect metrics directly from DB using prometheus exporters.