-
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
Avoid auto following leader system indices in CCR #72815
Avoid auto following leader system indices in CCR #72815
Conversation
Pinging @elastic/es-distributed (Team:Distributed) |
Related #67686 |
@jaymode I kind of regard this a bugfix, but to some extent it is only a bugfix towards a concept introduced in 7.x (system indices) so I suppose we could also regard this a breaking change requiring some level of BWC (like adding a flag to specify the behavior and remove this in 8.0). I seek your guidance on this subject given your experience with other system index related changes. |
Is it possible to make this change 8.0 only and just have a deprecation log/warning emitted in 7.x? I think we may find some users doing this for system indices like Kibana even if we do not advise it and it would be best not to break them in 7.x. |
Yes, that should be possible, @fcofdez will you open a PR to add the deprecations for 7.x (or 8+7.x). Then we can continue on this when that is merged (I prefer to get the deprecation in before doing the breaking change here). That also means this PR should have a breaking changes section. Given that we are adding the exclusions option in a separate PR I see no reason to add a flag or similar. |
This commits deprecates Auto-Follow of system indices Relates #72815
Sorry I lost track of this. I'll update the branch and I'll add the breaking changes section. Thanks for the ping. |
@elasticmachine run elasticsearch-ci/docs-check |
@henningandersen could you review this when you have the chance? Thanks! |
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.
assertThat(autoFollowResults.get(0).autoFollowExecutionResults, is(anEmptyMap())); | ||
} | ||
|
||
private List<AutoFollowCoordinator.AutoFollowResult> executeAutoFollow(String indexPattern, final ClusterState finalRemoteState) { |
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.
It would be nice to add a single test that also uses this method, that actually creates an index. It is sort of a test of the test, but given the complexity of this test method and the fact that if it just returns nothing, we are good, I think it is worthwhile.
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.
Added in d8a7c41
Relates #67686