decrease pending gc task count when return (eBay#345)

Author: JacksonYao287

conanfile.py

@@ -10,7 +10,7 @@
 
 class HomeObjectConan(ConanFile):
     name = "homeobject"
-    version = "2.7.6"
+    version = "2.7.7"
 
     homepage = "https://github.com/eBay/HomeObject"
     description = "Blob Store built on HomeReplication"

src/lib/homestore_backend/gc_manager.cpp

@@ -163,9 +163,7 @@ std::shared_ptr< GCManager::pdev_gc_actor > GCManager::get_pdev_gc_actor(uint32_
 
 bool GCManager::is_eligible_for_gc(chunk_id_t chunk_id) {
     auto chunk = m_chunk_selector->get_extend_vchunk(chunk_id);
-
     const auto defrag_blk_num = chunk->get_defrag_nblks();
-
     if (!defrag_blk_num) { return false; }
 
     // 1 if the chunk state is inuse, it is occupied by a open shard, so it can not be selected and we don't need gc it.
@@ -176,9 +174,7 @@ bool GCManager::is_eligible_for_gc(chunk_id_t chunk_id) {
     }
 
     const auto total_blk_num = chunk->get_total_blks();
-
     const auto gc_garbage_rate_threshold = HS_BACKEND_DYNAMIC_CONFIG(gc_garbage_rate_threshold);
-
     bool should_gc = 100 * defrag_blk_num >= total_blk_num * gc_garbage_rate_threshold;
 
     LOGDEBUG("gc scan chunk_id={}, use_blks={}, available_blks={}, total_blks={}, defrag_blks={}, should_gc={}",
@@ -1007,7 +1003,8 @@ bool GCManager::pdev_gc_actor::purge_reserved_chunk(chunk_id_t chunk) {
 void GCManager::pdev_gc_actor::handle_error_before_persisting_gc_metablk(chunk_id_t move_from_chunk,
                                                                          chunk_id_t move_to_chunk,
                                                                          folly::Promise< bool > task,
-                                                                         const uint64_t task_id, uint8_t priority) {
+                                                                         const uint64_t task_id, uint8_t priority,
+                                                                         const pg_id_t& pg_id) {
     LOGERROR(
         "gc task_id={}, move_from_chunk={} to move_to_chunk={} with priority={} failed before persisting gc metablk",
         task_id, move_from_chunk, move_to_chunk, priority);
@@ -1023,6 +1020,7 @@ void GCManager::pdev_gc_actor::handle_error_before_persisting_gc_metablk(chunk_i
         priority == static_cast< uint8_t >(task_priority::normal) ? de.failed_gc_task_count.fetch_add(1)
                                                                   : de.failed_egc_task_count.fetch_add(1);
     });
+    m_hs_home_object->gc_manager()->decr_pg_pending_gc_task(pg_id);
 }
 
 void GCManager::pdev_gc_actor::process_gc_task(chunk_id_t move_from_chunk, uint8_t priority,
@@ -1031,16 +1029,16 @@ void GCManager::pdev_gc_actor::process_gc_task(chunk_id_t move_from_chunk, uint8
              priority);
     auto start_time = std::chrono::steady_clock::now();
     auto vchunk = m_chunk_selector->get_extend_vchunk(move_from_chunk);
+    RELEASE_ASSERT(vchunk->m_pg_id.has_value(), "chunk_id={} is expected to belong to a pg, but not!", move_from_chunk);
+    const auto pg_id = vchunk->m_pg_id.value();
 
     if (vchunk->m_state != ChunkState::GC) {
         LOGWARN("gc task_id={}, move_from_chunk={} is expected to in GC state, but not!", task_id, move_from_chunk);
         task.setValue(false);
+        m_hs_home_object->gc_manager()->decr_pg_pending_gc_task(pg_id);
         return;
     }
 
-    RELEASE_ASSERT(vchunk->m_pg_id.has_value(), "chunk_id={} is expected to belong to a pg, but not!", move_from_chunk);
-    const auto pg_id = vchunk->m_pg_id.value();
-
     RELEASE_ASSERT(vchunk->m_v_chunk_id.has_value(), "pg={}, chunk_id={} is expected to have a vchunk id, but not!",
                    pg_id, move_from_chunk);
     const auto vchunk_id = vchunk->m_v_chunk_id.value();
@@ -1056,22 +1054,25 @@ void GCManager::pdev_gc_actor::process_gc_task(chunk_id_t move_from_chunk, uint8
 
     if (!purge_reserved_chunk(move_to_chunk)) {
         LOGWARN("gc task_id={}, can not purge move_to_chunk={}", task_id, move_to_chunk);
-        handle_error_before_persisting_gc_metablk(move_from_chunk, move_to_chunk, std::move(task), task_id, priority);
+        handle_error_before_persisting_gc_metablk(move_from_chunk, move_to_chunk, std::move(task), task_id, priority,
+                                                  pg_id);
         return;
     }
 
     if (!copy_valid_data(move_from_chunk, move_to_chunk, task_id)) {
         LOGWARN("gc task_id={}, failed to copy data from move_from_chunk={} to move_to_chunk={} with priority={}",
                 task_id, move_from_chunk, move_to_chunk, priority);
-        handle_error_before_persisting_gc_metablk(move_from_chunk, move_to_chunk, std::move(task), task_id, priority);
+        handle_error_before_persisting_gc_metablk(move_from_chunk, move_to_chunk, std::move(task), task_id, priority,
+                                                  pg_id);
         return;
     }
 
     std::vector< std::pair< BlobRouteByChunkKey, BlobRouteValue > > valid_blob_indexes;
     if (!get_blobs_to_replace(move_to_chunk, valid_blob_indexes)) {
         LOGWARN("gc task_id={}, failed to get valid blob indexes from gc index table for move_to_chunk={}", task_id,
                 move_to_chunk);
-        handle_error_before_persisting_gc_metablk(move_from_chunk, move_to_chunk, std::move(task), task_id, priority);
+        handle_error_before_persisting_gc_metablk(move_from_chunk, move_to_chunk, std::move(task), task_id, priority,
+                                                  pg_id);
         return;
     }
 

src/lib/homestore_backend/gc_manager.hpp

@@ -233,7 +233,7 @@ class GCManager {
 
         void handle_error_before_persisting_gc_metablk(chunk_id_t move_from_chunk, chunk_id_t move_to_chunk,
                                                        folly::Promise< bool > task, const uint64_t task_id,
-                                                       uint8_t priority);
+                                                       uint8_t priority, const pg_id_t& pg_id);
 
         pdev_gc_metrics& metrics() { return metrics_; }