Skip to content

Docs: Add metrics and configs for embedded kill tasks#18124

Merged
kfaraz merged 3 commits intoapache:masterfrom
kfaraz:add_embedded_kill_docs
Jun 12, 2025
Merged

Docs: Add metrics and configs for embedded kill tasks#18124
kfaraz merged 3 commits intoapache:masterfrom
kfaraz:add_embedded_kill_docs

Conversation

@kfaraz
Copy link
Copy Markdown
Contributor

@kfaraz kfaraz commented Jun 11, 2025

Docs changes for #18028

Changes

  • Document metrics and configs for embedded kill tasks
  • Remove duplicate configs for Coordinator auto-kill from data-management/delete.md
  • Fix up references

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • 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
Copy Markdown
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.

Lgtm, left two suggestions, thanks!

Comment thread docs/data-management/delete.md Outdated
- can keep up with a large number of unused segments in the cluster.
- take advantage of the segment metadata cache on the Overlord.

This feature can be used only if [segment metadata caching](../configuration/index.md#segment-metadata-cache-experimental) is enabled on the Overlord.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It would be better to move this prerequisite above, perhaps into a separate ::info section for visibility.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is it okay to put it into the same info section as a separate bullet?

##### Auto-kill unused segments (Experimental)

These configs pertain to the new embedded mode of running [kill tasks on the Overlord](../data-management/delete.md#auto-kill-data-on-the-overlord-experimental).

Copy link
Copy Markdown
Contributor

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 good to explicitly call out that none of the coordinator-based auto-kill configs apply to this overlord-based auto-kill feature (even in the combined coordinator+OL mode). Bringing this up because I had that assumption initially when reviewing #18028. The reasons are already cross-linked in data-management/delete.md, so it should be clear for anyone curious.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

updated

@kfaraz kfaraz merged commit 821769c into apache:master Jun 12, 2025
6 checks passed
@kfaraz
Copy link
Copy Markdown
Contributor Author

kfaraz commented Jun 12, 2025

Thanks for the review, @abhishekrb19 !

@kfaraz kfaraz deleted the add_embedded_kill_docs branch June 12, 2025 03:47
@capistrant
Copy link
Copy Markdown
Contributor

Thanks for documenting, Kashif!

jtuglu1 pushed a commit to jtuglu1/druid that referenced this pull request Jun 17, 2025
Docs changes for apache#18028

- Document metrics and configs for embedded kill tasks
- Remove duplicate configs for Coordinator auto-kill from `data-management/delete.md`
- Fix up references
@capistrant capistrant added this to the 34.0.0 milestone Jul 22, 2025
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.

3 participants