Skip to content

Refactor: Miscellaneous batch task cleanup#16730

Merged
kfaraz merged 2 commits intoapache:masterfrom
kfaraz:misc_task_cleanup
Jul 13, 2024
Merged

Refactor: Miscellaneous batch task cleanup#16730
kfaraz merged 2 commits intoapache:masterfrom
kfaraz:misc_task_cleanup

Conversation

@kfaraz
Copy link
Contributor

@kfaraz kfaraz commented Jul 12, 2024

Changes

  • No functional change
  • Remove unused method IndexTuningConfig.withPartitionsSpec()
  • Remove unused method ParallelIndexTuningConfig.withPartitionsSpec()
  • Remove redundant method CompactTask.emitIngestionModeMetrics()
  • Remove Clock argument from CompactionTask.createDataSchemasForInterval() as it was only needed
    for one test which was just verifying the value passed by the test itself. The code now uses a Stopwatch
    instead and test simply verifies that the metric has been emitted.
  • Other minor cleanup

@github-actions github-actions bot added Area - Batch Ingestion Area - Ingestion Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 labels Jul 12, 2024
Copy link
Contributor

@abhishekrb19 abhishekrb19 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

}

private static List<TimelineObjectHolder<String, DataSegment>> retrieveRelevantTimelineHolders(
private static Iterable<DataSegment> retrieveRelevantTimelineHolders(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the objects being returned is still relevant to the timeline, but does this method need a rename?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point, let me include that in the next PR.

@kfaraz kfaraz merged commit a618c5d into apache:master Jul 13, 2024
@kfaraz kfaraz deleted the misc_task_cleanup branch July 13, 2024 02:42
@kfaraz
Copy link
Contributor Author

kfaraz commented Jul 13, 2024

Thanks a lot for the review, @abhishekrb19 !

sreemanamala pushed a commit to sreemanamala/druid that referenced this pull request Aug 6, 2024
Changes
- No functional change
- Remove unused method `IndexTuningConfig.withPartitionsSpec()`
- Remove unused method `ParallelIndexTuningConfig.withPartitionsSpec()`
- Remove redundant method `CompactTask.emitIngestionModeMetrics()`
- Remove Clock argument from `CompactionTask.createDataSchemasForInterval()` as it was only needed
for one test which was just verifying the value passed by the test itself. The code now uses a `Stopwatch`
instead and test simply verifies that the metric has been emitted.
- Other minor cleanup changes
@kfaraz kfaraz added this to the 31.0.0 milestone Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area - Batch Ingestion Area - Ingestion Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants