-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Add configurable final stages to MSQ ingestion queries #16699
Conversation
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/MSQTaskQueryMaker.java
Outdated
Show resolved
Hide resolved
...ore/multi-stage-query/src/main/java/org/apache/druid/msq/sql/SegmentMorphFactoryCreator.java
Fixed
Show fixed
Hide fixed
...ore/multi-stage-query/src/main/java/org/apache/druid/msq/sql/SegmentMorphFactoryCreator.java
Fixed
Show fixed
Hide fixed
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java
Outdated
Show resolved
Hide resolved
.../multi-stage-query/src/main/java/org/apache/druid/msq/guice/MSQTerminalStageSpecFactory.java
Dismissed
Show dismissed
Hide dismissed
.../multi-stage-query/src/main/java/org/apache/druid/msq/guice/MSQTerminalStageSpecFactory.java
Dismissed
Show dismissed
Hide dismissed
DataSourceMSQDestination destination = (DataSourceMSQDestination) querySpec.getDestination(); | ||
TerminalStageSpec terminalStageSpec = destination.getTerminalStageSpec(); | ||
if (terminalStageSpec instanceof SegmentGenerationStageSpec) { | ||
return ((SegmentGenerationStageSpec) terminalStageSpec).constructFinalStage(queryId, queryDef, querySpec, jsonMapper); |
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.
can this be a interface method and we pass query def only ?
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 can inject the jsonMapper.
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 would also need the querySpec for the tuningConfig. Passing the jsonMapper here would also require SegmentGenerationStageSpec to not be a singleton class, and instead require a jsonMapper to be injected.
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java
Outdated
Show resolved
Hide resolved
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.
Changes LGTM
* Add a segmentMorphFactory to MSQ. * Add test * Make argument nullable * Fix Guice issues * Merge with master * Remove extra information * Fix tests * Create a utils class * Refactor segment generation * Fix javadoc * Refactor * Refactor * Fix injection
* Add a segmentMorphFactory to MSQ. * Add test * Make argument nullable * Fix Guice issues * Merge with master * Remove extra information * Fix tests * Create a utils class * Refactor segment generation * Fix javadoc * Refactor * Refactor * Fix injection
Description
Modifies the
DataSourceMSQDestination
to have aTerminalStageSpec
. This spec tells the controller how the segments are created. This allows queries to configure how segments are created after the queries have finished running.This will allow queries to add new
TerminalStageSpec
which allow queries to have other results instead of generating segments.The PR also adds a
SegmentGenerationStageSpec
to be used by default, which causes queries to create new segments with the query results.The SegmentGenerationTerminalStageSpec for ingestion is serialized as:This PR should not have a functional impact, and is meant to be used for other features in the future.
Backward Compatibility
The changes are backward compatible. The
DataSourceMSQDestination
created has a default value which uses theSegmentGenerationStageSpec
by default, since normally all ingestions would require a segment generation stage. If indexers/MM are upgraded first, the stage spec automatically deserializes to segment generation.This PR has: