Issue 6614 - CLI - Error when trying to display global DB stats with LMDB #6622
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…LMDB Bug description: Displaying global monitor stats fails with key error. Caused by BDB backend keys being used when MDB is the configured DB implementation. Fix description: Ensure backend and monitor keys match the configured DB implementation. Fixes: 389ds#6614 Reviewed by:
droideck
requested changes
Mar 4, 2025
| dbpages = int(ldbm_mon['nsslapd-db-pages-in-use'][0]) | ||
| dbcachefree = max(int(dbcachesize - (pagesize * dbpages)), 0) | ||
| dbcachefreeratio = dbcachefree/dbcachesize | ||
| if ldbm_monitor.inst_db_impl == DB_IMPL_BDB : |
| # Check for library specific attributes | ||
| if db_lib == 'bdb': | ||
| assert 'dbcachehits' in monitor | ||
| assert 'nsslapd-db-configured-locks' in monitor | ||
| elif db_lib == 'mdb': | ||
| pass | ||
| assert not 'dbcachehits' in monitor |
There was a problem hiding this comment.
assert 'dbcachehits' not in monitor is a preferred python style, IIRC (we might run a linter some time soon, so better to catch issues early)
src/lib389/lib389/monitor.py
Outdated
Comment on lines
121
to
134
| db_keys = { | ||
| DB_IMPL_BDB: [ | ||
| 'dbcachehits', 'dbcachetries', 'dbcachehitratio', | ||
| 'dbcachepagein', 'dbcachepageout', 'dbcacheroevict', | ||
| 'dbcacherwevict' | ||
| ], | ||
| DB_IMPL_MDB: [ | ||
| 'normalizeddncachetries', 'normalizeddncachehits', | ||
| 'normalizeddncachemisses', 'normalizeddncachehitratio', | ||
| 'normalizeddncacheevictions', 'currentnormalizeddncachesize', | ||
| 'maxnormalizeddncachesize', 'currentnormalizeddncachecount', | ||
| 'normalizeddncachethreadsize', 'normalizeddncachethreadslots' | ||
| ] | ||
| } |
There was a problem hiding this comment.
I think both db_keys and db_monitor_keys have more sense as a class variables.
So something like:
class MonitorLDBM(Monitor):
DB_KEYS = {
DB_IMPL_BDB: [
'dbcachehits', 'dbcachetries', 'dbcachehitratio',
...
],
DB_IMPL_MDB: [
'normalizeddncachetries', 'normalizeddncachehits',
...
]
}
DB_MONITOR_KEYS = {
DB_IMPL_BDB: [
'nsslapd-db-abort-rate', 'nsslapd-db-active-txns', 'nsslapd-db-cache-hit',
...
],
DB_IMPL_MDB: [
'dbenvmapmaxsize', 'dbenvmapsize', 'dbenvlastpageno',
...
]
}
def __init__(self, instance, dn=None):
super(MonitorLDBM, self).__init__(instance=instance)
self._dn = DN_MONITOR_LDBM
...And then in the constructor we can create a copy for the class instance:
self._backend_keys = list(self.DB_KEYS.get(self.inst_db_impl, []))
self._db_mon_keys = list(self.DB_MONITOR_KEYS.get(self.inst_db_impl, []))which later can be extended with other conditional variables.
I don't have a strong opinion here, though. But I think it'll be cleaner and more pythonic.:)
There was a problem hiding this comment.
I agree, it is cleaner. Thanks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bug description:
Displaying global monitor stats fails with key error. Caused by BDB backend keys being used when MDB is the configured DB implementation.
Fix description:
Ensure backend and monitor keys match the configured DB implementation.
Fixes: #6614
Reviewed by: