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

[dev.icinga.com #2618] re-add missing indexes from #1110 resolving performance issues on all supported rdbms #977

Closed
icinga-migration opened this issue May 16, 2012 · 20 comments
Milestone

Comments

@icinga-migration
Copy link

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

Created by tgelf on 2012-05-16 17:38:53 +00:00

Assignee: mfriedrich
Status: Resolved (closed on 2012-09-23 09:57:00 +00:00)
Target Version: 1.8
Last Update: 2014-12-08 14:37:43 +00:00 (in Redmine)

Icinga Version: 1.10.0
OS Version: any

The hostgroup_members table is important for Hostgroup-based permissions and has therefore a strong impact on the performance of many Icinga-Web installations. Here a few suggestions:

  • UNIQUE KEY `hostgroup_member_id` (`hostgroup_member_id`) -> this one is useless, hostgroup_member_id is the primary key and therefore already unique.
  • KEY `hostgroup_members_i_id_idx` (`instance_id`) -> there are many indexes like this to be found in the IDO schema. As MySQL is unable to use multiple indexes on the same table at once this will rarely be used. However, the only one doing massive (and insane) searches involving instance_id is the ido2db daemon himself, so better leave the indexes as they are.
  • Please add an index involving hostgroup_id and host_object_id, something like: CREATE INDEX 'hostgroup_members' ON icinga_hostgroup_members (hostgroup_id, host_object_id);

Cheers,
Thomas Gelf

Attachments

Changesets

2012-09-17 12:27:51 +00:00 by mfriedrich 6f37cd7

idoutils: re-add missing indexes from #1110 resolving performance issues on all supported rdbms #2618 - MF

thanks to Mr. Gelf nagging me down, finally.

refs #2618

Relations:

@icinga-migration
Copy link
Author

Updated by tgelf on 2012-05-16 18:06:17 +00:00

I digged a little bit farther, as this seemed pretty strange to me. Found the related commit:

https://git.icinga.org/?p=icinga-core.git;a=commitdiff;h=8c071d12cc5cb3a4f2a51d7fd3ee9812c476a992

Not only hostgroup_members have been affected, also a few other important indexes have been removed. At least the following ones make sense to me and shall be restored - but please give them a new name:

icinga_contactgroup_members:
UNIQUE KEY instance_id (contactgroup_id,contact_object_id)

icinga_hostgroup_members:
UNIQUE KEY instance_id (hostgroup_id,host_object_id)

icinga_host_contactgroups:
UNIQUE KEY instance_id (host_id,contactgroup_object_id)

icinga_host_parenthosts:
UNIQUE KEY instance_id (host_id,parent_host_object_id)

icinga_servicegroup_members:
UNIQUE KEY instance_id (servicegroup_id,service_object_id)

icinga_service_contactgroups:
UNIQUE KEY instance_id (service_id,contactgroup_object_id)

The commit shown before also affects other DB backends, please have a look at them too. I guess that what you wanted to achieve was removal of useless instance_id indexes (makes sense), but all of those named above didn't have any relation to the instance_id. No idea how they got such name.

This could have been the root cause of the massive performance drop experienced with version 1.5 in large Icinga-Web installations, notably in summaries and historic grids involving hostgroups (affects all those with hostgroup permissions) and contacts (notification history). They have been and still are terribly slow. This is at least true for 1.6, please note that I didn't test 1.7 in large environments yet.

Regards,
Thomas

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2012-05-16 18:10:53 +00:00

the goal was to get rid of unique constraints, which would prohibit the changed multiline inserts in the code and query diff. if you require the implicit indexes back in the game, name them accordingly.

@icinga-migration
Copy link
Author

Updated by tgelf on 2012-05-16 18:14:20 +00:00

I found some more constructs as the following one:

PRIMARY KEY (host_parenthost_id),
UNIQUE KEY host_parenthost_id (host_parenthost_id),

As noted in the initial comment (hostgroup-specific), this is completely useless - but slows INSERTs (and UPDATEs?) down, as each indexes has to be refreshed each time. A primary key is always UNIQUE - please remove all those UNIQUE KEYs. But please take care, remove only similar ones, not all of them ;-)

@icinga-migration
Copy link
Author

Updated by tgelf on 2012-05-16 18:36:33 +00:00

Hi Michael,

thank you for your fast response! Well, if you wanted to remove the UNIQUEness of those columns the right way would have been to replace UNIQUE KEY with KEY (or CREATE INDEX). It would be great if you could restore at least the dropped indexes, if uniqueness is not an option anymore.

However, I can immagine no reason why those UNIQUE KEYs shall break multiline inserts. Duplicates in those member-tables shall never happen. Unfortunately they seem to do so in some rare situations. Today I stumbled over an installation with icinga-core 1.6.1-2 from Ubuntu 12.04, they had >200.000 entries in icinga_hostgroup_members, with most of them having hostgroup_id=0. This wouldn't have happened if those UNIQUE constraints had still been in place. Blind guess: the fact that they have been an issue once may mean that they showed that there is something missbehaving. FYI, here some settings from that installation:

data_processing_options=66977597
config_output_options=2
clean_realtime_tables_on_core_startup=0
clean_config_tables_on_core_startup=0

IMHO the best solution would be trying to find out WHY ido2db had problems with those indexes. Otherwise at least the indexes (without setting them UNIQUE) shell be restored. But please consider the former option, allowing duplicates in member tables is IMO not sane.

Cheers,
Thomas

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2012-05-18 12:08:10 +00:00

well not letting the config tables cleaned on startup is your really own problem, that was not the scope when allowing that feature request to make sure that everything is truly unique

probably those indexes itsself must be re-added, but yet i am not really convinced that the unique constraint works as well, as the "insert on duplicate key update" query is not in place anymore.

@icinga-migration
Copy link
Author

Updated by tgelf on 2012-05-20 15:12:29 +00:00

dnsmichi wrote:

well not letting the config tables cleaned on startup is your really own problem, that was not the scope when allowing that feature request to make sure that everything is truly unique

censored

probably those indexes itsself must be re-added, ...

Definitively!

...but yet i am not really convinced that the unique constraint works as well, as the "insert on duplicate key update" query is not in place anymore.

IMO there is no valid reason for allowing such duplicates, neither with nor without those fantastic ON-DUPLICATE constructs. If ido2db stumbles over these contraints it would be great if we could find out, WHY it does so. Tt definitively shouldn't.

Cheers,
Thomas

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2012-05-20 16:04:19 +00:00

Definitively!

not in 1.7 tree then. such things require extensive testing.

IMO there is no valid reason for allowing such duplicates, neither with nor without those fantastic ON-DUPLICATE constructs.

fantastic? you are kidding right? those statements are the pure evil, as not in sql spec and therefore not portable to postgresql or oracle. that mess did cost me weeks of my life to figure out and write the very correct queries for all rdbms supported.

so please do not call them 'fantastic'.

If ido2db stumbles over these contraints it would be great if we could find out, WHY it does so. Tt definitively shouldn't.

mhm. test again with postgresql or oracle please. mysql is not the real problem, making things too easy aside from the sql specs.

@icinga-migration
Copy link
Author

Updated by tgelf on 2012-05-20 16:42:39 +00:00

dnsmichi wrote:

> Definitively!

not in 1.7 tree then. such things require extensive testing.

The indexes themselves as well? What about re-adding the indexes and postponing the decision related to the uniqueness of those indexes?

> IMO there is no valid reason for allowing such duplicates, neither with nor without those fantastic ON-DUPLICATE constructs.

fantastic? you are kidding right?

Of course I am. They are an alluring toy for lazy developers.

so please do not call them 'fantastic'.

A little bit of sarcasm sometimes doesn't hurt ;-)

> If ido2db stumbles over these contraints it would be great if we could find out, WHY it does so. Tt definitively shouldn't.

mhm. test again with postgresql or oracle please. mysql is not the real problem, making things too easy aside from the sql specs.

Unfortunately I haven't available neither one nor the other, I'll try to find someone who has. The indexes alone without the unique contraint obviously won't hurt there. If you've had problems with the constraint before 1.5 there may probably be some issue in ido2db. Therefore I strongly agree on postponing that decision, some intensive testing is required first. But it would be great to see the indexes for those m:n tables being re-added.

Regards,
Thomas

@icinga-migration
Copy link
Author

Updated by robe on 2012-08-25 16:13:38 +00:00

  • Icinga Version set to 1
  • (unknown custom field) set to 1
  • OS Version set to n/a
  • (unknown custom field) set to MySQL
  • (unknown custom field) set to 1

tgelf wrote:

I found some more constructs as the following one:

