feat(pit): Elasticsearch _pit support with engine-aware gating, plus integration and request-building test coverage#32
Open
marevol wants to merge 3 commits into
Open
Conversation
…hards serialization Correctness fixes to existing HTTP actions, verified against the OpenSearch REST layer. No new dependencies and no API changes. - HttpOpenIndexAction: serialize wait_for_active_shards ActiveShardCount.ALL as "all" (was "ALL", which the server rejects with HTTP 400) and forward indices options. This was the remaining call site still using ActiveShardCount.toString(). - HttpCloseIndexAction: forward wait_for_active_shards and indices options (both were dropped, so close could return before shards were ready). - HttpUpdateAction: remove version/version_type params. The _update endpoint rejects internal versioning (it uses if_seq_no/if_primary_term, already sent), and UpdateRequest's version setters throw UnsupportedOperationException, so the removed block was unreachable. - HttpIndicesAliasesAction: build the request body via the request's own toXContent so is_write_index/is_hidden/must_exist/routing are emitted and a remove_index action no longer emits an empty "aliases". - HttpRefreshAction, HttpFlushAction, HttpForceMergeAction, HttpDeleteIndexAction, HttpGetMappingsAction: forward indices options (ignore_unavailable, allow_no_indices, expand_wildcards). - HttpForceMergeAction: also forward primary_only. Adds unit tests asserting the emitted query params / body for each fix.
… later primary_only is unknown to OpenSearch 1.x and Elasticsearch 7/8, which reject unrecognized query parameters with HTTP 400 (the test_force_merge integration tests failed on those engines). Gate the parameter on the detected engine type, matching the version-conditional pattern already used by HttpGetMappingsAction. ActionTestUtils now stubs getEngineInfo() (with a per-engine-type overload) so version-conditional serialization can be asserted offline; HttpForceMergeActionTest covers both the OpenSearch 2.x/3.x (sent) and legacy (omitted) branches.
…ntegration and request-building test coverage
Make the Point-In-Time actions engine-aware so they work on both OpenSearch 2.x+
and Elasticsearch 7.10+/8.x, and fail clearly where a backend has no equivalent:
- CreatePit/DeletePit use POST/DELETE /_pit on Elasticsearch (mapping the
{"id":...} and {"succeeded":...} bodies to the OpenSearch response types) and keep
/_search/point_in_time on OpenSearch.
- Unsupported combinations fail the listener with a clear UnsupportedOperationException:
PIT on OpenSearch 1.x; GetAllPits and delete-all on Elasticsearch (no equivalent
endpoint); and deleting multiple PIT ids in one request on Elasticsearch (the _pit
endpoint accepts a single id, sent as {"id":"..."} rather than an array).
- The Elasticsearch _pit endpoint rejects allow_partial_search_results, so it is not
sent; the Elasticsearch response parsers now surface error bodies as failures instead
of returning an empty/negative response (the HTTP layer routes non-2xx through the
success path).
Test coverage for the previously unguarded request-building surface:
- Per-engine request-serialization unit tests for the PIT actions, and getCurlRequest
param/body tests for reindex, update_by_query, delete_by_query, resolve_index and
get_mappings. ActionTestUtils can now stub the reported engine type and read the
request URL.
- End-to-end integration tests across all five backends (OpenSearch 1/2/3,
Elasticsearch 7/8) for reindex, update_by_query, delete_by_query, resolve_index and
the PIT lifecycle, including the unsupported-engine assertions.
marevol
changed the base branch from
fix/action-request-param-correctness
to
main
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
1 participant
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.
Summary
Makes the Point-In-Time (PIT) actions work across engines and closes the biggest
regression-prevention gap found while reviewing the recent action work: the new
actions' request-building had no unit or integration coverage.
Stacked on #31 — this branch builds on
fix/action-request-param-correctness;please merge #31 first (the base retargets to
mainautomatically once it merges).Engine-aware PIT
CreatePit/DeletePitnow usePOST /{index}/_pitandDELETE /_pit, mapping the{"id":...}/{"succeeded":...}responses to theOpenSearch
CreatePitResponse/DeletePitResponsetypes./_search/point_in_time).UnsupportedOperationExceptionon the listener): PIT on OpenSearch 1.x;
GetAllPitsand delete-all on Elasticsearch;and multi-id delete on Elasticsearch (the
_pitendpoint takes a single id)._pitendpoint rejectsallow_partial_search_results(HTTP 400), so it is notsent. The ES response parsers now surface error bodies as failures instead of
returning an empty/negative response — the HTTP layer routes non-2xx through the success
path, so an unguarded parser would otherwise swallow the error.
Test coverage
getCurlRequestparam/body regression guards forreindex,update_by_query,delete_by_query,resolve_indexandget_mappings— these previously had onlyresponse-parsing tests, leaving the request-building logic (endpoints,
wait_for_completion,conflicts,keep_alive, PIT body, …) unguarded.ActionTestUtilsgains a per-enginetestClient(EngineType)stub ofgetEngineInfo()and a
url(...)accessor.Elasticsearch 7/8) for
reindex,update_by_query,delete_by_query,resolve_indexand the PIT lifecycle, including the unsupported-engine assertions.
Verification
mvn testexcluding the*ClientTestcontainers).engines (OS1/2/3, ES7/8) and pass; the ES
_pitrequest/response contract wasconfirmed against a live Elasticsearch 7.17 instance.