Skip to content

Return reloaded analyzers in _reload_search_ananlyzer response #43813

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

Merged
merged 8 commits into from
Jul 2, 2019
Merged

Return reloaded analyzers in _reload_search_ananlyzer response #43813

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0024968
Return reloaded analyzers in _reload_search_ananlyzer response
Jul 1, 2019
b3f3de1
Changing details section name in response
Jul 2, 2019
abaff61
checkstyle
Jul 2, 2019
d31a363
iter
Jul 2, 2019
0f376dd
Merge branch 'master' into fix-43804
Jul 2, 2019
14b1b39
Merge branch 'master' into fix-43804
Jul 2, 2019
d7136bd
Merge branch 'master' into fix-43804
elasticmachine Jul 2, 2019
c89d399
Merge branch 'master' into fix-43804
Jul 2, 2019
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
31 changes: 30 additions & 1 deletion docs/reference/indices/apis/reload-analyzers.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,33 @@ reload to ensure the new state of the file is reflected everywhere in the cluste
POST /my_index/_reload_search_analyzers
--------------------------------------------------
// CONSOLE
// TEST[s/^/PUT my_index\n/]
// TEST[continued]

The reload request returns information about the nodes it was executed on and the
analyzers that were reloaded:

[source,js]
--------------------------------------------------
{
"_shards" : {
"total" : 2,
"successful" : 2,
"failed" : 0
},
"reloaded_nodes" : [
Copy link
Contributor

@jtibshirani jtibshirani Jul 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

A few thoughts around the structure of this response:

  • I found it a little confusing that this section was named reloaded_nodes, since it's centered around indices instead of nodes (and we call these sections IndexDetails in the Java response). Maybe reloaded_indices would make more sense, or even just indices?
  • When there is one response section per index, we often return a map instead of a list, with the index name as the map key. For example, the 'get index' and 'get mapping' APIs follow this response format.

Copy link
Member Author

@cbuescher cbuescher Jul 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, the format of the response here is a bit different. The reason for this ist that although reloading is centered around indices, it needs to be executed on each node an index shard might live on (and not only once on each shard in a replica set). So this is potentially more than one node per index. While I think the list of nodeIds that were part of reloading for each index is not that important to the average user, it can provide valuable debug information e.g. for UI clients later on.

Regarding your second point: I know we use index names as keys in several places but if not necessary I consciously try to avoid it. I makes parsing of responses more difficult because the parser need to deal with an unbounded set of keys as field names. While we mange to get around it in the places you mentioned, I try to avoid it if possible in order to make the rest response easier to parse.

Copy link
Contributor

Choose a reason for hiding this comment

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

although reloading is centered around indices, it needs to be executed on each node an index shard might live on

That makes sense to me, I was just wondering about the name of the top-level key for the section (currently reloaded_nodes). The name seemed fitting when this section only contained node IDs, but now that it contains analyzer information as well, maybe there could be a better name? In Java this is called reloadedIndicesDetails, so I was bit surprised to see it called reloaded_nodes in the REST response as opposed to something like reloaded_indices.

I know we use index names as keys in several places but if not necessary I consciously try to avoid it.

I agree it makes parsing more difficult! To me it would be best to maintain consistency across APIs (even if it makes parsing a little harder) as opposed to having a mix of styles, since this is least surprising to users. However this may be a bigger conversation, and the API is marked as experimental and can be changed, so it doesn't seem like it should prevent merging.

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 see what you mean now with the top-level key, sorry that I missed that part. Will change to something more neutral and probably also allign the java API to that new key a bit.

{
"index" : "my_index",
"reloaded_analyzers" : [
"my_synonyms"
],
"reloaded_node_ids" : [
"mfdqTXn_T7SGr2Ho2KT8uw"
]
}
]
}
--------------------------------------------------
// TEST[continued]
// TESTRESPONSE[s/"total" : 2/"total" : $body._shards.total/]
// TESTRESPONSE[s/"successful" : 2/"successful" : $body._shards.successful/]
// TESTRESPONSE[s/mfdqTXn_T7SGr2Ho2KT8uw/$body.reloaded_nodes.0.reloaded_node_ids.0/]
11 changes: 8 additions & 3 deletions server/src/main/java/org/elasticsearch/index/mapper/MapperService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import com.carrotsearch.hppc.ObjectHashSet;
import com.carrotsearch.hppc.cursors.ObjectCursor;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.lucene.analysis.Analyzer;
Expand Down Expand Up @@ -847,19 +848,23 @@ protected Analyzer getWrappedAnalyzer(String fieldName) {
}
}

