You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "xichen01 (via GitHub)" <gi...@apache.org> on 2025/10/21 09:41:33 UTC

[PR] HDDS-13817. Add fixed-delay scheduling mode to SCMBlockDeletingService [ozone]

xichen01 opened a new pull request, #9185:
URL: https://github.com/apache/ozone/pull/9185

   ## What changes were proposed in this pull request?
   
   ### Context
   
   For detailed: https://issues.apache.org/jira/browse/HDDS-13817
   
   ### Changes
   
   1. Introduce SchedulingMode enum (`FIXED_RATE` (default, Consistent with the original code behavior), `FIXED_DELAY`) in BackgroundService
   2. Add configuration key 'hdds.scm.block.deleting.service.scheduling.mode' in ScmConfig
   3. Update SCMBlockDeletingService to use the configured scheduling mode
   
   
   
   ## What is the link to the Apache JIRA
   https://issues.apache.org/jira/browse/HDDS-13817
   
   ## How was this patch tested?
   
   https://github.com/xichen01/ozone/actions/runs/18675992516
   
   unit test
   


-- 
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@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13817. Add fixed-delay scheduling mode to SCMBlockDeletingService [ozone]

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #9185:
URL: https://github.com/apache/ozone/pull/9185#discussion_r2526269047


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfig.java:
##########
@@ -138,6 +139,15 @@ public class ScmConfig extends ReconfigurableConfig {
   )
   private int transactionToDNsCommitMapLimit = 5000000;
 
+  @Config(key = "hdds.scm.block.deleting.service.scheduling.mode",
+      defaultValue = "FIXED_RATE",
+      type = ConfigType.STRING,
+      tags = { ConfigTag.SCM, ConfigTag.DELETION },
+      description = "Scheduling mode for the block deleting service. For detailed, " +
+          "see org.apache.hadoop.hdds.utils.SchedulingMode"
+  )
+  private String blockDeletingServiceSchedulingMode = SchedulingMode.FIXED_RATE.name();

Review Comment:
   Had a offline discussion with Xi about the background of this improvement. It arise from a real observation data Xi has shared.  The rocksDB access could be very fast in most of cases, but there can be unexpected slow spikes sometimes.  
   
   A lot of Ozone services use the BackgroundService currently to schedule the task on fixed rate, SCMBlockDeletingService is one of them.  SCMBlockDeletingService uses DeletedBlockLogImpl, which updates deletion transactions in DB with a lock protected. Add/Get/HandleCommandResponseFromDN all competes this lock. If block deletion task runs one after another without any delay, it will hold the lock for the most of time, leave less time for Add operation which serves request from OM, and handle block deletion command response from DN, so these two types requests could pine up in SCM.  The new fixed delay can solve this problem, so every operation have the similar change to get the lock and proceed. 
   
   The cost of fixed delay is block deletion task will run less frequently from fixed rat, which I think is not a big problem. As if the task finishes very quickly,  then fixed delay and fixed rat are close. If the task take more time than expected, fixed delay is better than fixed rat,  it can better handle this case,  to have a smooth SCM overall activities experience. 
   Also with fixed delay, it's safe to tune the "hdds.scm.block.deleting.service.interval" value, we don't need to worry about whether the value is too short to cover one task run. 
   
   Actually,  I suggest we can consider to change the default mode to fixed delay, as fixed delay is more adaptive to different deletion cases. 
   



##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfig.java:
##########
@@ -138,6 +139,15 @@ public class ScmConfig extends ReconfigurableConfig {
   )
   private int transactionToDNsCommitMapLimit = 5000000;
 
+  @Config(key = "hdds.scm.block.deleting.service.scheduling.mode",
+      defaultValue = "FIXED_RATE",
+      type = ConfigType.STRING,
+      tags = { ConfigTag.SCM, ConfigTag.DELETION },
+      description = "Scheduling mode for the block deleting service. For detailed, " +
+          "see org.apache.hadoop.hdds.utils.SchedulingMode"
+  )
+  private String blockDeletingServiceSchedulingMode = SchedulingMode.FIXED_RATE.name();

Review Comment:
   Had a offline discussion with Xi about the background of this improvement. It arise from a real observation data Xi has shared.  The rocksDB access could be very fast in most of cases, but there can be unexpected slow spikes sometimes.  
   
   A lot of Ozone services use the BackgroundService currently to schedule the task on fixed rate, SCMBlockDeletingService is one of them.  SCMBlockDeletingService uses DeletedBlockLogImpl, which updates deletion transactions in DB with a lock protected. Add/Get/HandleCommandResponseFromDN all competes this lock. If block deletion task runs one after another without any delay, it will hold the lock for the most of time, leave less time for Add operation which serves request from OM, and handle block deletion command response from DN, so these two types requests could pine up in SCM.  The new fixed delay can solve this problem, so every operation have the similar change to get the lock and proceed. 
   
   The cost of fixed delay is block deletion task will run less frequently from fixed rat, which I think is not a big problem. As if the task finishes very quickly,  then fixed delay and fixed rat are close. If the task take more time than expected, fixed delay is better than fixed rat,  it can better handle this case,  to have a smooth SCM overall activities experience. 
   Also with fixed delay, it's safe to tune the "hdds.scm.block.deleting.service.interval" value, we don't need to worry about whether the value is too short to cover one task run. 
   
   Actually,  I suggest we can consider to change the default mode to fixed delay, as fixed delay is more adaptive to different deletion cases.  cc @ashishkumar50 
   



-- 
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@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13817. Add fixed-delay scheduling mode to SCMBlockDeletingService [ozone]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #9185: HDDS-13817. Add fixed-delay scheduling mode to SCMBlockDeletingService
URL: https://github.com/apache/ozone/pull/9185


-- 
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@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13817. Add fixed-delay scheduling mode to SCMBlockDeletingService [ozone]

Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9185:
URL: https://github.com/apache/ozone/pull/9185#discussion_r2460325097


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfig.java:
##########
@@ -138,6 +139,15 @@ public class ScmConfig extends ReconfigurableConfig {
   )
   private int transactionToDNsCommitMapLimit = 5000000;
 
+  @Config(key = "hdds.scm.block.deleting.service.scheduling.mode",
+      defaultValue = "FIXED_RATE",
+      type = ConfigType.STRING,
+      tags = { ConfigTag.SCM, ConfigTag.DELETION },
+      description = "Scheduling mode for the block deleting service. For detailed, " +
+          "see org.apache.hadoop.hdds.utils.SchedulingMode"
+  )
+  private String blockDeletingServiceSchedulingMode = SchedulingMode.FIXED_RATE.name();

Review Comment:
   If one sets both interval and also the scheduling Mode we would have conflicting configs which could get a bit confusing for user. I wouldn't want to introduce redundant configs which can confuse a user based on some convoluted logic.



-- 
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@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13817. Add fixed-delay scheduling mode to SCMBlockDeletingService [ozone]

Posted by "xichen01 (via GitHub)" <gi...@apache.org>.
xichen01 commented on PR #9185:
URL: https://github.com/apache/ozone/pull/9185#issuecomment-3448715183

   `SCMBlockDeletingService` holds a lock during execution, allowing other threads time to update the database.
   
   If the current `SCMBlockDeletingService` execution time exceeds the `BackgroundService` interval (60 seconds by default) for various reasons, `SCMBlockDeletingService`  will immediately execute the next scan, making it difficult for other threads to obtain `DeletedBlockLogImpl#lock` to update the DB.
   
   Therefore, "fixed delay" is more suitable for `SCMBlockDeletingService`. This way, regardless of the execution time of `SCMBlockDeletingService`, `SCMBlockDeletingService` will wait for the next execution after the interval.


-- 
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@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13817. Add fixed-delay scheduling mode to SCMBlockDeletingService [ozone]

