kfaraz commented on code in PR #20649:
URL: https://github.com/apache/druid/pull/20649#discussion_r4332769677
##########
docs/api-reference/supervisor-api.md:
##########
@@ -2288,13 +2288,22 @@ The following table lists the properties of a
supervisor spec:
|`dataSchema`|Object|The schema for the indexing task to use during ingestion.
See [`dataSchema`](../ingestion/ingestion-spec.md#dataschema) for more
information.|Yes|
|`tuningConfig`|Object|The tuning configuration object to define
performance-related settings for the supervisor and indexing tasks.|No|
-When you call this endpoint on an existing supervisor, the running supervisor
signals its tasks to stop reading and begin publishing, exiting itself. Druid
then uses the provided configuration from the request body to create a new
supervisor. Druid submits a new schema while retaining existing publishing
tasks and starts new tasks at the previous task offsets.
-This way, you can apply configuration changes without a pause in ingestion.
+When called on an existing supervisor, Druid compares the submitted spec with
the running one. If the change requires a restart, the supervisor signals its
tasks to stop reading and begin publishing, then exits. Druid creates a new
supervisor using the submitted spec, retaining existing publishing tasks and
starting new tasks from the previous task offsets. This allows configuration
changes without interrupting data processing.
+
+Otherwise, if the spec is unchanged or only `taskCount` differs while task
auto-scaling is enabled, Druid updates the stored spec without restarting the
supervisor.
Review Comment:
```suggestion
Otherwise, if the spec is unchanged or does not require a restart (e.g. only
`taskCount` differs while task auto-scaling is enabled), Druid updates the
stored spec without restarting the supervisor.
```
##########
indexing-service/src/test/java/org/apache/druid/indexing/overlord/supervisor/SupervisorResourceTest.java:
##########
@@ -317,6 +319,22 @@ public List<String> getDataSources()
Assertions.assertEquals(Map.of("id", "my-id", "modified", true,
"restarted", true), response.getEntity());
}
+ @Test
+ public void testSpecPost_skipRestartIfUnmodifiedQueryParam_defaultsToTrue()
throws NoSuchMethodException
+ {
+ // JAX-RS applies @DefaultValue only when binding an actual HTTP request,
so a direct call to
+ // specPost(spec, null, ...) can't exercise that default.
Review Comment:
Fair point, I suppose an embedded test could help here. But we can do that
in a follow up.
##########
docs/api-reference/supervisor-api.md:
##########
@@ -2516,6 +2527,7 @@ curl
"http://ROUTER_IP:ROUTER_PORT/druid/indexer/v1/supervisor?skipRestartIfUnmo
The response includes the following fields:
- `id`: The supervisor ID.
+- `modified`: A boolean indicating whether the submitted spec was persisted as
a change to the existing one.
Review Comment:
Reworded to clarify:
```suggestion
- `modified`: A boolean indicating whether the submitted spec was different
from the existing spec and was persisted as a new version.
```
--
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]
