Skip to content

Refactor: Remove type params from MetadataStorageActionHandler - #18479

Merged
kfaraz merged 11 commits into
apache:masterfrom
kfaraz:simplify_metadata_storage_handler
Sep 6, 2025
Merged

kfaraz merged 11 commits into
apache:masterfrom
kfaraz:simplify_metadata_storage_handler

Conversation

@kfaraz

@kfaraz kfaraz commented Sep 4, 2025 •

Copy link
Copy Markdown
Contributor

Description

The MetadataStorageActionHandler deals with persistence of Task and related objects.
But those model classes exist in indexing-service module which forces the MetadataStorageActionHandler
to handle generics unnecessarily. This makes the code cluttered and difficult to understand at times.

Changes

  • Move MetadataStorageActionHandler and factory classes to indexing-service module
  • Move implementations to respective extensions
  • Simplify class TaskInfo by removing type parameters
  • Add class TaskIdStatus
  • Remove class TaskStuff
  • Add DerbyTaskStorageModule to bind the MetadataStorageActionHandler to its default impl on CliOverlord
  • Remove references to the tasklog table as it had been disabled in Remove task action audit logging and druid_taskLog metadata table #16309

Developer notes

The MetadataStorageActionHandler extension point is being updated to have simpler APIs.
This is a breaking change for any SQL metadata extensions in the wild.
The changes for PostgreSQL, MySQL and SQLServer are already handled in this PR.


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.

@jtuglu1 jtuglu1 left a comment

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.

LGTM – thanks!

DateTime createdTime
)
{
this.taskId = taskId;

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'm assuming we don't want to perform validations on these fields?

@kfaraz kfaraz Sep 4, 2025 •

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.

Do you mean a null check?

@kfaraz
kfaraz requested a review from gianm September 5, 2025 02:44
@kfaraz kfaraz changed the title Remove type params from MetadataStorageActionHandler Refactor: Remove type params from MetadataStorageActionHandler Sep 5, 2025
@kfaraz
kfaraz merged commit 02338fc into apache:master Sep 6, 2025
62 checks passed
@kfaraz

kfaraz commented Sep 6, 2025

Copy link
Copy Markdown
Contributor Author

Thanks for the reviews, @jtuglu1 , @gianm !

@kfaraz
kfaraz deleted the simplify_metadata_storage_handler branch September 6, 2025 02:32
@cecemei cecemei added this to the 35.0.0 milestone Oct 21, 2025
riovic918data added a commit to riovic918data/druid that referenced this pull request Jun 12, 2026
…e#18479)

Description:
The `MetadataStorageActionHandler` deals with persistence of `Task` and related objects.
But those model classes exist in `indexing-service` module which forces the `MetadataStorageActionHandler`
to handle generics unnecessarily. This makes the code cluttered and difficult to understand at times.

Changes:
- Move `MetadataStorageActionHandler` and factory classes to `indexing-service` module
- Move implementations to respective extensions
- Simplify class `TaskInfo` by removing type parameters
- Add class `TaskIdStatus`
- Remove class `TaskStuff`
- Add `DerbyTaskStorageModule` to bind the `MetadataStorageActionHandler` to its default impl on `CliOverlord`
- Remove references to the tasklog table as it had been disabled in apache#16309
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