Remove dangling index auto import functionality#59698
Remove dangling index auto import functionality#59698pugnascotia merged 4 commits intoelastic:masterfrom
Conversation
Closes 48366.
pugnascotia
added
>breaking
:Distributed/Recovery
|
Pinging @elastic/es-distributed (:Distributed/Recovery) |
DaveCTurner
left a comment
DaveCTurner
left a comment
There was a problem hiding this comment.
Looks good, I left a few very minor comments.
| truly represents the latest state of the data when the index was still part | ||
| of the cluster. | ||
|
|
||
| WARNING: You should avoid situations that result in dangling indices (e.g. |
There was a problem hiding this comment.
I don't think we need this warning any more; at least, you probably already know you're doing something wrong before you get to the question of importing dangling indices.
| */ | ||
| void findNewAndAddDanglingIndices(final Metadata metadata) { | ||
| final IndexGraveyard graveyard = metadata.indexGraveyard(); | ||
| // Extracted from getDanglingIndices() as a package-private method to allow easier testing testing |
There was a problem hiding this comment.
Looks like the tests already mock out the ClusterService, so I think we could pass in the metadata via that mock, have them call getDanglingIndices() and thereby inline this. Or have DanglingIndicesState track a Supplier<ClusterState> rather than the full ClusterService and pass that in for the tests. Not sure which I prefer really, up to you.
| final Settings allocateSettings = Settings.builder().build(); | ||
|
|
||
| final ClusterService clusterServiceMock = mock(ClusterService.class); | ||
| when(clusterServiceMock.getSettings()).thenReturn(allocateSettings); |
There was a problem hiding this comment.
Don't think we need to mock this any more.
| when(clusterServiceMock.getSettings()).thenReturn(allocateSettings); |
| return danglingIndices; | ||
| } catch (IOException e) { | ||
| logger.warn("failed to list dangling indices", e); | ||
| logger.warn("Failed to list dangling indices", e); |
There was a problem hiding this comment.
I prefer the original:
| logger.warn("Failed to list dangling indices", e); | |
| logger.warn("failed to list dangling indices", e); |
| * their state written on disk, but don't exists in the metadata of the cluster). | ||
| */ | ||
| public class DanglingIndicesState implements ClusterStateListener { | ||
| public class DanglingIndicesState { |
|
Thanks for the review @DaveCTurner, I've addressed your comments. |
DaveCTurner
left a comment
DaveCTurner
left a comment
There was a problem hiding this comment.
Checkstyle seems unhappy but LGTM once CI is green.
…ndex-auto-import-v2
|
@elasticmachine run elasticsearch-ci/1 |
Adds an 8.0 breaking change for PR #59698.
4 participants
Closes #48366. Remove all traces of automatically importing dangling indices. This change will not be backported, though this functionality is deprecated as of 7.9.0.