Skip to content

Commit

Permalink
segment_meta_cstore: to_iobuf optimizations
Browse files Browse the repository at this point in the history
previously to_iobuf was achieved by constructing a temporary cstore,
appending all the segments and serializing it. this is done so that
to_iobuf can be const, in turn to be able to use it in
base_manifest::serialize.

To minimize operations and memory, this commits adds the ability to
create a column_store that shares all the iobuf with the *this object,
and then serialize it.

sharing is done by constructing on top of deltafor_encoder::share()
const a series of unsafe_alias() const methods, responsible to construct
the subobjects.

this new schema saves time by not re-encoding the segment_meta, and
saves memory by sharing the underlying iobuf objects, to write them
directly in the final serde iobuf
  • Loading branch information
andijcr committed May 17, 2023
1 parent 56ec997 commit c1ee47f
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 9 deletions.
33 changes: 24 additions & 9 deletions src/v/cloud_storage/segment_meta_cstore.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "model/fundamental.h"
#include "model/metadata.h"
#include "model/timestamp.h"
#include "serde/serde.h"
#include "utils/delta_for.h"

#include <bitset>
Expand Down Expand Up @@ -733,6 +734,16 @@ class column_store
member_fields());
}

auto unsafe_alias() const -> column_store {
auto tmp = column_store{};
details::tuple_map(
[](auto& lhs, auto const& rhs) { lhs = rhs.unsafe_alias(); },
tmp.columns(),
columns());
tmp._hints = _hints;
return tmp;
}

private:
gauge_col_t _is_compacted{};
gauge_col_t _size_bytes{};
Expand Down Expand Up @@ -959,6 +970,17 @@ class segment_meta_cstore::impl
_col = serde::read_nested<column_store>(in, h._bytes_left_limit);
}

auto to_iobuf() const {
flush_write_buffer();
auto tmp = impl{};
tmp._col = _col.unsafe_alias();
return serde::to_iobuf(std::move(tmp));
}

void from_iobuf(iobuf in) {
*this = serde::from_iobuf<impl>(std::move(in));
}

private:
void flush_write_buffer() const {
if (_write_buffer.empty()) {
Expand Down Expand Up @@ -1051,15 +1073,8 @@ void segment_meta_cstore::from_iobuf(iobuf in) {
// NOTE: this process is not optimal memory-wise, but it's simple and
// correct. It would require some rewrite on serde to accept a type& out
// parameter.
*_impl = serde::from_iobuf<segment_meta_cstore::impl>(std::move(in));
_impl->from_iobuf(std::move(in));
}

iobuf segment_meta_cstore::to_iobuf() const {
impl tmp;
for (auto s : *this) {
tmp.append(s);
}

return serde::to_iobuf(std::move(tmp));
}
iobuf segment_meta_cstore::to_iobuf() const { return _impl->to_iobuf(); }
} // namespace cloud_storage
25 changes: 25 additions & 0 deletions src/v/cloud_storage/segment_meta_cstore.h
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,20 @@ class segment_meta_column_frame

auto serde_fields() { return std::tie(_head, _tail, _size, _last_row); }

/// Returns a frame that shares the underlying iobuf
/// The method itself is not unsafe, but the returned object can modify the
/// original object, so users need to think about the use case
auto unsafe_alias() const -> self_t {
auto tmp = self_t{};
tmp._head = _head;
if (_tail.has_value()) {
tmp._tail = _tail->unsafe_alias();
}
tmp._size = _size;
tmp._last_row = _last_row;
return tmp;
}

private:
template<class PredT>
const_iterator pred_search(value_t value) const {
Expand Down Expand Up @@ -720,6 +734,17 @@ class segment_meta_column_impl
}
}

/// Returns a column that shares the underlying iobuf
/// The method itself is not unsafe, but the returned object can modify the
/// original object, so users need to think about the use case
auto unsafe_alias() const -> Derived {
auto tmp = Derived{};
for (auto& e : _frames) {
tmp._frames.push_back(e.unsafe_alias());
}
return tmp;
}

protected:
template<class PredT>
const_iterator pred_search_impl(value_t value) const {
Expand Down
16 changes: 16 additions & 0 deletions src/v/utils/delta_for.h
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,22 @@ class deltafor_encoder
}
}

/// Returns a deltafor_encoder that shares the underlying iobuf
/// The method itself is not unsafe, but the returned object can modify the
/// original object, so users need to think about the use case
auto unsafe_alias() const -> deltafor_encoder {
auto tmp = deltafor_encoder{};
tmp._initial = _initial;
tmp._last = _last;
tmp._data = share();
tmp._cnt = _cnt;

if constexpr (!use_nttp_deltastep) {
tmp._delta = _delta;
}
return tmp;
}

private:
template<typename T>
void _pack_as(const row_t& input) {
Expand Down

0 comments on commit c1ee47f

Please sign in to comment.