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.

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

Fix caching for PreConfiguredTokenFilter #50912

Merged
merged 3 commits into from
Jan 16, 2020
Merged

Conversation

matriv
Copy link
Contributor

@matriv matriv commented Jan 13, 2020

The PreConfiguredTokenFilter#singletonWithVersion uses the version
internaly for the token filter factories but it registers only one
instance in the cahce and not one instance per version. This can lead
to exceptions like the one described in #50734 since the singleton is
created and cached using the version created of the first index
that is processed.

Remove the singletonWithVersion() methods and use the
elasticsearchVersion() methods instead.

Fixes: #50734

The `PreConfiguredTokenFilter#singletonWithVersion` uses the version
internaly for the token filter factories but it registers only one
instance in the cahce and not one instance per version. This can lead
to exceptions like the one described in elastic#50734 since the version created
of the first index creates and caches the singleton.

Remove the `singletonWithVersion()` methods and use the
`elasticsearchVersion()` methods instead.

Fixes: elastic#50734
@elasticmachine
Copy link
Collaborator

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

assertSame(tff_v1_1, tff_v1_2);


Version version2 = Version.V_7_2_1;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any idea to use something like this: https://github.com/elastic/elasticsearch/pull/50912/files#diff-5a875cd5f076862a2984f3815807fc6aR95 but get a different random Version where the Lucene Version is also different?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think VersionUtils.getFirstVersion() will always return a version with a different lucene version to Version.CURRENT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably it would work, but there is no actual guarantee that the first released version (that's what getFirstVersion() does) has a different Lucene version.

Copy link
Contributor

@romseygeek romseygeek left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @matriv!

assertSame(tff_v1_1, tff_v1_2);


Version version2 = Version.V_7_2_1;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think VersionUtils.getFirstVersion() will always return a version with a different lucene version to Version.CURRENT?

@polyfractal polyfractal added v7.7.0 and removed v7.6.0 labels Jan 15, 2020
@matriv
Copy link
Contributor Author

matriv commented Jan 15, 2020

@elasticmachine run elasticsearch-ci/default-distro
@elasticmachine run elasticsearch-ci/1

@matriv matriv merged commit 24e1858 into elastic:master Jan 16, 2020
@matriv matriv deleted the fix-50734 branch January 16, 2020 11:04
@matriv matriv added the v7.6.1 label Jan 16, 2020
matriv added a commit to matriv/elasticsearch that referenced this pull request Jan 16, 2020
The PreConfiguredTokenFilter#singletonWithVersion uses the version
internaly for the token filter factories but it registers only one
instance in the cahce and not one instance per version. This can lead
to exceptions like the one described in elastic#50734 since the singleton is
created and cached using the version created of the first index
that is processed.

Remove the singletonWithVersion() methods and use the
elasticsearchVersion() methods instead.

Fixes: elastic#50734
(cherry picked from commit 24e1858)
matriv added a commit to matriv/elasticsearch that referenced this pull request Jan 16, 2020
The PreConfiguredTokenFilter#singletonWithVersion uses the version
internaly for the token filter factories but it registers only one
instance in the cahce and not one instance per version. This can lead
to exceptions like the one described in elastic#50734 since the singleton is
created and cached using the version created of the first index
that is processed.

Remove the singletonWithVersion() methods and use the
elasticsearchVersion() methods instead.

Fixes: elastic#50734
(cherry picked from commit 24e1858)
matriv added a commit to matriv/elasticsearch that referenced this pull request Jan 16, 2020
The PreConfiguredTokenFilter#singletonWithVersion uses the version
internaly for the token filter factories but it registers only one
instance in the cahce and not one instance per version. This can lead
to exceptions like the one described in elastic#50734 since the singleton is
created and cached using the version created of the first index
that is processed.

Remove the singletonWithVersion() methods and use the
elasticsearchVersion() methods instead.

Fixes: elastic#50734
(cherry picked from commit 24e1858)
matriv added a commit that referenced this pull request Jan 16, 2020
The PreConfiguredTokenFilter#singletonWithVersion uses the version
internally for the token filter factories but it registers only one
instance in the cache and not one instance per version. This can lead
to exceptions like the one described in #50734 since the singleton is
created and cached using the version created of the first index
that is processed.

Remove the singletonWithVersion() methods and use the
elasticsearchVersion() methods instead.

Fixes: #50734
(cherry picked from commit 24e1858)
matriv added a commit that referenced this pull request Jan 16, 2020
The PreConfiguredTokenFilter#singletonWithVersion uses the version
internally for the token filter factories but it registers only one
instance in the cache and not one instance per version. This can lead
to exceptions like the one described in #50734 since the singleton is
created and cached using the version created of the first index
that is processed.

Remove the singletonWithVersion() methods and use the
elasticsearchVersion() methods instead.

Fixes: #50734
(cherry picked from commit 24e1858)
matriv added a commit that referenced this pull request Jan 16, 2020
The PreConfiguredTokenFilter#singletonWithVersion uses the version
internally for the token filter factories but it registers only one
instance in the cache and not one instance per version. This can lead
to exceptions like the one described in #50734 since the singleton is
created and cached using the version created of the first index
that is processed.

Remove the singletonWithVersion() methods and use the
elasticsearchVersion() methods instead.

Fixes: #50734
(cherry picked from commit 24e1858)
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this pull request Jan 23, 2020
The PreConfiguredTokenFilter#singletonWithVersion uses the version
internaly for the token filter factories but it registers only one
instance in the cahce and not one instance per version. This can lead
to exceptions like the one described in elastic#50734 since the singleton is
created and cached using the version created of the first index
that is processed.

Remove the singletonWithVersion() methods and use the
elasticsearchVersion() methods instead.

Fixes: elastic#50734
@polyfractal polyfractal added v7.6.0 and removed v7.6.1 labels Feb 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Standard token filter removal causes exceptions after upgrade
6 participants