cloud_storage: use fragvec to hold replaced segments #16160
Conversation
Observed a large 1 to 2 MB allocation due to 50K plus segments in the replaced segments field of the manifest. Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
| const fragmented_vector<lw_segment_meta> source_backlog | ||
| = _manifest->lw_replaced_segments(); |
There was a problem hiding this comment.
nit: since lw_replaced_segments now returns fragmented vector, should we use const auto here?
| const auto backlog_size = source_backlog.size(); | ||
| fragmented_vector<lw_segment_meta> backlog; | ||
| for (const auto& m : source_backlog) { | ||
| auto it = _manifest->find(m.base_offset); | ||
| if (it == _manifest->end()) { | ||
| backlog.push_back(m); | ||
| continue; | ||
| } | ||
| auto m_name = _manifest->generate_remote_segment_name( | ||
| cloud_storage::partition_manifest::lw_segment_meta::convert(m)); | ||
| auto s_name = _manifest->generate_remote_segment_name(*it); | ||
| // The segment will have the same path as the one we have in | ||
| // manifest in S3 so if we will delete it the data will be lost. | ||
| if (m_name == s_name) { | ||
| vlog( | ||
| _logger.error, | ||
| "The replaced segment name {} collides with the segment " | ||
| "{} " | ||
| "in the manifest. It will be removed to prevent the data " | ||
| "loss.", | ||
| m_name, | ||
| s_name); | ||
| continue; | ||
| } | ||
| backlog.push_back(m); | ||
| } |
There was a problem hiding this comment.
nit: copy_if with a predicate seems like it would be easier to read as the predicate just has to evaluate to true or false.
|
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/43936#018d20b5-6f42-4166-8d43-574a55dea299 ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/43936#018d20b5-6f3c-4e09-98a0-a73a29a3d9a8 |
|
/ci-repeat |
|
Looks like this is taking so long because BK waited an hour to get a worker machine. |
|
new failures in https://buildkite.com/redpanda/redpanda/builds/43958#018d22af-1fa1-4b4d-b74a-e78cdae56f80: |
|
The DT failure is a duplicate of #14149 which is an issue that pops up at shutdown. |
|
/backport v23.3.x |
|
/backport v23.2.x |
|
Failed to create a backport PR to v23.2.x branch. I tried: |
Observed a large 1 to 2 MB allocation due to 50K plus segments in the replaced segments field of the manifest.
Backports Required
Release Notes
Bug Fixes