KIKIMR-18779 trigger forget exported blobs seldom

7 files changed, 39 insertions, 21 deletions

diff --git a/ydb/core/tx/columnshard/blob_manager.cpp b/ydb/core/tx/columnshard/blob_manager.cpp
index 495df341b8a..b1f9d682e97 100644
--- a/ydb/core/tx/columnshard/blob_manager.cpp
+++ b/ydb/core/tx/columnshard/blob_manager.cpp
@@ -690,9 +690,13 @@ TEvictedBlob TBlobManager::GetDropped(const TUnifiedBlobId& blobId, TEvictMetada
     return {};
 }
 
-void TBlobManager::GetCleanupBlobs(THashMap<TString, THashSet<TEvictedBlob>>& tierBlobs) const {
+void TBlobManager::GetCleanupBlobs(THashMap<TString, THashSet<TEvictedBlob>>& tierBlobs,
+                                   const THashSet<TUnifiedBlobId>& allowList) const {
     TStringBuilder strBlobs;
     for (auto& [evict, meta] : DroppedEvictedBlobs) {
+        if (!allowList.empty() && !allowList.contains(evict.Blob)) {
+            continue;
+        }
         if (evict.State != EEvictState::EVICTING) {
             strBlobs << "'" << evict.Blob << "' ";
             auto& tierName = meta.GetTierName();
@@ -734,10 +738,10 @@ bool TBlobManager::BlobInUse(const NOlap::TUnifiedBlobId& blobId) const {
     return BlobsUseCount.contains(blobId);
 }
 
-void TBlobManager::SetBlobInUse(const TUnifiedBlobId& blobId, bool inUse) {
+bool TBlobManager::SetBlobInUse(const TUnifiedBlobId& blobId, bool inUse) {
     if (inUse) {
         BlobsUseCount[blobId]++;
-        return;
+        return true;
     }
 
     auto useIt = BlobsUseCount.find(blobId);
@@ -746,7 +750,7 @@ void TBlobManager::SetBlobInUse(const TUnifiedBlobId& blobId, bool inUse) {
 
     if (useIt->second > 0) {
         // Blob is still in use
-        return;
+        return false;
     }
 
     LOG_S_DEBUG("BlobManager at tablet " << TabletInfo->TabletID << " Blob " << blobId << " is no longer in use");
@@ -773,6 +777,7 @@ void TBlobManager::SetBlobInUse(const TUnifiedBlobId& blobId, bool inUse) {
             }
         }
     }
+    return true;
 }
 
 bool TBlobManager::ExtractEvicted(TEvictedBlob& evict, TEvictMetadata& meta, bool fromDropped /*= false*/) {
diff --git a/ydb/core/tx/columnshard/blob_manager.h b/ydb/core/tx/columnshard/blob_manager.h
index f0fb3b6bffe..cdcd6dac5cb 100644
--- a/ydb/core/tx/columnshard/blob_manager.h
+++ b/ydb/core/tx/columnshard/blob_manager.h
@@ -98,7 +98,8 @@ public:
     virtual bool LoadOneToOneExport(IBlobManagerDb& db, THashSet<TUnifiedBlobId>& droppedEvicting) = 0;
     virtual TEvictedBlob GetEvicted(const TUnifiedBlobId& blob, TEvictMetadata& meta) = 0;
     virtual TEvictedBlob GetDropped(const TUnifiedBlobId& blobId, TEvictMetadata& meta) = 0;
-    virtual void GetCleanupBlobs(THashMap<TString, THashSet<TEvictedBlob>>& tierBlobs) const = 0;
+    virtual void GetCleanupBlobs(THashMap<TString, THashSet<TEvictedBlob>>& tierBlobs,
+                                const THashSet<TUnifiedBlobId>& allowList = {}) const = 0;
     virtual void GetReexportBlobs(THashMap<TString, THashSet<TEvictedBlob>>& tierBlobs) const = 0;
     virtual bool HasExternBlobs() const = 0;
 };
@@ -249,7 +250,8 @@ public:
     bool LoadOneToOneExport(IBlobManagerDb& db, THashSet<TUnifiedBlobId>& droppedEvicting) override;
     TEvictedBlob GetEvicted(const TUnifiedBlobId& blobId, TEvictMetadata& meta) override;
     TEvictedBlob GetDropped(const TUnifiedBlobId& blobId, TEvictMetadata& meta) override;
-    void GetCleanupBlobs(THashMap<TString, THashSet<TEvictedBlob>>& tierBlobs) const override;
+    void GetCleanupBlobs(THashMap<TString, THashSet<TEvictedBlob>>& tierBlobs,
+                        const THashSet<TUnifiedBlobId>& allowList = {}) const override;
     void GetReexportBlobs(THashMap<TString, THashSet<TEvictedBlob>>& tierBlobs) const override;
 
     bool HasExternBlobs() const override {
@@ -257,7 +259,7 @@ public:
     }
 
     // Implementation of IBlobInUseTracker
-    void SetBlobInUse(const TUnifiedBlobId& blobId, bool inUse) override;
+    bool SetBlobInUse(const TUnifiedBlobId& blobId, bool inUse) override;
     bool BlobInUse(const NOlap::TUnifiedBlobId& blobId) const override;
 
 private:
diff --git a/ydb/core/tx/columnshard/columnshard.cpp b/ydb/core/tx/columnshard/columnshard.cpp
index ef345651034..849f291e94c 100644
--- a/ydb/core/tx/columnshard/columnshard.cpp
+++ b/ydb/core/tx/columnshard/columnshard.cpp
@@ -111,7 +111,7 @@ void TColumnShard::Handle(TEvPrivate::TEvReadFinished::TPtr& ev, const TActorCon
     Y_UNUSED(ctx);
     ui64 readCookie = ev->Get()->RequestCookie;
     LOG_S_DEBUG("Finished read cookie: " << readCookie << " at tablet " << TabletID());
-    InFlightReadsTracker.RemoveInFlightRequest(ev->Get()->RequestCookie, *BlobManager);
+    auto blobs = InFlightReadsTracker.RemoveInFlightRequest(ev->Get()->RequestCookie, *BlobManager);
 
     ui64 txId = ev->Get()->TxId;
     if (ScanTxInFlight.contains(txId)) {
@@ -121,8 +121,10 @@ void TColumnShard::Handle(TEvPrivate::TEvReadFinished::TPtr& ev, const TActorCon
         SetCounter(COUNTER_SCAN_IN_FLY, ScanTxInFlight.size());
     }
 
-    // Cleanup just freed dropped exported blobs
-    CleanForgottenBlobs(ctx);
+    if (blobs.size()) {
+        // Cleanup just freed blobs (dropped exported ones)
+        CleanForgottenBlobs(ctx, blobs);
+    }
 }
 
 void TColumnShard::Handle(TEvPrivate::TEvPeriodicWakeup::TPtr& ev, const TActorContext& ctx) {
diff --git a/ydb/core/tx/columnshard/columnshard_impl.cpp b/ydb/core/tx/columnshard/columnshard_impl.cpp
index 30d13b7a300..7c307793a6e 100644
--- a/ydb/core/tx/columnshard/columnshard_impl.cpp
+++ b/ydb/core/tx/columnshard/columnshard_impl.cpp
@@ -655,7 +655,7 @@ void TColumnShard::EnqueueBackgroundActivities(bool periodic, TBackgroundActivit
     if (activity.HasCleanup()) {
         if (auto event = SetupCleanup()) {
             ctx.Send(SelfId(), event.release());
-        } else {
+        } else if (periodic) {
             // Small cleanup (no index changes)
             CleanForgottenBlobs(ctx);
         }
@@ -946,10 +946,12 @@ void TColumnShard::MapExternBlobs(const TActorContext& /*ctx*/, NOlap::TReadMeta
     }
 }
 
-void TColumnShard::CleanForgottenBlobs(const TActorContext& ctx) {
+void TColumnShard::CleanForgottenBlobs(const TActorContext& ctx, const THashSet<TUnifiedBlobId>& allowList) {
     THashMap<TString, THashSet<NOlap::TEvictedBlob>> tierBlobsToForget;
-    BlobManager->GetCleanupBlobs(tierBlobsToForget);
-    ForgetBlobs(ctx, tierBlobsToForget);
+    BlobManager->GetCleanupBlobs(tierBlobsToForget, allowList);
+    if (tierBlobsToForget.size()) {
+        ForgetBlobs(ctx, tierBlobsToForget);
+    }
 }
 
 void TColumnShard::Reexport(const TActorContext& ctx) {
diff --git a/ydb/core/tx/columnshard/columnshard_impl.h b/ydb/core/tx/columnshard/columnshard_impl.h
index 5942d140612..05eec6340b2 100644
--- a/ydb/core/tx/columnshard/columnshard_impl.h
+++ b/ydb/core/tx/columnshard/columnshard_impl.h
@@ -579,7 +579,7 @@ private:
 
     void EnqueueProgressTx(const TActorContext& ctx);
     void EnqueueBackgroundActivities(bool periodic = false, TBackgroundActivity activity = TBackgroundActivity::All());
-    void CleanForgottenBlobs(const TActorContext& ctx);
+    void CleanForgottenBlobs(const TActorContext& ctx, const THashSet<TUnifiedBlobId>& allowList = {});
 
     void UpdateSchemaSeqNo(const TMessageSeqNo& seqNo, NTabletFlatExecutor::TTransactionContext& txc);
     void ProtectSchemaSeqNo(const NKikimrTxColumnShard::TSchemaSeqNo& seqNoProto, NTabletFlatExecutor::TTransactionContext& txc);
diff --git a/ydb/core/tx/columnshard/inflight_request_tracker.h b/ydb/core/tx/columnshard/inflight_request_tracker.h
index c8b95d9ea5f..b98c6826c00 100644
--- a/ydb/core/tx/columnshard/inflight_request_tracker.h
+++ b/ydb/core/tx/columnshard/inflight_request_tracker.h
@@ -14,7 +14,7 @@ public:
     // it's ref count. This will prevent the blob from beeing physically deleted when DeleteBlob() is called
     // until all the references are released.
     // NOTE: this ref counts are in-memory only, so the blobs can be deleted if tablet restarts
-    virtual void SetBlobInUse(const NOlap::TUnifiedBlobId& blobId, bool inUse) = 0;
+    virtual bool SetBlobInUse(const NOlap::TUnifiedBlobId& blobId, bool inUse) = 0;
     virtual bool BlobInUse(const NOlap::TUnifiedBlobId& blobId) const = 0;
 };
 
@@ -40,10 +40,12 @@ public:
     }
 
     // Forget completed request
-    void RemoveInFlightRequest(ui64 cookie, IBlobInUseTracker& blobTracker) {
+    THashSet<NOlap::TUnifiedBlobId> RemoveInFlightRequest(ui64 cookie, IBlobInUseTracker& blobTracker) {
         Y_VERIFY(RequestsMeta.contains(cookie), "Unknown request cookie %" PRIu64, cookie);
         const auto& readMetaList = RequestsMeta[cookie];
 
+        THashSet<NOlap::TUnifiedBlobId> freedBlobs;
+
         for (const auto& readMetaBase : readMetaList) {
             NOlap::TReadMetadata::TConstPtr readMeta = std::dynamic_pointer_cast<const NOlap::TReadMetadata>(readMetaBase);
 
@@ -61,16 +63,21 @@ public:
                     it->second--;
                 }
                 for (auto& rec : portion.Records) {
-                    blobTracker.SetBlobInUse(rec.BlobRange.BlobId, false);
+                    if (blobTracker.SetBlobInUse(rec.BlobRange.BlobId, false)) {
+                        freedBlobs.emplace(rec.BlobRange.BlobId);
+                    }
                 }
             }
 
             for (const auto& committedBlob : readMeta->CommittedBlobs) {
-                blobTracker.SetBlobInUse(committedBlob.GetBlobId(), false);
+                if (blobTracker.SetBlobInUse(committedBlob.GetBlobId(), false)) {
+                    freedBlobs.emplace(committedBlob.GetBlobId());
+                }
             }
         }
 
         RequestsMeta.erase(cookie);
+        return freedBlobs;
     }
 
     // Checks if the portion is in use by any in-flight request
diff --git a/ydb/core/tx/columnshard/ut_schema/ut_columnshard_schema.cpp b/ydb/core/tx/columnshard/ut_schema/ut_columnshard_schema.cpp
index 2a504a75339..aea9ddf1249 100644
--- a/ydb/core/tx/columnshard/ut_schema/ut_columnshard_schema.cpp
+++ b/ydb/core/tx/columnshard/ut_schema/ut_columnshard_schema.cpp
@@ -685,7 +685,7 @@ std::vector<std::pair<ui32, ui64>> TestTiers(bool reboots, const std::vector<TSt
                     deplayedExports += numExports;
                     numExports = 0;
                 } else if (numForgets) {
-                    counter.CaptureForgetResponse = reboots ? 2 : 1;
+                    counter.CaptureForgetResponse = 1;
                     deplayedForgets += numForgets;
                     numForgets = 0;
                 }
@@ -1491,7 +1491,7 @@ Y_UNIT_TEST_SUITE(TColumnShardTestSchema) {
         TestExport(false, {}, 2);
     }
 
-    Y_UNIT_TEST(RebootForgettWithLostAnswer) {
+    Y_UNIT_TEST(RebootForgetWithLostAnswer) {
         TestExport(true, {}, 2);
     }