siddharthteotia commented on code in PR #8694:
URL: https://github.com/apache/pinot/pull/8694#discussion_r871985051


##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java:
##########
@@ -72,6 +73,8 @@ public abstract class BaseTableDataManager implements 
TableDataManager {
   private static final Logger LOGGER = 
LoggerFactory.getLogger(BaseTableDataManager.class);
 
   protected final ConcurrentHashMap<String, SegmentDataManager> 
_segmentDataManagerMap = new ConcurrentHashMap<>();
+  // Semaphore to restrict the maximum number of parallel segment downloads 
for a table.
+  private Semaphore _segmentDownloadSemaphore;

Review Comment:
   My suggestion was not to reuse the same `object/instance` (which will be 
problematic like you mentioned0 but the implementation instead and instantiate 
it here with the required number of permits for download because generically, 
semaphore just needs those 2 configs and can be instantiated separately by each 
different user for different purpose.
   
   But this is not super necessary. The current approach is fine 



-- 
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