Upgrade to Elasticsearch 7.10 - #3688
Closed
koetsier wants to merge 29 commits into
Closed
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request upgrades the Ruby Elasticsearch client to 7.10 and updates the codebase to be compatible with Elasticsearch 7’s removal of document types and changed response shapes (notably hits.total), while keeping conditional compatibility helpers for 6.x semantics.
Changes:
- Upgrade
elasticsearchgem to7.10and remove/adjust ES6-era type usage across indexing, mapping sync, and tests. - Introduce
ElasticsearchClientandElasticsearchResponsehelpers to centralize ES6/ES7 compatibility (types, mappings,hits.total). - Update unit/integration specs and schema config to reflect ES7 request/response patterns and analyzer changes.
Reviewed changes
Copilot reviewed 45 out of 46 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/unit/time_based_index_cleanup_spec.rb | Updates WebMock stub to match ES7 search request method. |
| spec/unit/tasks/indices_spec.rb | Removes typed mapping expectations for ES7 client. |
| spec/unit/schema/index_schema_parser_spec.rb | Adapts mapping expectations to typeless ES7 mappings. |
| spec/unit/index/elasticsearch_processor_spec.rb | Adjusts processor tests to no longer rely on _type. |
| spec/unit/index_group_spec.rb | Updates alias endpoint stubs for get_alias(name: ...). |
| spec/unit/govuk_index/presenters/elasticsearch_presenter_spec.rb | Removes _type from expected bulk identifiers. |
| spec/unit/govuk_index/presenters/elasticsearch_delete_presenter_spec.rb | Removes _type from expected bulk identifiers. |
| spec/unit/elasticsearch_index_spec.rb | Updates search/scroll stubs for ES7 client behaviour. |
| spec/unit/elasticsearch_client_spec.rb | Adds coverage for ES6/ES7 compatibility wrapper behaviour. |
| spec/support/integration_test_helper.rb | Updates integration helpers to use compatibility wrapper + adjusts WebMock allowlist. |
| spec/spec_helper.rb | Introduces :unit/:integration metadata and forces ES7 for unit tests. |
| spec/integration/search/search_spec.rb | Adds index reset to avoid ES scoring nondeterminism. |
| spec/integration/search/booster_spec.rb | Relaxes ordering assertion to reduce flakiness. |
| spec/integration/scroll_enumerator_spec.rb | Updates sort field away from removed _uid. |
| spec/integration/schema_synchroniser_spec.rb | Handles ES6 vs ES7 mapping response shape in assertions. |
| spec/integration/metasearch_index/inserter_spec.rb | Adjusts best-bet document id/type handling for ES7. |
| spec/integration/metasearch_index/deleter_spec.rb | Removes typed fetch for ES7. |
| spec/integration/govuk_index/unpublishing_message_processing_spec.rb | Removes typed fetch for ES7. |
| lib/tasks/indices.rake | Uses compatibility helpers when syncing mappings. |
| lib/tasks/delete.rake | Switches delete-by-link task to use compatibility wrapper. |
| lib/sitemap/generator.rb | Uses ES6/7-compatible total-hits extraction. |
| lib/search/query_components/popularity.rb | Makes painless script safe when field is missing. |
| lib/search/query_components/booster.rb | Ensures core query can be array (flattened) for should. |
| lib/search/presenters/result_set_presenter.rb | Uses ES6/7-compatible total-hits extraction. |
| lib/search/aggregate_example_fetcher.rb | Uses ES6/7-compatible total-hits extraction. |
| lib/scroll_enumerator.rb | Uses ES6/7-compatible total-hits extraction. |
| lib/schema/index_schema.rb | Emits typeless mappings via compatibility helper. |
| lib/schema_synchroniser.rb | Delegates mapping updates to compatibility wrapper. |
| lib/rummager.rb | Requires new ES compatibility helper files. |
| lib/metasearch_index/inserter.rb | Removes _type from bulk identifiers. |
| lib/metasearch_index/deleter.rb | Removes _type from bulk identifiers. |
| lib/indexer/popularity_lookup.rb | Uses ES6/7-compatible total-hits extraction. |
| lib/index/elasticsearch_processor.rb | Wraps bulk action identifiers for ES6/7 compatibility. |
| lib/index/client.rb | Minor simplification of get implementation. |
| lib/index.rb | Removes typed search/get usage; routes searches via compatibility helper. |
| lib/index_group.rb | Uses get_alias(name: ...) and handles 404 for missing alias. |
| lib/govuk_index/supertype_job.rb | Removes typed identifier construction for bulk ops. |
| lib/govuk_index/presenters/elasticsearch_identity.rb | Removes _type from identifiers. |
| lib/govuk_index/presenters/elasticsearch_delete_presenter.rb | Removes typed get usage for deletes. |
| lib/govuk_index/popularity_job.rb | Removes typed identifier construction for bulk ops. |
| lib/govuk_index/page_traffic_job.rb | Removes _type injection into bulk identifiers. |
| lib/elasticsearch_response.rb | New helper to normalize ES6/ES7 hits.total. |
| lib/elasticsearch_client.rb | New helper to normalize ES6/ES7 client parameters and mappings. |
| Gemfile.lock | Upgrades Elasticsearch gems to 7.10.0. |
| Gemfile | Pins Elasticsearch gem to 7.10. |
| config/schema/elasticsearch_schema.yml | Adjusts analyzer filter chains for ES7 compatibility. |
Comments suppressed due to low confidence (1)
spec/unit/index_group_spec.rb:44
- With
get_alias(name: @name), Elasticsearch returns 404 when the alias does not exist. This test currently stubs a 200 response, so it doesn't exercise the NotFound-rescue path that the production code now relies on. Stubbing a 404 here better matches Elasticsearch behaviour and improves coverage of the new branch.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
koetsier
force-pushed
the
upgrade_to_es7_new
branch
3 times, most recently
from
July 2, 2026 11:16
a5f389b to
10bca9f
Compare
koetsier
force-pushed
the
upgrade_to_es7_new
branch
from
July 7, 2026 14:26
10bca9f to
1c130e5
Compare
koetsier
force-pushed
the
upgrade_to_es7_new
branch
from
July 15, 2026 09:19
677691f to
581c59e
Compare
Elasticsearch 7+ does not allow nested 'should' queries
# Conflicts: # Gemfile # Gemfile.lock
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
…7.10 client. The Elasticsearch client gem has been updated and now issues POST requests for search operations instead of GET. # Conflicts: # spec/unit/elasticsearch_index_spec.rb
… 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.
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
koetsier
force-pushed
the
upgrade_to_es7_new
branch
from
July 24, 2026 10:42
581c59e to
6864bee
Compare
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
ElasticsearchClientclass 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
ElasticsearchClientclass can be removed.This PR builds on #3548, which upgrades the client gem
Jira: https://gov-uk.atlassian.net/browse/SCH-2026