c/partition_recovery_manager: avoid oversized allocations when logging #24476
Conversation
| auto ostr = make_iobuf_ref_output_stream(buf); | ||
| co_await mat.partition_manifest.serialize_json(ostr); | ||
| auto istr = make_iobuf_input_stream(std::move(buf)); | ||
| auto truncated = co_await istr.read_exactly( |
There was a problem hiding this comment.
what happens if you don't truncate?
arbitrary truncation looks odd especially if we ever come to rely on this information for debugging; if it is useless (and we are fine dropping it arbitrarily) then why are we printing it at all?
There was a problem hiding this comment.
what happens if you don't truncate?
- It's needs additional effort to log without oversized allocations (although it absolutely is possible)
- Potentially it also bloats the logs
My reasoning was that if something is large enough to cause an oversized allocation (128Kb) it may also significantly bloat logs as there may be thousands of partitions to recover.
As for how useful this log line is, that's why I would like @Lazin's opinion, maybe he remembers.
There was a problem hiding this comment.
dump 128kb to a single line is intense. surely there is a better solution than arbitrary truncation and dumping everything.
|
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/59388#01939d08-1d89-47d0-9e4f-c10e0067c37e |
Do not dump partition manifest JSON as it can be very large
bashtanov
force-pushed
the
avoid-oversized-allocation-when-logging
branch
from
5bd49fb to
28ec1df
Compare
Partition manifest logging was removed in redpanda-data#24476 to avoid oversized allocations and very long log lines. This is to re-add it, but without segments data.
Partition manifest logging was removed in redpanda-data#24476 to avoid oversized allocations and very long log lines. This is to re-add it, with segment data limited to last segment only.
https://redpandadata.atlassian.net/browse/CORE-8238
Do not dump the manifest JSON as it may be very large.
Backports Required
Release Notes