cluster: serde_fields for various structs

[oleiman]

In #22782, we aim to require explicit serde_fields for serde structs that would otherwise use aggregate field order. This is one in a series of pull requests that add serde_fields to existing structs, to be followed by a PR to remove the aggregate order fallback from serde itself.

The primary acceptance criterion for this PR should be "field order unchanged". Any functional change is a bug.

Backports Required

• none - not a bug fix
• none - this is a backport
• none - issue does not exist in previous branches
• none - papercut/not impactful enough to backport
• v24.2.x
• v24.1.x
• v23.3.x

Release Notes

• none

[dotnwat]

i wonder why serde doesn't encode static fields

[oleiman]

presumably you could, but I suppose it's strictly wasted bits since the value wouldn't change per instance. or maybe more straightforwardly, "the same reason statics don't appear in structured bindings".

[dotnwat]

oh yeh i think it makes sense that they do not appear in the encodings!

[oleiman]

force push to fix merge conflict

[vbotbuildovich]

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/53784#01919f3c-809c-4102-8f14-d104caa7a996

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/53823#0191a421-7168-486c-9bc9-b364a6d551a7

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/53823#0191a421-716b-4700-a66b-2230c1ab4724

[oleiman]

serialization_rt_test_rpunit::serde_reflection_roundtrip is demonstrably flaky (I can reproduce this locally). That seems...wrong. Failure looks like this:

/home/orenleiman/co/redpanda/src/v/cluster/tests/serialization_rt_test.cc(786): Entering test case "serde_reflection_roundtrip"
Failed serde roundtrip on N7cluster29configuration_with_assignmentINS_19topic_configurationEEE
/home/orenleiman/co/redpanda/src/v/cluster/tests/serialization_rt_test.cc(502): fatal error: in "serde_reflection_roundtrip": critical check original == from_serde has failed
Dump 780 bytes:
  00000000 | 00 00 06 03 00 00 02 00  6e 00 00 00 14 00 00 00  | ........n.......
  00000010 | 69 7a 68 56 37 30 77 44  63 71 49 62 42 4d 79 77  | izhV70wDcqIbBMyw
  00000020 | 6d 66 43 38 14 00 00 00  41 63 5a 63 30 31 42 4e  | mfC8....AcZc01BN
  00000030 | 64 35 6f 44 53 6e 46 65  65 66 47 64 61 00 00 00  | d5oDSnFeefGda...
  00000040 | 08 00 09 00 31 00 00 00  00 00 01 00 00 00 00 00  | ....1...........
  00000050 | 00 ff 01 00 9f 37 6f a3  8e ab 3b 00 01 00 00 00  | .....7o...;.....
/home/orenleiman/co/redpanda/src/v/cluster/tests/serialization_rt_test.cc(786): Leaving test case "serde_reflection_roundtrip"; testing time: 364us

Which looks genuine to me. Trying this out on dev right now.

[oleiman]

Same failure on dev, specific to configuration_with_assignment<topic_configuration> aka topic_configuration_assignment.

[oleiman]

This ought to fix it: #23149

[oleiman]

/ci-repeat 1

[dotnwat]

@oleiman Curious why the force push removed this serde_fields() method?

https://github.com/redpanda-data/redpanda/compare/fd06016c90f15837c926d8141b1c390f4dd6a6b1..9f93962754e97eee718a06ca07d4bb1ae122d7a7#diff-a37731b2d2f3426c8e6eebf80be94745c8a7dfd547ccd9f6c60688f57f28ca4dL125

[oleiman]

@oleiman Curious why the force push removed this serde_fields() method?

https://github.com/redpanda-data/redpanda/compare/fd06016c90f15837c926d8141b1c390f4dd6a6b1..9f93962754e97eee718a06ca07d4bb1ae122d7a7#diff-a37731b2d2f3426c8e6eebf80be94745c8a7dfd547ccd9f6c60688f57f28ca4dL125

Oh yeah, ha, that's confusing. The reason is that it was added already upstream:

redpanda/src/v/cluster/data_migration_types.h

Line 134 in 5f53ab4

auto serde_fields() { return std::tie(hint); }