Skip to content

MDEV-31344: fast status variable lookup#4751

Draft
dmitry416 wants to merge 3 commits into
MariaDB:mainfrom
dmitry416:MDEV-31344
Draft

MDEV-31344: fast status variable lookup#4751
dmitry416 wants to merge 3 commits into
MariaDB:mainfrom
dmitry416:MDEV-31344

Conversation

@dmitry416
Copy link
Copy Markdown

Summary

  • Added a hash table for status variables
  • Now the variable is searched first in the hash table, and if it is not found, it is searched using the old path.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 7, 2026

CLA assistant check
All committers have signed the CLA.

@gkodinov gkodinov added the External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements. label Mar 9, 2026
Copy link
Copy Markdown
Member

@gkodinov gkodinov left a comment

Choose a reason for hiding this comment

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

Thank you for your work. This is a preliminary review.

Please make sure the buildbot servers compile and run tests successfully to completion.

Also, the whole diff needs work:

  • the hash should not be filled in only at init time. It should also be filled in when status variables array version is increased (e.g. load a plugin etc).
  • hash access should be protected under LOCK_all_status_vars.
  • you need a test that covers the change as much as possible.
  • to be able to test you might need some new status variable counters to observe the hash usage vs scans.

I would start with an exact design document of what the diff is trying to do. Then I'd add regression test to ensure that all the functional and non-functional requirements are tested. And then compare the implementation to the design spec and the tests and make sure it delivers.

@gkodinov gkodinov marked this pull request as draft April 6, 2026 11:20
@gkodinov
Copy link
Copy Markdown
Member

gkodinov commented Apr 6, 2026

Closing due to inactivity: lack of reaction to my review comments from 3 weeks ago. If you intend to keep working on this, please turn it back from draft to open.

@FooBarrior
Copy link
Copy Markdown
Contributor

@gkodinov,

the hash should not be filled in only at init time. It should also be filled in when status variables array version is increased (e.g. load a plugin etc).

The thing is it's only practical to populate the hash from non-plugin vars: plugin vars are always array or func types, which are not supported by hash.

But we agreed with @dmitry416 that we'll do it in add_status_var, just for wholesomeness of the solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements.

Development

Successfully merging this pull request may close these issues.

4 participants