-
Notifications
You must be signed in to change notification settings - Fork 3.8k
feat: MSQ supervisors using mostFragmentedFirst policy and minor compaction can scale taskCount down for minor compactions #19412
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
2332d3f
e40c45d
745b17f
e3ba963
9e91fbd
17dd69c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,6 +32,7 @@ | |
| import org.apache.druid.client.indexing.ClientCompactionTaskGranularitySpec; | ||
| import org.apache.druid.client.indexing.ClientCompactionTaskQuery; | ||
| import org.apache.druid.client.indexing.ClientCompactionTaskQueryTuningConfig; | ||
| import org.apache.druid.client.indexing.ClientMSQContext; | ||
| import org.apache.druid.client.indexing.ClientMinorCompactionInputSpec; | ||
| import org.apache.druid.common.guava.FutureUtils; | ||
| import org.apache.druid.common.utils.IdUtils; | ||
|
|
@@ -41,6 +42,7 @@ | |
| import org.apache.druid.java.util.common.granularity.Granularity; | ||
| import org.apache.druid.java.util.common.granularity.GranularityType; | ||
| import org.apache.druid.java.util.common.logger.Logger; | ||
| import org.apache.druid.query.QueryContext; | ||
| import org.apache.druid.query.SegmentDescriptor; | ||
| import org.apache.druid.query.aggregation.AggregatorFactory; | ||
| import org.apache.druid.rpc.indexing.OverlordClient; | ||
|
|
@@ -294,6 +296,17 @@ private int submitCompactionTasks( | |
| return numSubmittedTasks; | ||
| } | ||
|
|
||
| /** | ||
| * Default percent applied to {@code maxNumTasks} for MSQ minor compactions | ||
| * when the supervisor's {@code taskContext} does not specify | ||
| * {@link ClientMSQContext#CTX_MINOR_COMPACTION_TASK_PERCENT}. The default of | ||
| * 100 disables scaling, matching the opt-in shape of the minor compaction | ||
| * feature itself (which is gated by non-zero | ||
| * {@code minUncompactedBytesPercentForFullCompaction} or | ||
| * {@code minUncompactedRowsPercentForFullCompaction} on the policy). | ||
| */ | ||
| public static final int DEFAULT_MINOR_COMPACTION_TASK_PERCENT = 100; | ||
|
|
||
| /** | ||
| * Creates a {@link ClientCompactionTaskQuery} which can be submitted to an | ||
| * {@link OverlordClient} to start a compaction task. | ||
|
|
@@ -448,6 +461,56 @@ public Map<String, AutoCompactionSnapshot> getAutoCompactionSnapshot() | |
| return autoCompactionSnapshotPerDataSource.get(); | ||
| } | ||
|
|
||
| /** | ||
| * Reduces {@code maxNumTasks} on the task context for MSQ minor compactions | ||
| * by the percent specified under | ||
| * {@link ClientMSQContext#CTX_MINOR_COMPACTION_TASK_PERCENT} in the same | ||
| * context, defaulting to {@link #DEFAULT_MINOR_COMPACTION_TASK_PERCENT} when | ||
| * absent. The scaled value is floored at {@link ClientMSQContext#DEFAULT_MAX_NUM_TASKS} | ||
| * (the MSQ minimum of 1 controller + 1 worker). No-op when the engine is | ||
| * native, the mode is full, or the percent is 100. | ||
| * | ||
| * <p>The percent is validated at config-acceptance time by | ||
| * {@link ClientCompactionRunnerInfo#validateMinorCompactionTaskPercentForMSQ}, | ||
| * so an out-of-range value here indicates a bug. | ||
| */ | ||
| private static void maybeScaleMaxNumTasksForMinorCompaction( | ||
| Map<String, Object> context, | ||
| CompactionMode compactionMode, | ||
| ClientCompactionRunnerInfo compactionRunner | ||
| ) | ||
| { | ||
| if (compactionMode != CompactionMode.UNCOMPACTED_SEGMENTS_ONLY | ||
| || !CompactionEngine.MSQ.equals(compactionRunner.getType())) { | ||
| return; | ||
| } | ||
|
|
||
| final int percent = QueryContext.of(context).getInt( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Validate minorCompactionTaskPercent when accepting the compaction config The new context key is only parsed and range-checked while creating a minor MSQ compaction task. Supervisor config validation still accepts values like 0, 200, or a non-numeric string, so the API can persist an invalid supervisor and it will later fail job creation repeatedly once a minor compaction candidate appears. Please add validation alongside the existing MSQ maxNumTasks validation paths, including CascadingReindexingTemplate if applicable, so bad configs are rejected before scheduling. |
||
| ClientMSQContext.CTX_MINOR_COMPACTION_TASK_PERCENT, | ||
| DEFAULT_MINOR_COMPACTION_TASK_PERCENT | ||
| ); | ||
| if (percent < 1 || percent > 100) { | ||
| throw DruidException.defensive( | ||
| "'%s'[%d] must be between 1 and 100; should have been rejected at config-acceptance time", | ||
| ClientMSQContext.CTX_MINOR_COMPACTION_TASK_PERCENT, | ||
| percent | ||
| ); | ||
| } | ||
| if (percent == 100) { | ||
| return; | ||
| } | ||
|
|
||
| final int originalMaxNumTasks = QueryContext.of(context).getInt( | ||
| ClientMSQContext.CTX_MAX_NUM_TASKS, | ||
| ClientMSQContext.DEFAULT_MAX_NUM_TASKS | ||
| ); | ||
| final int scaledMaxNumTasks = Math.max( | ||
| ClientMSQContext.DEFAULT_MAX_NUM_TASKS, | ||
| (int) Math.round(originalMaxNumTasks * percent / 100.0) | ||
| ); | ||
| context.put(ClientMSQContext.CTX_MAX_NUM_TASKS, scaledMaxNumTasks); | ||
| } | ||
|
|
||
| private static ClientCompactionTaskQuery compactSegments( | ||
| CompactionCandidate entry, | ||
| Eligibility eligibility, | ||
|
|
@@ -480,6 +543,8 @@ private static ClientCompactionTaskQuery compactSegments( | |
| context.put(COMPACTION_POLICY_RESULT, eligibility.getReason()); | ||
| } | ||
|
|
||
| maybeScaleMaxNumTasksForMinorCompaction(context, compactionMode, compactionRunner); | ||
|
|
||
| String taskIdPrefix = compactionMode == CompactionMode.UNCOMPACTED_SEGMENTS_ONLY | ||
| ? TASK_ID_PREFIX + "-minor" | ||
| : TASK_ID_PREFIX; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Reject fractional minorCompactionTaskPercent values
This validation uses
QueryContext.getInt, which accepts anyNumberby callingintValue(). JSON values like50.9deserialize asDoubleand are silently truncated to50, so malformed configs can be persisted despite the new contract requiring an integer between 1 and 100. Please reject non-integralNumbervalues, or parse with exact integer conversion, before accepting the config.