public synchronized void reloadSearchAnalyzers(AnalysisRegistry registry) throws IOException {
public synchronized List<String> reloadSearchAnalyzers(AnalysisRegistry registry) throws IOException {
logger.info("reloading search analyzers");
// refresh indexAnalyzers and search analyzers
final Map<String, TokenizerFactory> tokenizerFactories = registry.buildTokenizerFactories(indexSettings);
final Map<String, CharFilterFactory> charFilterFactories = registry.buildCharFilterFactories(indexSettings);
final Map<String, TokenFilterFactory> tokenFilterFactories = registry.buildTokenFilterFactories(indexSettings);
final Map<String, Settings> settings = indexSettings.getSettings().getGroups("index.analysis.analyzer");
final List<String> reloadedAnalyzers = new ArrayList<>();
for (NamedAnalyzer namedAnalyzer : indexAnalyzers.getAnalyzers().values()) {
if (namedAnalyzer.analyzer() instanceof ReloadableCustomAnalyzer) {
ReloadableCustomAnalyzer analyzer = (ReloadableCustomAnalyzer) namedAnalyzer.analyzer();
Settings analyzerSettings = settings.get(namedAnalyzer.name());
analyzer.reload(namedAnalyzer.name(), analyzerSettings, tokenizerFactories, charFilterFactories, tokenFilterFactories);
String analyzerName = namedAnalyzer.name();
Settings analyzerSettings = settings.get(analyzerName);
analyzer.reload(analyzerName, analyzerSettings, tokenizerFactories, charFilterFactories, tokenFilterFactories);
reloadedAnalyzers.add(analyzerName);
}
}
return reloadedAnalyzers;
}
}
70 changes: 54 additions & 16 deletions ...lugin/core/src/main/java/org/elasticsearch/xpack/core/action/ReloadAnalyzersResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,20 @@
import org.elasticsearch.action.support.DefaultShardOperationFailedException;
import org.elasticsearch.action.support.broadcast.BroadcastResponse;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.xpack.core.action.TransportReloadAnalyzersAction.ReloadResult;

import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;

import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;

