Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the way Wazuh DB cleans open databases #22155

Merged
merged 3 commits into from
Feb 27, 2024

Conversation

TomasTurina
Copy link
Member

@TomasTurina TomasTurina commented Feb 26, 2024

Related issue
#22130

Description

It was discovered that the wdb_pool_size variable could contain an invalid value, which could cause an excessive number of file descriptors to be opened and the wazuh-db daemon to use excessive memory due to an incorrect filter in the database cleaning. This was because the wdb_close function (where wdb_pool_size was decremented) is not always called with a valid database pointer, which could lead to decrementing the number of wdb_pool_size when it was not necessary.

This PR replaces the wdb_open_count variable with wdb_pool_size. This new variable corresponds to the number of nodes that are stored in wdb_pool where all open objects are stored, whether the database is already open or not.

This new variable is used as a filter in the wdb_close_old function instead of wdb_pool_size. Now, every time a new node is created/deleted, this new variable is modified, so it will always correspond to the current number of databases stored in memory.

Tests

  • Compilation without warnings in every supported platform
    • Linux
    • Windows
    • MAC OS X
  • Source installation
  • Package installation
  • Source upgrade
  • Package upgrade

@TomasTurina TomasTurina self-assigned this Feb 26, 2024
@TomasTurina TomasTurina linked an issue Feb 26, 2024 that may be closed by this pull request
Copy link
Member

@vikman90 vikman90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TomasTurina I've understood where the issue was. Thanks.

The aspect I'd like to tweak is regarding the wdb_pool_size counter. It's equivalent to rbtree_size(wdb_pool.nodes) but less costly. However, it jeopardizes encapsulation by allowing someone to write directly to that variable.

My proposal is to incorporate pool_size within wdb_pool, and have wdb_pool_size() as a function,

@vikman90 vikman90 force-pushed the 22130-fix-wazuhdb-fd-clean branch from 37ac398 to b5c85cf Compare February 27, 2024 08:16
Copy link
Member

@JcabreraC JcabreraC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM !

@vikman90 vikman90 merged commit 61c66a5 into 4.7.3 Feb 27, 2024
60 of 62 checks passed
@vikman90 vikman90 deleted the 22130-fix-wazuhdb-fd-clean branch February 27, 2024 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DB Error Logs in Footprints
3 participants