Skip to content

Conversation

@kfaraz
Copy link
Contributor

@kfaraz kfaraz commented Dec 26, 2025

Changes


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.

@kfaraz kfaraz changed the title Migrate ITCustomCoordinatorDutiesTest to embedded tests Migrate ITAppendBatchIndexTest, ITCustomCoordinatorDutiesTest to embedded tests Dec 26, 2025
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.

Lgtm, thanks!

druid.coordinator.dutyGroups=["cleanupMetadata"]
druid.coordinator.cleanupMetadata.duties=["killSupervisors"]
druid.coordinator.cleanupMetadata.duty.killSupervisors.durationToRetain=PT0M
druid.coordinator.cleanupMetadata.period=PT10S
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
druid.coordinator.cleanupMetadata.period=PT0.1S

Perhaps we can just remove the config block here, since it’s likely to go out of sync with the source of truth and have readers refer to KillSupervisorsCustomDutyTest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

@kfaraz
Copy link
Contributor Author

kfaraz commented Dec 26, 2025

Thanks for the review, @abhishekrb19 !

@kfaraz
Copy link
Contributor Author

kfaraz commented Dec 26, 2025

@abhishekrb19 , I have updated the docs.

I also realised that the cds-X-disabled test groups are not needed anymore since all the tests have been migrated.

@abhishekrb19
Copy link
Contributor

I also realised that the cds-X-disabled test groups are not needed anymore since all the tests have been migrated.

Sweet. For posterity, maybe link the PRs in the summary where this migration was completed.

@kfaraz kfaraz merged commit c5a85b4 into apache:master Dec 27, 2025
122 of 127 checks passed
@kfaraz kfaraz deleted the migrate_more_its branch December 27, 2025 04:07
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.

2 participants