Add SameIntervalMergeTask for easier usage of MergeTask #3981
Conversation
|
@kaijianding, this generally looks good to me. Could you include a test, ideally one for behavior and one for serde of the task itself? |
|
ut added, @gianm |
| @JsonSubTypes.Type(name = "convert_segment_sub", value = ConvertSegmentTask.SubTask.class) | ||
| @JsonSubTypes.Type(name = "convert_segment_sub", value = ConvertSegmentTask.SubTask.class), | ||
| @JsonSubTypes.Type(name = "same_interval_merge", value = SameIntervalMergeTask.class), | ||
| @JsonSubTypes.Type(name = "same_interval_merge_sub", value = SameIntervalMergeTask.SubTask.class) |
There was a problem hiding this comment.
why is SubTask needed here?
There was a problem hiding this comment.
After test, I verified it is not need here, I removed it from here and also remove other not needed code.
|
still 👍 after latest changes. thanks for identifying and removing the unneeded code. |
|
@kaijianding you removed overriding of isReady(..) as well ... i thought it was good because segment list is automatically figured out so no need to fetch segment list again and match as is done in MergeTaskBase.isReady(..) ? |
|
SubTask.run() is called directly inside SameIntervalMergeTask.run() after SameIntervalMergeTask.isReady() called. |
|
@kaijianding yeah, its fine then. |
We generate several small segments each hour in realtime node, and want to merge all yesterday"s segments to only 1 segment.
MergeTask needs DataSegment list to be provided, which is painful in actual use.