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

[dev.icinga.com #3406] many ido2db memory leaks on dbi_result_free and others #1151

Closed
icinga-migration opened this issue Oct 29, 2012 · 10 comments

Comments

Projects
None yet
1 participant
@icinga-migration
Copy link
Member

commented Oct 29, 2012

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

Created by abraxas on 2012-10-29 01:44:45 +00:00

Assignee: mfriedrich
Status: Resolved (closed on 2012-11-09 16:55:13 +00:00)
Target Version: 1.8.2
Last Update: 2014-12-08 14:35:04 +00:00 (in Redmine)

Icinga Version: 1.8.0
OS Version: RHEL 6.3 x86_64

In our companies icinga installation we are facing memory leaks that also lead to high cpu load of the ido2db process. The result is an increased latency on icinga-core and a drop in checks performed per 5 minutes over time.

Last week we moved from postgresql to mysql database backend and faced the same problem, so I started a code review. I found several memory leaks in libdbi handling as well as general allocation/error handling issues.

The patch included in the bug report has been tested over the weekend and seems stable. Further testing is required to ensure stabliity of the code. The fix included does affect both postgresql and mysql handling though it has only been tested on mysql.

Formal findings (102 fixed leaks):

dbqueries.c: insert_or_update - result is not freed between insert and update
db.c: ido2db_handle_enable_disable: temporary string array 'es' is not freed
dbhandlers.c: improper error handling - on several occasions neither dbi_result_free nor free of temporary arrays is called

A part of this issue is covered in Bug #3405 (which has been reported today), however this patch fixed additional leaks so i reported as separate issue.

Attachments

Changesets

2012-10-29 18:31:20 +00:00 by mfriedrich 59bec47

idoutils: fix many memory leaks in ido2db on dbi_result_free and others (thx Klaus Wagner) refs #3406 #3405

2012-11-01 13:07:33 +00:00 by mfriedrich 5344f68

idoutils: dbi_result_free on unsuccesful queries as well (refs #3408 #3406)

2012-11-01 13:33:01 +00:00 by mfriedrich f3df924

idoutils: move dbi_result_free into insert_or_update functions (refs #3408 #3406)

Relations:

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 29, 2012

Updated by mfriedrich on 2012-10-29 18:01:55 +00:00

  • Status changed from New to Assigned
  • Assigned to set to mfriedrich
  • Target Version set to 1.8.2

i'll try to have look this week, thanks.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 29, 2012

Updated by mfriedrich on 2012-10-29 18:25:02 +00:00

  • the db_hello would explain a lot, leaking memory when connection drops out everytime. i've modified that a bit, in order to also dbi_free the result handle, like the rest is done.

  • dbqueries - true that, as it sources from the change to the update/select/insert foo in #3008
    the last cleanup of the result handle happens outside in the dbhandler functions then again. so patch is valid.

  • db.c - my fault, experimental function, which is never called. patch was missing int x; declaration, added that as well (your code base could still have that one, i am building on top of 1.8.1+).

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 29, 2012

Updated by mfriedrich on 2012-10-29 18:28:42 +00:00

  • Subject changed from Ido2db memory leaks (including patch) to many ido2db memory leaks on dbi_result_free and others
@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 30, 2012

Updated by abraxas on 2012-10-30 15:10:02 +00:00

My patch missed the int x; declaration. Seems I fixed that after pulling the patch from local repo.

I want to add that today we switched back to postgresql backend and the memory usage is stable as well (albeit higher in total, but that could be libdbi/postgresql handling).

Looking forward to see this in next release.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 30, 2012

Updated by mfriedrich on 2012-10-30 15:25:58 +00:00

afaik libdbi is built again 8.1 api of pgsql (the newer versions link against 8.4 iirc). on rhel5 i had the problem with the old libdbi packages, linking the old libpq api, leaking memory as well. see #2741 for the note.

regarding your patch - hopefully the kudos given are ok?

1.8.2 will be due once some core/classic ui issues are solved as well.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 30, 2012

Updated by mfriedrich on 2012-10-30 21:11:12 +00:00

runs since 5 hours against pgsql without noticable issues (debian 6.0.6 x64). though to note, pgsql is from backports, 9.1

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 31, 2012

Updated by abraxas on 2012-10-31 09:03:10 +00:00

I have been looking into 2741 before I started to look in the code and we had a higher memory consumption when using 9.1 client libs then the RHEL based build on 8.4.

Now we stick to the OS bundled libraries (postgresql server version is 9.1 though) which have now been running for over a day without any raise in memory consumption. Looks good from our side, and yes kudos are appreciated :).

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Nov 1, 2012

Updated by mfriedrich on 2012-11-01 14:37:06 +00:00

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

yeah, pgsql libs on rhel are not nice.

i consider the fully fixed tree in 'test/ido' ready for some more longrun tests.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Nov 9, 2012

Updated by mfriedrich on 2012-11-09 16:55:13 +00:00

  • Status changed from 7 to Resolved
@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Dec 8, 2014

Updated by mfriedrich on 2014-12-08 14:35:04 +00:00

  • Project changed from 18 to Core, Classic UI, IDOUtils
  • Category changed from 79 to IDOUtils
  • OS Version set to RHEL 6.3 x86_64
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.