Skip to content

Deprecation check for : in Cluster/Index name#36185

Merged
AthenaEryma merged 3 commits intoelastic:6.xfrom
AthenaEryma:depcheck/colon-in-names
Dec 4, 2018
Merged

Deprecation check for : in Cluster/Index name#36185
AthenaEryma merged 3 commits intoelastic:6.xfrom
AthenaEryma:depcheck/colon-in-names

Conversation

@AthenaEryma
Copy link
Copy Markdown
Contributor

@AthenaEryma AthenaEryma commented Dec 3, 2018

Adds a deprecation check for cluster and index names that contain :,
which is illegal in 7.0.

Relates to #36024 and #26247

Adds a deprecation check for cluster and index names that contain `:`,
which is illegal in 7.0.
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-core-features

int currentOpenShards = state.getMetaData().getTotalOpenIndexShards();

if (currentOpenShards >= maxShardsInCluster) {
if (nodeCount != 0 && currentOpenShards >= maxShardsInCluster) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a fix for bug that was revealed by the test for the new cluster-level deprecation check, but I sincerely doubt it would ever be hit in practice and I didn't think it was worth its own PR.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: IMO nodeCount > 0 && ... reads better, else i am asking myself how this value can ever be negative.

static DeprecationIssue indexNameCheck(IndexMetaData indexMetaData) {
String clusterName = indexMetaData.getIndex().getName();
if (clusterName.contains(":")) {
return new DeprecationIssue(DeprecationIssue.Level.CRITICAL,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there guidance on what is Critical vs. Warning ?

Copy link
Copy Markdown
Contributor Author

@AthenaEryma AthenaEryma Dec 3, 2018

Choose a reason for hiding this comment

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

I believe Critical means "if you upgrade without fixing this your cluster will fail", whereas Warning means "you will get some deprecation warnings, and/or behavior probably won't be what you want".

This looks like it should be Critical for the cluster name but Warning for the index name - I'll fix that.

Copy link
Copy Markdown
Contributor

@jakelandis jakelandis left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

LGTM

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants