Skip to content

Commit

Permalink
compression: Allocate memory for compressEnd
Browse files Browse the repository at this point in the history
Before this change the code assumed that the destination temp. buffer
always had enough free space for LZ4F_compressEnd operation. This change
checks the free size before calling LZ4F_compressEnd and allocates a new
buffer if necessary.

Note that the free space required is computed using LZ4F_compressBound
with a srcSize of 0. By looking at LZ4F_compressEnd it may appear that
we can just check if 12 bytes are free in the destination buffer, whereas
LZ4F_compressBound always assumes at least 65443 ie 65535 + block header
size (4) + frame footer (4).

However, lz4frame docstring for LZ4F_compressBound states that:

"When srcSize==0, LZ4F_compressBound() provides an upper bound for
LZ4F_flush() and LZ4F_compressEnd() instead."

Similar directions are mentioned on LZ4 gh issues, so it is safer to be
conservative and possibly allocate the extra bytes.

(cherry picked from commit 1493c6d)
  • Loading branch information
abhijat authored and vbotbuildovich committed Mar 18, 2024
1 parent ab88ad0 commit 541d1ef
Showing 1 changed file with 10 additions and 0 deletions.
10 changes: 10 additions & 0 deletions src/v/compression/internal/lz4_frame_compressor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,16 @@ iobuf lz4_frame_compressor::compress(const iobuf& b) {
output_cursor += code;
}

if (const auto sz_for_compress_end = LZ4F_compressBound(0, &prefs);
output_sz - output_cursor < sz_for_compress_end) {
obuf.trim(output_cursor);
ret.append(std::move(obuf));
obuf = ss::temporary_buffer<char>(sz_for_compress_end);
output = obuf.get_write();
output_sz = obuf.size();
output_cursor = 0;
}

code = LZ4F_compressEnd(
ctx, output + output_cursor, output_sz - output_cursor, nullptr);
check_lz4_error("lz4f_compressend:{}", code);
Expand Down

0 comments on commit 541d1ef

Please sign in to comment.