Upgrade to Elasticsearch 7 - #3714
Draft
koetsier wants to merge 30 commits into
Draft
Conversation
koetsier
force-pushed
the
es7_upgrade_latest
branch
from
July 29, 2026 09:27
3039b9b to
8514efb
Compare
There was a problem hiding this comment.
Pull request overview
This PR upgrades the Search API’s Elasticsearch integration to be compatible with both legacy Elasticsearch 6.8 and newer Elasticsearch 7.10/OpenSearch by introducing compatibility wrappers, removing mapping-type usage where required, and updating query/response handling and tests accordingly.
Changes:
- Introduces
ElasticsearchClientandElasticsearchResponseto abstract ES6/ES7 API differences (mapping types,hits.totalshape, request patterns). - Updates indexing/search/mapping/delete flows and associated unit/integration tests to remove ES “type” usage where appropriate.
- Tightens integration test determinism for scoring-related tests by recreating indices before certain scenarios.
Reviewed changes
Copilot reviewed 50 out of 51 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/unit/time_based_index_cleanup_spec.rb | Updates WebMock to expect POST for _search. |
| spec/unit/tasks/indices_spec.rb | Updates mapping sync expectations to typeless put-mapping calls. |
| spec/unit/services_spec.rb | Adds tests for default-port normalization in Services.elasticsearch. |
| spec/unit/search/aggregate_example_fetcher_spec.rb | Updates expected query shape when must is nil. |
| spec/unit/schema/index_schema_parser_spec.rb | Updates expectations for typeless mappings structure. |
| spec/unit/index/elasticsearch_processor_spec.rb | Removes _type from identifiers used in bulk operations. |
| spec/unit/index_group_spec.rb | Adjusts alias lookup stubs to named alias endpoint. |
| spec/unit/govuk_index/presenters/elasticsearch_presenter_spec.rb | Removes _type from expected identifier. |
| spec/unit/govuk_index/presenters/elasticsearch_delete_presenter_spec.rb | Removes _type from expected identifier. |
| spec/unit/elasticsearch_index_spec.rb | Updates WebMock stubs to match ES7-style endpoints/verbs and scroll URL shape. |
| spec/unit/elasticsearch_client_spec.rb | Adds unit coverage for ES6 vs ES7 behavior switching and wrappers. |
| spec/support/rank_eval_test_helpers.rb | Updates _rank_eval stub verb and removes _type from hits. |
| spec/support/integration_test_helper.rb | Adds helper to recreate indices; updates insert/delete/get helpers for typeless APIs. |
| spec/spec_helper.rb | Switches derived metadata to :integration/:unit flags and forces ES7 in unit specs by default. |
| spec/integration/search/search_spec.rb | Recreates indices before scoring-sensitive test. |
| spec/integration/search/booster_spec.rb | Recreates indices and relaxes ordering assertion to reduce flakiness. |
| spec/integration/search/best_bets_spec.rb | Recreates indices before best-bet scenarios. |
| spec/integration/scroll_enumerator_spec.rb | Updates test data and sort field away from _uid. |
| spec/integration/schema_synchroniser_spec.rb | Makes mapping assertion conditional on ES6 vs ES7 mapping shape. |
| spec/integration/metasearch_index/inserter_spec.rb | Adjusts metasearch document ID and adds document_type. |
| spec/integration/metasearch_index/deleter_spec.rb | Updates fetch helper call to typeless get. |
| spec/integration/govuk_index/unpublishing_message_processing_spec.rb | Updates fetch helper call to typeless get. |
| lib/tasks/indices.rake | Routes mapping sync through ElasticsearchClient.mappings_properties. |
| lib/tasks/delete.rake | Routes deletes through ElasticsearchClient.delete. |
| lib/sitemap/generator.rb | Uses ElasticsearchResponse for total-hit extraction. |
| lib/services.rb | Adds default-port normalization for Elasticsearch hosts. |
| lib/search/query_components/popularity.rb | Makes painless script resilient to missing popularity field. |
| lib/search/query_components/booster.rb | Flattens core query to ensure should is an array. |
| lib/search/presenters/result_set_presenter.rb | Uses ElasticsearchResponse for total-hit extraction. |
| lib/search/aggregate_example_fetcher.rb | Avoids invalid must: nil queries; uses ElasticsearchResponse for totals. |
| lib/scroll_enumerator.rb | Uses ElasticsearchResponse for total-hit extraction. |
| lib/schema/index_schema.rb | Delegates ES6/ES7 mapping shape to ElasticsearchClient.compatible_mappings. |
| lib/schema_synchroniser.rb | Routes put-mapping through ElasticsearchClient.put_mapping. |
| lib/rummager.rb | Requires new ES compatibility wrapper classes. |
| lib/metasearch_index/inserter.rb | Removes _type from bulk identifier. |
| lib/metasearch_index/deleter.rb | Removes _type from bulk identifier. |
| lib/indexer/popularity_lookup.rb | Uses ElasticsearchResponse for total-hit extraction. |
| lib/index/elasticsearch_processor.rb | Wraps bulk identifiers via ElasticsearchClient.compatible_identifier. |
| lib/index/client.rb | Simplifies get call (but currently drops ES6 type handling). |
| lib/index.rb | Removes type from get-by-id and routes searches through ElasticsearchClient.search. |
| lib/index_group.rb | Uses named-alias lookup and treats missing alias as empty state. |
| lib/govuk_index/supertype_job.rb | Removes _type from bulk identifiers. |
| lib/govuk_index/presenters/elasticsearch_identity.rb | Removes _type from identifiers. |
| lib/govuk_index/presenters/elasticsearch_delete_presenter.rb | Removes typeless get call parameters (but currently drops ES6 type handling). |
| lib/govuk_index/popularity_job.rb | Removes _type from bulk identifiers. |
| lib/govuk_index/page_traffic_job.rb | Stops forcing _type into bulk identifiers. |
| lib/elasticsearch_response.rb | Adds ES6/ES7-compatible total-hits accessor. |
| lib/elasticsearch_client.rb | Adds ES6/ES7 wrappers for search/index/delete/put_mapping and mapping/identifier shaping. |
| Gemfile.lock | Locks elasticsearch gem to 7.10.x. |
| Gemfile | Pins elasticsearch gem to 7.10. |
| config/schema/elasticsearch_schema.yml | Removes standard token filter from analyzers to align with ES7 expectations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
hannako
reviewed
Jul 29, 2026
hannako
left a comment
hannako
left a comment
Contributor
There was a problem hiding this comment.
So much work here Jos, thank you for breaking it into really easy to follow commits.
I've made a bunch of comments which are mainly a request for links to documentation and a question about running unit tests against 6.8 as well as 7.0
| ) | ||
| end | ||
|
|
||
| def self.with_default_port(uri) |
Contributor
There was a problem hiding this comment.
This change looks different to what you've explained in the commit message. Do both 6.8 and 7.10 default to 9200 for http and 443 for https?
Contributor
Author
There was a problem hiding this comment.
This bit adds an explicit port number if there is none, so 9200 for http and 443 for https. This means that if we don't specify a port number it uses the correct one.
Contributor
There was a problem hiding this comment.
But do both versions of Elasticsearch use the same defaults?
This change upgrades the Elasticsearch client library to version 7.10. In future commits the code will be updated to make it compatible.
This filter is deprecated, as it no longer provides any functional behavior. https://stackoverflow.com/questions/76108163/getting-error-the-standard-token-filter-has-been-removed-when-running-a-que
koetsier
force-pushed
the
es7_upgrade_latest
branch
from
July 30, 2026 10:08
8514efb to
acde6f2
Compare
…7.10 client. Both POST and GET are valid: https://www.elastic.co/docs/api/doc/elasticsearch/operation/operation-search. The Elasticsearch client gem has been updated and now issues POST requests for search operations instead of GET.
… client.
When performing a scroll search, the Elasticsearch client may now use
either of the following endpoint formats:
$HOSTNAME/_search/scroll/{scroll_id}?scroll=1m
$HOSTNAME/_search/scroll?scroll=1m&scroll_id={scroll_id}
Both URL patterns are supported by Elasticsearch 6.8.
Elasticsearch 7.10 appears to validate bool.should clauses more strictly than Elasticsearch 6.8, rejecting arrays of arrays where query clauses are expected. Flatten the should query to ensure compatibility. Documentation: https://www.elastic.co/guide/en/elasticsearch/reference/7.10/query-dsl-bool-query.html
Although _uid is still present in Elasticsearch 6.8, it is deprecated. All usages have been updated to _id to ensure forward compatibility with future Elasticsearch versions. Reference: https://www.elastic.co/guide/en/elasticsearch/reference/6.8/mapping-uid-field.html # Conflicts: # lib/tasks/export.rake
Use a fallback when doc['popularity'] is empty by applying POPULARITY_OFFSET directly; otherwise add the offset to the existing value. This is necessary because Elasticsearch 7+ requires the property to exist if accessed popularity data is missing.
Introduce an ElasticsearchResponse helper to normalize the hits.total
field across Elasticsearch 6.x and 7.x, and update callers to use the
shared helper instead of accessing the response structure directly.
In Elasticsearch 6.x, the hits.total field is returned as a simple integer:
{
"hits": {
"total": 123,
"hits": [
...
]
}
}
In Elasticsearch 7.x, hits.total became an object that includes both
the count and information about whether it’s exact:
{
"hits": {
"total": {
"value": 123,
"relation": "eq"
},
"hits": [
...
]
}
}
…GET requests The _all parameter is optional in Elasticsearch 6 but is no longer supported in Elasticsearch 7. Remove _all from all GET requests to maintain compatibility with the upcoming Elasticsearch 7 upgrade.
Introduce an ElasticsearchClient helper to detect whether the application is connected to Elasticsearch 6 or 7. Support overriding the detected version via environment variables
Configure RSpec to directly use :integration tags for specs under spec/integration,
Unit tests do not allow connecting to a live Elasticsearch instance which means the compatibility class ElasticsearchClient cannot use it to determine which version to use Furthermore, unit tests use mocks to abstract away the Elasticsearch server and they cannot easily be made compatible for both Since we're going to migrate to Elasticsearch 7, it makes sense to ensure they are compatible with that version and so we ensure the ElasticsearchClient class assumes Elasticsearch 7 is present
The unit tests assume Elasticsearch 7 is used. Elasticsearch 7 no longer supports the _type parameter and so these need to be removed.
Introduce ElasticsearchClient.search so we can seamlessly migrate from Elasticsearch 6 to 7. The wrapper conditionally adds the legacy _type parameter required by ES6 and omits it for ES7, where _type is no longer supported. Update Index#raw_search to use the wrapper instead of passing _type directly.
Introduce ElasticsearchClient.search so we can seamlessly migrate from Elasticsearch 6 to 7. The wrapper conditionally adds the legacy _type parameter required by ES6 and omits it for ES7, where _type is no longer supported.
Introduce ElasticsearchClient.compatible_mappings to generate index mappings that work across Elasticsearch versions. For ES6, mappings are wrapped under the legacy "generic-document" type, while ES7 uses the updated format with "properties" at the root level. Update IndexSchema to use this helper instead of hardcoding ES6-style type mappings, ensuring consistent behavior during the migration.
Introduce ElasticsearchClient.put_mapping to handle differences in mapping APIs between Elasticsearch 6 and 7. The helper conditionally includes the legacy type: "generic-document" parameter for ES6 and omits it for ES7, where types are no longer supported. Update SchemaSynchroniser to use the compatibility layer instead of calling the Elasticsearch client directly. Adjust integration test expectations to account for the different mapping structure in ES7 versus ES6.
Introduce ElasticsearchClient.delete so we can seamlessly migrate from Elasticsearch 6 to 7. The wrapper conditionally adds the legacy _type parameter required by ES6 and omits it for ES7, where _type is no longer supported.
Add ElasticsearchClient.mappings_properties to normalize mapping format differences between Elasticsearch versions. For ES6, mappings are nested under "generic-document", while ES7 exposes "properties" at the top level. Update the indices rake task to extract the correct mapping structure via this helper before passing it to SchemaSynchroniser, ensuring schema synchronization works across both Elasticsearch 6 and 7 during migration.
…migration Update bulk indexing and deletion flows to remove _type from document identifiers, since Elasticsearch 7 no longer supports types. Introduce ElasticsearchClient.compatible_identifier to conditionally include _type only for ES6, ensuring bulk index and delete actions remain compatible across both ES versions during migration. This affects bulk processor actions, indexers, and related presenters that build Elasticsearch bulk request payloads.
GET _alias without a name resolves across all indices, including system indices (.kibana*, .security*, etc.), which triggers a deprecation warning in Elasticsearch 7. Restrict the request to a named alias to avoid scanning system indices and align with Elasticsearch system index protection behavior. Docs: https://www.elastic.co/guide/en/elasticsearch/reference/7.10/migrating-7.10.html#breaking_710_indices_changes
…tion Everywhere we use the ElasticsearchProcessor for bulk actions. This ensures consistency and compatibility between ES version 6 and 7
Ordering by _id is going to be deprecated. Use another field instead.
The boost (0.3) is applied equally to service_manual_guide and service_manual_topic so the order of those two is not certain. The test should only test that the cma_case comes first
expect_document_is_in_rummager does not work when there are spaces in the id field
Deleted documents can continue to influence Lucene scoring statistics, so simply deleting documents is not sufficient for test isolation. Recreating indices ensures a clean state and stable search scores.
Elasticsearch client 6.8 defaults HTTPS URLs without a port to 443, but the Elasticsearch 7.10 client defaults to 9200. We usually do not specify the port in the ELASTICSEARCH_URI variable when deployed and so this will break This commit explicitly adds the appropriate default port when missing.
As part of upgrading from Elasticsearch to Opensearch, the query
fetching the aggregate examples needs to be tweaked.
The old query used 'query: { bool: { must: nil } }' which is no
longer considered valid. Instead the query section should simply
be omitted
Removed _type parameter and expect POST instead of GET request for the rank_eval call.
koetsier
force-pushed
the
es7_upgrade_latest
branch
from
July 30, 2026 10:32
acde6f2 to
179c928
Compare
hannako
approved these changes
Jul 30, 2026
hannako
left a comment
hannako
left a comment
Contributor
There was a problem hiding this comment.
This looks good - do you want to set it to draft and we can merge when you're back?
hannako
marked this pull request as draft
August 3, 2026 11:53
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR ensures Search API can use the legacy Elasticseach 6.8 cluster as well as a newer Elasticsearch 7.10 cluster.
It uses the ElasticsearchClient class to query the cluster to determine the Elasticsearch version and then provides some simple abstractions to ensure compatibility.
Once we fully migrated to Elasticsearch 7.10, the ElasticsearchClient class can be removed.
This PR builds on #3548, which upgrades the client gem
Jira: https://gov-uk.atlassian.net/browse/SCH-2026