tibrewalpratik17 commented on code in PR #12960:
URL: https://github.com/apache/pinot/pull/12960#discussion_r1604868126
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotTaskManager.java:
##########
@@ -89,6 +89,8 @@ public class PinotTaskManager extends
ControllerPeriodicTask<Void> {
public final static String LEAD_CONTROLLER_MANAGER_KEY =
"LeadControllerManager";
public final static String SCHEDULE_KEY = "schedule";
public final static String MINION_INSTANCE_TAG_CONFIG = "minionInstanceTag";
+ public final static String MINION_ALLOW_DOWNLOAD_FROM_SERVER =
"allowDownloadFromServer";
+ public final static boolean DEFAULT_MINION_ALLOW_DOWNLOAD_FROM_SERVER =
false;
Review Comment:
Keeping it true may affect other custom tasks folks might have written which
might be okay to fail but impact servers a lot if server starts to tar. Maybe
we can make it default true as a cluster level config and keep option of
overriding to false at task level. Wdyt? I'd still suggest to do in a follow-up
so that we can get this feature in as a separate patch and in a follow-up we
can introduce a minion-cluster level config (gives option of easy to revert
that particular patch).
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/generator/BaseTaskGenerator.java:
##########
@@ -131,4 +140,45 @@ public void generateTasks(List<TableConfig> tableConfigs,
List<PinotTaskConfig>
public String getMinionInstanceTag(TableConfig tableConfig) {
return TaskGeneratorUtils.extractMinionInstanceTag(tableConfig,
getTaskType());
}
+
+ @Override
+ public boolean isAllowDownloadFromServer(TableConfig tableConfig) {
+ return
TaskGeneratorUtils.extractMinionAllowDownloadFromServer(tableConfig,
getTaskType());
+ }
+
+ public List<URI> getSegmentServerURIs(TableConfig tableConfig, String
segmentName) {
+ // TODO add validation check if allowDownloadFromServer is enabled then
peer segment download scheme should be set
Review Comment:
yeah my bad! i kept it for my tracking, forgot to cleanup. Let me add.
--
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]
