Skip to content

Auto-reload synonym analyzers on synonyms updates #96886

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

Merged

Conversation

mayya-sharipova
Copy link
Contributor

@mayya-sharipova mayya-sharipova commented Jun 15, 2023

Synonym Management API project

On changes of synonyms in a synonym set, auto-reload analyzers.
Note that currently all updateable analyzers will be reloaded, even
those that are not relevant for a synonyms set being updated.

Responses from PUT and DELETE synonyms set are enhanced to
contain reload analyzers details.

For a PUT synonyms set request, an example of response:
PUT _synonyms/set1

{
    "result": "created",
    "reload_analyzers_details": {
        "_shards": {
            "total": 0,
            "successful": 0,
            "failed": 0
        },
        "reload_details": []
    }
}

or an update:

{
    "result": "updated",
    "reload_analyzers_details": {
        "_shards": {
            "total": 2,
            "successful": 1,
            "failed": 0
        },
        "reload_details": [
            {
                "index": "index1",
                "reloaded_analyzers": [
                    "my_analyzer"
                ],
                "reloaded_node_ids": [
                    "vxKVyIRJQuO4DPj8Y70ijg"
                ]
            }
        ]
    }
}

For a DELETE synonyms set request, if the deleted synonyms set is in use, a response
will contain reload failures, like below:

{
    "acknowledged": true,
    "reload_analyzers_details": {
        "_shards": {
            "total": 2,
            "successful": 0,
            "failed": 1,
            "failures": [
                {
                    "shard": 0,
                    "index": "index1",
                    "status": "NOT_FOUND",
                    "reason": {
                        "type": "resource_not_found_exception",
                        "reason": "Synonym set [set1] not found"
                    }
                }
            ]
        },
        "reload_details": []
    }
}

And it is left for a user to fix affected indices.

Synonym Management API project

On changes of synonyms in a synonym set, auto-reload analyzers.
Note that currently all updateable analyzers will be reloaded, even
those that are not relevant for a synonyms set being updated.
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

- match: { reload_analyzers_details.reload_details.0.index: "my_index" }
- match: { reload_analyzers_details.reload_details.0.reloaded_analyzers.0 : "my_analyzer" }

# Confirm that the index analyzers are reloaded
Copy link
Member

Choose a reason for hiding this comment

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

I like this test ❤️

: UpdateSynonymsResultStatus.UPDATED;

// auto-reload all reloadable analyzers (currently only those that use updateable synonym or keyword_marker filters)
// TODO: reload only those analyzers that use this synonymsSet
Copy link
Member

Choose a reason for hiding this comment

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

What are your thoughts for reloading just the analyzers for the specific syonyms set? Do it in a follow up PR? Or do we need some additional infra for retrieving the analyzers that use it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, we can do this in a follow-up. It is not trivial how to do this efficiently.

@mayya-sharipova mayya-sharipova merged commit b508ee7 into elastic:main Jun 19, 2023
@mayya-sharipova mayya-sharipova deleted the reload-analyzers-synonyms-change2 branch June 19, 2023 14:50
mayya-sharipova added a commit to mayya-sharipova/elasticsearch that referenced this pull request Jun 22, 2023
This PR adds a new optional parameter "resource" for ReloadAnalyzersRequest.
If used, only analyzers that use this specific "resource" will be reload.
This parameter is not documented, for internal use only.

PR elastic#96886 introduced auto-reload of analyzers on synonyms index change. The problem
was that reloading was applied broady for all indices that contained reloadable
analyzers. This PR improves this, so when a particular synonyms set changes,
only analyzers that use this synonyms set  will auto-reloaded. Note that shard
requests will still be sent to all indices shards, as only on a shard we can
decide if analyzers need to be reloaded.
mayya-sharipova added a commit that referenced this pull request Jun 22, 2023
This PR adds a new optional parameter "resource" for ReloadAnalyzersRequest.
If used, only analyzers that use this specific "resource" will be reload.
This parameter is not documented, for internal use only.

PR #96886 introduced auto-reload of analyzers on synonyms index change. The problem
was that reloading was applied broadly for all indices that contained reloadable
analyzers. This PR improves this, so when a particular synonyms set changes,
only analyzers that use this synonyms set  will auto-reloaded. Note that shard
requests will still be sent to all indices shards, as only on a shard we can
decide if analyzers need to be reloaded.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>non-issue :Search Relevance/Analysis How text is split into tokens Team:Search Meta label for search team v8.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants