Skip refresh for unused segments in metadata cache #16990

findingrish · 2024-09-03T05:35:14Z

Parent issue: #14989

In the metadata cache, unused segment could be marked for refresh leading to unnecessary metadata queries.

One such scenario is:

Compaction task generates new set of segments for an interval leading to existing segments being marked as unused.
Those segments are being unloaded from historical's.
MetadataCache receives segment removal callback from historical's and marks the datasource for refresh.
The segment and schema poll updates the schema cache with schema for used segments.
The refresh call doesn't find schema for some segments and marks them to be refreshed in the next cycle.
In the next refresh call metadata query is issued for unused segments.

This PR tries to place a check for unused segments before marking it for refresh.

This PR has:

cryptoe · 2024-09-03T06:24:34Z

server/src/main/java/org/apache/druid/segment/metadata/CoordinatorSegmentMetadataCache.java

+          ImmutableDruidDataSource druidDataSource =
+              sqlSegmentsMetadataManager.getImmutableDataSourceWithUsedSegments(segmentId.getDataSource());
+
+          if (druidDataSource != null && druidDataSource.getSegment(segmentId) != null) {


Can we remove the segment if the segment is unused when we receive a callback from the inventory view.?

Whenever SqlSegmentsMetadataManager polls the database and returns the latest list of used segments, we should check that list and remove any segments from cache which are not in that list.

This removal should not be triggered based on any callback from server inventory view. Metadata store should be the source of truth on whether a segment is used or not.

Also, I was under the impression that this feature is already included. Can you confirm, @findingrish ?

Whenever SqlSegmentsMetadataManager polls the database and returns the latest list of used segments, we should check that list and remove any segments from cache which are not in that list.

This doesn't happen currently. Callbacks from the inventory view is the source of truth for the metadata cache.
The reason being that segments in the metadata cache are representative of the available segments loaded on various data servers in the cluster.

I feel that this would be an expensive operation as well.

Callbacks from the inventory view is the source of truth for the metadata cache.

I am not sure I understand. Isn't the schema coming from the metadata store?

I am not sure I understand. Isn't the schema coming from the metadata store?

That is partly correct, we still rely on metadata query to fetch schema for segments published from MSQ job.
The datasource schema is still computed from the segments which are available.

Yes, my understanding is that we should be doing the following:

Every minute (or less), coordinator polls the metadata store to get the list of used segments and their schemas

Coordinator populates its cache with this information

Coordinator gets rid of any segment not found in the above list

If any used segment that the Coordinator received in the latest poll does not have the schema populated, we fire a metadata query to get that schema and populate it back into the DB.

Subsequent polls will get schema for that segment also.

If any service requests schema for a used segment, coordinator sends back an OK response

If any service requests schema for an unused segment, coordinator sends back a NOT FOUND response.

Let me know if this aligns with what we need and what we currently have.

Traditionally, coordinator has been aware of "used" segments only (both overshadowed and non-overshadowed). We should continue to maintain those semantics.

Coordinator does also maintain an inventory view which might show unused segments on some server. But in the immediately next run, the Coordinator unloads the unused segments.

I don't think we can base the schema building logic entirely on segments polled from the database since realtime segment schema isn't present in the metadata database.

Also, in the present mechanism datasource schema is rebuilt only on segment addition/removal. In the proposed approach the datasource schema would be rebuilt after each poll leading to overhead?

Can we remove the segment if the segment is unused when we receive a callback from the inventory view.?

@cryptoe I think there could be a scenario where this would still fail.

Segment s1 got added to foo and the cache tries to refresh it.

The cache is building the schema for foo and in parallel s2 was marked unused.

The caches marks s2 to be refreshed in the next cycle.

Lets assume that the segment removal callback didn't make it to the Coordinator until the next refresh cycle got triggered.

s2 an unused segment will be refreshed in that cycle.

I don't think we can base the schema building logic entirely on segments polled from the database since realtime segment schema isn't present in the metadata database.

That's a fair point. How does the coordinator currently distinguish between a realtime segment and a published segment?

Also, in the present mechanism datasource schema is rebuilt only on segment addition/removal. In the proposed approach the datasource schema would be rebuilt after each poll leading to overhead?

Don't we already do that? If not, how is the schema polled from the DB used?

How does the coordinator currently distinguish between a realtime segment and a published segment?

Both segments are added to the timeline, the difference is in refreshing the segment schema.
Traditionally, a realtime segment would be refreshed every minute by issuing metadata query to tasks serving the segment whereas a published segment would be refreshed only once after the segment has been added.

In the current scheme, schema for published segment is obtained from the database whereas realtime segment schema is periodically pushed to the Coordinator from tasks.

Don't we already do that? If not, how is the schema polled from the DB used?

No we don't do this as of now. The segment schema polled from the database is cached and used on-demand ie. when a segment is added or datasource schema is being rebuilt.

…t-refresh

… refresh

…t-refresh

* Skip refresh for unused segments in metadata cache * Cover the condition where a used segment missing schema is marked for refresh * Fix test

)" This reverts commit b0e07bf.

* Skip refresh for unused segments in metadata cache * Cover the condition where a used segment missing schema is marked for refresh * Fix test

Skip refresh for unused segments in metadata cache

40721dc

kfaraz approved these changes Sep 3, 2024

View reviewed changes

cryptoe reviewed Sep 3, 2024

View reviewed changes

cryptoe approved these changes Sep 6, 2024

View reviewed changes

findingrish added 5 commits September 9, 2024 09:35

Merge remote-tracking branch 'upstream/master' into fix-unused-segmen…

7b1670e

…t-refresh

Cover the condition where a used segment missing schema is marked for…

8ccc6f2

… refresh

Merge remote-tracking branch 'upstream/master' into fix-unused-segmen…

0df0f2c

…t-refresh

Fix test

9eae5d1

Merge remote-tracking branch 'upstream/master' into fix-unused-segmen…

9e085d3

…t-refresh

cryptoe merged commit a18f582 into apache:master Sep 12, 2024
88 of 90 checks passed

cryptoe added this to the 31.0.0 milestone Sep 12, 2024

cecemei added a commit to cecemei/druid that referenced this pull request Sep 14, 2024

Revert "Skip refresh for unused segments in metadata cache (apache#16990

028cf80

)" This reverts commit b0e07bf.

findingrish mentioned this pull request Sep 16, 2024

[Backport] Skip refresh for unused segments in metadata cache #17079

Merged

kfaraz mentioned this pull request Oct 11, 2024

[DRAFT] 31.0.0 Release Notes #17332

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Skip refresh for unused segments in metadata cache #16990

Skip refresh for unused segments in metadata cache #16990

findingrish commented Sep 3, 2024 •

edited

Loading

cryptoe Sep 3, 2024

kfaraz Sep 3, 2024 •

edited

Loading

findingrish Sep 4, 2024 •

edited

Loading

kfaraz Sep 4, 2024

findingrish Sep 4, 2024 •

edited

Loading

kfaraz Sep 4, 2024 •

edited

Loading

findingrish Sep 4, 2024

findingrish Sep 4, 2024

kfaraz Sep 5, 2024 •

edited

Loading

findingrish Sep 5, 2024 •

edited

Loading

Skip refresh for unused segments in metadata cache #16990

Skip refresh for unused segments in metadata cache #16990

Conversation

findingrish commented Sep 3, 2024 • edited Loading

cryptoe Sep 3, 2024

Choose a reason for hiding this comment

kfaraz Sep 3, 2024 • edited Loading

Choose a reason for hiding this comment

findingrish Sep 4, 2024 • edited Loading

Choose a reason for hiding this comment

kfaraz Sep 4, 2024

Choose a reason for hiding this comment

findingrish Sep 4, 2024 • edited Loading

Choose a reason for hiding this comment

kfaraz Sep 4, 2024 • edited Loading

Choose a reason for hiding this comment

findingrish Sep 4, 2024

Choose a reason for hiding this comment

findingrish Sep 4, 2024

Choose a reason for hiding this comment

kfaraz Sep 5, 2024 • edited Loading

Choose a reason for hiding this comment

findingrish Sep 5, 2024 • edited Loading

Choose a reason for hiding this comment

findingrish commented Sep 3, 2024 •

edited

Loading

kfaraz Sep 3, 2024 •

edited

Loading

findingrish Sep 4, 2024 •

edited

Loading

findingrish Sep 4, 2024 •

edited

Loading

kfaraz Sep 4, 2024 •

edited

Loading

kfaraz Sep 5, 2024 •

edited

Loading

findingrish Sep 5, 2024 •

edited

Loading