Posted by "xichen01 (via GitHub)" <gi...@apache.org>.
xichen01 commented on PR #9185:
URL: https://github.com/apache/ozone/pull/9185#issuecomment-3548694280

   @sumitagrawl 
   
   > fixed_rate: after current iteration for getting blocks, it will wait for 1 min for next iteration and schedule again. (This is existing implementation and default for new implementation). This is already solving the issue in existing code.
   
   The current implementation does not execute in this way, it will not wait for 1 min for next iteration, it might not wait and would proceed directly to the next iteration.
   
   The current `BackgroundService` executes `PeriodicalTask#run` via `scheduleWithFixedDelay`.
   `PeriodicalTask#run` does not execute the task directly but instead uses `CompletableFuture.runAsync`.
   Therefore, `scheduleWithFixedDelay` only submits the task according to a fixed delay; the task itself will not execute according to the fixed delay. 
   So, If the previous task takes 10 seconds, the next task will execute after 50 seconds. If the previous task takes 60 seconds, the next task will execute immediately.
   
   you can add this test into `TestBackgroundService` to observe the output
   
   ```java
   @Test
     @Timeout(60)
     public void testSchedulingIntervalWithLongRunningTasks() throws InterruptedException {
   
       backgroundService = new BackgroundService(
           "TimingTestService", 3, TimeUnit.SECONDS, 1, 60) {
         @Override
         public BackgroundTaskQueue getTasks() {
           BackgroundTaskQueue newQueue = new BackgroundTaskQueue();
   
           newQueue.add(() -> {
             System.out.printf("%n[%s] Task start%n", Time.formatTime(System.currentTimeMillis()));
             Thread.sleep(5_000);
             System.out.printf("[%s] Task end%n", Time.formatTime(System.currentTimeMillis()));
             return BackgroundTaskResult.EmptyTaskResult.newResult();
           });
   
           return newQueue;
         }
       };
   
       backgroundService.start();
       Thread.sleep(60_000);
       backgroundService.shutdown();
     }
   ```


-- 
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@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13817. Add fixed-delay scheduling mode to SCMBlockDeletingService [ozone]

Posted by "xichen01 (via GitHub)" <gi...@apache.org>.
xichen01 commented on code in PR #9185:
URL: https://github.com/apache/ozone/pull/9185#discussion_r2463935779


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfig.java:
##########
@@ -138,6 +139,15 @@ public class ScmConfig extends ReconfigurableConfig {
   )
   private int transactionToDNsCommitMapLimit = 5000000;
 
+  @Config(key = "hdds.scm.block.deleting.service.scheduling.mode",
+      defaultValue = "FIXED_RATE",
+      type = ConfigType.STRING,
+      tags = { ConfigTag.SCM, ConfigTag.DELETION },
+      description = "Scheduling mode for the block deleting service. For detailed, " +
+          "see org.apache.hadoop.hdds.utils.SchedulingMode"
+  )
+  private String blockDeletingServiceSchedulingMode = SchedulingMode.FIXED_RATE.name();

Review Comment:
   `BackgroundService` interval and mode are two independent parameters, just like scheduleAtFixedRate and scheduleAtFixedRate of `ScheduledExecutorService` 
   
   > I wouldn't want to introduce redundant configs which can confuse a user based on some convoluted logic.
   
   In most cases, the default configuration is sufficient for users and users do not need to understand every configuration. When users have special needs, they can adjust specific parameters to meet their needs.



-- 
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@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13817. Add fixed-delay scheduling mode to SCMBlockDeletingService [ozone]

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #9185:
URL: https://github.com/apache/ozone/pull/9185#discussion_r2526064677


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/BackgroundService.java:
##########
@@ -150,6 +180,18 @@ public void run() {
         execTaskCompletion();
       }
 
