-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Restrict input stream modification in existing seekable stream supervisors #17955
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
Changes from 5 commits
155aa3b
349e3fa
162da49
ea56bdf
a9095cb
7a9bcd5
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -24,14 +24,18 @@ | |||||
| import com.fasterxml.jackson.annotation.JsonIgnore; | ||||||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||||||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||||||
| import org.apache.druid.error.DruidException; | ||||||
| import org.apache.druid.error.InvalidInput; | ||||||
| import org.apache.druid.guice.annotations.Json; | ||||||
| import org.apache.druid.indexing.kafka.KafkaIndexTaskClientFactory; | ||||||
| import org.apache.druid.indexing.overlord.IndexerMetadataStorageCoordinator; | ||||||
| import org.apache.druid.indexing.overlord.TaskMaster; | ||||||
| import org.apache.druid.indexing.overlord.TaskStorage; | ||||||
| import org.apache.druid.indexing.overlord.supervisor.Supervisor; | ||||||
| import org.apache.druid.indexing.overlord.supervisor.SupervisorSpec; | ||||||
| import org.apache.druid.indexing.overlord.supervisor.SupervisorStateManagerConfig; | ||||||
| import org.apache.druid.indexing.seekablestream.supervisor.SeekableStreamSupervisorSpec; | ||||||
| import org.apache.druid.java.util.common.StringUtils; | ||||||
| import org.apache.druid.java.util.emitter.service.ServiceEmitter; | ||||||
| import org.apache.druid.java.util.metrics.DruidMonitorSchedulerConfig; | ||||||
| import org.apache.druid.segment.incremental.RowIngestionMetersFactory; | ||||||
|
|
@@ -176,6 +180,47 @@ protected KafkaSupervisorSpec toggleSuspend(boolean suspend) | |||||
| ); | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Extends {@link SeekableStreamSupervisorSpec#validateSpecUpdateTo} to ensure that the proposed spec and current spec are either both multi-topic or both single-topic. | ||||||
| * <p> | ||||||
| * getSource() returns the same string (exampleTopic) for "topicPattern=exampleTopic" and "topic=exampleTopic". | ||||||
| * This override prevents this case from being considered a valid update. | ||||||
| * </p> | ||||||
| * @param proposedSpec the proposed supervisor spec | ||||||
| * @throws DruidException if the proposed spec is not a Kafka spec or if the proposed spec changes from multi-topic to single-topic or vice versa | ||||||
| */ | ||||||
| @Override | ||||||
| public void validateSpecUpdateTo(SupervisorSpec proposedSpec) throws DruidException | ||||||
| { | ||||||
| if (!(proposedSpec instanceof KafkaSupervisorSpec)) { | ||||||
| throw InvalidInput.exception( | ||||||
| StringUtils.format("Cannot evolve to [%s] from [%s]", getClass().getName(), proposedSpec.getClass().getName()) | ||||||
|
||||||
| StringUtils.format("Cannot evolve to [%s] from [%s]", getClass().getName(), proposedSpec.getClass().getName()) | |
| "Cannot change spec from type[%s] to type[%s]", getClass().getName(), proposedSpec.getClass().getName() |
Outdated
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.
We shouldn't need to override this method.
We can either use the constant defined in SeekableStreamSupervisorSpec as is or a different String altogether.
A cleaner way to include the type name in that String would be to have a placeholder for type and fill it with getType() while formatting.
Uh oh!
There was an error while loading. Please reload this page.
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.
Thanks for adding this.
Maybe you can put it in a list format for better readability like
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.
Updated with list
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.
Thanks!