Add RollupV2 cluster metadata behind feature-flag#64680
Add RollupV2 cluster metadata behind feature-flag#64680talevy merged 5 commits intoelastic:masterfrom
Conversation
|
Pinging @elastic/es-analytics-geo (:Analytics/Rollup) |
d4ad68e to
fbf53c3
Compare
this commit sets the foundation for following rollup-v2-related commits to master. The intention of this metadata is to be used by both the upcoming RollupV2 action that will create new indices along with their associated cluster metadata. This metadata is to be used by the SearchService when filtering shards in the can-match phase to decide which of the indices belonging to an original index should be queried.
fbf53c3 to
d287131
Compare
server/src/main/java/org/elasticsearch/cluster/metadata/RollupGroup.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/cluster/metadata/RollupGroup.java
Show resolved
Hide resolved
| } else if ("false".equals(property)) { | ||
| ROLLUPV2_FEATURE_FLAG_REGISTERED = false; | ||
| } else if (property == null) { | ||
| ROLLUPV2_FEATURE_FLAG_REGISTERED = null; |
There was a problem hiding this comment.
Shouldn't we just default to false for now? Is there a benefit for making this nullable throughout the code?
There was a problem hiding this comment.
Unclear. I kind of chose the "keep feature-flag behavior consistent" route. Instead of asking which is better, I decided to just follow the same pattern that was used by Autoscaling
csoulios
left a comment
There was a problem hiding this comment.
LGTM!
I am still not 100% sure if we need more metadata in the cluster state, but we can start with those and change later if needed.
Thanks Tal!
|
@csoulios there will be more that I can think of that I left out for now since my POC was not leveraging them. One detail that is missing: Type of interval - Calendar vs. Fixed. These can be added as needed, will be very minor A lot of the details needed to decide between different indices in the rollup group tend to be related to which fields are accessible, and for which date interval |
server/src/main/java/org/elasticsearch/cluster/metadata/RollupGroup.java
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/common/time/WriteableZoneId.java
Outdated
Show resolved
Hide resolved
| // a map from index-name to the date interval used in the associated index | ||
| private Map<String, DateHistogramInterval> dateInterval; | ||
| // a map from index-name to timezone used in the associated index | ||
| private Map<String, WriteableZoneId> dateTimezone; |
There was a problem hiding this comment.
Can we condense this down to maybe just one map with a value that holds the interval and the time zone?
There was a problem hiding this comment.
very much so. Once all the data is being used by the search-resolution code, I suspect the contents of these maps will change 👍 and be simplified into one Map containing a larger Object holding the necessary information in one place
…neId.java Co-authored-by: Mark Tozzi <mark.tozzi@gmail.com>
this commit sets the foundation for following rollup-v2-related commits to master. The intention of this metadata is to be used by both the upcoming RollupV2 action that will create new indices along with their associated cluster metadata. This metadata is to be used by the SearchService when filtering shards in the can-match phase to decide which of the indices belonging to an original index should be queried. Co-authored-by: Mark Tozzi <mark.tozzi@gmail.com>
This commit lowers the minimum compatiblity of RollupMetadata since elastic#64892 was merged into 7.11. relates to elastic#64680.
this commit sets the foundation for following rollup-v2-related
commits to master.
The intention of this metadata is to be used by both the upcoming
RollupV2 action that will create new indices along with their associated
cluster metadata. This metadata is to be used by the SearchService when
filtering shards in the can-match phase to decide which of the indices
belonging to an original index should be queried.
7.x (7.11) backport: #64892