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

fix: fix defrag stats #1740

Merged
merged 3 commits into from
Aug 27, 2023
Merged

fix: fix defrag stats #1740

merged 3 commits into from
Aug 27, 2023

Conversation

dranikpg
Copy link
Contributor

@dranikpg dranikpg commented Aug 25, 2023

Fixes a bug from the hash defrag PR: Successful re-allocations are not reported back.

Should we count a hash re-allocation as one (currently doing so) or also count every internal re-allocation (each StringMap key, etc.) ?

Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
kostasrim
kostasrim previously approved these changes Aug 25, 2023
src/core/compact_object.cc Outdated Show resolved Hide resolved
@kostasrim
Copy link
Contributor

Should we count a hash re-allocation as one (currently doing so)

What is being done on other data structures?

@dranikpg
Copy link
Contributor Author

What is being done on other data structures?

Nothing, this is a follow up on #1727

Co-authored-by: Kostas Kyrimis  <kostaskyrim@gmail.com>
Signed-off-by: Vladislav <vladislav.oleshko@gmail.com>
@@ -634,7 +634,7 @@ TEST_F(CompactObjectTest, DefragHash) {

// Trigger re-allocation
cobj_.InitRobj(OBJ_HASH, kEncodingListPack, target_lp);
cobj_.DefragIfNeeded(0.8);
CHECK(cobj_.DefragIfNeeded(0.8));
Copy link
Collaborator

Choose a reason for hiding this comment

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

assert_true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true

Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
@dranikpg dranikpg requested a review from romange August 25, 2023 10:55
Copy link
Contributor

@royjacobson royjacobson left a comment

Choose a reason for hiding this comment

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

LGTM

@dranikpg dranikpg merged commit 75fdc16 into dragonflydb:main Aug 27, 2023
7 checks passed
kostasrim pushed a commit that referenced this pull request Aug 29, 2023
* fix: fix defrag stats

Signed-off-by: Vladislav <vladislav.oleshko@gmail.com>
@dranikpg dranikpg deleted the fix-defrag-stats branch September 15, 2023 06:37
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.

4 participants