Skip to content
This repository has been archived by the owner on Jan 15, 2019. It is now read-only.

[dev.icinga.com #2439] Host/Service Infobox and execution time not correct displayed/calculated #710

Closed
icinga-migration opened this issue Mar 14, 2012 · 19 comments

Comments

@icinga-migration
Copy link

This issue has been migrated from Redmine: https://dev.icinga.com/issues/2439

Created by ossmon on 2012-03-14 08:08:53 +00:00

Assignee: sgronewold
Status: Resolved (closed on 2012-04-27 14:27:30 +00:00)
Target Version: 1.7
Last Update: 2012-04-27 14:27:30 +00:00 (in Redmine)


Icinga-Web 1.6.2/Icinga-Core 1.6.1

In the new Infobox (Host ans Service), the execution time of the check command is displayed with 4 values (last call, avg, min and max).
The problem is that all the 4 values are always the same (see screenshot).
For the average, min and max, i would expect other values.

Attachments

Changesets

2012-04-27 13:28:21 +00:00 by jmosshammer 5af32a3

Applied sgronewold's patch for removing min/max/avg latencies
fixes #2439

2012-05-02 07:25:12 +00:00 by jmosshammer 5d83134

Fixed issues in TO views and  strict legacyview (fixes #2571, refs #2439)
@icinga-migration
Copy link
Author

Updated by jmosshammer on 2012-04-17 12:40:34 +00:00

  • Category set to Interface Features
  • Assigned to set to sgronewold
  • Target Version set to 1.7

@icinga-migration
Copy link
Author

Updated by sgronewold on 2012-04-25 17:45:38 +00:00

  • Status changed from New to Closed
  • Done % changed from 0 to 100

Fixed. The table icinga_hoststatus/icinga_servicestatus was used for the calculation of min, max and average instead of icinga_hostchecks/icinga_servicechecks.

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2012-04-25 17:49:57 +00:00

*checks? is that really necessary? letting those being inserted is a performance killer, just for icinga-web.

currently, we mark *checks tables as not-a-requirement for web and reporting.

@icinga-migration
Copy link
Author

Updated by sgronewold on 2012-04-25 18:17:55 +00:00

I might be wrong but i think there is no other way to calculate the min,max,avg of execution time or latency.

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2012-04-25 18:50:46 +00:00

max min avg on ALL hosts/services can be done with queries on host/servicestatus joined objects. like the cgis do it. this is NOT on a per host/service basis.

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2012-04-26 06:56:38 +00:00

  • Status changed from Closed to Assigned
  • Done % changed from 100 to 50

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2012-04-26 06:58:56 +00:00

using the hostchecks and service checks table is an absolute NOGO. we are lucky not to use those - the cgis do not use such a combined dataset either.

if you apply that fix, my systems will not work with it - filling up checks tables on larger systems is a performance killer, and just for the sole reason of calculating one single row for icinga-web does not find the argument right.

just for explaination

  • now you are fetching a row from the hostchecks for one host, calculating over the min/max/avg execution time over time
  • cgis fetch all hosts, run through and calculate an overall min/max/avg execution time

cgis use status.dat for that, and do not parse the logfiles, nor retention.dat

so, what's the point and argument to implement that feature, introduced the *checks tables dependency?
if you think it is right, you need to adapt the requirements plus propose new values for idomod.cfg, update all the tuning and hints guides, plus tell the user why his/here system should suffer from bloated checks tables to generate some stats out of that.

@icinga-migration
Copy link
Author

Updated by davidg on 2012-04-26 07:13:02 +00:00

dnsmichi wrote:

using the hostchecks and service checks table is an absolute NOGO. we are lucky not to use those - the cgis do not use such a combined dataset either.

if you apply that fix, my systems will not work with it - filling up checks tables on larger systems is a performance killer, and just for the sole reason of calculating one single row for icinga-web does not find the argument right.

just for explaination

* now you are fetching a row from the hostchecks for one host, calculating over the min/max/avg execution time over time
* cgis fetch all hosts, run through and calculate an overall min/max/avg execution time

cgis use status.dat for that, and do not parse the logfiles, nor retention.dat

so, what's the point and argument to implement that feature, introduced the *checks tables dependency?
if you think it is right, you need to adapt the requirements plus propose new values for idomod.cfg, update all the tuning and hints guides, plus tell the user why his/here system should suffer from bloated checks tables to generate some stats out of that.

I absolutely agree with dnsmichi here! Less DB usage is preferred.

D.

@icinga-migration
Copy link
Author

Updated by sgronewold on 2012-04-26 08:51:02 +00:00

just for explaination

now you are fetching a row from the hostchecks for one host, calculating over the min/max/avg execution time over time
cgis fetch all hosts, run through and calculate an overall min/max/avg execution time
cgis use status.dat for that, and do not parse the logfiles, nor retention.dat

The overall min/max/avg execution time is not the problem. The new web interface displays these values correct and doesn't use the hostchecks/servicechecks table for it.

The problem is, that the min/max/avg values for single objects are displayed wrong, overall values are fine. Calculating these values for single objects is of course always an expensive action, so maybe we should throw out this feature from the new web? Another solution would be adding an extra button for people who want to see the min/max/min execution time or latency for a single host/service.

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2012-04-26 08:57:37 +00:00

when did that fall into -web as feature anyway? that changes dependencies all over.

i vote for removing it, if the dependency stays.

@icinga-migration
Copy link
Author

Updated by sgronewold on 2012-04-26 09:39:55 +00:00

i vote for removing it, if the dependency stays.

i agree with you. it is not worth adding this feature for killing the performance and adding dependencies. if nobody else has a different opinion i will supply a git patch removing this feature.

@icinga-migration
Copy link
Author

Updated by Tommi on 2012-04-26 18:38:40 +00:00

This Patch is really a performance killing stuff, because the checks tables are usually big. I have 300entries per check in service check table (with 10min check interval and default cleaning interval). 1000 checks are a middlesized installation. Pls try your resulting SQL for yourself with ~300000 entries in service checks table and probably your sql repeated 50time for your first page.

There are several other ways,you can reach your approach. i already suggested some alternatives.

@icinga-migration
Copy link
Author

Updated by sgronewold on 2012-04-26 21:42:46 +00:00

  • File added 0001-Bug-2439.patch
  • Done % changed from 50 to 90

Yeah I got your approaches from the email but I thought about the problem again and I think it's not worth keeping the feature. I mean the average execution_time of all time for example is not really a usable value. Your suggestion with the time interval was good but I don't think someone would really use this function either.

So, we should throw out those values...

I attached the git patch which is doing that. I would appreciate if someone commits that, if you agree with it. Cheers.

@icinga-migration
Copy link
Author

Updated by jmosshammer on 2012-04-27 07:47:23 +00:00

I'd suggest only removing the fields from the interface, which doesn't affect existing queries (i think your patch would crash, the regular info boxes as used in the grids.

Take a look at:
app/modules/Web/config/simple_data_provider.xml

This file defines which fields are displayed in the infobox of a regular grid

@icinga-migration
Copy link
Author

Updated by sgronewold on 2012-04-27 08:38:11 +00:00

The patch is tested and works. You'll find the according queries in app/modules/Cronks/lib/js/Cronks/Tackle/Information/Head.js, which I changed.

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2012-04-27 08:49:51 +00:00

oh, this was introduced with tackle (which is now disabled by default)?

@icinga-migration
Copy link
Author

Updated by jmosshammer on 2012-04-27 13:01:13 +00:00

No, he's right. We switched to the tackle view in 1.7 for displaying user information.
Sorry, I forgot that. But please update the simpleprovider entries, too, so we have no more references to those fields

@icinga-migration
Copy link
Author

Updated by sgronewold on 2012-04-27 13:08:48 +00:00

There are no simpleprovider entries for the min/max/avg values so that might be alright...

@icinga-migration
Copy link
Author

Updated by jmosshammer on 2012-04-27 14:27:30 +00:00

  • Status changed from Assigned to Resolved
  • Done % changed from 90 to 100

Applied in changeset 5af32a3.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant