vvivekiyer commented on code in PR #12786:
URL: https://github.com/apache/pinot/pull/12786#discussion_r1550933003
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotTaskManager.java:
##########
@@ -485,23 +485,24 @@ public void registerTaskGenerator(PinotTaskGenerator
taskGenerator) {
* Returns a map from the task type to the list of tasks scheduled.
*/
public synchronized Map<String, List<String>> scheduleTasks() {
- return
scheduleTasks(_pinotHelixResourceManager.getAllTables(CommonConstants.DEFAULT_DATABASE),
false);
+ return
scheduleTasks(_pinotHelixResourceManager.getAllTables(CommonConstants.DEFAULT_DATABASE),
false, null);
}
/**
* Public API to schedule tasks (all task types) for all tables in given
database.
* It might be called from the non-leader controller.
* Returns a map from the task type to the list of tasks scheduled.
*/
- public synchronized Map<String, List<String>>
scheduleTasksForDatabase(String database) {
- return scheduleTasks(_pinotHelixResourceManager.getAllTables(database),
false);
+ public synchronized Map<String, List<String>>
scheduleTasksForDatabase(String database, String minionInstanceTag) {
Review Comment:
Add `@Nullable` annotation here and in other places.
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotTaskManager.java:
##########
@@ -624,7 +626,15 @@ private List<String> scheduleTask(PinotTaskGenerator
taskGenerator, List<TableCo
* controller. Returns a map from the task type to the list of tasks
scheduled.
*/
public synchronized Map<String, List<String>> scheduleTasks(String
tableNameWithType) {
- return scheduleTasks(Collections.singletonList(tableNameWithType), false);
+ return scheduleTasks(Collections.singletonList(tableNameWithType), false,
null);
+ }
+
+ /**
+ * Public API to schedule tasks (all task types) for the given table on a
specific instance tag.
+ * It might be called from the non-leader controller. Returns a map from the
task type to the list of tasks scheduled.
+ */
+ public synchronized Map<String, List<String>> scheduleTasks(String
tableNameWithType, String minionInstanceTag) {
Review Comment:
Can we not add minionInstanceTag to the public method above? It's only
invoked from PinotTaskRestletResource and a bunch of tests.
IMO, there are already too many scheduleTask methods making this code
difficult to read.
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