Expand All @@ -28,16 +30,20 @@
*/
public class ReloadAnalyzersResponse extends BroadcastResponse {

private final Map<String, List<String>> reloadedIndicesNodes;
private final Map<String, IndexDetails> reloadedIndicesDetails;

public ReloadAnalyzersResponse() {
reloadedIndicesNodes = Collections.emptyMap();
reloadedIndicesDetails = Collections.emptyMap();
}

public ReloadAnalyzersResponse(int totalShards, int successfulShards, int failedShards,
List<DefaultShardOperationFailedException> shardFailures, Map<String, List<String>> reloadedIndicesNodes) {
List<DefaultShardOperationFailedException> shardFailures, Map<String, IndexDetails> reloadedIndicesNodes) {
super(totalShards, successfulShards, failedShards, shardFailures);
this.reloadedIndicesNodes = reloadedIndicesNodes;
this.reloadedIndicesDetails = reloadedIndicesNodes;
}

public final Map<String, IndexDetails> getReloadedIndicesDetails() {
return this.reloadedIndicesDetails;
}

/**
Expand All @@ -46,10 +52,12 @@ public ReloadAnalyzersResponse(int totalShards, int successfulShards, int failed
@Override
protected void addCustomXContentFields(XContentBuilder builder, Params params) throws IOException {
builder.startArray("reloaded_nodes");
for (Entry<String, List<String>> indexNodesReloaded : reloadedIndicesNodes.entrySet()) {
for (Entry<String, IndexDetails> indexDetails : reloadedIndicesDetails.entrySet()) {
builder.startObject();
builder.field("index", indexNodesReloaded.getKey());
builder.field("reloaded_node_ids", indexNodesReloaded.getValue());
IndexDetails value = indexDetails.getValue();
builder.field("index", value.getIndexName());
builder.field("reloaded_analyzers", value.getReloadedAnalyzers());
builder.field("reloaded_node_ids", value.getReloadedIndicesNodes());
builder.endObject();
}
builder.endArray();
Expand All @@ -59,31 +67,61 @@ protected void addCustomXContentFields(XContentBuilder builder, Params params) t
private static final ConstructingObjectParser<ReloadAnalyzersResponse, Void> PARSER = new ConstructingObjectParser<>("reload_analyzer",
true, arg -> {
BroadcastResponse response = (BroadcastResponse) arg[0];
List<Tuple<String, List<String>>> results = (List<Tuple<String, List<String>>>) arg[1];
Map<String, List<String>> reloadedNodeIds = new HashMap<>();
for (Tuple<String, List<String>> result : results) {
reloadedNodeIds.put(result.v1(), result.v2());
List<IndexDetails> results = (List<IndexDetails>) arg[1];
Map<String, IndexDetails> reloadedNodeIds = new HashMap<>();
for (IndexDetails result : results) {
reloadedNodeIds.put(result.getIndexName(), result);
}
return new ReloadAnalyzersResponse(response.getTotalShards(), response.getSuccessfulShards(), response.getFailedShards(),
Arrays.asList(response.getShardFailures()), reloadedNodeIds);
});

@SuppressWarnings({ "unchecked" })
private static final ConstructingObjectParser<Tuple<String, List<String>>, Void> ENTRY_PARSER = new ConstructingObjectParser<>(
private static final ConstructingObjectParser<IndexDetails, Void> ENTRY_PARSER = new ConstructingObjectParser<>(
"reload_analyzer.entry", true, arg -> {
String index = (String) arg[0];
List<String> nodeIds = (List<String>) arg[1];
return new Tuple<>(index, nodeIds);
return new IndexDetails((String) arg[0], new HashSet<>((List<String>) arg[1]), new HashSet<>((List<String>) arg[2]));
});

static {
declareBroadcastFields(PARSER);
PARSER.declareObjectArray(constructorArg(), ENTRY_PARSER, new ParseField("reloaded_nodes"));
ENTRY_PARSER.declareString(constructorArg(), new ParseField("index"));
ENTRY_PARSER.declareStringArray(constructorArg(), new ParseField("reloaded_analyzers"));
ENTRY_PARSER.declareStringArray(constructorArg(), new ParseField("reloaded_node_ids"));
}

public static ReloadAnalyzersResponse fromXContent(XContentParser parser) {
return PARSER.apply(parser, null);
}

public static class IndexDetails {

private final String indexName;
private final Set<String> reloadedIndicesNodes;
private final Set<String> reloadedAnalyzers;

IndexDetails(String name, Set<String> reloadedIndicesNodes, Set<String> reloadedAnalyzers) {
this.indexName = name;
this.reloadedIndicesNodes = reloadedIndicesNodes;
this.reloadedAnalyzers = reloadedAnalyzers;
}

public String getIndexName() {
return indexName;
}

public Set<String> getReloadedIndicesNodes() {
return reloadedIndicesNodes;
}

public Set<String> getReloadedAnalyzers() {
return reloadedAnalyzers;
}

void merge(ReloadResult other) {
assert this.indexName == other.index;
this.reloadedAnalyzers.addAll(other.reloadedSearchAnalyzers);
this.reloadedIndicesNodes.add(other.nodeId);
}
}
}
29 changes: 17 additions & 12 deletions ...ore/src/main/java/org/elasticsearch/xpack/core/action/TransportReloadAnalyzersAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.elasticsearch.indices.IndicesService;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TransportService;
import org.elasticsearch.xpack.core.action.ReloadAnalyzersResponse.IndexDetails;
import org.elasticsearch.xpack.core.action.TransportReloadAnalyzersAction.ReloadResult;

import java.io.IOException;
Expand Down Expand Up @@ -67,18 +68,18 @@ protected ReloadResult readShardResult(StreamInput in) throws IOException {
@Override
protected ReloadAnalyzersResponse newResponse(ReloadAnalyzersRequest request, int totalShards, int successfulShards, int failedShards,
List<ReloadResult> responses, List<DefaultShardOperationFailedException> shardFailures, ClusterState clusterState) {
Map<String, List<String>> reloadedIndicesNodes = new HashMap<String, List<String>>();
Map<String, IndexDetails> reloadedIndicesDetails = new HashMap<String, IndexDetails>();
for (ReloadResult result : responses) {
if (reloadedIndicesNodes.containsKey(result.index)) {
List<String> nodes = reloadedIndicesNodes.get(result.index);
nodes.add(result.nodeId);
if (reloadedIndicesDetails.containsKey(result.index)) {
reloadedIndicesDetails.get(result.index).merge(result);;
} else {
List<String> nodes = new ArrayList<>();
nodes.add(result.nodeId);
reloadedIndicesNodes.put(result.index, nodes);
HashSet<String> nodeIds = new HashSet<String>();
nodeIds.add(result.nodeId);
IndexDetails details = new IndexDetails(result.index, nodeIds, new HashSet<String>(result.reloadedSearchAnalyzers));
reloadedIndicesDetails.put(result.index, details);
}
}
return new ReloadAnalyzersResponse(totalShards, successfulShards, failedShards, shardFailures, reloadedIndicesNodes);
return new ReloadAnalyzersResponse(totalShards, successfulShards, failedShards, shardFailures, reloadedIndicesDetails);
}

@Override
Expand All @@ -92,17 +93,19 @@ protected ReloadAnalyzersRequest readRequestFrom(StreamInput in) throws IOExcept
protected ReloadResult shardOperation(ReloadAnalyzersRequest request, ShardRouting shardRouting) throws IOException {
logger.info("reloading analyzers for index shard " + shardRouting);
IndexService indexService = indicesService.indexService(shardRouting.index());
indexService.mapperService().reloadSearchAnalyzers(indicesService.getAnalysis());
return new ReloadResult(shardRouting.index().getName(), shardRouting.currentNodeId());
List<String> reloadedSearchAnalyzers = indexService.mapperService().reloadSearchAnalyzers(indicesService.getAnalysis());
return new ReloadResult(shardRouting.index().getName(), shardRouting.currentNodeId(), reloadedSearchAnalyzers);
}

public static final class ReloadResult implements Streamable {
static final class ReloadResult implements Streamable {
String index;
String nodeId;
List<String> reloadedSearchAnalyzers;

private ReloadResult(String index, String nodeId) {
private ReloadResult(String index, String nodeId, List<String> reloadedSearchAnalyzers) {
this.index = index;
this.nodeId = nodeId;
this.reloadedSearchAnalyzers = reloadedSearchAnalyzers;
}

private ReloadResult() {
Expand All @@ -112,12 +115,14 @@ private ReloadResult() {
public void readFrom(StreamInput in) throws IOException {
this.index = in.readString();
this.nodeId = in.readString();
this.reloadedSearchAnalyzers = in.readStringList();
}

@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeString(index);
out.writeString(nodeId);
out.writeStringCollection(this.reloadedSearchAnalyzers);
}
}

Expand Down
19 changes: 12 additions & 7 deletions .../core/src/test/java/org/elasticsearch/xpack/core/action/ReloadAnalyzersResponseTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,31 @@
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.test.AbstractBroadcastResponseTestCase;
import org.elasticsearch.xpack.core.action.ReloadAnalyzersResponse;
import org.elasticsearch.xpack.core.action.ReloadAnalyzersResponse.IndexDetails;

import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

public class ReloadAnalyzersResponseTests extends AbstractBroadcastResponseTestCase<ReloadAnalyzersResponse> {

@Override
protected ReloadAnalyzersResponse createTestInstance(int totalShards, int successfulShards, int failedShards,
List<DefaultShardOperationFailedException> failures) {
Map<String, List<String>> reloadedIndicesNodes = new HashMap<>();
Map<String, IndexDetails> reloadedIndicesDetails = new HashMap<>();
int randomIndices = randomIntBetween(0, 5);
for (int i = 0; i < randomIndices; i++) {
List<String> randomNodeIds = Arrays.asList(generateRandomStringArray(5, 5, false, true));
reloadedIndicesNodes.put(randomAlphaOfLengthBetween(5, 10), randomNodeIds);
String name = randomAlphaOfLengthBetween(5, 10);
Set<String> reloadedIndicesNodes = new HashSet<>(Arrays.asList(generateRandomStringArray(5, 5, false, true)));
Set<String> reloadedAnalyzers = new HashSet<>(Arrays.asList(generateRandomStringArray(5, 5, false, true)));
reloadedIndicesDetails.put(name, new IndexDetails(name, reloadedIndicesNodes, reloadedAnalyzers));
}
return new ReloadAnalyzersResponse(totalShards, successfulShards, failedShards, failures, reloadedIndicesNodes);
return new ReloadAnalyzersResponse(totalShards, successfulShards, failedShards, failures, reloadedIndicesDetails);
}

@Override
Expand All @@ -39,12 +43,13 @@ protected ReloadAnalyzersResponse doParseInstance(XContentParser parser) throws

@Override
public void testToXContent() {
Map<String, List<String>> reloadedIndicesNodes = Collections.singletonMap("index", Collections.singletonList("nodeId"));
Map<String, IndexDetails> reloadedIndicesNodes = Collections.singletonMap("index",
new IndexDetails("index", Collections.singleton("nodeId"), Collections.singleton("my_analyzer")));
ReloadAnalyzersResponse response = new ReloadAnalyzersResponse(10, 5, 5, null, reloadedIndicesNodes);
String output = Strings.toString(response);
assertEquals(
"{\"_shards\":{\"total\":10,\"successful\":5,\"failed\":5},"
+ "\"reloaded_nodes\":[{\"index\":\"index\",\"reloaded_node_ids\":[\"nodeId\"]}]"
+ "\"reloaded_nodes\":[{\"index\":\"index\",\"reloaded_analyzers\":[\"my_analyzer\"],\"reloaded_node_ids\":[\"nodeId\"]}]"
+ "}",
output);
}
Expand Down
Loading