Skip to content

Conversation

@tlrx
Copy link
Member

@tlrx tlrx commented Jul 16, 2020

This pull request adds the basic infrastructure that creates an index template for the future .snapshots-N system indices. Those indices will contain documents about blobs stored in remote snapshot repositories.

This change will go in master first and will be backported along with future changes once the feature is stable enough.

@tlrx tlrx changed the title Add searchable snapshots index template for snapshot cache index Add searchable snapshots index template for snapshot cache indices Jul 16, 2020
@tlrx tlrx added :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >enhancement v8.0.0 labels Jul 16, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (:Distributed/Snapshot/Restore)

@elasticmachine elasticmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Jul 16, 2020
@dakrone
Copy link
Member

dakrone commented Jul 16, 2020

@tlrx should this be using the legacy templates, given that they will be deprecated for 8.0? Since this is a new thing, I think it'd make more sense to start off using composable index templates so that it doesn't have to be updated at a later time

@tlrx
Copy link
Member Author

tlrx commented Jul 16, 2020

@dakrone Thanks for spotting this. I missed the information that composable index templates will replace index templates. I'm going to update the code here to avoid using legacy templates.

@tlrx tlrx requested review from DaveCTurner and dakrone July 17, 2020 11:18
Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

LGTM, I left three really minor things, thanks Tanguy!

"aliases": {
".snapshots": {}
},
"settings": {
Copy link
Member

Choose a reason for hiding this comment

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

Should .snapshots indices also have index.hidden: true set in their settings? (I can't remember right now whether dot-prefixed indices automatically are hidden)

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

I left one request regarding naming; otherwise this is all quite unfamiliar territory to me but looks reasonable.

// version 1: initial
static final int INDEX_TEMPLATE_VERSION = 1;

static final String SNAPSHOTS_CACHE_TEMPLATE_NAME = ".snapshots";
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we call this something more specific, e.g. .snapshot-blob-cache? I worry that we might want to introduce other snapshot-related system indices in future, so the choice of a generic name now will result in confusion and/or a painful migration later.

@tlrx
Copy link
Member Author

tlrx commented Aug 27, 2020

We decided to use a dedicated system index, implemented in #60522. I'm closing this issue.

Thanks for the feedback, and sorry for the noise 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >enhancement Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants