Skip to content

Change lzf to handle values larger than UINT32_MAX#9776

Merged
oranagra merged 3 commits intoredis:unstablefrom
sundb:fix-lzf-4gb
Nov 16, 2021
Merged

Change lzf to handle values larger than UINT32_MAX#9776
oranagra merged 3 commits intoredis:unstablefrom
sundb:fix-lzf-4gb

Conversation

@sundb
Copy link
Collaborator

@sundb sundb commented Nov 13, 2021

Describe of feature

Redis supports inserting data over 4GB into string (and recently for lists too, see #9357), But LZF compression used in RDB files (see rdbcompression config), and in quicklist (see list-compress-depth config) does not support compress/decompress data over UINT32_MAX, which will result in corrupting the rdb after compression.

Internal changes

  1. Modify the unsigned int parameter of lzf_compress/lzf_decompress to size_t.
  2. Modify the variable types in lzf_compress involving offsets and lengths to size_t.
  3. Set LZF_USE_OFFSETS to 0.
    When LZF_USE_OFFSETS is 1, lzf store offset into LZF_HSLOT(32bit).
    Even in 64-bit, LZF_USE_OFFSETS defaults to 1, because lzf assumes that it only compresses and decompresses data smaller than UINT32_MAX.
    But now we need to make lzf support 64-bit, turning on LZF_USE_OFFSETS will make it impossible to store 64-bit
    offsets or pointers.
    BTW, disable LZF_USE_OFFSETS also brings a few performance improvements.

Test

  1. Add test for compress/decompress string large than UINT32_MAX.
  2. Add unittest for compress/decompress quicklistNode.

Implements #9732.

@oranagra oranagra linked an issue Nov 14, 2021 that may be closed by this pull request
oranagra pushed a commit that referenced this pull request Nov 16, 2021
…ore than 100mb (#9784)

This is a preparation step in order to add a new test in quicklist.c see #9776
Copy link
Member

@oranagra oranagra left a comment

Choose a reason for hiding this comment

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

LGTM, minor comment.

@oranagra oranagra added the release-notes indication that this issue needs to be mentioned in the release notes label Nov 16, 2021
@sundb sundb changed the title WIP: Change lzf to handle values larger than UINT32_MAX Change lzf to handle values larger than UINT32_MAX Nov 16, 2021
@oranagra oranagra merged commit 985430b into redis:unstable Nov 16, 2021
@sundb sundb deleted the fix-lzf-4gb branch November 16, 2021 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes indication that this issue needs to be mentioned in the release notes

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Change lzf_compress to handle values larger than 4GB.

2 participants