-
Notifications
You must be signed in to change notification settings - Fork 25.5k
Force rollover on write to true when data stream indices list is empty #133347
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
Conversation
Pinging @elastic/es-data-management (Team:Data Management) |
Hi @jbaiera, I've created a changelog YAML for you. |
qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/FailureStoreUpgradeIT.java
Outdated
Show resolved
Hide resolved
qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/FailureStoreUpgradeIT.java
Show resolved
Hide resolved
CI is failing because rollover on write is forced true on any data stream index set which is empty, but this is not allowed because data streams created for CCR may not have rollover on write enabled. As such, I have a fix coming in which should simply update the default places in deserialization that we set the flag and make them default to true on an empty index list. |
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.
LGTM
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.
LGTM
dataStreamOptions, | ||
new DataStreamIndices(BACKING_INDEX_PREFIX, List.copyOf(indices), rolloverOnWrite, autoShardingEvent), | ||
new DataStreamIndices(FAILURE_STORE_PREFIX, List.copyOf(failureIndices), false, null) | ||
new DataStreamIndices(FAILURE_STORE_PREFIX, List.copyOf(failureIndices), failureIndices.isEmpty(), null) |
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.
Is the reason we force rolloverOnWrite
as a param to the next constructor here instead other constructor to bypass the CCR error you mentioned? If so I find it a little subtle to read, but I am not sure how to do it more explicitly.
Disregard this, I think I mis-read the constructor arguments, there are too many of them
390818d
to
d7fbb12
Compare
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
elastic#133347) Updates the serialization and construction logic for failure store components to bias rollover on write to true when no indices are present in the failure store index set. --------- Co-authored-by: elasticsearchmachine <[email protected]> Co-authored-by: Keith Massey <[email protected]> (cherry picked from commit 5ae52bf)
elastic#133347) Updates the serialization and construction logic for failure store components to bias rollover on write to true when no indices are present in the failure store index set. --------- Co-authored-by: elasticsearchmachine <[email protected]> Co-authored-by: Keith Massey <[email protected]>
#133347) (#133406) Updates the serialization and construction logic for failure store components to bias rollover on write to true when no indices are present in the failure store index set. --------- (cherry picked from commit 5ae52bf) Co-authored-by: elasticsearchmachine <[email protected]> Co-authored-by: Keith Massey <[email protected]>
If a data stream indices list is ever empty, then it cannot accept any write traffic. This is a state that is not allowed in the normal backing indices, but as we add more index sets to a data stream that are lazily created like failure store, this property should not only reflect any state that was marked via a lazy rollover API call, but also be forced on when no indices exist so that they may be initialized on first write.
An upgrade test is present to ensure that failure store indices created on versions from before their support are able to be enabled and used.
Fixes #133176