-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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
add support for write index resolution when creating/updating documents #31520
Changes from 25 commits
9e86921
df8fabd
7c75916
113ebc5
fb7c168
c1d2d4e
b585835
42893bf
54d0a3d
b5e5e81
d4a5ebd
2f8a004
9238cb5
8ae0967
e8cf8c8
2f2af95
3df26bd
5d47213
00eb394
7c753db
f1032e4
347149e
e58a3fb
222ae13
ae372f3
b54efe4
cda911f
eb097c2
cc969d1
284d3ee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,7 +42,6 @@ | |
|
||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.Collection; | ||
import java.util.Collections; | ||
import java.util.HashMap; | ||
import java.util.HashSet; | ||
|
@@ -103,7 +102,7 @@ public String[] concreteIndexNames(ClusterState state, IndicesOptions options, S | |
return concreteIndexNames(context, indexExpressions); | ||
} | ||
|
||
/** | ||
/** | ||
* Translates the provided index expression into actual concrete indices, properly deduplicated. | ||
* | ||
* @param state the cluster state containing all the data to resolve to expressions to concrete indices | ||
|
@@ -117,7 +116,7 @@ public String[] concreteIndexNames(ClusterState state, IndicesOptions options, S | |
* indices options in the context don't allow such a case. | ||
*/ | ||
public Index[] concreteIndices(ClusterState state, IndicesOptions options, String... indexExpressions) { | ||
Context context = new Context(state, options); | ||
Context context = new Context(state, options, false, false); | ||
return concreteIndices(context, indexExpressions); | ||
} | ||
|
||
|
@@ -193,30 +192,44 @@ Index[] concreteIndices(Context context, String... indexExpressions) { | |
} | ||
} | ||
|
||
Collection<IndexMetaData> resolvedIndices = aliasOrIndex.getIndices(); | ||
if (resolvedIndices.size() > 1 && !options.allowAliasesToMultipleIndices()) { | ||
String[] indexNames = new String[resolvedIndices.size()]; | ||
int i = 0; | ||
for (IndexMetaData indexMetaData : resolvedIndices) { | ||
indexNames[i++] = indexMetaData.getIndex().getName(); | ||
if (aliasOrIndex.isAlias() && context.isResolveToWriteIndex()) { | ||
AliasOrIndex.Alias alias = (AliasOrIndex.Alias) aliasOrIndex; | ||
IndexMetaData writeIndex = alias.getWriteIndex(); | ||
if (writeIndex == null) { | ||
if (alias.getIndices().size() > 1) { | ||
throw new IllegalArgumentException("Alias [" + alias.getAliasName() + | ||
"] points to multiple indices with none set as a write-index [is_write_index=true]"); | ||
} else { | ||
throw new IllegalArgumentException("Alias [" + alias.getAliasName() + "] points to an index [" | ||
+ alias.getIndices().get(0).getIndex().getName() + "] with [is_write_index=false]"); | ||
} | ||
} | ||
throw new IllegalArgumentException("Alias [" + expression + "] has more than one indices associated with it [" + | ||
concreteIndices.add(writeIndex.getIndex()); | ||
} else { | ||
if (aliasOrIndex.getIndices().size() > 1 && !options.allowAliasesToMultipleIndices()) { | ||
String[] indexNames = new String[aliasOrIndex.getIndices().size()]; | ||
int i = 0; | ||
for (IndexMetaData indexMetaData : aliasOrIndex.getIndices()) { | ||
indexNames[i++] = indexMetaData.getIndex().getName(); | ||
} | ||
throw new IllegalArgumentException("Alias [" + expression + "] has more than one indices associated with it [" + | ||
Arrays.toString(indexNames) + "], can't execute a single index op"); | ||
} | ||
} | ||
|
||
for (IndexMetaData index : resolvedIndices) { | ||
if (index.getState() == IndexMetaData.State.CLOSE) { | ||
if (failClosed) { | ||
throw new IndexClosedException(index.getIndex()); | ||
} else { | ||
if (options.forbidClosedIndices() == false) { | ||
concreteIndices.add(index.getIndex()); | ||
for (IndexMetaData index : aliasOrIndex.getIndices()) { | ||
if (index.getState() == IndexMetaData.State.CLOSE) { | ||
if (failClosed) { | ||
throw new IndexClosedException(index.getIndex()); | ||
} else { | ||
if (options.forbidClosedIndices() == false) { | ||
concreteIndices.add(index.getIndex()); | ||
} | ||
} | ||
} else if (index.getState() == IndexMetaData.State.OPEN) { | ||
concreteIndices.add(index.getIndex()); | ||
} else { | ||
throw new IllegalStateException("index state [" + index.getState() + "] not supported"); | ||
} | ||
} else if (index.getState() == IndexMetaData.State.OPEN) { | ||
concreteIndices.add(index.getIndex()); | ||
} else { | ||
throw new IllegalStateException("index state [" + index.getState() + "] not supported"); | ||
} | ||
} | ||
} | ||
|
@@ -255,6 +268,28 @@ public Index concreteSingleIndex(ClusterState state, IndicesRequest request) { | |
return indices[0]; | ||
} | ||
|
||
/** | ||
* Utility method that allows to resolve an index expression to its corresponding single write index. | ||
* | ||
* @param state the cluster state containing all the data to resolve to expression to a concrete index | ||
* @param request The request that defines how the an alias or an index need to be resolved to a concrete index | ||
* and the expression that can be resolved to an alias or an index name. | ||
* @throws IllegalArgumentException if the index resolution does not lead to an index, or leads to more than one index | ||
* @return the write index obtained as a result of the index resolution | ||
*/ | ||
public Index concreteWriteIndex(ClusterState state, IndicesRequest request) { | ||
if (request.indices() == null || (request.indices() != null && request.indices().length != 1)) { | ||
throw new IllegalArgumentException("indices request must specify a single index expression"); | ||
} | ||
Context context = new Context(state, request.indicesOptions(), false, true); | ||
Index[] indices = concreteIndices(context, request.indices()[0]); | ||
if (indices.length != 1) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💯 |
||
throw new IllegalArgumentException("The index expression [" + request.indices()[0] + | ||
"] and options provided did not point to a single write-index"); | ||
} | ||
return indices[0]; | ||
} | ||
|
||
/** | ||
* @return whether the specified alias or index exists. If the alias or index contains datemath then that is resolved too. | ||
*/ | ||
|
@@ -292,7 +327,7 @@ public String[] indexAliases(ClusterState state, String index, Predicate<AliasMe | |
String... expressions) { | ||
// expand the aliases wildcard | ||
List<String> resolvedExpressions = expressions != null ? Arrays.asList(expressions) : Collections.emptyList(); | ||
Context context = new Context(state, IndicesOptions.lenientExpandOpen(), true); | ||
Context context = new Context(state, IndicesOptions.lenientExpandOpen(), true, false); | ||
for (ExpressionResolver expressionResolver : expressionResolvers) { | ||
resolvedExpressions = expressionResolver.resolve(context, resolvedExpressions); | ||
} | ||
|
@@ -512,24 +547,26 @@ static final class Context { | |
private final IndicesOptions options; | ||
private final long startTime; | ||
private final boolean preserveAliases; | ||
private final boolean resolveToWriteIndex; | ||
|
||
Context(ClusterState state, IndicesOptions options) { | ||
this(state, options, System.currentTimeMillis()); | ||
} | ||
|
||
Context(ClusterState state, IndicesOptions options, boolean preserveAliases) { | ||
this(state, options, System.currentTimeMillis(), preserveAliases); | ||
Context(ClusterState state, IndicesOptions options, boolean preserveAliases, boolean resolveToWriteIndex) { | ||
this(state, options, System.currentTimeMillis(), preserveAliases, resolveToWriteIndex); | ||
} | ||
|
||
Context(ClusterState state, IndicesOptions options, long startTime) { | ||
this(state, options, startTime, false); | ||
this(state, options, startTime, false, false); | ||
} | ||
|
||
Context(ClusterState state, IndicesOptions options, long startTime, boolean preserveAliases) { | ||
Context(ClusterState state, IndicesOptions options, long startTime, boolean preserveAliases, boolean resolveToWriteIndex) { | ||
this.state = state; | ||
this.options = options; | ||
this.startTime = startTime; | ||
this.preserveAliases = preserveAliases; | ||
this.resolveToWriteIndex = resolveToWriteIndex; | ||
} | ||
|
||
public ClusterState getState() { | ||
|
@@ -552,6 +589,14 @@ public long getStartTime() { | |
boolean isPreserveAliases() { | ||
return preserveAliases; | ||
} | ||
|
||
/** | ||
* This is used to require that aliases resolve to their write-index. It is currently not used in conjunction | ||
* with <code>preserveAliases</code>. | ||
*/ | ||
boolean isResolveToWriteIndex() { | ||
return resolveToWriteIndex; | ||
} | ||
} | ||
|
||
private interface ExpressionResolver { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -471,6 +471,42 @@ public String[] getConcreteAllClosedIndices() { | |
return allClosedIndices; | ||
} | ||
|
||
/** | ||
* Returns indexing routing for the given <code>aliasOrIndex</code>. Resolves routing from the alias metadata used | ||
* in the write index. | ||
*/ | ||
public String resolveWriteIndexRouting(@Nullable String routing, String aliasOrIndex) { | ||
if (aliasOrIndex == null) { | ||
return routing; | ||
} | ||
|
||
AliasOrIndex result = getAliasAndIndexLookup().get(aliasOrIndex); | ||
if (result == null || result.isAlias() == false) { | ||
return routing; | ||
} | ||
AliasOrIndex.Alias alias = (AliasOrIndex.Alias) result; | ||
IndexMetaData writeIndex = alias.getWriteIndex(); | ||
if (writeIndex == null) { | ||
throw new IllegalArgumentException("alias [" + aliasOrIndex + "] does not have a write index"); | ||
} | ||
AliasMetaData aliasMd = writeIndex.getAliases().get(alias.getAliasName()); | ||
if (aliasMd.indexRouting() != null) { | ||
if (aliasMd.indexRouting().indexOf(',') != -1) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we should validate that when people set is_write_index to true as well There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that to do this validation justice, it will have to be looked at as something that can be invalid for both read and write. Since this check is similar in both. For this reason, I would feel better if that stricter validation be looked at outside this PR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok. I agree it's not straight forward. Let's have another PR (I tend to say that multiple routing values changes the default for is_write_index and disallows setting it) |
||
throw new IllegalArgumentException("index/alias [" + aliasOrIndex + "] provided with routing value [" | ||
+ aliasMd.getIndexRouting() + "] that resolved to several routing values, rejecting operation"); | ||
} | ||
if (routing != null) { | ||
if (!routing.equals(aliasMd.indexRouting())) { | ||
throw new IllegalArgumentException("Alias [" + aliasOrIndex + "] has index routing associated with it [" | ||
+ aliasMd.indexRouting() + "], and was provided with routing value [" + routing + "], rejecting operation"); | ||
} | ||
} | ||
// Alias routing overrides the parent routing (if any). | ||
return aliasMd.indexRouting(); | ||
} | ||
return routing; | ||
} | ||
|
||
/** | ||
* Returns indexing routing for the given index. | ||
*/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you can make this statement - it may be that all of them have their write index flag set to false. I think we can have a generic statement like "no write index is defined for alias X. The write index may be explicitly disabled using
is_write_index=false
or the alias points to multiple indices without one being designated as a write index".There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will make more generic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
saying it points to multiple indices with none set as a write-index [is_write_index=true] does not conflict with that statement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't this covered by "the alias points to multiple indices without one being designated as a write index"?