From 3e7b2b56e00d9c9a5e3f1ace5f62b4e5897c68a8 Mon Sep 17 00:00:00 2001 From: Alexey Bashtanov Date: Tue, 6 Jan 2026 17:09:37 +0000 Subject: [PATCH 1/2] cloud_storange: base all serde classes on serde::envelope To remove serde dependency on refelction in future --- src/v/cloud_storage/segment_meta_cstore.cc | 2 +- .../tests/partition_manifest_test.cc | 16 +++++-------- src/v/cloud_storage/types.h | 24 +++++++++++++++---- 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/src/v/cloud_storage/segment_meta_cstore.cc b/src/v/cloud_storage/segment_meta_cstore.cc index a4ddd73eda089..2a7b8d91fea16 100644 --- a/src/v/cloud_storage/segment_meta_cstore.cc +++ b/src/v/cloud_storage/segment_meta_cstore.cc @@ -221,7 +221,7 @@ class column_store }; static_assert( - reflection::arity() + std::tuple_size_v().serde_fields())> == std::tuple_size_v, "segment_meta has a field that is not in segment_meta_accessors. check " "also that the members of column_store match the members of " diff --git a/src/v/cloud_storage/tests/partition_manifest_test.cc b/src/v/cloud_storage/tests/partition_manifest_test.cc index 1a136ff140ff0..bb2f7ca3acce9 100644 --- a/src/v/cloud_storage/tests/partition_manifest_test.cc +++ b/src/v/cloud_storage/tests/partition_manifest_test.cc @@ -1544,11 +1544,9 @@ struct metadata_stm_segment }; namespace old { -struct segment_meta_v0 { - using value_t = segment_meta_v0; - static constexpr serde::version_t redpanda_serde_version = 0; - static constexpr serde::version_t redpanda_serde_compat_version = 0; - +struct segment_meta_v0 + : serde:: + envelope, serde::compat_version<0>> { bool is_compacted; size_t size_bytes; model::offset base_offset; @@ -1570,11 +1568,9 @@ struct segment_meta_v0 { auto operator<=>(const segment_meta_v0&) const = default; }; -struct segment_meta_v1 { - using value_t = segment_meta_v1; - static constexpr serde::version_t redpanda_serde_version = 1; - static constexpr serde::version_t redpanda_serde_compat_version = 0; - +struct segment_meta_v1 + : serde:: + envelope, serde::compat_version<0>> { bool is_compacted; size_t size_bytes; model::offset base_offset; diff --git a/src/v/cloud_storage/types.h b/src/v/cloud_storage/types.h index 2299d3249f14d..248b3b11ad19c 100644 --- a/src/v/cloud_storage/types.h +++ b/src/v/cloud_storage/types.h @@ -85,11 +85,8 @@ struct offset_range { model::offset end_rp; }; -struct segment_meta { - using value_t = segment_meta; - static constexpr serde::version_t redpanda_serde_version = 3; - static constexpr serde::version_t redpanda_serde_compat_version = 0; - +struct segment_meta + : serde::envelope, serde::compat_version<0>> { bool is_compacted; size_t size_bytes; model::offset base_offset; @@ -111,6 +108,23 @@ struct segment_meta { /// Size of the metadata (optional) uint64_t metadata_size_hint{0}; + auto serde_fields() { + return std::tie( + is_compacted, + size_bytes, + base_offset, + committed_offset, + base_timestamp, + max_timestamp, + delta_offset, + ntp_revision, + archiver_term, + segment_term, + delta_offset_end, + sname_format, + metadata_size_hint); + } + kafka::offset base_kafka_offset() const { // Manifests created with the old version of redpanda won't have the // delta_offset field. In this case the value will be initialized to From 1177b3dc766c495997cd713000afb7508eab5fa9 Mon Sep 17 00:00:00 2001 From: Alexey Bashtanov Date: Tue, 6 Jan 2026 17:18:41 +0000 Subject: [PATCH 2/2] serde: do not use reflection, require classes to inherit envelope There is support for classes that can be written/read by serde, but do not depend on serde::envelope. These still need version and compat_version static members, but when read/written in automatic mode instead of serde_field() rely on reflection. Remove such support. --- src/v/serde/BUILD | 2 - src/v/serde/envelope.h | 8 +--- src/v/serde/envelope_for_each_field.h | 21 ++------- src/v/serde/test/fuzz.cc | 7 ++- src/v/serde/test/serde_test.cc | 64 +++++++++------------------ 5 files changed, 29 insertions(+), 73 deletions(-) diff --git a/src/v/serde/BUILD b/src/v/serde/BUILD index 6196994e6b45e..d7d0e0adca3e3 100644 --- a/src/v/serde/BUILD +++ b/src/v/serde/BUILD @@ -24,8 +24,6 @@ redpanda_cc_library( deps = [ "//src/v/bytes:iobuf_parser", "//src/v/hashing:crc32c", - "//src/v/reflection:arity", - "//src/v/reflection:to_tuple", "//src/v/ssx:sformat", ], ) diff --git a/src/v/serde/envelope.h b/src/v/serde/envelope.h index 2639cd91301e5..2c14e1f4b5077 100644 --- a/src/v/serde/envelope.h +++ b/src/v/serde/envelope.h @@ -98,16 +98,10 @@ inline constexpr std::size_t checksum_envelope_header_size = envelope_header_size + 4; template -concept is_envelope = requires { - { T::redpanda_serde_version } -> std::same_as; - { T::redpanda_serde_compat_version } -> std::same_as; -}; +concept is_envelope = T::redpanda_inherits_from_envelope; template concept is_checksum_envelope = is_envelope && T::redpanda_serde_build_checksum; -template -concept inherits_from_envelope = T::redpanda_inherits_from_envelope; - } // namespace serde diff --git a/src/v/serde/envelope_for_each_field.h b/src/v/serde/envelope_for_each_field.h index 919b535a7c25c..237e48bcaadd9 100644 --- a/src/v/serde/envelope_for_each_field.h +++ b/src/v/serde/envelope_for_each_field.h @@ -9,8 +9,6 @@ #pragma once -#include "reflection/arity.h" -#include "reflection/to_tuple.h" #include "serde/envelope.h" #include @@ -29,26 +27,13 @@ concept check_for_more_fn = requires(Fn&& fn, int& f) { template inline auto envelope_for_each_field(T& t, Fn&& fn) { - if constexpr (inherits_from_envelope>) { - std::apply( - [&](auto&&... args) { (fn(args), ...); }, envelope_to_tuple(t)); - } else { - std::apply( - [&](auto&&... args) { (fn(args), ...); }, reflection::to_tuple(t)); - } + std::apply([&](auto&&... args) { (fn(args), ...); }, envelope_to_tuple(t)); } template inline auto envelope_for_each_field(T& t, Fn&& fn) { - if constexpr (inherits_from_envelope>) { - std::apply( - [&](auto&&... args) { (void)(fn(args) && ...); }, - envelope_to_tuple(t)); - } else { - std::apply( - [&](auto&&... args) { (void)(fn(args) && ...); }, - reflection::to_tuple(t)); - } + std::apply( + [&](auto&&... args) { (void)(fn(args) && ...); }, envelope_to_tuple(t)); } } // namespace serde diff --git a/src/v/serde/test/fuzz.cc b/src/v/serde/test/fuzz.cc index b9ced0527db5c..a878409bfc3bb 100644 --- a/src/v/serde/test/fuzz.cc +++ b/src/v/serde/test/fuzz.cc @@ -18,13 +18,16 @@ bool eq( return ((std::get(a) == std::get(b)) && ...); } +template +constexpr size_t arity + = std::tuple_size_v()))>; + template bool operator==(T1 const& a, T2 const& b) { return eq( envelope_to_tuple(a), envelope_to_tuple(b), - std::make_index_sequence() - 1, reflection::arity() - 1)>()); + std::make_index_sequence, arity)>()); } struct data_gen { diff --git a/src/v/serde/test/serde_test.cc b/src/v/serde/test/serde_test.cc index 48cc1e22f6b1e..2ead4e1ff7976 100644 --- a/src/v/serde/test/serde_test.cc +++ b/src/v/serde/test/serde_test.cc @@ -117,23 +117,9 @@ struct test_msg1_new auto serde_fields() { return std::tie(_a, _m, _b, _c); } }; -struct test_msg1_new_manual { - using value_t = test_msg1_new_manual; - static constexpr auto redpanda_serde_version = serde::version_t{10}; - static constexpr auto redpanda_serde_compat_version = serde::version_t{5}; - - bool operator==(const test_msg1_new_manual&) const = default; - - int _a; - test_msg0 _m; - int _b, _c; -}; - struct not_an_envelope {}; static_assert(!serde::is_envelope); static_assert(serde::is_envelope); -static_assert(serde::inherits_from_envelope); -static_assert(!serde::inherits_from_envelope); static_assert(test_msg1::redpanda_serde_version == 4); static_assert(test_msg1::redpanda_serde_compat_version == 0); @@ -143,16 +129,6 @@ SEASTAR_THREAD_TEST_CASE(incompatible_version_throws) { serde::serde_exception); } -SEASTAR_THREAD_TEST_CASE(manual_and_envelope_equal) { - const auto roundtrip = serde::from_iobuf( - serde::to_iobuf( - test_msg1_new{ - ._a = 77, ._m = test_msg0{._i = 2, ._j = 3}, ._b = 88, ._c = 99})); - const auto check = test_msg1_new_manual{ - ._a = 77, ._m = test_msg0{._i = 2, ._j = 3}, ._b = 88, ._c = 99}; - BOOST_CHECK(roundtrip == check); -} - SEASTAR_THREAD_TEST_CASE(reserve_test) { auto b = iobuf(); auto p = b.reserve(10); @@ -648,17 +624,17 @@ SEASTAR_THREAD_TEST_CASE(serde_checksum_envelope_test) { serde::compat_version<2>> { bool operator==(const checksummed&) const = default; - std::vector data_; + std::vector data_; auto serde_fields() { return std::tie(data_); } }; const auto obj = checksummed{ .data_ = { - test_msg1_new_manual{ + test_msg1_new{ ._a = 1, ._m = test_msg0{._i = 33, ._j = 44}, ._b = 2, ._c = 3}, - test_msg1_new_manual{ + test_msg1_new{ ._a = 4, ._m = test_msg0{._i = 55, ._j = 66}, ._b = 5, ._c = 6}, - test_msg1_new_manual{ + test_msg1_new{ ._a = 7, ._m = test_msg0{._i = 77, ._j = 88}, ._b = 8, ._c = 9}}}; const auto vec = std::vector{obj, obj}; BOOST_CHECK( @@ -670,7 +646,7 @@ struct old_no_cs envelope, serde::compat_version<2>> { bool operator==(const old_no_cs&) const = default; - std::vector data_; + std::vector data_; auto serde_fields() { return std::tie(data_); } }; struct new_cs @@ -696,27 +672,27 @@ struct new_cs bool operator==(const new_cs&) const = default; - std::vector data_; + std::vector data_; auto serde_fields() { return std::tie(data_); } }; SEASTAR_THREAD_TEST_CASE(serde_checksum_update) { const auto old_obj = old_no_cs{ .data_ = { - test_msg1_new_manual{ + test_msg1_new{ ._a = 1, ._m = test_msg0{._i = 33, ._j = 44}, ._b = 2, ._c = 3}, - test_msg1_new_manual{ + test_msg1_new{ ._a = 4, ._m = test_msg0{._i = 55, ._j = 66}, ._b = 5, ._c = 6}, - test_msg1_new_manual{ + test_msg1_new{ ._a = 7, ._m = test_msg0{._i = 77, ._j = 88}, ._b = 8, ._c = 9}}}; const auto new_obj = new_cs{ .data_ = { - test_msg1_new_manual{ + test_msg1_new{ ._a = 1, ._m = test_msg0{._i = 33, ._j = 44}, ._b = 2, ._c = 3}, - test_msg1_new_manual{ + test_msg1_new{ ._a = 4, ._m = test_msg0{._i = 55, ._j = 66}, ._b = 5, ._c = 6}, - test_msg1_new_manual{ + test_msg1_new{ ._a = 7, ._m = test_msg0{._i = 77, ._j = 88}, ._b = 8, ._c = 9}}}; const auto old_vec = std::vector{old_obj, old_obj}; @@ -731,7 +707,7 @@ struct old_cs checksum_envelope, serde::compat_version<2>> { bool operator==(const old_cs&) const = default; - std::vector data_; + std::vector data_; auto serde_fields() { return std::tie(data_); } }; struct new_no_cs @@ -742,7 +718,7 @@ struct new_no_cs } serde::checksum_t unchecked_dummy_checksum_{0U}; - std::vector data_; + std::vector data_; auto serde_fields() { return std::tie(unchecked_dummy_checksum_, data_); } }; @@ -750,20 +726,20 @@ struct new_no_cs SEASTAR_THREAD_TEST_CASE(serde_checksum_update_1) { const auto old_obj = old_cs{ .data_ = { - test_msg1_new_manual{ + test_msg1_new{ ._a = 1, ._m = test_msg0{._i = 33, ._j = 44}, ._b = 2, ._c = 3}, - test_msg1_new_manual{ + test_msg1_new{ ._a = 4, ._m = test_msg0{._i = 55, ._j = 66}, ._b = 5, ._c = 6}, - test_msg1_new_manual{ + test_msg1_new{ ._a = 7, ._m = test_msg0{._i = 77, ._j = 88}, ._b = 8, ._c = 9}}}; const auto new_obj = new_no_cs{ .data_ = { - test_msg1_new_manual{ + test_msg1_new{ ._a = 1, ._m = test_msg0{._i = 33, ._j = 44}, ._b = 2, ._c = 3}, - test_msg1_new_manual{ + test_msg1_new{ ._a = 4, ._m = test_msg0{._i = 55, ._j = 66}, ._b = 5, ._c = 6}, - test_msg1_new_manual{ + test_msg1_new{ ._a = 7, ._m = test_msg0{._i = 77, ._j = 88}, ._b = 8, ._c = 9}}}; const auto old_vec = std::vector{old_obj, old_obj};