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

Add feature to automatically remove compaction configurations for inactive datasources #11232

Merged
merged 10 commits into from
May 12, 2021

Conversation

maytasm
Copy link
Contributor

@maytasm maytasm commented May 11, 2021

Add feature to automatically remove compaction configurations for inactive datasources

Description

We currently already have tasklog auto cleanup (#3677) and audit logs auto cleanup (#11084). This PR adds a similar auto cleanup but for the compaction configurations (which are stored in the druid config table) to auto clean up compaction configs belonging to datasource that is no longer active -- meaning that the datasource has no used/unused segments.

This is useful when Druid user has a high churn of task / datasource in a short amount of time causing the metadata store size to grow uncontrollably.

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

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.

Would a better name for such config flags be druid.coordinator.autocleanup.x or something instead of druid.coordinator.kill.x ?

@maytasm
Copy link
Contributor Author

maytasm commented May 11, 2021

Would a better name for such config flags be druid.coordinator.autocleanup.x or something instead of druid.coordinator.kill.x ?

This is to be consistent with the other similar existing functionality (i.e. druid.coordinator.kill.supervisor.on, druid.coordinator.kill.audit.on, druid.coordinator.kill.pendingSegments.on, etc)

* under the License.
*/

package org.apache.druid.java.util;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you choose to put this class in this package?

I didn't find an equivalent class in the java.util package.

I think for now it might be better to move this to a private class in KillCompactionConfig which appears to be the only place where this is used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it would be useful for people using the RetryUtils (which is also in org.apache.druid.java.util). RetryUtils.retry retry condition requires an exception to be thrown and applying a predicate to the thrown exception. For cases where the task method does not throw an exception, the method can throw this RetryableException so that the RetryUtils can then retry

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, this is what I suspected, but the package naming threw me off. Can you please add javadocs explaining this use case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

import java.util.function.Function;
import java.util.stream.Collectors;

public class KillCompactionConfig implements CoordinatorDuty
Copy link
Contributor

Choose a reason for hiding this comment

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

javadocs please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@suneet-s suneet-s left a comment

Choose a reason for hiding this comment

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

LGTM after CI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants