Skip to content

Emit event when a datasource has been dropped from the coordinator#18497

Merged
cryptoe merged 16 commits intoapache:masterfrom
uds5501:emitting-datasource-drop-event
Sep 9, 2025
Merged

Emit event when a datasource has been dropped from the coordinator#18497
cryptoe merged 16 commits intoapache:masterfrom
uds5501:emitting-datasource-drop-event

Conversation

@uds5501
Copy link
Copy Markdown
Contributor

@uds5501 uds5501 commented Sep 8, 2025

Follow up PR for #18495

It introduces a new event segment/schemaCache/datasource/dropped/count that is emitted when the data source is dropped from the coordinator and the cache refresh in broker acknowledges it.

Copy link
Copy Markdown
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.

Since we already have this PR, do we still need the one disabling the test?

Comment thread server/src/main/java/org/apache/druid/segment/metadata/Metric.java Outdated
Comment thread docs/operations/metrics.md Outdated
Copy link
Copy Markdown
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.

@uds5501 , can we re-enable the flaky test as a part of this PR?

@uds5501
Copy link
Copy Markdown
Contributor Author

uds5501 commented Sep 9, 2025

@kfaraz yeah let me do that.

.hasDimension(DruidMetrics.DATASOURCE, dataSource),
agg -> agg.hasSumAtLeast(numSegments)
);
broker.latchableEmitter().waitForEvent(
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.

Maybe we should we look for aggregate where the sum is equal to numSegments.
After this change, do we even need to wait to wait for the load success.
@uds5501 , could you try to push a change with the coordinator part commented out?
If not needed, we can remove that check completely.

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.

Tested locally, the tests seems to be working fine without the coordinator wait, removed in the latest comment.
Also updated the EmbeddedBroker as part of the numSegments change.

Comment thread services/src/test/java/org/apache/druid/testing/embedded/EmbeddedClusterApis.java Outdated
Copy link
Copy Markdown
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.

Looks good, left some minor suggestions.

@kfaraz kfaraz self-requested a review September 9, 2025 12:06
@cryptoe cryptoe merged commit 69be831 into apache:master Sep 9, 2025
82 of 84 checks passed
@cryptoe
Copy link
Copy Markdown
Contributor

cryptoe commented Sep 9, 2025

One input source IT was running. The patch does not change anything there hence merged the PR.

@cecemei cecemei added this to the 35.0.0 milestone Oct 21, 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.

4 participants