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.

Sign up for GitHub

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

[DOCS] add comment to clarify cluster name resolution #34014

Merged
merged 2 commits into from
Sep 25, 2018
Merged

[DOCS] add comment to clarify cluster name resolution #34014

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
79dbec6
[DOCS] add comment to clarify cluster name resolution
javanna Sep 24, 2018
b308664
iter
javanna Sep 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,9 +231,10 @@ public Map<String, List<String>> groupClusterIndices(String[] requestIndices, Pr
List<String> clusters = clusterNameResolver.resolveClusterNames(remoteClusterNames, remoteClusterName);
if (clusters.isEmpty() == false) {
if (indexExists.test(index)) {
// we use : as a separator for remote clusters. might conflict if there is an index that is actually named
// remote_cluster_alias:index_name - for this case we fail the request. the user can easily change the cluster alias
// if that happens
//We use ":" as a separator for remote clusters. There may be a conflict if there is an index that is named
//remote_cluster_alias:index_name - for this case we fail the request. the user can easily change the cluster alias
//if that happens. Note that ":" can be part of indices names up to 6.last and part of aliases names up to 7.last.
//It will be possible to remove this check from 8.0 on.
throw new IllegalArgumentException("Can not filter indices; index " + index +
" exists but there is also a remote cluster named: " + remoteClusterName);
}
Expand All @@ -242,6 +243,10 @@ public Map<String, List<String>> groupClusterIndices(String[] requestIndices, Pr
perClusterIndices.computeIfAbsent(clusterName, k -> new ArrayList<>()).add(indexName);
}
} else {
//Indices can be created with ":" in their names only up to 5.6, and still be there up to 6.last, but aliases may be
Copy link
Member

Choose a reason for hiding this comment

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

I think both alias and index names can have : in the name until 6.last? https://github.com/elastic/elasticsearch/blob/6.x/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java#L170

This validation method is both used for alias and index names. From 6.0 we log a deprecation warning, but not fail it. From 7.0 we fail such index or alias nems

Copy link
Member Author

Choose a reason for hiding this comment

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

you are right, I thought that we introduced index name validation for that earlier compared to aliases. I will update.

Copy link
Member Author

Choose a reason for hiding this comment

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

I pushed an update, thanks for double checking this Martijn.

//created with ":" in their names up to 6.last (although deprecated) and still be around in 7.x.
//That's why we need to be lenient here and treat the index as local although it contains ":"
//It will be possible to remove such leniency and assume that no local indices contain ":" only from 8.0 on.
perClusterIndices.computeIfAbsent(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY, k -> new ArrayList<>()).add(index);
}
} else {
Expand Down