Skip to content
This repository has been archived by the owner. It is now read-only.

[dev.icinga.com #3722] add hash compare for conf reading (cgi enhancement) #1213

Closed
icinga-migration opened this issue Feb 20, 2013 · 8 comments
Closed
Milestone

Comments

@icinga-migration
Copy link
Member

@icinga-migration icinga-migration commented Feb 20, 2013

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

Created by mfriedrich on 2013-02-20 18:07:23 +00:00

Assignee: mfriedrich
Status: Resolved (closed on 2013-04-06 21:56:29 +00:00)
Target Version: 1.9
Last Update: 2014-12-08 09:15:16 +00:00 (in Redmine)


this patch by gunnar adds hashes to the objects, being populated when reading objects data and adding them to memory.

  • hash compare
  • if hashes are the same, fallback to strcmp again
  • saving the hash compare value, and only running it once

this is mainly a useful thing when combined with icinga2 compat component and huge amount of checks. as this introduced a change to include/objects.h the ugly method with attributes at the end to stay nagios compatible applies. but as a matter of fact, it can be be a basis to add that to the core as well (the strcmp's are all over the place).

Attachments

Changesets

2013-03-11 19:37:26 +00:00 by (unknown) de79fd5

increase performance with hash comparing hosts/services to show/filter

refs #3722
@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Mar 11, 2013

Updated by mfriedrich on 2013-03-11 19:01:05 +00:00

added the patch, but encapsulated all objects.h modifications into NSCGI #ifdef's in order to only use that for the cgis now. it's ugly, but what else is beautiful in the current code basis ...

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Mar 11, 2013

Updated by mfriedrich on 2013-03-11 19:04:11 +00:00

ok, not really. just keep the hash implementation like it is now, as this will also be used within the current config reading routine already (add_host) saving us a bunch of strcmp's already.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Mar 11, 2013

Updated by mfriedrich on 2013-03-11 19:23:16 +00:00

my dev box - 4k services, 200 hosts. showing all items (no result limit).

$ export REQUEST_METHOD=GET ; export QUERY_STRING="host=all&limit=0&start=1"; export REMOTE_USER=icingademo; time cgi/status.cgi

without the patch

real    0m13.473s
user    0m1.008s
sys     0m0.628s

with the patch

real    0m4.140s
user    0m1.012s
sys     0m0.604s
@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Mar 11, 2013

Updated by mfriedrich on 2013-03-11 20:29:53 +00:00

  • Status changed from Assigned to 7
  • Done % changed from 0 to 100

since it's also a core change, it lives in the core dev tree, as well as 'next'.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Mar 14, 2013

Updated by ricardo on 2013-03-14 23:09:13 +00:00

wow, this seems quite an improvement. will check this at my work next week.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Mar 15, 2013

Updated by mfriedrich on 2013-03-15 08:55:59 +00:00

test it on the weekend during CLT. gunnar is using the classic ui with icinga 2 compat. you'll get a better idea with 100k services.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Apr 6, 2013

Updated by mfriedrich on 2013-04-06 21:56:29 +00:00

  • Status changed from 7 to Resolved
@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Dec 8, 2014

Updated by mfriedrich on 2014-12-08 09:15:16 +00:00

  • Project changed from 19 to Core, Classic UI, IDOUtils
  • Category set to Classic UI
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
1 participant
You can’t perform that action at this time.