xiangfu0 commented on code in PR #20036:
URL: https://github.com/apache/pinot/pull/20036#discussion_r4430647429
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/generator/PinotTaskGenerator.java:
##########
@@ -97,6 +98,16 @@ default String getMinionInstanceTag(TableConfig tableConfig)
{
return CommonConstants.Helix.UNTAGGED_MINION_INSTANCE;
}
+ /// Validates the effective minion instance tag before scheduled task
generation.
+ /// The effective tag includes any scheduling request override. The default
implementation permits every tag.
+ /// Implementations must only validate: scheduling paths may invoke this
hook more than once.
+ ///
+ /// @param tableConfig table whose tasks are being scheduled
+ /// @param minionInstanceTag effective tag, which may be null
+ /// @throws IllegalArgumentException if the tag is incompatible with the
table's tasks
Review Comment:
A rejection here does more than skip the table. If every table of a task
type is rejected, `scheduleTask` throws, and `doScheduleTasks` rethrows that,
so a multi-type `POST /tasks/schedule?tableName=T&minionInstanceTag=X` (no
`taskType`) fails as a whole. In a local repro with a lax type and a strict
type on one table, the lax task was submitted first. The strict rejection then
turned the request into a `RuntimeException`, and the lax task name was dropped
from the response. With the opposite HashMap order, the lax type would never be
scheduled. The mechanism already existed, but this hook makes it reachable
through a routine override. Consider documenting this in `@throws`, or
recording a rejection as a per-table skip rather than a type-level failure.
AI-generated review via IntentLab ยท Model: Claude Opus 5.5 (claude-opus-5-5)
--
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]
