Skip to content
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 regular indices in frozen tier #70141

Conversation

henningandersen
Copy link
Contributor

The frozen tier will be dedicated for partially cached searchable
snapshots. This PR ensures that we do not allow allocating regular
indices (including fully cached searchable snapshots) to the frozen
tier.

The frozen tier will be dedicated for partially cached searchable
snapshots. This PR ensures that we do not allow allocating regular
indices (including fully cached searchable snapshots) to the frozen
tier.
@elasticmachine elasticmachine added Team:Distributed Meta label for distributed team Team:Data Management Meta label for data/management team labels Mar 9, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (Team:Core/Features)

henningandersen added a commit to henningandersen/elasticsearch that referenced this pull request Mar 9, 2021
A setting validator can declare settings that the validation depends on,
but when updating index settings, we eagerly validate the settings
before submitting the cluster state update and here we do not know the
existing settings. With this commit, we ensure that the pre-validation
works with such validators, letting the validator validate syntax
(validate(value)) only.

Relates elastic#70141.
Blob cache index not on frozen.
When cloning frozen have to specify the tier
Comment on lines 567 to 571
if (validateDependencies) {
setting.get(settings);
} else {
setting.validateWithoutDependencies(settings);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change and the associated change in SecureSettings and tests have been split into #70144

These tests are going to be touched by other work, will wait merging
this until that is done.
Copy link
Contributor Author

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

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

Note to self to remove AwaitsFix before merge. Those tests are fixed by #70158, which I will integrate here before merging.

Now done.

@henningandersen
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/bwc

Old 7.12/7.13 clusters will create the .snapshot-blob-cache index with
tier preference including frozen, which causes cluster state updates
to fail when upgrading. Disabling bwc until this is in 7.12.
Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

I've just left one small comment, otherwise looking good. I did not review the dependent setting validation logic.


public static boolean isSearchableSnapshotStore(Settings indexSettings) {
return SNAPSHOT_DIRECTORY_FACTORY_KEY.equals(INDEX_STORE_TYPE_SETTING.get(indexSettings));
}

public static boolean isPartialSearchableSnapshotIndex(Map<Setting<?>, Object> indexSettings) {
return SNAPSHOT_DIRECTORY_FACTORY_KEY.equals(indexSettings.get(INDEX_STORE_TYPE_SETTING))
&& (boolean) indexSettings.get(SNAPSHOT_PARTIAL_SETTING);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this method throw a NPE if indexSettings does not contain SNAPSHOT_PARTIAL_SETTING?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it could, but the expectation is that the map contains the two settings. I have added assertions to enforce this.

@henningandersen
Copy link
Contributor Author

Thanks for reviewing, @dakrone and @ywelsch , I integrated the settings changes from #70144 so they no longer appear in this PR. This is ready for another round.

Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

LGTM

@henningandersen henningandersen merged commit dad23ed into elastic:master Mar 11, 2021
henningandersen added a commit to henningandersen/elasticsearch that referenced this pull request Mar 11, 2021
The frozen tier will be dedicated for partially cached searchable
snapshots. This PR ensures that we do not allow allocating regular
indices (including fully cached searchable snapshots) to the frozen
tier.
henningandersen added a commit that referenced this pull request Mar 11, 2021
The frozen tier will be dedicated for partially cached searchable
snapshots. This PR ensures that we do not allow allocating regular
indices (including fully cached searchable snapshots) to the frozen
tier.
henningandersen added a commit to henningandersen/elasticsearch that referenced this pull request Mar 11, 2021
The frozen tier will be dedicated for partially cached searchable
snapshots. This PR ensures that we do not allow allocating regular
indices (including fully cached searchable snapshots) to the frozen
tier.
henningandersen added a commit that referenced this pull request Mar 11, 2021
The frozen tier will be dedicated for partially cached searchable
snapshots. This PR ensures that we do not allow allocating regular
indices (including fully cached searchable snapshots) to the frozen
tier.
henningandersen added a commit to henningandersen/elasticsearch that referenced this pull request Mar 11, 2021
Reenable bwc tests now that elastic#70141 has been backported.
henningandersen added a commit that referenced this pull request Mar 11, 2021
Reenable bwc tests now that #70141 has been backported.
henningandersen added a commit that referenced this pull request Mar 11, 2021
Reenable bwc tests now that #70141 has been backported.
dakrone added a commit to dakrone/elasticsearch that referenced this pull request Mar 29, 2021
This commit converts the index metadata of searchable snapshot indices using the `shared_cache`
storage type to:

- Remove all the `index.routing.allocation.(include|exclude|require)._tier` settings
- Sets `index.routing.allocation.include._tier_preference` to `data_frozen` automatically when the index metadata is read

This is in preperation to enforcing that the `_tier_preference` setting is always set to
`data_frozen` for shared cache SBIs.

Relates to elastic#70846, elastic#71013, elastic#70786, elastic#70141
dakrone added a commit that referenced this pull request Mar 31, 2021
This commit converts the index metadata of searchable snapshot indices using the `shared_cache`
storage type to:

- Remove all the `index.routing.allocation.(include|exclude|require)._tier` settings
- Sets `index.routing.allocation.include._tier_preference` to `data_frozen` automatically when the index metadata is read

This is in preperation to enforcing that the `_tier_preference` setting is always set to
`data_frozen` for shared cache SBIs.

Relates to #70846, #71013, #70786, #70141
dakrone added a commit to dakrone/elasticsearch that referenced this pull request Mar 31, 2021
This commit converts the index metadata of searchable snapshot indices using the `shared_cache`
storage type to:

- Remove all the `index.routing.allocation.(include|exclude|require)._tier` settings
- Sets `index.routing.allocation.include._tier_preference` to `data_frozen` automatically when the index metadata is read

This is in preperation to enforcing that the `_tier_preference` setting is always set to
`data_frozen` for shared cache SBIs.

Relates to elastic#70846, elastic#71013, elastic#70786, elastic#70141
dakrone added a commit that referenced this pull request Mar 31, 2021
…#71129)

This commit converts the index metadata of searchable snapshot indices using the `shared_cache`
storage type to:

- Remove all the `index.routing.allocation.(include|exclude|require)._tier` settings
- Sets `index.routing.allocation.include._tier_preference` to `data_frozen` automatically when the index metadata is read

This is in preperation to enforcing that the `_tier_preference` setting is always set to
`data_frozen` for shared cache SBIs.

Relates to #70846, #71013, #70786, #70141
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >non-issue Team:Data Management Meta label for data/management team Team:Distributed Meta label for distributed team v7.12.0 v7.13.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants