krishan1390 commented on code in PR #16931:
URL: https://github.com/apache/pinot/pull/16931#discussion_r2401088875


##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java:
##########
@@ -156,10 +156,15 @@ public abstract class BaseTableDataManager implements 
TableDataManager {
   protected volatile boolean _shutDown;
   protected volatile boolean _isDeleted;
 
+  // Executor service to process segment refresh if asynchronous handling is 
enabled
+  @Nullable
+  protected ExecutorService _segmentRefreshExecutor;

Review Comment:
   I think all existing tests will have _segmentRefreshExecutor disabled. So we 
need a test where this is enabled and we validate that segments are eventually 
refreshed. 



##########
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerConfig.java:
##########
@@ -252,6 +257,12 @@ public int getMaxParallelRefreshThreads() {
     return _serverConfig.getProperty(MAX_PARALLEL_REFRESH_THREADS, 1);
   }
 
+  @Override
+  public boolean isEnableSegmentRefreshAsynchronousHandling() {

Review Comment:
   can we enable this config without a server restart ? 



##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/InstanceDataManager.java:
##########
@@ -169,6 +169,11 @@ void reloadSegments(String tableNameWithType, List<String> 
segmentNames, boolean
    */
   int getMaxParallelRefreshThreads();
 
+  /**
+   * Returns true if background processing for SEGMENT_REFRESH is enabled, 
false otherwise

Review Comment:
   are there any valid cases where we wouldn't want the feature to be enabled ? 
if yes, would be useful to document that



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to