Skip to content

Conversation

gmarouli
Copy link
Contributor

The IndexAbstraction interface provides the methods getWriteIndex() and getIndices() to retrieve the backing indices and its subclasses implement them accordingly.

For the failure store, we implemented equivalent ways to retrieve the failure indices but only in the DataStream subclass. Until now this was sufficient but looking at #118614, I think the code could be simplified and be more robust if we pulled them up to IndexAbstraction; considering that a data stream alias can also be resolved to failure store indices.

I believe that if we add these methods we could easily retrieve the failure store indices after we check isDataStreamRelated() without needing to know how to resolve them.

With this PR I we add the equivalent failure indices retrieval methods, with a twist, we retrieve them lazily. Considering that the failure indices are not as often needed, we retrieve them lazily from the metadata and we do not burden all data stream aliases. So we propose the following:

Backing Indices Failure Indices
Index getWriteIndex() Index getWriteFailureIndex(Metadata metadata)
List<Index> getIndices() List<Index> getFailureIndices(Metadata metadata)

Furthermore, we renamed the following DataStream methods to be more symmetrical:

  • DataStreamIndices getBackingIndices() -> DataStreamIndices getDataComponent()
  • DataStreamIndices getFailureIndices() -> DataStreamIndices getFailureComponent()
  • List<Index> getFailureIndices() -> List<Index> getFailureIndices() this is symmetrical with the List<Index> getIndices().
  • Index getFailureStoreWriteIndex() -> Index getWriteFailureIndex() so it can be an overloaded version of Index getWriteFailureIndex(Metadata metadata).

Bonus, now the failure store retrieval can be tested easier.

@gmarouli gmarouli added >refactoring :Data Management/Data streams Data streams and their lifecycles labels Dec 31, 2024
@gmarouli gmarouli requested a review from jbaiera December 31, 2024 16:37
@elasticsearchmachine elasticsearchmachine added the Team:Data Management Meta label for data/management team label Dec 31, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@elasticsearchmachine elasticsearchmachine added the serverless-linked Added by automation, don't add manually label Jan 3, 2025
Copy link
Member

@jbaiera jbaiera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Sorry, this fell off my radar for a bit there, but the code looks so much cleaner after all this, thanks!

Copy link
Contributor

@gwbrown gwbrown left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, please go ahead and merge this ASAP so I can merge this into #119915, which currently duplicates much of this in a less complete, less tested way. Nice work!

@gmarouli gmarouli merged commit 2ee7ca4 into elastic:main Jan 17, 2025
16 checks passed
@gmarouli gmarouli deleted the failure-store/index-abstraction-refactoring branch January 17, 2025 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Data Management/Data streams Data streams and their lifecycles >refactoring serverless-linked Added by automation, don't add manually Team:Data Management Meta label for data/management team v8.18.0 v9.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants