[I] should we start the compaction_checker_range thread if it's not configed. [kvrocks]

["13015517713 (via GitHub)" <gi...@apache.org>]

13015517713 opened a new issue, #2305:
URL: https://github.com/apache/kvrocks/issues/2305

   ### Search before asking
   
   - [X] I had searched in the [issues](https://github.com/apache/kvrocks/issues) and found no similar issues.
   
   
   ### Motivation
   
   When running Server.start(), kvrocks will launch a thread for compaction check. Some considerations are as follows.
   
   -  If there is no checker configured, maybe it is no need to start the thread to check frequently. Only if it is enabled, then create the thread.
   
   - The checker for pubsubfiles is independent of the configuration of the normal checker (the code is like this). If there is no configured checker, kvrocks also should do compression for pubsub.  (not sure I understand that correctly.)
   
   Corresponding code block is in server.cc, Server().Start().
   ```
   compaction_checker_thread_ = GET_OR_RET(util::CreateThread("compact-check", [this] {
       uint64_t counter = 0;
       int64_t last_compact_date = 0;
       CompactionChecker compaction_checker{this->storage};
   
       while (!stop_) {
         // Sleep first
         std::this_thread::sleep_for(std::chrono::milliseconds(100));
   
         // To guarantee accessing DB safely
         auto guard = storage->ReadLockGuard();
         if (storage->IsClosing()) continue;
   
         if (!is_loading_ && ++counter % 600 == 0  // check every minute
             && config_->compaction_checker_range.Enabled()) {
           auto now_hours = util::GetTimeStamp<std::chrono::hours>();
           if (now_hours >= config_->compaction_checker_range.start &&
               now_hours <= config_->compaction_checker_range.stop) {
             std::vector<std::string> cf_names = {engine::kMetadataColumnFamilyName, engine::kSubkeyColumnFamilyName,
                                                  engine::kZSetScoreColumnFamilyName, engine::kStreamColumnFamilyName};
             for (const auto &cf_name : cf_names) {
               compaction_checker.PickCompactionFiles(cf_name);
             }
           }
           // compact once per day
           if (now_hours != 0 && last_compact_date != now_hours / 24) {
             last_compact_date = now_hours / 24;
             compaction_checker.CompactPropagateAndPubSubFiles();
           }
         }
       }
     }));
   ```
   
   ### Solution
   
   _No response_
   
   ### Are you willing to submit a PR?
   
   - [ ] I'm willing to submit a PR!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] should we start the compaction_checker_range thread if it's not configed. [kvrocks]

["git-hulk (via GitHub)" <gi...@apache.org>]

git-hulk commented on issue #2305:
URL: https://github.com/apache/kvrocks/issues/2305#issuecomment-2104785275

   @13015517713 Thanks for bringing up this discussion. It's unnecessary to check if disabled since the compaction will be totally depended on the rocksdb.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] should we start the compaction_checker_range thread if it's not configed. [kvrocks]

["git-hulk (via GitHub)" <gi...@apache.org>]

git-hulk commented on issue #2305:
URL: https://github.com/apache/kvrocks/issues/2305#issuecomment-2104805788

   Yes, it's good to leave as it is to keep the codes simple.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] should we start the compaction_checker_range thread if it's not configed. [kvrocks]

["mapleFU (via GitHub)" <gi...@apache.org>]

mapleFU commented on issue #2305:
URL: https://github.com/apache/kvrocks/issues/2305#issuecomment-2104800017

   The range isn't read-only, and the thread is not so high-cost. I think it's ok to not dynamically enable it. But if we can have a timer task system, it would also be ok


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org