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.
  • Loading branch information
abhijat committed Mar 18, 2024
1 parent 0ecbec8 commit 1493c6d
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 1493c6d

Please sign in to comment.