From 869b518dcaf014f1933ed2dddfdbe400419d97c2 Mon Sep 17 00:00:00 2001
From: Benny Halevy <bhalevy@scylladb.com>
Date: Tue, 27 Aug 2019 11:20:44 +0300
Subject: [PATCH] sstables: auto-delete unsealed sstables

Fixes #4807

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Message-Id: <20190827082044.27223-1-bhalevy@scylladb.com>
---
 sstables/sstables.cc |  8 +++++++-
 sstables/sstables.hh | 10 +++++++---
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/sstables/sstables.cc b/sstables/sstables.cc
index 26ec2690dc..68cfb54fc2 100644
--- a/sstables/sstables.cc
+++ b/sstables/sstables.cc
@@ -973,6 +973,9 @@ future<> sstable::seal_sstable() {
             }).then([this, dir_f] () mutable {
                 return sstable_write_io_check([&] { return dir_f.close(); });
             }).then([this, dir_f] {
+                if (_marked_for_deletion == mark_for_deletion::implicit) {
+                    _marked_for_deletion = mark_for_deletion::none;
+                }
                 // If this point was reached, sstable should be safe in disk.
                 sstlog.debug("SSTable with generation {} of {}.{} was sealed successfully.", _generation, _schema->ks_name(), _schema->cf_name());
             });
@@ -2377,6 +2380,8 @@ future<> sstable::seal_sstable(bool backup)
 sstable_writer sstable::get_writer(const schema& s, uint64_t estimated_partitions,
         const sstable_writer_config& cfg, encoding_stats enc_stats, const io_priority_class& pc, shard_id shard)
 {
+    // Mark sstable for implicit deletion if destructed before it is sealed.
+    _marked_for_deletion = mark_for_deletion::implicit;
     return sstable_writer(*this, s, estimated_partitions, cfg, enc_stats, pc, shard);
 }
 
@@ -2861,13 +2866,14 @@ sstable::~sstable() {
         });
     }
 
-    if (_marked_for_deletion) {
+    if (_marked_for_deletion != mark_for_deletion::none) {
         // We need to delete the on-disk files for this table. Since this is a
         // destructor, we can't wait for this to finish, or return any errors,
         // but just need to do our best. If a deletion fails for some reason we
         // log and ignore this failure, because on startup we'll again try to
         // clean up unused sstables, and because we'll never reuse the same
         // generation number anyway.
+        sstlog.debug("Deleting sstable that is {}marked for deletion", _marked_for_deletion == mark_for_deletion::implicit ? "implicitly " : "");
         try {
             // FIXME:
             // - Longer term fix is to hand off deletion of sstables to a manager that can
diff --git a/sstables/sstables.hh b/sstables/sstables.hh
index 54f1ab8b51..e1a9f1dd33 100644
--- a/sstables/sstables.hh
+++ b/sstables/sstables.hh
@@ -299,11 +299,11 @@ public:
     // guaranteed that all of its on-disk files will be deleted as soon as
     // the in-memory object is destroyed.
     void mark_for_deletion() {
-        _marked_for_deletion = true;
+        _marked_for_deletion = mark_for_deletion::marked;
     }
 
     bool marked_for_deletion() const {
-        return _marked_for_deletion;
+        return _marked_for_deletion == mark_for_deletion::marked;
     }
 
     void add_ancestor(int64_t generation) {
@@ -532,7 +532,11 @@ private:
 
     filter_tracker _filter_tracker;
 
-    bool _marked_for_deletion = false;
+    enum class mark_for_deletion {
+        implicit = -1,
+        none = 0,
+        marked = 1
+    } _marked_for_deletion = mark_for_deletion::none;
 
     gc_clock::time_point _now;