+      if (schedulingMode.shouldSleepAfterCompletion()) {
+        try {
+          long delayMillis = getIntervalMillis();
+          LOG.debug("Waiting {} ms after task completion before next execution", delayMillis);
+          Thread.sleep(delayMillis);
+        } catch (InterruptedException e) {
+          LOG.warn("Interrupted while waiting for fixed interval");
+          execTaskCompletion();

Review Comment:
   execTaskCompletion() is called after future.join().  I think we don't need to call it here again. 



-- 
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@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13817. Add fixed-delay scheduling mode to SCMBlockDeletingService [ozone]

Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on PR #9185:
URL: https://github.com/apache/ozone/pull/9185#issuecomment-3551352306

   @xichen01 Yes, its not working as expected. https://issues.apache.org/jira/browse/HDDS-6696 is created for same earlier in backlog as can see this history.
   
   Fixing this bug will resolve the problem. And fix will be just move the future.join() at end of run() method,
   ```
   try {
           future.join();
         } catch (RuntimeException e) {
           LOG.error("Background service execution failed.", e);
         } finally {
           execTaskCompletion();
         }
   ```
   
   This will resolve the related bug and problem of no-wait for next schedule. IMO, we need not support 2 different mechanism of FIXED_RATE, And Fixed_Delay, can follow only Fixed_Delay with the fix.
   


-- 
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@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13817. Add fixed-delay scheduling mode to SCMBlockDeletingService [ozone]

Posted by "xichen01 (via GitHub)" <gi...@apache.org>.
xichen01 commented on PR #9185:
URL: https://github.com/apache/ozone/pull/9185#issuecomment-3595473573

   > @xichen01, since @sumitagrawl's suggestion works too, we can go with this approach as it's simpler and can fix the problem. What do you think?
   It's ok to change the behavior of all current BackgroundService subclasses since it's buggy currently. We don't need to maintain two modes.
   
   This is also an alternative approach. If this modification is merged into master, the current branch can be closed.
   


-- 
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@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13817. Add fixed-delay scheduling mode to SCMBlockDeletingService [ozone]

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #9185:
URL: https://github.com/apache/ozone/pull/9185#discussion_r2498425103


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/BackgroundService.java:
##########
@@ -116,9 +135,20 @@ public synchronized void start() {
     if (exec == null || exec.isShutdown() || exec.isTerminated()) {
       initExecutorAndThreadGroup();
     }
-    LOG.info("Starting service {} with interval {} {}", serviceName,
-        interval, unit.name().toLowerCase());
-    exec.scheduleWithFixedDelay(service, 0, interval, unit);
+    
+    if (schedulingMode == SchedulingMode.FIXED_DELAY) {
+      LOG.info("Starting service {} with fixed delay {} {} after task completion", 
+          serviceName, interval, unit.name().toLowerCase());
+      // Use minimal delay for scheduleWithFixedDelay, actual interval controlled in run()
+      exec.scheduleWithFixedDelay(service, 0, 1, TimeUnit.MILLISECONDS);

Review Comment:
   We should use the real interval and unit here.  



-- 
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@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13817. Add fixed-delay scheduling mode to SCMBlockDeletingService [ozone]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9185:
URL: https://github.com/apache/ozone/pull/9185#issuecomment-3684593498

   This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.


-- 
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@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13817. Add fixed-delay scheduling mode to SCMBlockDeletingService [ozone]

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on PR #9185:
URL: https://github.com/apache/ozone/pull/9185#issuecomment-3570394359

   @xichen01,  since @sumitagrawl's suggestion works too, we can go with this approach as it's simpler and can fix the problem.  What do you think?   
   It's ok to change the behavior of all current BackgroundService subclasses since it's buggy currently. We don't need to maintain two modes. 


-- 
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@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13817. Add fixed-delay scheduling mode to SCMBlockDeletingService [ozone]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9185:
URL: https://github.com/apache/ozone/pull/9185#issuecomment-3700890692

   Thank you for your contribution. This PR is being closed due to inactivity. If needed, feel free to reopen it.


-- 
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@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13817. Add fixed-delay scheduling mode to SCMBlockDeletingService [ozone]

Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #9185:
URL: https://github.com/apache/ozone/pull/9185#discussion_r2536383793


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfig.java:
##########
@@ -138,6 +139,15 @@ public class ScmConfig extends ReconfigurableConfig {
   )
   private int transactionToDNsCommitMapLimit = 5000000;
 
+  @Config(key = "hdds.scm.block.deleting.service.scheduling.mode",
+      defaultValue = "FIXED_RATE",
+      type = ConfigType.STRING,
+      tags = { ConfigTag.SCM, ConfigTag.DELETION },
+      description = "Scheduling mode for the block deleting service. For detailed, " +
+          "see org.apache.hadoop.hdds.utils.SchedulingMode"
+  )
+  private String blockDeletingServiceSchedulingMode = SchedulingMode.FIXED_RATE.name();

Review Comment:
   @xichen01 fixed_delay can make service run continuously if previous iteration is taking time. This is useful for case to quickly clear-up blocks running continuously, as wait time can reduce to 1 milli-second for iteration taking time.
   
   But this is not solving problem as mentioned in JIRA as can create continuous running for large block deletion as fixed delay is just 1 milli-second.



-- 
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@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13817. Add fixed-delay scheduling mode to SCMBlockDeletingService [ozone]

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #9185:
URL: https://github.com/apache/ozone/pull/9185#discussion_r2498425103


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/BackgroundService.java:
##########
@@ -116,9 +135,20 @@ public synchronized void start() {
     if (exec == null || exec.isShutdown() || exec.isTerminated()) {
       initExecutorAndThreadGroup();
     }
-    LOG.info("Starting service {} with interval {} {}", serviceName,
-        interval, unit.name().toLowerCase());
-    exec.scheduleWithFixedDelay(service, 0, interval, unit);
+    
+    if (schedulingMode == SchedulingMode.FIXED_DELAY) {
+      LOG.info("Starting service {} with fixed delay {} {} after task completion", 
+          serviceName, interval, unit.name().toLowerCase());
+      // Use minimal delay for scheduleWithFixedDelay, actual interval controlled in run()
+      exec.scheduleWithFixedDelay(service, 0, 1, TimeUnit.MILLISECONDS);

Review Comment:
   We should use the real interval and unit here.  



-- 
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@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13817. Add fixed-delay scheduling mode to SCMBlockDeletingService [ozone]

Posted by "xichen01 (via GitHub)" <gi...@apache.org>.
xichen01 commented on PR #9185:
URL: https://github.com/apache/ozone/pull/9185#issuecomment-3448710136

   > @xichen01 Thanks for the patch, overall LGTM. Can you specify with how much data and interval you are hitting this? Because ReadOnlyIterator should not take more than a few seconds to read millions of record.
   
   If the number of keys in the DB exceeds tens of millions, or even hundreds of millions, I've observed that RocksDB iterators can sometimes become very slow. 
   
   This is one example.
   
   <img width="1398" height="410" alt="image" src="https://github.com/user-attachments/assets/121da021-9813-4b59-9e6a-e01d4c340b08" />
   


-- 
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@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13817. Add fixed-delay scheduling mode to SCMBlockDeletingService [ozone]

Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on PR #9185:
URL: https://github.com/apache/ozone/pull/9185#issuecomment-3547461580

   @ChenSammi @swamirishi @xichen01 
   Existing logic have problem as below,
   - future.join() is done at begining of new task to ensure previous task is completed. So, this lead to below behavior,


-- 
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@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13817. Add fixed-delay scheduling mode to SCMBlockDeletingService [ozone]

Posted by "xichen01 (via GitHub)" <gi...@apache.org>.
xichen01 commented on PR #9185:
URL: https://github.com/apache/ozone/pull/9185#issuecomment-3558799528

   > @xichen01 Yes, its not working as expected. https://issues.apache.org/jira/browse/HDDS-6696 is created for same earlier in backlog as can see this history.
   //...
   > This will resolve the related bug and problem of no-wait for next schedule. IMO, we need not support 2 different mechanism of FIXED_RATE, And Fixed_Delay, can follow only Fixed_Delay with the fix.
   
   @sumitagrawl 
   
   Yes, this is a easy way to fix this issue, but this will directly change the behavior of all current `BackgroundService` subclasses (even though their behavior was previously buggy). My main purpose in adding these two modes was to make the changes more controllable. However, this introduces complexity.
   This is a matter of trade offs, How do you think?


-- 
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@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org