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.
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 "Feature States" for managing snapshots of system indices #63513
Introduce "Feature States" for managing snapshots of system indices #63513
Changes from all commits
65903ab
6d32a65
37e4f4b
1fefb26
1869e91
ac2961f
febcdd7
6561940
c16e472
0c53a22
f0ef426
1f4ecfe
f824abe
040fda6
013b764
9120ffa
26e94f6
df3deff
ce24b74
9382885
261f5c0
89d8e19
c207993
08406bd
d2b517b
93af221
b6392af
e4a4580
b7ef98e
9197bb8
8d6cbcb
27df301
a344a65
2a5c42a
2ff22b1
7ecabdc
f069cf1
93c96df
9a1ee0f
06898ba
b86dcf6
4b5cb08
1966575
e3efb29
c8ed77a
e9b25ac
a8398fa
87fde96
8ccbba6
410b8bb
c262c0b
952ffb0
aa32eb5
7ba77bd
d544f1e
d2adae1
88c6c41
c3f8813
64f60d0
ab4106a
031e86e
9b6d1ff
f24ac78
c8ca4a1
c16d60f
3b0fb7f
cfbfbcb
d0b3a44
091ca4c
05fd6d7
4af0a24
439679d
376e264
3c3b1b8
d36f6c8
ef88902
10f2802
e9b0f80
2e54c94
7300c0c
2b9945b
cdc3bd1
9520873
a0baac2
262760f
b416193
2dde730
6831671
510ab18
20c6b98
15b4903
792d735
b375b4b
85b90e8
3909cfc
86ed638
0a70ae1
9f16920
2e652fa
178e941
afdf54c
8c324fd
eec5785
1b99941
055adf2
290b203
f7a0dc2
774f11c
82cfdac
46f5eed
8be4e74
1bec52b
59ab240
b938323
201eec6
7ceecae
6c4669f
596c9ac
d677aeb
8946c94
1476d8d
884983c
1dc1a80
1e37dc6
8f038ee
898a3b3
fe203f6
dc2ac03
1e65d12
fda3a9c
455f762
ba8aba3
4c5ce56
eeda4e1
b78fbcd
facf8fa
4bc6bae
56fbb20
6b1cf06
12d1b25
699c2ae
d503868
f2d7bbe
ef0dda4
6d9edd8
2b52037
38022dd
c5d68ea
873de63
6dd31f1
66bc5cd
90bf18d
dd8889f
d300586
b636420
6930e00
d4267d4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
EDIT: please revert noisy format changes in this PR, they're all over the place now that I read through it in full :)
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.
There isn't a great place to leave this comment but this is in the section about what the
cluster state
contains. IMO we should change the wording above to say global state instead of cluster state. Alternatively we can move this out of being included in the cluster state and say that global state also includes data stored in system indices.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.
I have concerns regarding the consistency of what an empty array means. For indices within a CreateSnapshotRequest, it means all open indices. The behavior for features states is different and this feels wrong to me.
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.
That's fair and a good point - my intent with making it that way was to be able to say "I want to snapshot/restore my cluster state, and no system indices". Do you have any suggestions for an alternate way of doing that?
Alternatively, is that even something we need at all? This feels like something that should be possible given you can omit features, but I don't necessarily have a use case in mind.
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.
A few items that I can think of would be introducing a special
none
option, completely omittingfeature_states
, or+
-
and*
support.Regarding whether we need it, I guess there could be a case where someone just wants to restore how they did previously and only include items from the cluster state.
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.
My favorite of those is the
none
option I think. Do you think["none"]
is acceptable, with an error ifnone
is present but isn't the only value in the array? Just using a bare string complicates parsing.When feature states are completely omitted I think we should continue to base the default on
include_global_state
, which if that'strue
means "all feature states" (rather similar to how omitting theindices
field implicitly means "all indices", at least on the restore side).Introducing +/- support would also be workable, but I think that makes usage more complex. I don't like pattern support here, since feature names don't fall into patterns like indices often do - I think that would just make it more confusing (e.g. "Do I have to say
enrich*
to restore all my enrich indices?").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.
I agree that the
none
option is the best and also agree that it must be the only item in the array in that case.