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

Require ZSTD >= 1.4.0, greatly simplifying related code #13362

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pdillinger
Copy link
Contributor

@pdillinger pdillinger commented Jan 31, 2025

Summary: Leading up to some compression code refactoring, we have a bit of an ifdef nightmare in compression.h relating to zstd support. With the major release RocksDB 10.0.0 coming up, it is a good time to clean up much of this tech debt by requiring zstd >= 1.4.0 (April 2019) if building RocksDB with ZSTD support. For example, Ubuntu 20, the first LTS version to properly support C++17 in its built-in gcc, comes with zstd version 1.4.4. This should not be a significant limitation.

  • Almost all of the ZSTD_VERSION_NUMBER checks are simplified to just ZSTD, though
    • ROCKSDB_ZSTD_DDICT still needs to be separate because of dependency on ZSTD_STATIC_LINKING_ONLY (added to fbcode_config_platform010.sh by the way)
    • Similar for ZDICT_finalizeDictionary, which is only generally available in >= 1.4.5
  • Eliminate deprecated kZSTDNotFinalCompression
  • Reduce some cases of unnecessary copying definitions across #if branches (e.g. ZSTDUncompressCachedData)

Test Plan: minor unit test updates. make check on several build variants with/without zstd and with/without ZSTD_STATIC_LINKING_ONLY

Summary: Leading up to some compression code refactoring, we have a bit
of an ifdef nightmare in compression.h relating to zstd support. With
the major release RocksDB 10.0.0 coming up, it is a good time to clean
up much of this tech debt by requiring zstd >= 1.4.0 (April 2019) if
building RocksDB with ZSTD support.
* Most of the `ZSTD_VERSION_NUMBER` checks are simplified to just
  `ZSTD`, though
  * `ROCKSDB_ZSTD_DDICT` still needs to be separate because of
    dependency on `ZSTD_STATIC_LINKING_ONLY` (added to
    fbcode_config_platform010.sh by the way)
* Eliminate deprecated `kZSTDNotFinalCompression`
* Reduce some cases of unnecessary copying definitions across `#if`
  branches (e.g. `ZSTDUncompressCachedData`)

Test Plan: minor unit test updates. `make check` on several build
variants with/without zstd and with/without `ZSTD_STATIC_LINKING_ONLY`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants