(pinot) branch master updated: Split reload and refresh executor wrappers in BaseTableDataManager (#18312)

[xi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

xiangfu0 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new 9581549d548 Split reload and refresh executor wrappers in BaseTableDataManager (#18312)
9581549d548 is described below

commit 9581549d548a3124ef856fde9963d4873c7cc778
Author: Jhow <44...@users.noreply.github.com>
AuthorDate: Sun Apr 26 15:02:39 2026 -0700

    Split reload and refresh executor wrappers in BaseTableDataManager (#18312)
    
    * Split reload and refresh executor wrappers in BaseTableDataManager
    
    Reload and refresh now use separately-wrapped ExecutorService fields
    backed by the same underlying thread pool. Separation lets subclasses
    decorate one path without affecting the other (e.g. to add cancellation
    or tracing on reload without intercepting async refresh submissions).
    
    - Add SegmentOperationsTaskType.RELOAD and REFRESH (split from the
      combined REFRESH_OR_RELOAD).
    - Route refresh submissions through _segmentRefreshExecutor and reload
      submissions through _segmentReloadExecutor.
    
    * docs: comment
    
    * docs: add back the deprecated task type
    
    * legacy _segmentReloadRefreshExecutor
---
 .../pinot/core/data/manager/BaseTableDataManager.java | 19 +++++++++++++++----
 .../core/data/manager/SegmentOperationsTaskType.java  |  9 +++++++++
 ...seTableDataManagerEnqueueSegmentToReplaceTest.java |  4 ++--
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java b/pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java
index e35ad1a8ddf..9bf3d151cf4 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java
@@ -142,7 +142,14 @@ public abstract class BaseTableDataManager implements TableDataManager {
   protected File _resourceTmpDir;
   protected Logger _logger;
   protected SegmentReloadSemaphore _segmentReloadSemaphore;
+  /**
+   * @deprecated Use {@link #_segmentReloadExecutor} or {@link #_segmentRefreshExecutor} instead. Kept for binary
+   * compatibility with downstream extensions; refers to the shared underlying executor pool.
+   */
+  @Deprecated
   protected ExecutorService _segmentReloadRefreshExecutor;
+  protected ExecutorService _segmentReloadExecutor;
+  protected ExecutorService _segmentRefreshExecutor;
   @Nullable
   protected ExecutorService _segmentPreloadExecutor;
   protected AuthProvider _authProvider;
@@ -188,8 +195,12 @@ public abstract class BaseTableDataManager implements TableDataManager {
     _propertyStore = helixManager.getHelixPropertyStore();
     _segmentLocks = segmentLocks;
     _segmentReloadSemaphore = segmentReloadSemaphore;
-    _segmentReloadRefreshExecutor = new SegmentOperationsExecutorService(segmentReloadRefreshExecutor,
-        SegmentOperationsTaskType.REFRESH_OR_RELOAD, tableConfig.getTableName());
+    _segmentReloadRefreshExecutor = segmentReloadRefreshExecutor;
+    // These two executors are using the same underlying thread pool, only that they're wrapped with different decorator
+    _segmentReloadExecutor = new SegmentOperationsExecutorService(segmentReloadRefreshExecutor,
+        SegmentOperationsTaskType.RELOAD, tableConfig.getTableName());
+    _segmentRefreshExecutor = new SegmentOperationsExecutorService(segmentReloadRefreshExecutor,
+        SegmentOperationsTaskType.REFRESH, tableConfig.getTableName());
     _segmentPreloadExecutor = segmentPreloadExecutor != null
         ? new SegmentOperationsExecutorService(segmentPreloadExecutor, SegmentOperationsTaskType.PRELOAD,
         tableConfig.getTableName())
@@ -498,7 +509,7 @@ public abstract class BaseTableDataManager implements TableDataManager {
       return;
     }
     _logger.info("Enqueuing segment: {} to be replaced", segmentName);
-    _segmentReloadRefreshExecutor.submit(() -> {
+    _segmentRefreshExecutor.submit(() -> {
       try {
         replaceSegmentInternal(segmentName);
       } catch (Exception e) {
@@ -948,7 +959,7 @@ public abstract class BaseTableDataManager implements TableDataManager {
           _reloadJobStatusCache.recordFailure(reloadJobId, segmentName, t);
         }
       }
-    }, _segmentReloadRefreshExecutor)).toArray(CompletableFuture[]::new)).get();
+    }, _segmentReloadExecutor)).toArray(CompletableFuture[]::new)).get();
     if (sampleException.get() != null) {
       throw new RuntimeException(
           String.format("Failed to reload %d/%d segments: %s in table: %s", failedSegments.size(),
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/data/manager/SegmentOperationsTaskType.java b/pinot-core/src/main/java/org/apache/pinot/core/data/manager/SegmentOperationsTaskType.java
index a97248e8bb7..abc602cf1cf 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/data/manager/SegmentOperationsTaskType.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/data/manager/SegmentOperationsTaskType.java
@@ -28,6 +28,15 @@ package org.apache.pinot.core.data.manager;
 public class SegmentOperationsTaskType {
   public static final SegmentOperationsTaskType CONSUMER =
       new SegmentOperationsTaskType("CONSUMER");
+  public static final SegmentOperationsTaskType REFRESH =
+      new SegmentOperationsTaskType("REFRESH");
+  public static final SegmentOperationsTaskType RELOAD =
+      new SegmentOperationsTaskType("RELOAD");
+  /**
+   * @deprecated Use {@link #REFRESH} or {@link #RELOAD} instead. Segment refresh and reload operations are now
+   * tracked as separate task types for independent throttling.
+   */
+  @Deprecated
   public static final SegmentOperationsTaskType REFRESH_OR_RELOAD =
       new SegmentOperationsTaskType("REFRESH_OR_RELOAD");
   public static final SegmentOperationsTaskType PRELOAD =
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/data/manager/BaseTableDataManagerEnqueueSegmentToReplaceTest.java b/pinot-core/src/test/java/org/apache/pinot/core/data/manager/BaseTableDataManagerEnqueueSegmentToReplaceTest.java
index 4bda671eb65..97fbe88e48a 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/data/manager/BaseTableDataManagerEnqueueSegmentToReplaceTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/data/manager/BaseTableDataManagerEnqueueSegmentToReplaceTest.java
@@ -236,7 +236,7 @@ public class BaseTableDataManagerEnqueueSegmentToReplaceTest {
     }).when(spyExecutor).submit(any(Runnable.class));
 
     // Replace the executor in the table data manager
-    _tableDataManager._segmentReloadRefreshExecutor = spyExecutor;
+    _tableDataManager._segmentRefreshExecutor = spyExecutor;
 
     // Test the method
     _tableDataManager.replaceSegment(SEGMENT_NAME);
@@ -262,7 +262,7 @@ public class BaseTableDataManagerEnqueueSegmentToReplaceTest {
   @Test
   public void testExecutorUsage() throws Exception {
     ExecutorService mockExecutor = mock(ExecutorService.class);
-    _tableDataManager._segmentReloadRefreshExecutor = mockExecutor;
+    _tableDataManager._segmentRefreshExecutor = mockExecutor;
 
     // Test the method
     _tableDataManager.replaceSegment(SEGMENT_NAME);


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org