-
Notifications
You must be signed in to change notification settings - Fork 24.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce data stream options and failure store configuration classes #109515
Merged
elasticsearchmachine
merged 22 commits into
elastic:main
from
gmarouli:add-data-stream-options
Sep 11, 2024
Merged
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
da3bab3
Introduce data stream options and failure store configuration classes
gmarouli 6352d2e
Fix format
gmarouli 9e41720
Fix license
gmarouli 9f19c4a
Remove unnecessary getter
gmarouli 47cca13
Fix wrong field name
gmarouli 4fcd3af
Add lifecycle to show how they combine
gmarouli 89c950b
Merge branch 'main' into add-data-stream-options
elasticmachine 9b1984e
Merge branch 'main' into add-data-stream-options
elasticmachine fbc03be
Update server/src/main/java/org/elasticsearch/cluster/metadata/DataSt…
gmarouli 2efb558
Merge branch 'main' into add-data-stream-options
elasticmachine 3fcf60c
Merge branch 'main' into add-data-stream-options
gmarouli a0453dc
Review comments
gmarouli dcc04fd
Merge branch 'main' into add-data-stream-options
elasticmachine 28777ee
Merge branch 'main' into add-data-stream-options
elasticmachine 8613f63
Merge branch 'main' into add-data-stream-options
elasticmachine 24aa320
Merge with main
gmarouli dd769d0
Use constructor instead of static method.
gmarouli 9f1fd8c
Merge branch 'main' into add-data-stream-options
elasticmachine a26bb6d
Remove lifecycle from the data stream options
gmarouli f31a50a
Merge branch 'main' into add-data-stream-options
gmarouli dc4cb37
Merge branch 'main' into add-data-stream-options
elasticmachine 1d3d566
Merge branch 'main' into add-data-stream-options
elasticmachine File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
76 changes: 76 additions & 0 deletions
76
server/src/main/java/org/elasticsearch/cluster/metadata/DataStreamFailureStore.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
package org.elasticsearch.cluster.metadata; | ||
|
||
import org.elasticsearch.cluster.Diff; | ||
import org.elasticsearch.cluster.SimpleDiffable; | ||
import org.elasticsearch.common.Strings; | ||
import org.elasticsearch.common.io.stream.StreamInput; | ||
import org.elasticsearch.common.io.stream.StreamOutput; | ||
import org.elasticsearch.xcontent.ConstructingObjectParser; | ||
import org.elasticsearch.xcontent.ParseField; | ||
import org.elasticsearch.xcontent.ToXContentObject; | ||
import org.elasticsearch.xcontent.XContentBuilder; | ||
import org.elasticsearch.xcontent.XContentParser; | ||
|
||
import java.io.IOException; | ||
|
||
/** | ||
* Holds the data stream failure store metadata that enable or disable the failure store of a data stream. Currently, it | ||
* supports the following configurations: | ||
* - enabled | ||
*/ | ||
public record DataStreamFailureStore(boolean enabled) implements SimpleDiffable<DataStreamFailureStore>, ToXContentObject { | ||
|
||
public static final ParseField ENABLED_FIELD = new ParseField("enabled"); | ||
|
||
public static final ConstructingObjectParser<DataStreamFailureStore, Void> PARSER = new ConstructingObjectParser<>( | ||
"failure_store", | ||
false, | ||
(args, unused) -> new DataStreamFailureStore(args[0] == null || (Boolean) args[0]) | ||
); | ||
|
||
static { | ||
PARSER.declareBoolean(ConstructingObjectParser.constructorArg(), ENABLED_FIELD); | ||
} | ||
|
||
public DataStreamFailureStore() { | ||
this(true); | ||
} | ||
nielsbauman marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
public DataStreamFailureStore(StreamInput in) throws IOException { | ||
this(in.readBoolean()); | ||
} | ||
|
||
public static Diff<DataStreamFailureStore> readDiffFrom(StreamInput in) throws IOException { | ||
return SimpleDiffable.readDiffFrom(DataStreamFailureStore::new, in); | ||
} | ||
|
||
@Override | ||
public void writeTo(StreamOutput out) throws IOException { | ||
out.writeBoolean(enabled); | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return Strings.toString(this, true, true); | ||
} | ||
|
||
@Override | ||
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { | ||
builder.startObject(); | ||
builder.field(ENABLED_FIELD.getPreferredName(), enabled); | ||
builder.endObject(); | ||
return builder; | ||
} | ||
|
||
public static DataStreamFailureStore fromXContent(XContentParser parser) throws IOException { | ||
return PARSER.parse(parser, null); | ||
} | ||
} |
93 changes: 93 additions & 0 deletions
93
server/src/main/java/org/elasticsearch/cluster/metadata/DataStreamOptions.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,93 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
package org.elasticsearch.cluster.metadata; | ||
|
||
import org.elasticsearch.cluster.Diff; | ||
import org.elasticsearch.cluster.SimpleDiffable; | ||
import org.elasticsearch.common.Strings; | ||
import org.elasticsearch.common.io.stream.StreamInput; | ||
import org.elasticsearch.common.io.stream.StreamOutput; | ||
import org.elasticsearch.core.Nullable; | ||
import org.elasticsearch.xcontent.ConstructingObjectParser; | ||
import org.elasticsearch.xcontent.ObjectParser; | ||
import org.elasticsearch.xcontent.ParseField; | ||
import org.elasticsearch.xcontent.ToXContentObject; | ||
import org.elasticsearch.xcontent.XContentBuilder; | ||
import org.elasticsearch.xcontent.XContentParser; | ||
|
||
import java.io.IOException; | ||
|
||
/** | ||
* Holds data stream dedicated configuration options such as failure store, (in the future lifecycle). Currently, it | ||
* supports the following configurations: | ||
* - failure store | ||
*/ | ||
public record DataStreamOptions(@Nullable DataStreamFailureStore failureStore) | ||
implements | ||
SimpleDiffable<DataStreamOptions>, | ||
ToXContentObject { | ||
|
||
public static final ParseField FAILURE_STORE_FIELD = new ParseField("failure_store"); | ||
|
||
public static final ConstructingObjectParser<DataStreamOptions, Void> PARSER = new ConstructingObjectParser<>( | ||
"options", | ||
false, | ||
(args, unused) -> new DataStreamOptions((DataStreamFailureStore) args[0]) | ||
); | ||
|
||
static { | ||
PARSER.declareField( | ||
ConstructingObjectParser.optionalConstructorArg(), | ||
(p, c) -> DataStreamFailureStore.fromXContent(p), | ||
FAILURE_STORE_FIELD, | ||
ObjectParser.ValueType.OBJECT_OR_NULL | ||
); | ||
} | ||
|
||
public DataStreamOptions() { | ||
this(null); | ||
} | ||
|
||
public static DataStreamOptions read(StreamInput in) throws IOException { | ||
return new DataStreamOptions(in.readOptionalWriteable(DataStreamFailureStore::new)); | ||
} | ||
|
||
@Nullable | ||
public DataStreamFailureStore getFailureStore() { | ||
return failureStore; | ||
} | ||
|
||
public static Diff<DataStreamOptions> readDiffFrom(StreamInput in) throws IOException { | ||
return SimpleDiffable.readDiffFrom(DataStreamOptions::read, in); | ||
} | ||
|
||
@Override | ||
public void writeTo(StreamOutput out) throws IOException { | ||
out.writeOptionalWriteable(failureStore); | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return Strings.toString(this, true, true); | ||
} | ||
|
||
@Override | ||
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { | ||
builder.startObject(); | ||
if (failureStore != null) { | ||
builder.field(FAILURE_STORE_FIELD.getPreferredName(), failureStore); | ||
} | ||
builder.endObject(); | ||
return builder; | ||
} | ||
|
||
public static DataStreamOptions fromXContent(XContentParser parser) throws IOException { | ||
return PARSER.parse(parser, null); | ||
} | ||
} |
42 changes: 42 additions & 0 deletions
42
server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamFailureStoreTests.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
package org.elasticsearch.cluster.metadata; | ||
|
||
import org.elasticsearch.common.io.stream.Writeable; | ||
import org.elasticsearch.test.AbstractXContentSerializingTestCase; | ||
import org.elasticsearch.xcontent.XContentParser; | ||
|
||
import java.io.IOException; | ||
|
||
public class DataStreamFailureStoreTests extends AbstractXContentSerializingTestCase<DataStreamFailureStore> { | ||
|
||
@Override | ||
protected Writeable.Reader<DataStreamFailureStore> instanceReader() { | ||
return DataStreamFailureStore::new; | ||
} | ||
|
||
@Override | ||
protected DataStreamFailureStore createTestInstance() { | ||
return randomFailureStore(); | ||
} | ||
|
||
@Override | ||
protected DataStreamFailureStore mutateInstance(DataStreamFailureStore instance) throws IOException { | ||
return new DataStreamFailureStore(instance.enabled() == false); | ||
} | ||
|
||
@Override | ||
protected DataStreamFailureStore doParseInstance(XContentParser parser) throws IOException { | ||
return DataStreamFailureStore.fromXContent(parser); | ||
} | ||
|
||
static DataStreamFailureStore randomFailureStore() { | ||
return new DataStreamFailureStore(randomBoolean()); | ||
} | ||
} |
44 changes: 44 additions & 0 deletions
44
server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamOptionsTests.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
package org.elasticsearch.cluster.metadata; | ||
|
||
import org.elasticsearch.common.io.stream.Writeable; | ||
import org.elasticsearch.test.AbstractXContentSerializingTestCase; | ||
import org.elasticsearch.xcontent.XContentParser; | ||
|
||
import java.io.IOException; | ||
|
||
public class DataStreamOptionsTests extends AbstractXContentSerializingTestCase<DataStreamOptions> { | ||
|
||
@Override | ||
protected Writeable.Reader<DataStreamOptions> instanceReader() { | ||
return DataStreamOptions::read; | ||
} | ||
|
||
@Override | ||
protected DataStreamOptions createTestInstance() { | ||
return new DataStreamOptions(randomBoolean() ? null : DataStreamFailureStoreTests.randomFailureStore()); | ||
} | ||
|
||
@Override | ||
protected DataStreamOptions mutateInstance(DataStreamOptions instance) throws IOException { | ||
var failureStore = instance.getFailureStore(); | ||
if (failureStore == null) { | ||
failureStore = DataStreamFailureStoreTests.randomFailureStore(); | ||
} else { | ||
failureStore = randomBoolean() ? null : new DataStreamFailureStore(failureStore.enabled() == false); | ||
} | ||
return new DataStreamOptions(failureStore); | ||
} | ||
|
||
@Override | ||
protected DataStreamOptions doParseInstance(XContentParser parser) throws IOException { | ||
return DataStreamOptions.fromXContent(parser); | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One of the things we're talking about doing is adding a cluster setting that will contain index patterns which will state that failure store is enabled for any matching indices by default. The plan is to fall back to using that cluster setting in the case that failure stores haven't been explicitly enabled or disabled via templates at creation time, or in the state.
To save on headache going forward, should this boolean be an optional
Boolean
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! thinking further about this, I would expect that the following to be the not explicitly set:
This follows the way that the lifecycle works. In template composition, a user can set
failure_store: null
to remove any configuration set by previous templates.Since this is a new configuration type we could always decide we would like to change the way we work with this, but I thought we should at least consider it.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see, since there is only one property we can just assume the entire section will be removed from options if it's not present. I'm good with that