refactor: make supervisor diff+restart algorithm smarter on submission by jtuglu1 · Pull Request #19541 · apache/druid

jtuglu1 · 2026-06-01T22:21:22Z

Description

Restarting a supervisor spec after submission sometimes isn't always needed and can be expensive for large supervisors. The current skip restart logic is very bare-bones (doing effectively a byte-level check between the 2 specs) instead of allowing each spec to override its own restart requirements, often causing some false positive restarts. This diff can also be affected by false-positive diffs like ioConfig.taskCount being updated while auto-scaling is configured, whitespace changes, etc.

This updates the restart-required checking logic by allowing supervisors to implement their own restart requirement checks (plus some sane defaults) along with proper comparators, and stop treating each supervisor spec as an opaque ball of text. Also adds builders to keep things clean.

Notably, this doesn't change the spec persistence behavior: byte-level differing spec updates are persisted into the DB, but may not necessarily invoke a supervisor restart.

Release note

Make supervisor diff+restart algorithm smarter on submission

This PR has:

…sion

Copilot

Pull request overview

This PR refactors supervisor “diff + restart” behavior so that supervisor specs can decide whether a changed submission actually requires a restart, while still persisting byte-level spec changes to metadata storage even when a restart is skipped. It introduces a SupervisorSpec.requireRestart(old) hook, implements smarter restart logic for seekable-stream supervisors, and adds IOConfig/spec builders plus equality semantics to support structured comparisons rather than opaque JSON byte diffs.

Changes:

Add SupervisorSpec.requireRestart(SupervisorSpec old) (default: restart on any non-identical change) and use it from SupervisorManager.shouldUpdateSupervisor.
Persist spec updates even when restart is skipped via SupervisorManager.updateSupervisorSpecWithoutRestart, wired into SupervisorResource POST handling.
Introduce/extend builders and equals/hashCode implementations across seekable-stream specs/configs (Kafka/Kinesis/RabbitStream) and update tests accordingly.

Reviewed changes

Copilot reviewed 34 out of 34 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
server/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorSpec.java	Adds `requireRestart(old)` hook to let specs participate in restart decisions.
indexing-service/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorManager.java	Splits “spec changed” vs “restart required”, adds no-restart persistence path.
indexing-service/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorResource.java	Persists changed specs even when restart is skipped.
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorSpec.java	Implements structured restart decision + adds `toBuilder()` and spec equality/hash.
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorIOConfig.java	Adds `equals/hashCode` and `toBuilder()` for IOConfig comparisons/copying.
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorIngestionSpec.java	Adds `equals/hashCode` so spec equality can be structured.
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SupervisorIOConfigBuilder.java	Enhances builder (copyFromBase, more fields) and adds a default IOConfig builder.
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/LagAggregator.java	Makes default lag aggregator comparable across serde (stateless equality).
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/IdleConfig.java	Adds `equals/hashCode` for structured comparisons.
extensions-core/kinesis-indexing-service/src/main/java/org/apache/druid/indexing/kinesis/supervisor/KinesisSupervisorSpec.java	Adds `toBuilder()` to support restart comparison/copy patterns.
extensions-core/kinesis-indexing-service/src/main/java/org/apache/druid/indexing/kinesis/supervisor/KinesisSupervisorIOConfig.java	Adds `equals/hashCode` and `toBuilder()` for IOConfig comparisons/copying.
extensions-core/kinesis-indexing-service/src/main/java/org/apache/druid/indexing/kinesis/supervisor/KinesisSupervisorTuningConfig.java	Adds `equals/hashCode` to ensure tuning changes affect comparisons.
extensions-core/kinesis-indexing-service/src/main/java/org/apache/druid/indexing/kinesis/supervisor/KinesisIOConfigBuilder.java	New builder for Kinesis IOConfig with copyFrom/build support.
extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisorSpec.java	Adds `toBuilder()` to support restart comparison/copy patterns.
extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisorIOConfig.java	Adds `equals/hashCode` and `toBuilder()` for IOConfig comparisons/copying.
extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisorTuningConfig.java	Adds `equals/hashCode` to ensure tuning changes affect comparisons.
extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/supervisor/KafkaIOConfigBuilder.java	Expands builder (copyFrom + additional fields), relies on base bounded/serverPriority fields.
extensions-contrib/rabbit-stream-indexing-service/src/main/java/org/apache/druid/indexing/rabbitstream/supervisor/RabbitStreamSupervisorSpec.java	Adds `toBuilder()` to support restart comparison/copy patterns.
extensions-contrib/rabbit-stream-indexing-service/src/main/java/org/apache/druid/indexing/rabbitstream/supervisor/RabbitStreamSupervisorIOConfig.java	Adds `equals/hashCode` and `toBuilder()` for IOConfig comparisons/copying.
extensions-contrib/rabbit-stream-indexing-service/src/main/java/org/apache/druid/indexing/rabbitstream/supervisor/RabbitStreamSupervisorTuningConfig.java	Adds `equals/hashCode` to ensure tuning changes affect comparisons.
extensions-contrib/rabbit-stream-indexing-service/src/main/java/org/apache/druid/indexing/rabbitstream/supervisor/RabbitStreamIOConfigBuilder.java	New builder for RabbitStream IOConfig with copyFrom/build support.
indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorTestBase.java	Updates test spec to support `toBuilder()` and uses IOConfig builders.
indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorStateTest.java	Replaces inline IOConfig anonymous classes with builders for stable equality.
indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorSpecTest.java	Adds `requireRestart` tests; updates IOConfig construction to builder pattern.
indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorIOConfigTest.java	Adds equality/hashCode tests for IOConfig + related value types.
indexing-service/src/test/java/org/apache/druid/indexing/overlord/supervisor/SupervisorResourceTest.java	Adjusts POST expectations to include no-restart persistence call.
indexing-service/src/test/java/org/apache/druid/indexing/overlord/supervisor/SupervisorManagerTest.java	Adds tests for `updateSupervisorSpecWithoutRestart` and introduces versioned test spec.
extensions-core/kinesis-indexing-service/src/test/java/org/apache/druid/indexing/kinesis/supervisor/KinesisSupervisorTest.java	Updates tests to use new Kinesis IOConfig builder.
extensions-core/kinesis-indexing-service/src/test/java/org/apache/druid/indexing/kinesis/supervisor/KinesisSupervisorIOConfigTest.java	Adds equality/hashCode tests and builder-based construction.
extensions-core/kinesis-indexing-service/src/test/java/org/apache/druid/indexing/kinesis/KinesisSamplerSpecTest.java	Updates sampler tests to use Kinesis IOConfig builder.
extensions-core/kafka-indexing-service/src/test/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisorTest.java	Updates tests to use Kafka IOConfig builder.
extensions-core/kafka-indexing-service/src/test/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisorIOConfigTest.java	Adds equality/hashCode tests and builder-based construction.
extensions-contrib/rabbit-stream-indexing-service/src/test/java/org/apache/druid/indexing/rabbitstream/supervisor/RabbitStreamSupervisorTest.java	Updates tests to use RabbitStream IOConfig builder.
extensions-contrib/rabbit-stream-indexing-service/src/test/java/org/apache/druid/indexing/rabbitstream/supervisor/RabbitStreamSupervisorIOConfigTest.java	Adds equality/hashCode tests and builder-based construction.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

FrankChen021

Severity	Findings
P0	0
P1	1
P2	2
P3	0
Total	3

Severity	Findings
P0	0
P1	1
P2	2
P3	0
Total	3

Reviewed 34 of 34 changed files.

This is an automated review by Codex GPT-5.5

FrankChen021

Severity	Findings
P0	0
P1	0
P2	2
P3	0
Total	2

Reviewed 36 of 36 changed files.

This is an automated review by Codex GPT-5.5

cecemei

overall looks good to me, left a few minor comments

FrankChen021

I have reviewed the code for correctness, edge cases, concurrency, and integration risks; no issues found.

Reviewed 38 of 38 changed files.

This is an automated review by Codex GPT-5.5

kfaraz

Thanks for the PR, @jtuglu1 ! Overall makes sense, left some suggestions.

kfaraz · 2026-06-12T07:23:15Z

+   */
+  public SupervisorIOConfigBuilder<?, ?> toBuilder()
+  {
+    return new SupervisorIOConfigBuilder.DefaultSupervisorIOConfigBuilder().copyFromBase(this);


If sub-classes must always override this method, we should make it abstract.

kfaraz · 2026-06-12T07:32:02Z

      return new LagStats(maxLag, totalLag, avgLag);
    }
+
+    // Stateless: all instances are equal. Needed because Jackson creates fresh instances on


Alternatively, you can make the constructor private and add a static @JsonCreator method which just returns the DEFAULT instance.

kfaraz · 2026-06-12T07:39:35Z

+   */
+  public Builder<?> toBuilder()
+  {
+    throw new UnsupportedOperationException("No builder is available for this supervisor spec.");


Can this method be abstract instead?

kfaraz · 2026-06-12T07:43:59Z

+      this.taskStorage = spec.taskStorage;
+      this.taskMaster = spec.taskMaster;
+      this.indexerMetadataStorageCoordinator = spec.indexerMetadataStorageCoordinator;
+      this.indexTaskClientFactory = spec.indexTaskClientFactory;
+      this.mapper = spec.mapper;
+      this.emitter = spec.emitter;
+      this.monitorSchedulerConfig = spec.monitorSchedulerConfig;
+      this.rowIngestionMetersFactory = spec.rowIngestionMetersFactory;
+      this.supervisorStateManagerConfig = spec.supervisorStateManagerConfig;


Will the builder ever use these dependencies?
The output of the build() method will only ever be used for an equality check which should not involve the services anyway. So the build() method could simply pass these as null, atleast for now.

kfaraz · 2026-06-12T08:20:13Z

    synchronized (lock) {
      Preconditions.checkState(started, "SupervisorManager not started");
-      try {
-        byte[] specAsBytes = jsonMapper.writeValueAsBytes(spec);


Maybe rename this method from shouldUpdateSupervisor to shouldRestartSupervisor to avoid ambiguity.

Alternatively, add a new method shouldRestartSupervisor and leave shouldUpdateSupervisor unchanged.

FrankChen021

Severity	Findings
P0	0
P1	0
P2	1
P3	0
Total	1

Reviewed 43 of 43 changed files.

This is an automated review by Codex GPT-5.5

FrankChen021

I have reviewed the code for correctness, edge cases, concurrency, and integration risks; no issues found.

Reviewed 43 of 43 changed files.

This is an automated review by Codex GPT-5.5

FrankChen021

Severity	Findings
P0	0
P1	0
P2	1
P3	0
Total	1

Found 1 issue.

Reviewed 43 of 43 changed files.

This is an automated review by Codex GPT-5.5

FrankChen021 · 2026-06-30T14:08:34Z

+      // The effective (merged) spec is what will be persisted, so validate that transition exactly once.
+      current.rhs.validateSpecUpdateTo(spec);
+
+      if (!current.rhs.requireRestart(spec)) {


[P2] No-restart task-count updates leave the live supervisor stale

When an autoscaling supervisor is resubmitted with only an effective taskCount change, for example after autoscaling has moved the current ioConfig.taskCount to 5 and the new POST is merged back to taskCountStart 2, SeekableStreamSupervisorSpec.requireRestart masks taskCount differences whenever either side has autoscaling enabled. This branch then persists the new spec and swaps only the manager's stored spec, but it does not restart current.lhs or update the running SeekableStreamSupervisor's ioConfig. The API can return modified=true,restarted=false while ingestion continues using the old taskCount until a later autoscaler action or Overlord restart. Treat this reset as restart-required, or explicitly propagate the new taskCount into the running supervisor before taking the no-restart path.

FrankChen021

I have reviewed the updated changes for correctness, edge cases, concurrency, and integration risks; no issues found.

Reviewed 44 of 44 changed files.

This is an automated review by Codex GPT-5.5

github-actions Bot added Area - Streaming Ingestion Area - Ingestion labels Jun 1, 2026

jtuglu1 force-pushed the fix-supervisor-restart-algo-on-submission branch from b6ae435 to d3e9127 Compare June 1, 2026 22:22

abhishekrb19 requested a review from aho135 June 1, 2026 22:41

github-advanced-security AI found potential problems Jun 1, 2026

View reviewed changes

Comment thread ...n/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorSpec.java Fixed

jtuglu1 force-pushed the fix-supervisor-restart-algo-on-submission branch 2 times, most recently from 1edda80 to b18c3b3 Compare June 1, 2026 23:08

github-advanced-security AI found potential problems Jun 1, 2026

View reviewed changes

Comment thread ...va/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorSpecTest.java Fixed

jtuglu1 force-pushed the fix-supervisor-restart-algo-on-submission branch 2 times, most recently from d773ffa to 82dc32b Compare June 2, 2026 02:25

refactor: make supervisor diff+restart algorithm smarter on re-submis…

fa855ef

…sion

jtuglu1 force-pushed the fix-supervisor-restart-algo-on-submission branch from 82dc32b to fa855ef Compare June 2, 2026 03:19

kfaraz requested a review from Copilot June 2, 2026 07:33

Copilot started reviewing on behalf of kfaraz June 2, 2026 07:33 View session

Copilot AI reviewed Jun 2, 2026

View reviewed changes

FrankChen021 reviewed Jun 2, 2026

View reviewed changes

more stuff

835a2ac

FrankChen021 reviewed Jun 3, 2026

View reviewed changes

Comment thread ...g-service/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorManager.java Outdated

Comment thread ...n/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorSpec.java Outdated

cecemei reviewed Jun 3, 2026

View reviewed changes

jtuglu1 force-pushed the fix-supervisor-restart-algo-on-submission branch from 19edbf8 to 5168809 Compare June 3, 2026 22:30

FrankChen021 reviewed Jun 4, 2026

View reviewed changes

jtuglu1 added this to the 38.0.0 milestone Jun 5, 2026

jtuglu1 requested a review from cecemei June 5, 2026 22:23

jtuglu1 mentioned this pull request Jun 8, 2026

feat: introduce stopGracefullyOnNewSpec for Supervisors #19568

Closed

2 tasks

kfaraz reviewed Jun 12, 2026

View reviewed changes

jtuglu1 force-pushed the fix-supervisor-restart-algo-on-submission branch 2 times, most recently from 76ad757 to 4948912 Compare June 16, 2026 07:22

FrankChen021 reviewed Jun 16, 2026

View reviewed changes

Comment thread ...service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/LagAggregator.java Outdated

jtuglu1 requested review from FrankChen021 and kfaraz June 18, 2026 03:34

more stuff

ef7bb59

jtuglu1 force-pushed the fix-supervisor-restart-algo-on-submission branch from 4948912 to ef7bb59 Compare June 18, 2026 04:02

FrankChen021 reviewed Jun 18, 2026

View reviewed changes

FrankChen021 reviewed Jun 30, 2026

View reviewed changes

more cleanup

f16d41b

jtuglu1 force-pushed the fix-supervisor-restart-algo-on-submission branch from c932e9c to f16d41b Compare June 30, 2026 19:30

FrankChen021 reviewed Jul 1, 2026

View reviewed changes

Uh oh!

Conversation

jtuglu1 commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Release note

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

FrankChen021 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

FrankChen021 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cecemei left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

FrankChen021 left a comment

Choose a reason for hiding this comment

Uh oh!

kfaraz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kfaraz Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

kfaraz Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

kfaraz Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

kfaraz Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kfaraz Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

kfaraz Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

FrankChen021 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

FrankChen021 left a comment

Choose a reason for hiding this comment

Uh oh!

FrankChen021 left a comment

Choose a reason for hiding this comment

Uh oh!

FrankChen021 Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

FrankChen021 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

jtuglu1 commented Jun 1, 2026 •

edited

Loading