Support choosing the downsampling method in data stream lifecycle

[gmarouli]

Following #136813, we expose to data stream lifecycle the new sampling method config in the downsampling API. This will allow users to configure the sampling method directly in the lifecycle configuration. For example:

PUT _data_stream/my-ds/_lifecycle
{
  "data_retention": "10d",
  "downsampling_method": "last_value",
  "downsampling": [
     {
       "after": "1d",
       "fixed_interval": "5m
     }
  ]
}

Specification PR: elastic/elasticsearch-specification#5594

[elasticsearchmachine]

Pinging @elastic/es-storage-engine (Team:StorageEngine)

[elasticsearchmachine]

Pinging @elastic/es-data-management (Team:Data Management)

[elasticsearchmachine]

Hi @gmarouli, I've created a changelog YAML for you.

[nielsbauman]

Thanks again for working on this, Mary! I posted some minor comments, so hopefully it doesn't take you too long to get through them.

[nielsbauman]

What exactly is the reasoning behind this sampling method logic? Why are we specifying the sampling method from the source index metadata? Is there a reason we can't just always pass the requestedDownsamplingMethod? I'm not saying it's wrong, it's just unexpected to me. Could you explain your thinking at least in a comment in the code and optionally here in the PR?

[gmarouli]

The goal of this is to make data stream lifecycle handle changes of the sampling method gracefully. Consider the following scenario.

1. User has a data stream with a couple of downsampling rounds, and there are backing indices that are half way in their downsampled journey, for example they have been downsampled once, but there are still more rounds that apply to them.
2. User decides to change the sampling method of their lifecycle for this data stream.
3. If we use the requested method to downsample an already downsampled index, this will fail. DLM will report the errors for this indices until it's time to delete them.

The same thing applies for ILM, but in ILM a user can create a new policy with the new sampling method so it will only be applied to the newest backing indices.

Considering that we want data stream lifecycle to work seamlessly, we thought that using the downsampling method of the source index when available is closer to how the user is expecting it to work (considering the limitations).

Does this make sense?

[nielsbauman]

Yep, that definitely makes sense. Thanks for the explanation :). Could you put that explanation somewhere in this method?

[nielsbauman]

Perhaps you were already planning this, but I'd recommend adding some docs somewhere that describe this update behavior. I don't know downsampling that well, so TBH I have trouble understanding this test (i.e. why the 5m index gets deleted), but that's probably on me.

[gmarouli]

That is a good point, I added it within the test just like the previous tests has a similar comment. Let me know if it's clear now.

[nielsbauman]

I was actually referring to adding some documentation for users on how DLM handles changing the downsampling method. But I appreciate this a lot too! I still think there's value in documenting this for users, what do you think?

[gmarouli]

I agree, my concern is that we need to see documentation more holistically, not only on the API level. I will add a note there, but the sampling method needs to be documented and explained properly.

[gmarouli]

There is a documentation task in the meta issue: #128357

[nielsbauman]

Should we have a test somewhere that verifies that the downsampling method is resolved correctly with certain permutations of component templates, index templates, and lifecycles? Or do you think that's already covered sufficiently by the ResettableValue infrastructure? This could be either in a YAML or Java REST tests, although the latter feels like a better choice to me.

[gmarouli]

Good point, we should extend template composition tests to include this too. However, I do not think this should be a YAML or a REST test, it feels too low level for that. We already have

elasticsearch/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java

Line 1092 in 1d383f6

public void testResolveLifecycle() throws Exception {

. I would add the sampling method here. What do you think?

[nielsbauman]

Mostly LGTM, we just have one or two outstanding discussions to resolve, after that I'll approve :)