Skip to content
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

Remove index_realtime and index_realtime_appenderator tasks. #15717

Closed
wants to merge 16 commits into from

Conversation

gianm
Copy link
Contributor

@gianm gianm commented Jan 18, 2024

index_realtime tasks were removed from the documentation in #13107. Even at that time, they weren't really documented per se— just mentioned. They existed solely to support Tranquility, which is an obsolete ingestion method that predates migration of Druid to ASF and is no longer being maintained. Tranquility docs were de-linked from the sidebars and other docs earlier, in #11134. Only a stub remains today, so people with links to the page can see that it's no longer recommended.

index_realtime_appenderator tasks existed in the code base, but were never documented, nor as far as I am aware were they used for any purpose.

Since they are both obsolete, this patch removes both task types completely, as well as removes all supporting code that was otherwise unused. It also updates the stub doc for Tranquility to be firmer that it is not compatible. (Previously, the stub doc said it wasn't recommended, and pointed out that it is built against an ancient 0.9.2 version of Druid.)

This patch also has some related metrics changes:

  • rename FireDepartmentMetrics to SegmentGenerationMetrics
  • removes a useless call to fireDepartmentMetrics.incrementProcessed() in SinglePhaseSubTask (this metric was not being emitted anyway)

index_realtime tasks were removed from the documentation in apache#13107. Even
at that time, they weren't really documented per se— just mentioned. They
existed solely to support Tranquility, which is an obsolete ingestion
method that predates migration of Druid to ASF and is no longer being
maintained. Tranquility docs were also de-linked from the sidebars and
the other doc pages in apache#11134. Only a stub remains, so people with
links to the page can see that it's no longer recommended.

index_realtime_appenderator tasks existed in the code base, but were
never documented, nor as far as I am aware were they used for any purpose.

This patch removes both task types completely, as well as removes all
supporting code that was otherwise unused. It also updates the stub
doc for Tranquility to be firmer that it is not compatible. (Previously,
the stub doc said it wasn't recommended, and pointed out that it is
built against an ancient 0.9.2 version of Druid.)
Copy link
Contributor

@kfaraz kfaraz left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 !!

@@ -46,8 +42,6 @@ public class RealtimeTuningConfig implements AppenderatorConfig
{
private static final Period DEFAULT_INTERMEDIATE_PERSIST_PERIOD = new Period("PT10M");
Copy link
Contributor

Choose a reason for hiding this comment

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

I think most of these constants have been moved to SeekableStreamIndexTaskTuningConfig. Do we still need this class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, no, we don't need it. I went through all the trouble of adjusting all the call sites to not use this, but then forgot to delete it. Fixed!

@gianm
Copy link
Contributor Author

gianm commented Jan 19, 2024

Hmm. ITUnionQueryTest needs to be rewritten somehow. It is making sure union datasources work with realtime tasks, and uses an index_realtime to do so. It probably needs to be moved to use Kafka.

Copy link

This pull request has been marked as stale due to 60 days of inactivity.
It will be closed in 4 weeks if no further activity occurs. If you think
that's incorrect or this pull request should instead be reviewed, please simply
write any comment. Even if closed, you can still revive the PR at any time or
discuss it on the [email protected] list.
Thank you for your contributions.

Copy link

This pull request has been marked as stale due to 60 days of inactivity.
It will be closed in 4 weeks if no further activity occurs. If you think
that's incorrect or this pull request should instead be reviewed, please simply
write any comment. Even if closed, you can still revive the PR at any time or
discuss it on the [email protected] list.
Thank you for your contributions.

@gianm
Copy link
Contributor Author

gianm commented Jun 26, 2024

Closed in favor of #16602.

@gianm gianm closed this Jun 26, 2024
@gianm gianm deleted the realtime-rampage branch June 26, 2024 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants