Add timeout to synonyms put APIs to wait for synonyms system index to be searchable #126314
Conversation
carlosdelest
added
>bug
:Search Relevance/Analysis
|
Hi @carlosdelest, I've created a changelog YAML for you. |
| @@ -15,11 +15,6 @@ setup: | |||
|
|
|||
| - match: { result: "created" } | |||
|
|
|||
| - do: | |||
There was a problem hiding this comment.
We no longer need to do an explicit wait in tests, as it is done in the PUT operations themselves
| @@ -120,67 +132,73 @@ public void testCreateTooManySynonymsUsingRuleUpdates() throws InterruptedExcept | |||
| int rulesToUpdate = randomIntBetween(1, 10); | |||
| int synonymsToCreate = maxSynonymSets - rulesToUpdate; | |||
| String synonymSetId = randomIdentifier(); | |||
| synonymsManagementAPIService.putSynonymsSet(synonymSetId, randomSynonymsSet(synonymsToCreate), new ActionListener<>() { | |||
There was a problem hiding this comment.
This method is not changed, the reformatting confused the diff
| @@ -34,14 +36,16 @@ public class PutSynonymRuleAction extends ActionType<SynonymUpdateResponse> { | |||
| public static final PutSynonymRuleAction INSTANCE = new PutSynonymRuleAction(); | |||
| public static final String NAME = "cluster:admin/synonym_rules/put"; | |||
|
|
|||
| public static final TimeValue DEFAULT_TIMEOUT = TimeValue.timeValueSeconds(10); | |||
There was a problem hiding this comment.
We could have used the default ACK timeout (30s) - seemed a bit too much, happy to hear thoughts
| @@ -343,7 +353,13 @@ public void putSynonymsSet(String synonymSetId, SynonymRule[] synonymsSet, Actio | |||
| ? UpdateSynonymsResultStatus.CREATED | |||
| : UpdateSynonymsResultStatus.UPDATED; | |||
|
|
|||
| reloadAnalyzers(synonymSetId, false, bulkInsertResponseListener, updateSynonymsResultStatus); | |||
| ensureSynonymsSearchableAndReloadAnalyzers( | |||
There was a problem hiding this comment.
Put actions need to ensure that synonyms are searchable before reloading
| @@ -85,3 +79,69 @@ setup: | |||
| rule_id: "test-id-0" | |||
| body: | |||
| synonyms: "i-phone, iphone" | |||
|
|
|||
| --- | |||
| "Timeout can be specified": | |||
There was a problem hiding this comment.
These general validation tests were missing - adding them and the timeout validation tests
| @@ -283,7 +283,7 @@ public class InternalUsers { | |||
| UsernamesField.SYNONYMS_USER_NAME, | |||
| new RoleDescriptor( | |||
| UsernamesField.SYNONYMS_ROLE_NAME, | |||
| null, | |||
| new String[] { "monitor" }, | |||
There was a problem hiding this comment.
Retrieving cluster health means having monitor cluster privilege.
|
Pinging @elastic/es-search-relevance (Team:Search Relevance) |
carlosdelest
changed the title
|
I agree with @benwtrent , and also added my comments on the issue. |
|
Closing in favour of using |
Closes #121441
Synonyms APIs use a system index to store synonyms as individual documents. These documents need to be searched in order to be retrieved from the analyzers.
When the system index has not been created yet, it is possible that creating the index need some time for the replicas to be available for search. Thus, an analyzer that is reloaded after the synonyms are created won't be able to retrieve the synonyms, and as a consequence we won't have synonyms for the index until a reload analyzer operation is performed.
This PR fixes this issue by retrieving the synonyms index health, and waiting for it to be green before reloading analyzers. As this can time out due to the index not being available, a default timeout is introduced (10s) that can be overriden in the request, with -1 meaning not to check the synonyms index availability.