FrankChen021 commented on code in PR #20579:
URL: https://github.com/apache/druid/pull/20579#discussion_r4271609664
##########
server/src/main/java/org/apache/druid/server/compaction/CompactionSlotManager.java:
##########
@@ -304,9 +306,7 @@ public static int getMaxTaskSlotsForNativeCompactionTask(
*/
public static int getMaxTaskSlotsForMSQCompactionTask(@Nullable Map<String,
Object> context)
{
- return context == null
- ? ClientMSQContext.DEFAULT_MAX_NUM_TASKS
- : (int) context.getOrDefault(ClientMSQContext.CTX_MAX_NUM_TASKS,
ClientMSQContext.DEFAULT_MAX_NUM_TASKS);
+ return
QueryContext.of(context).getOrDefault(QueryContextParameters.MAX_NUM_TASKS);
Review Comment:
[P2] Do not under-reserve slots for invalid maxNumTasks
**Finding:** After removing the descriptor lower-bound constraint, this
helper returns 0, 1, or a negative maxNumTasks directly. It is used while
reserving slots for running or submitted compaction tasks, so an invalid value
can under-reserve capacity—and a negative value can increase available
capacity—allowing the coordinator to overschedule before task validation
rejects the task.
**Suggestion:** Keep an equivalent local guard and parse-error fallback that
substitutes the default task count until compaction validation has rejected the
invalid task.
##########
server/src/main/java/org/apache/druid/client/indexing/ClientCompactionRunnerInfo.java:
##########
@@ -242,8 +243,7 @@ public static CompactionConfigValidationResult
validateRollupForMSQ(
public static CompactionConfigValidationResult
validateMaxNumTasksForMSQ(Map<String, Object> context)
{
if (context != null) {
- int maxNumTasks = QueryContext.of(context)
-
.getInt(ClientMSQContext.CTX_MAX_NUM_TASKS,
ClientMSQContext.DEFAULT_MAX_NUM_TASKS);
+ final int maxNumTasks =
QueryContext.of(context).getOrDefault(QueryContextParameters.MAX_NUM_TASKS);
Review Comment:
[P2] Preserve validation results for malformed maxNumTasks
**Finding:** Values such as 1.5, a non-numeric string, or an out-of-range
integer still make QueryContextParameter.parse throw BadQueryContextException.
This method is used by compaction supervisor and task validation, which expect
a CompactionConfigValidationResult, so malformed task contexts now escape as
exceptions instead of producing the existing invalid-spec diagnostic.
**Suggestion:** Retain the BadQueryContextException boundary here and
convert parse failures into a failed CompactionConfigValidationResult as before.
--
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]