> PRIMARY KEY (host_parenthost_id),
> UNIQUE KEY host_parenthost_id (host_parenthost_id),

As noted in the initial comment (hostgroup-specific), this is completely useless - but slows INSERTs (and UPDATEs?) down, as each indexes has to be refreshed each time. A primary key is always UNIQUE - please remove all those UNIQUE KEYs. But please take care, remove only similar ones, not all of them ;-)

This is adressed in #3018

@icinga-migration
Copy link
Author

Updated by robe on 2012-08-25 16:15:54 +00:00

Thomas - can you show me the diff between the upstream schema and the one you use in production? I'm interested in which indexes are necessary for icinga-web to perform in large setups. We can easily readd them, I just want to have some documentation on why they're necessary.

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2012-09-16 14:26:00 +00:00

  • File added unique_key_drop.patch

    $ git show 8c071d1

attached is the diff which did cause that probably.

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2012-09-16 14:26:40 +00:00

  • Target Version set to 1.8

@thomas

any possible patch from your side?

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2012-09-16 15:43:46 +00:00

related to #1110

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2012-09-17 12:20:41 +00:00

  • Category set to 24
  • Status changed from New to Assigned
  • Assigned to set to mfriedrich

i've nailed that down to the following pretty much generic re-add of indexes (oracle - wipe table prefix)

-- -----------------------------------------
-- #2618
-- -----------------------------------------

CREATE INDEX cntgrpmbrs_cgid_coid ON icinga_contactgroup_members (contactgroup_id,contact_object_id);
CREATE INDEX hstgrpmbrs_hgid_hoid ON icinga_hostgroup_members (hostgroup_id,host_object_id);
CREATE INDEX hstcntgrps_hid_cgoid ON icinga_host_contactgroups (host_id,contactgroup_object_id);
CREATE INDEX hstprnthsts_hid_phoid ON icinga_host_parenthosts (host_id,parent_host_object_id);
CREATE INDEX runtimevars_iid_varn ON icinga_runtimevariables (instance_id,varname);
CREATE INDEX sgmbrs_sgid_soid ON icinga_servicegroup_members (servicegroup_id,service_object_id);
CREATE INDEX scgrps_sid_cgoid ON icinga_service_contactgroups (service_id,contactgroup_object_id);
CREATE INDEX tperiod_tid_d_ss_es ON icinga_timeperiod_timeranges (timeperiod_id,day,start_sec,end_sec);

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2012-09-17 12:21:29 +00:00

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2012-09-17 12:27:39 +00:00

oh well, oracle will of course use the script defined tablespace for the indexes in that case, and not just put that into the main tablespace.

-- -----------------------------------------
-- #2618
-- -----------------------------------------

CREATE INDEX cntgrpmbrs_cgid_coid ON contactgroup_members (contactgroup_id,contact_object_id) tablespace &&IDXTBS;
CREATE INDEX hstgrpmbrs_hgid_hoid ON hostgroup_members (hostgroup_id,host_object_id) tablespace &&IDXTBS;
CREATE INDEX hstcntgrps_hid_cgoid ON host_contactgroups (host_id,contactgroup_object_id) tablespace &&IDXTBS;
CREATE INDEX hstprnthsts_hid_phoid ON host_parenthosts (host_id,parent_host_object_id) tablespace &&IDXTBS;
CREATE INDEX runtimevars_iid_varn ON runtimevariables (instance_id,varname) tablespace &&IDXTBS;
CREATE INDEX sgmbrs_sgid_soid ON servicegroup_members (servicegroup_id,service_object_id) tablespace &&IDXTBS;
CREATE INDEX scgrps_sid_cgoid ON service_contactgroups (service_id,contactgroup_object_id) tablespace &&IDXTBS;
CREATE INDEX tperiod_tid_d_ss_es ON timeperiod_timeranges (timeperiod_id,day,start_sec,end_sec) tablespace &&IDXTBS;

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2012-09-17 12:34:31 +00:00

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

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2012-09-17 16:20:03 +00:00

  • Done % changed from 80 to 100

now up in next, master, r1.8 for testers.

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2012-09-23 09:57:00 +00:00

  • Status changed from 7 to Resolved

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2014-12-08 14:37:43 +00:00

  • Project changed from 18 to Core, Classic UI, IDOUtils
  • Category changed from 24 to IDOUtils
  • Icinga Version changed from 1 to 1
  • OS Version set to any

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