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

Adding bloom defrag hits and misses to info, added check these metrics get incremented in defrag test #33

Merged
merged 1 commit into from
Dec 13, 2024

Conversation

zackcam
Copy link
Contributor

@zackcam zackcam commented Dec 11, 2024

Added a new section in info specifically for defrag. Added two new fields in this section of defrag hits and misses that are incremented in bloom_callback.
Added a check in defrag test to make sure these metrics are actually incremented

@@ -101,3 +103,17 @@ def check_values_present(self, scale_names, num_items_inserted_per_object):
# All items should be present so we compare with an array of length num items where all items are 1
expected_results = [1] * num_items
assert results == expected_results, f"Unexpected results for scale {scale}: {results}"

def verfiy_bloom_defrag_metrics(self):
Copy link
Member

@KarthikSubbarao KarthikSubbarao Dec 12, 2024

Choose a reason for hiding this comment

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

One suggestion here.

Copy link
Member

@KarthikSubbarao KarthikSubbarao Dec 12, 2024

Choose a reason for hiding this comment

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

Actually, can we simply use this instead?

    def info_obj(self):
        """Return a ValkeyInfo object for the current client info"""
        return ValkeyInfo(self.info('all'))

You can add a custom function to accept a custom section name (MODULE, BF etc) and return the result. The caller site can access what they want from

self.info[<custom-metric>]

Copy link
Contributor

@YueTang-Vanessa YueTang-Vanessa left a comment

Choose a reason for hiding this comment

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

LGTM

…s get incremented in defrag test

Signed-off-by: zackcam <[email protected]>
@zackcam zackcam reopened this Dec 12, 2024
@KarthikSubbarao KarthikSubbarao merged commit 3589d30 into valkey-io:unstable Dec 13, 2024
11 checks passed
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.

3 participants