r/offset_translator: remove unsafe bootstrap code

[ztlpn]

When the corresponding kvstore state is not found, we should recover offset_translator state from the log, not use configuration_manager state - the latter is incorrect because there are other batch types contributing to offset delta, not just configuration batches. This code is a vestige from the time when the separate offset_translator component was just introduced and its kvstore state needed to be bootstrapped from configuration_manager state.

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
• v23.3.x
• v23.2.x
• v23.1.x

Release Notes

• none

[dotnwat]

recover offset_translator state from the log, not use configuration_manager state - the latter is incorrect because there are other batch types contributing to offset delta, not just configuration batches.

oh nice find. sounds like this could have (or was?) been a real headache!

This code is a vestige from the time when the separate offset_translator component was just introduced and its kvstore state needed to be bootstrapped from configuration_manager state.

Anything to do to remove the old bits or make them unlikely to be used incorrectly going forward?

[dotnwat]

Looks like a little build error

/__w/redpanda/redpanda/src/v/archival/tests/ntp_archiver_test.cc:960:56: error: too many arguments to function call, expected single argument 'reset', have 2 arguments
    tr.start(raft::offset_translator::must_reset::yes, {}).get();
    ~~~~~~~~                                           ^~
/__w/redpanda/redpanda/src/v/raft/offset_translator.h:64:18: note: 'start' declared here
    ss::future<> start(must_reset reset);

[ztlpn]

sounds like this could have (or was?) been a real headache!

Right, to my knowledge this issue hasn't occurred in real deployments - we are lucky that we checkpoint the state at the earliest possible opportunity, so that empty kvstore state is unlikely to occur for existing partitions. I was reminded of this piece of code when I was thinking about how to reset a buggy offset translator map that has been accidentally persisted - in theory just nuking kvstore state should be enough, but currently this is not the case.

a little build error

🤦‍♂️ fixed

[ztlpn]

Failures:

• CI Failure (wrong value on nodes) in ClusterConfigAliasTest.test_aliasing #15117
• CI Failure (Failed to query spillover manifest) in ShadowIndexingWhileBusyTest.test_create_or_delete_topics_while_busy #11735
• CI Failure (fatal error: in "test_stale_reader": critical check it->size_bytes != s.bytes.size() has failed) in cloud_storage_rpfixture test_stale_reader #15679
• CI Failure (assert len(orig_hwms) == 4 AssertionError) in DisablingPartitionsTest.test_disable #15949
• CI Failure (Failed to make list_offsets request, unknown broker) in DeleteRecordsTest.test_delete_records_segment_deletion #14641

Also pandatriage problem: https://github.com/redpanda-data/vtools/issues/2423

[vbotbuildovich]

new failures in https://buildkite.com/redpanda/redpanda/builds/43406#018cd622-adf9-41c8-ad82-1564496e42db:

"rptest.tests.recovery_mode_test.DisablingPartitionsTest.test_disable"

[vbotbuildovich]

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/43406#018cd622-adf9-41c8-ad82-1564496e42db

[vbotbuildovich]

/backport v23.3.x

[vbotbuildovich]

/backport v23.2.x

[vbotbuildovich]

/backport v23.1.x