zstd: Update zstd to 1.5.6 for cmake 4 compatability#65915
zstd: Update zstd to 1.5.6 for cmake 4 compatability#65915SrinivasaBharath merged 2 commits intoceph:mainfrom
Conversation
d7e1a48 to
9df8408
Compare
|
jenkins test make check arm64 |
|
jenkins test make check arm64 |
cbodley
left a comment
There was a problem hiding this comment.
the rgw suite has coverage of all compressor plugins, we can use that to validate
There was a problem hiding this comment.
i'd suggest split this change into two commits.
- the first commit migrates from the undeprecated APIs.
- and the second one bumps up the submodule.
in the commit message of the first commit, we'd better explain why the API was deprecated by referencing the related release notes, and explain the new APIs are available by the minimal supported zstd version.
also, would be better to
- take this opportunity to use
ZSTD_createCCtx()in favor ofZSTD_createCStream(). - use
ZSTD_CCtx_reset(cctx, ZSTD_reset_session_and_parameters)instead ofZSTD_CCtx_reset()andZSTD_CCtx_refCDict() - check the return value of these API calls.
9df8408 to
7546375
Compare
|
I broke this change into two commits as requested |
7546375 to
a233639
Compare
tchaikov
left a comment
There was a problem hiding this comment.
lgtm with a nit. but it's not a blocker.
| int compress(const ceph::buffer::list &src, ceph::buffer::list &dst, std::optional<int32_t> &compressor_message) override { | ||
| ZSTD_CStream *s = ZSTD_createCStream(); | ||
| ZSTD_initCStream_srcSize(s, cct->_conf->compressor_zstd_level, src.length()); | ||
| ZSTD_CCtx *s = ZSTD_createCCtx(); |
There was a problem hiding this comment.
nit. could use unique_ptr with a customized deleter.
a233639 to
d8a10d3
Compare
|
jenkins test make check |
Update ZstdCompressor to use the modern ZSTD compression context API and improve error handling: - Replace deprecated ZSTD_createCStream() with ZSTD_createCCtx() - Use ZSTD_CCtx_reset() with ZSTD_reset_session_and_parameters flag instead of separate ZSTD_reset_session_only and ZSTD_CCtx_refCDict() calls. This consolidates session and parameter reset into a single operation. - Add proper return value checking for all ZSTD API calls using ZSTD_isError() to catch compression failures - Ensure proper cleanup with ZSTD_freeCCtx() on all error paths to prevent memory leaks - Update corresponding free function from ZSTD_freeCStream() to ZSTD_freeCCtx() These changes align with ZSTD's recommended API usage patterns and improve robustness by properly handling potential compression errors. Fixes: https://tracker.ceph.com/issues/73522 Signed-off-by: Edwin Rodriguez <edwin.rodriguez1@ibm.com>
Update zstd submodule to version 1.5.6 for improved performance, bug fixes, and modern API support. Fixes: https://tracker.ceph.com/issues/73522 Signed-off-by: Edwin Rodriguez <edwin.rodriguez1@ibm.com>
d8a10d3 to
7795b1c
Compare
|
jenkins test make check |
|
How do I re-submit this change to qa for testing? |
|
@edwinzrodriguez i don't think it's been picked up for integration tests after checking the comments and labels in this PR. why do you think it has been tested in lab? |
|
This is an automated message by src/script/redmine-upkeep.py. I found one or more
The referenced tickets are: Those tickets do not reference this merged Pull Request. If this Pull Request merge resolves any of those tickets, please update the "Pull Request ID" field on each ticket. A future run of this script will appropriately update them. Update Log: https://github.com/ceph/ceph/actions/runs/19524553816 |
cmake 4 drops support for cmake_minimum_required < 3.5 This update bumps zstd to support newer cmake version
Fixes: https://tracker.ceph.com/issues/73522
Contribution Guidelines
To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an
xbetween the brackets:[x]. Spaces and capitalization matter when checking off items this way.Checklist
Show available Jenkins commands
jenkins test classic perfJenkins Job | Jenkins Job Definitionjenkins test crimson perfJenkins Job | Jenkins Job Definitionjenkins test signedJenkins Job | Jenkins Job Definitionjenkins test make checkJenkins Job | Jenkins Job Definitionjenkins test make check arm64Jenkins Job | Jenkins Job Definitionjenkins test submodulesJenkins Job | Jenkins Job Definitionjenkins test dashboardJenkins Job | Jenkins Job Definitionjenkins test dashboard cephadmJenkins Job | Jenkins Job Definitionjenkins test apiJenkins Job | Jenkins Job Definitionjenkins test docsReadTheDocs | Github Workflow Definitionjenkins test ceph-volume allJenkins Jobs | Jenkins Jobs Definitionjenkins test windowsJenkins Job | Jenkins Job Definitionjenkins test rook e2eJenkins Job | Jenkins Job DefinitionYou must only issue one Jenkins command per-comment. Jenkins does not understand
comments with more than one command.