Add "is_overshadowed" column to sys.segments table

[surekhasaharan]

Description

sys.segments table retrieves published segments from coordinator (segments which are in metadata store with used=1), and marks all those segments with is_published=true. The published segments can include overshadowed segments if compaction is underway. Currently, sys.segments table gets all the segments from coordinator where used=1, there might be some segments which are overshadowed and still have used=1 in coordinator as there can be a lag before used=0 is set for overshadowed segments, at which point they will not be returned from coordinator. But, if those overshadowed segments are not handled properly, the sys.segments table may show dangling segments for a while.

Motivation

sys.segments table contains overshadowed segments with their is_published column is set to true, which may not be the correct behavior because those overshadowed segments are going to be marked used=0 once compaction finishes. And sys.segments queries would be showing phantom segments for sometime. A better behavior for sys.segments is to add another column called is_overshadowed which marks the overshadowed segments with is_overshadowed=true in the query results, until they are removed completely from the table. So users can get the list of segments which should be available by querying is_published && !is_overshadowed

Proposed changes

1. A new column called is_overshadowed will be added to the sys.segments virtual table.
2. A new query param includeOvershadowedStatus will be added to coordinator api
   /druid/coordinator/v1/metadata/segments. So the proposed API is

GET /druid/coordinator/v1/metadata/segments?includeOvershadowedStatus

is_overshadowed flag for a segment is set to true if the given segment is published and is completely overshadowed by some other published segment .Currently, is_overshadowed is always false for unpublished segments, although this may change in the future. You can filter for segments that "should be published" by filtering for is_published = 1 AND is_overshadowed = 0. Segments can briefly be both published and overshadowed if they were recently replaced, but have not been unpublished yet.

Without includeOvershadowedStatus query param, the api returns a stream of DataSegment objects and with this param, it will return a stream of DataSegment plus overshadow flag.
It would be a new data structure something like SegmentWithOvershadowedStatus with 2 properties:

DataSegment dataSegment;
boolean isOvershadowed;

The MetadataSegmentView would store SegmentWithOvershadowedStatus object in the publishedSegments in case of cacheEnabled, else will pass along the stream of SegmentWithOvershadowedStatus to SystemSchema where the is_overshadowed column of sys.segments would be filled with the boolean value per segment.

Rationale

Current approach seems to have no or minimal impact on broker memory as the the overshadow info per segment is being retrieved in coordinator. Other way thought of was to create a VersionedIntervalTimeline in broker, but was discarded due to additional memory overhead it would introduce in broker to maintain a timeline for all published segments.

Operational impact

No operational impact I can think of.

[jihoonson]

Hmm, maybe is_published is not the best name because overshadowed segments should have been published. How about adding a new column indicating this like is_queryable (for non-overshadowed segments)? or is_overshadowed?

[gianm]

It might be fine to call them "not published" especially if we define it right (like, "any segments that have used = true in the MD store and are not overshadowed"). I'm thinking that is the most useful definition for what this column is likely to be used for: finding all segments active in the metadata store. (Overshadowed ones aren't really active since they don't do anything)

[jihoonson]

Do you mean adding a definition to doc? I'm afraid if ppl would check it before using the system schema.
Also, I'm not sure it's most useful. I think most ppl wouldn't care what are in metadata store, but would care what segments are currently being queried. Finally, it's not intuitive and I think it would highly likely mislead users. Why not just calling it is_used if you want to match it to used in metadata store?

[jihoonson]

Well, it shouldn't be called just is_used because it's used segments - overshadowed segments.

[gianm]

is_published is meant to mean "has bee published to the metadata store" according to the docs. And to my heart, in that I do think that's a useful thing for there to be a field for. I guess the question is what does that really mean?

1. All segments with used = true in the metadata store (what it means today)
2. All segments with used = true in the metadata store, that are also not overshadowed by any other segments with used = true in the metadata store (what this proposal is suggesting changing it to)

I think (2) makes sense, since as this proposal points out, (1) is unintuitive when doing an overwrite of a set of segments. The old set and new set will both have used = true for a while, but for all practical purposes the old set is considered overshadowed at that point.

Also, I'm not sure it's most useful. I think most ppl wouldn't care what are in metadata store, but would care what segments are currently being queried.

This is sorta like is_available. Except I think that one also includes some overshadowed segments, so it's not really what's "currently being queried". (I'm not 100% sure about that but maybe 80% sure)

Maybe what is happening here is that we want versions of both is_published and is_available that take into account whether the segments have been overshadowed or not.

[jihoonson]

Hmm, ok. It looks that there are some misunderstandings and confusions. First of all, I think it makes sense to keep is_published column as it is. But, I also want to add to a new column for non-overshadowed segments in system schema (regardless of their used value), like is_queryable, is_available, or is_overshadowed (this is opposite to first 2 names). To keep both columns, I think users can avoid confusions and query more flexibly.

I also have some questions for the meaning of "published segments".

is_published is meant to mean "has bee published to the metadata store" according to the docs.

I think this means any segments in the segments table of the metadata store regardless of their used flag. This also matches to the meaning of "publishing segments" in indexing tasks. But, you said,

All segments with used = true in the metadata store (what it means today)

This isn't exactly same with the meaning of is_published since overshadowed segments can still be in the metadata store with used = false. I'm not sure what you mean by "what it means today". Is there any document about this? I feel is_active or is_enabled is more appropriate for these segments.

is_available (or is_queryable) is a bit different. These segments would have mix of used if some historicals haven't unannounced them yet. Also, some segments with used = true in metadata store can be missing in these segments.

This is sorta like is_available. Except I think that one also includes some overshadowed segments, so it's not really what's "currently being queried". (I'm not 100% sure about that but maybe 80% sure)

Yeah, if you're thinking the lag in brokers until they refresh their cache, it might not be "currently" being queried.

[surekhasaharan]

Below statement is correct :

All segments with used = true in the metadata store (what it means today)

the is_published col in segments table represents "all segments in MD store with used=1".

is_published is meant to mean "has bee published to the metadata store" according to the docs.

The doc here says above, but I think it's incomplete, as it omits the part about used=1.

@jihoonson Did you mean to suggest that the is_published should include all segments from MD store (irrespective of their used flag value) ? I think we should keep the existing behavior where it only shows segments in MD store with used=1 as I don't see any use of showing segments with used=0. In addition, if we enhance the behavior to also exclude overshadowed segments from this column, then I'd be okay renaming it to is_active, is_queryable or some other name we can agree on. Then the definition of this to_be_named col becomes "All non-overshadowed segments in MD store with used=1".

[leventov]

@surekhasaharan could you please clarify the motivation "Showing overshadowed segments as part of published segments with is_published=true may not be correct behavior" - showing where and by whom?

@jihoonson

But, I also want to add to a new column for non-overshadowed segments in system schema (regardless of their used value), like is_queryable, is_available, or is_overshadowed (this is opposite to first 2 names).

There is already an is_available column, isn't it?

[surekhasaharan]

@surekhasaharan could you please clarify the motivation "Showing overshadowed segments as part of published segments with is_published=true may not be correct behavior" - showing where and by whom?

Sorry that it's not clear, will edit the issue to clarify. What I meant to say is sys.segments table stores metadata about segments, one of the column it has is is_published, which stores the information about whether a segment is published in metadata store with used=1 or not. And it contains overshadowed segments and their is_published column is set to true, which may not be the correct behavior because those overshadowed segments are going to be marked used=0 once compaction finishes.

[jihoonson]

@leventov oh, I missed it's already there. Thanks.

I talked with @gianm and @surekhasaharan offline and now it's more clear to me. Here is the current behavior.

• is_available means that a segment is served by historicals or a realtime task. Overshadowing is not considered. Updated whenever a historical or realtime task announces a new segment or unannounces an existing one.
• is_published means that used is set to true for a segment in metadata store. Overshadowing is not considered. Updated in background (metadata store is polled every 60s).
  • Unpublishing segments can mean setting used to false and it sounds good to me.

The issue here is that both columns are not considering overshadowing such that it makes debugging harder.

Probably, in many use cases, we want to know what segments are not loaded yet and why. To figure this out, we need to know what segment is published and is not fully overshadowed only by other segments in metadata store. This information is not being served yet, but it looks worth to add a new column. We came up with a name of is_active for this property. Does this make sense?

[gianm]

This suggested terminology sounds reasonable to me - in summary, not changing the meaning of is_available or is_published, but adding a new is_active property that basically means is_published && is_not_overshadowed_by_any_other_published_segment.

[leventov]

Is this right that sys.segments table is a virtual table that Druid processes queries against for the SQL interface to Druid? It doesn't exist in the metadata store (or any other real SQL database Druid depends on)?

Or not, since this message

What I meant to say is sys.segments table stores metadata about segments, one of the column it has is is_published, which stores the information about whether a segment is published in metadata store with used=1 or not. And it contains overshadowed segments and their is_published column is set to true, which may not be the correct behavior because those overshadowed segments are going to be marked used=0 once compaction finishes.

Contains the verb "stores"?

[leventov]

I don't like is_active. Active is a very overloaded term, and it's not obvious what's the difference between "active" and "published". I would suggest is_overshadowed, and if somebody wants "active", they need to write is_published = true AND is_overshadowed = false. is_overshadowed is defined as "overshadowed by other published segments that are loaded in the cluster". I suggest to add it only in sys.segments, but not in metadata store. In the codebase, all occurrences of "overshadowed" should be aligned with the above definition. "Overshadowed by published but not yet loaded segments" should be called somehow else (if it is used in the codebase), like "preOvershadowed".

[surekhasaharan]

Is this right that sys.segments table is a virtual table that Druid processes queries against for the SQL interface to Druid? It doesn't exist in the metadata store (or any other real SQL database Druid depends on)?

Yes this seems right, I guess "contains" conveys the meaning better than "stores" ? Not sure on which verb to use.

[surekhasaharan]

I would suggest is_overshadowed, and if somebody wants "active", they need to write is_published = true AND is_overshadowed = false.

We were trying to make it easier for the user to get active segments just by writing is_active=true and documenting what is_published vs is_active implies. Do you have any other suggested name instead of is_active ? Yeah we can always create a is_overshadowed col, and let user query the way you suggest, but thought the other approach might be more convenient for users.

I suggest to add it only in sys.segments, but not in metadata store.

That's the plan.

[jihoonson]

@leventov I agree that is_active is not the best term. That actually represents that the segment is supposed to be loaded or not, so that users can understand why some segments are not loaded, or notice that some segments should be loaded but they are not. Do you have a better suggestion?

Regarding is_overshadowed, I don't think it's a good idea to restrict its definition to such a narrow scope because it's being used in various situations in our code base as well as documents. So, for example, the coordinator searches for segments which are loaded by historicals and overshadowed by published segments to tidy up old segments. Brokers searches for non-overshadowed segments among available segments in tasks or historicals for query execution. Even in historicals, they searches for non-overshadowed segments among the segments loaded in itself (note that two segments of different versions but for the same interval can exist in the same historical before the coordinator cleans up the old one). I don't think it's worth to define different terms for various cases. It would make things more complicated.

[gianm]

is_overshadowed is defined as "overshadowed by other published segments that are loaded in the cluster"

Any particular reason you’re thinking it makes sense to define is_overshadowed as “overshadowed by other published segments that are loaded”? (I assume this would mean: segments with used = true and that are available on some server)

I’m thinking “overshadowed by some published segments” would be more useful in a debugging scenario. It allows you to write is_published && !is_overshadowed to mean “should be available”. In turn, comparing that to is_available helps narrow down where the current cluster state (what’s available) is different from the desired cluster state (what should be available).

[leventov]

Perhaps is_actual is better than is_active.

Regarding the semantics of is_overshadowed, you all are right, confusing it with the temporal state of being loaded on some servers is probably a wrong idea.

Maybe we need both:

• is_actual for published and not overshadowed by loaded segments, in other words, "segments that can and should be queried now".
• is_overshadowed to provide ability to introspect into is_published && !is_overshadowed and is_published && !is_overshadowed && !is_available views.

[jihoonson]

"segments that can and should be queried now".

I think this statement is a bit incorrect because segments which are currently being generated by stream ingestion tasks should also be able to be queried but is_actual doesn't include them.

[leventov]

Yes. Also, the part "can" is wrong, because is_available is responsible for "can". A segment may be "actual" (as per the definition above) but not available at the same time.

[surekhasaharan]

Thanks everyone for the discussion and your inputs on this. I am thinking of adding a column is_overshadowed to sys.segments at this time. Not adding any is_active or is_actual col to keep things clear and easy to understand and since you can get the is_actual by doing is_published && !is_overshadowed.

[surekhasaharan]

I have updated the issue with proposal components which talks about the way I am thinking of implementing is_overshadowed column addition to sys.segments.

[gianm]

@surekhasaharan Can you please add the definition of is_overshadowed to the proposal? (To the "Proposed changes" section.) It sounds like we are settling on something like 'published segments that are overshadowed by some other published segment'. I would be fine with it being false for unpublished segments, if that makes it easier to implement. If that's what you do, then just make sure to include it in the docs, please. Maybe document it as something like:

is_overshadowed is true for segments that are published and that are overshadowed by other published segments. Currently, is_overshadowed is always false for unpublished segments, although this may change in the future. You can filter for segments that "should be published" by filtering for is_published = 1 AND is_overshadowed = 0. Segments can briefly be both published and overshadowed if they were recently replaced, but have not been unpublished yet.

I am ok with leaving is_published and is_available unchanged, and adding an is_overshadowed defined as above. I'm also fine with leaving out active/actual at this time and potentially adding them in a later change. If this is what you're proposing @surekhasaharan then I am 👍 on it.

[surekhasaharan]

thanks @gianm for articulating the definition of is_overshadowed . Added the definition of is_overshadowed in the proposal. Yeah, the impression I got from the discussion is that is_overshadowed means "overshadowed by other published segments" and I am planing to leave is_overshadowed false for unpublished segments. Will add this definition to the sys.segments docs.

[jon-wei]

The current proposal (add is_overshadowed representing published segments that are overshadowed by other published segments, with overshadowing info provided by the coordinator when polling) looks good to me.

[leventov]

@surekhasaharan what do you think about "includeOvershadowedStatus", "SegmentWithOvershadowedStatus" instead of "overshadowInfo"?

[surekhasaharan]

@leventov includeOvershadowedStatus instead of includeOvershadowInfo seems fine to me for the query param in coordinator api. So the api would be GET /druid/coordinator/v1/metadata/segments?includeOvershadowedStatus. Did you mean to name the class "SegmentWithOvershadowedStatus " instead of "SegmentWithOvershadowInfo"? Yeah that also sounds good.

[leventov]

@surekhasaharan then the proposal looks good to me too.

If you plan to rename "used segments" to "published segments" across the codebase, I think it would be easier to do this after #7306 is merged, see #7306 (comment).

[surekhasaharan]

@leventov I do not plan on renaming "used" to "published" with this issue, this is solely focused on adding the overshadowed support. Sure, it would be best to change that terminology as a follow up PR after #7306.

[vogievetsky]

Looks like it is there now and in console also

Can this be closed?

[surekhasaharan]

Closed by #7425