Merge pull request #7832 from abhijat/prevent-trimming-segment-on-rem…

…ote-partition-stop cloud_storage: filter stopped segments on trim
redpanda-data · Dec 20, 2022 · faab40f · faab40f
2 parents b211fb0 + 0b4f769
commit faab40f
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 2 deletions.
diff --git a/src/v/cloud_storage/materialized_segments.cc b/src/v/cloud_storage/materialized_segments.cc
@@ -294,6 +294,10 @@ void materialized_segments::trim_segments(std::optional<size_t> target_free) {
  */
 void materialized_segments::maybe_trim_segment(
   materialized_segment_state& st, offload_list_t& to_offload) {
+    if (st.segment->is_stopped()) {
+        return;
+    }
+
     if (st.segment->download_in_progress()) {
         return;
     }
@@ -309,8 +313,19 @@ void materialized_segments::maybe_trim_segment(
         // this will delete and unlink the object from
         // _materialized collection
         if (st.parent) {
-            to_offload.push_back(
-              std::make_pair(st.parent.get(), st.base_rp_offset()));
+            if (
+              st.parent->state()
+              == remote_partition::partition_state::running) {
+                to_offload.push_back(
+                  std::make_pair(st.parent.get(), st.base_rp_offset()));
+            } else {
+                vlog(
+                  cst_log.info,
+                  "Materialized segment {} offset {} not offloaded, partition "
+                  "is stopping",
+                  st.ntp(),
+                  st.base_rp_offset());
+            }
         } else {
             // This cannot happen, because materialized_segment_state
             // is only instantiated by remote_partition and will

diff --git a/src/v/cloud_storage/remote_partition.cc b/src/v/cloud_storage/remote_partition.cc
@@ -578,6 +578,7 @@ remote_partition::aborted_transactions(offset_range offsets) {
 }
 
 ss::future<> remote_partition::stop() {
+    _state = partition_state::stopping;
     vlog(_ctxlog.debug, "remote partition stop {} segments", _segments.size());
 
     co_await _gate.close();
@@ -597,6 +598,7 @@ ss::future<> remote_partition::stop() {
     // stopping readers is fast, the queue is not usually long, and destroying
     // partitions is relatively infrequent.
     co_await materialized().flush_evicted();
+    _state = partition_state::stopped;
 }
 
 /// Return reader back to segment_state

diff --git a/src/v/cloud_storage/remote_partition.h b/src/v/cloud_storage/remote_partition.h
@@ -122,6 +122,9 @@ class remote_partition
     /// Hook for materialized_segment to notify us when a segment is evicted
     void offload_segment(model::offset);
 
+    enum class partition_state { running, stopping, stopped };
+    partition_state state() const { return _state; }
+
 private:
     friend struct materialized_segment_state;
 
@@ -185,6 +188,8 @@ class remote_partition
 
     segment_map_t _segments;
     partition_probe _probe;
+
+    partition_state _state{partition_state::running};
 };
 
 } // namespace cloud_storage