Skip to content

Add support to defrag ebuckets incrementally#13842

Merged
sundb merged 47 commits intoredis:unstablefrom
sundb:incrementail_defrag_ebuckets
May 18, 2025
Merged

Add support to defrag ebuckets incrementally#13842
sundb merged 47 commits intoredis:unstablefrom
sundb:incrementail_defrag_ebuckets

Conversation

@sundb
Copy link
Copy Markdown
Collaborator

@sundb sundb commented Mar 4, 2025

In PR #13229, we introduced the ebucket for HFE.
Before this PR, when updating eitems stored in ebuckets, the lack of incremental fragmentation support for non-kvstore data structures (until PR #13814) meant that we had to reverse lookup the position of the eitem in the ebucket and then perform the update.
This approach was inefficient as it often required frequent traversals of the segment list to locate and update the item.

To address this issue, in this PR, This PR implements incremental fragmentation for hash dict ebuckets and server.hexpires.
By incrementally defrag the ebuckets, we also perform defragmentation for the associated items, eliminates the need for frequent traversals of the segment list for defragging the eitem.

@sundb sundb marked this pull request as ready for review March 6, 2025 03:08
sundb and others added 4 commits March 12, 2025 14:55
Co-authored-by: Moti Cohen <moticless@gmail.com>
Co-authored-by: Moti Cohen <moticless@gmail.com>
sundb and others added 2 commits March 12, 2025 17:39
Co-authored-by: Moti Cohen <moticless@gmail.com>
@sundb sundb requested a review from moticless May 13, 2025 07:11
Copy link
Copy Markdown
Collaborator

@moticless moticless left a comment

Choose a reason for hiding this comment

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

LGTM

sundb and others added 2 commits May 14, 2025 21:37
Co-authored-by: Moti Cohen <moticless@gmail.com>
Co-authored-by: Moti Cohen <moticless@gmail.com>
@snyk-io
Copy link
Copy Markdown

snyk-io bot commented May 14, 2025

🎉 Snyk checks have passed. No issues have been found so far.

security/snyk check is complete. No issues have been found. (View Details)

license/snyk check is complete. No issues have been found. (View Details)

@sundb sundb requested a review from Copilot May 16, 2025 01:45
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements incremental defragmentation for hash dict ebuckets and server.hexpires to improve defragmentation efficiency and reduce costly segment traversals. Key changes include:

  • Updates to testing in tests/unit/memefficiency.tcl with revised assertions and new configurations.
  • Refactoring of defrag APIs in src/rax., src/module.c, and src/ebuckets. to include an additional privdata parameter.
  • Extensive modifications in src/defrag.c to add new defrag stages, including support for hexpires and hash fields with TTL.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/unit/memefficiency.tcl Updated test procedures and expected results for defrag incremental support.
src/rax.h, src/rax.c Updated callback signature to include privdata for defrag node callbacks.
src/module.c Adjusted module defrag callbacks to account for the new signature.
src/ebuckets.h, src/ebuckets.c Introduced new defrag APIs (e.g., ebScanDefrag) and refactored list/rax defrag.
src/defrag.c Revised defrag strategies for hash objects, added hexpires defrag stages, and refined callbacks.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@sundb sundb added the state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten label May 17, 2025
@sundb sundb merged commit 5d0d64b into redis:unstable May 18, 2025
18 checks passed
@github-project-automation github-project-automation bot moved this from Todo to Done in Redis 8.2 May 18, 2025
@sundb sundb removed the state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten label May 18, 2025
@sundb sundb deleted the incrementail_defrag_ebuckets branch August 7, 2025 02:08
sundb added a commit that referenced this pull request Sep 8, 2025
This PR fixes three defrag issues.

1. Fix the issue that forget to update cgroup_ref_node when the consume
group was reallocated.
This crash was introduced by #14130
In this PR, when performing defragmentation on `s->cgroups` using
`defragRadixTree()`, we no longer rely on the automatic data
defragmentation of `defragRadixTree()`. Instead, we manually defragment
the consumer group and then update its reference in `s->cgroups`.

2. Fix a use-after-free issue caused by updating dictionary keys after
HFE key is reallocated.
This issue was introduced by #13842

3. Fix the issue that forgot to be updated NextSegHdr->firstSeg when the
first segment was reallocated.
This issue was introduced by #13842

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
sundb added a commit that referenced this pull request Sep 11, 2025
From the malloc-stats reports of both failures and successes, we can see
that the additional fragments mainly come from bin24.
By analyzing the fragments mainly from the entries of the dict, since
`large_ebrax` test uses a dictionary with 1600 elements, it will move a
large number of entries during the rehashing process, and we will not
perform defragmentation on the dict entries.

In #13842 we changed to use two dicts
alternately to generate frag. Normally, the entries should also
alternate, but rehashing disrupted this, which resulted in bin24 frag
that can't be defragged.

## Solution
In this PR, the length of a single dictionary was reduced from 1600 to
500 to avoid excessive rehashing, and the threshold was also lowered.

---------

Co-authored-by: oranagra <oran@redislabs.com>
YaacovHazan pushed a commit to YaacovHazan/redis that referenced this pull request Sep 28, 2025
This PR fixes three defrag issues.

1. Fix the issue that forget to update cgroup_ref_node when the consume
group was reallocated.
This crash was introduced by redis#14130
In this PR, when performing defragmentation on `s->cgroups` using
`defragRadixTree()`, we no longer rely on the automatic data
defragmentation of `defragRadixTree()`. Instead, we manually defragment
the consumer group and then update its reference in `s->cgroups`.

2. Fix a use-after-free issue caused by updating dictionary keys after
HFE key is reallocated.
This issue was introduced by redis#13842

3. Fix the issue that forgot to be updated NextSegHdr->firstSeg when the
first segment was reallocated.
This issue was introduced by redis#13842

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants