fix(opensearch): default to OpenSearch if distribution detection fails#5700
fix(opensearch): default to OpenSearch if distribution detection fails#5700saketh-pallempati wants to merge 2 commits into
Conversation
saketh-pallempati
requested review from
KarstenSchnitter,
chenqi0805,
dinujoh,
dlvenable,
engechas,
graytaylor0,
kkondaka,
oeyh,
san81,
sb2k16 and
srikanthjg
as code owners
|
Please re run the DCO check |
| LOG.warn("Version number is null in Elasticsearch API response. Proceeding to default."); | ||
| } | ||
| } catch (final Exception e) { | ||
| LOG.warn("Failed to get cluster info using Elasticsearch client. Will proceed with default behavior.", e); |
There was a problem hiding this comment.
This log could maybe indicate that the default behavior is to assume that it is OpenSearch, although you do have that log later on
| "Defaulting to OpenSearch distribution. Consider setting 'distribution_version' in the configuration if this is not an OpenSearch cluster."); | ||
| return Pair.of(OPENSEARCH_DISTRIBUTION, OPENSEARCH_POINT_IN_TIME_SUPPORT_VERSION_CUTOFF); | ||
| } else { | ||
| final String configuredDistributionName = openSearchSourceConfiguration.getDistributionVersion().name(); |
There was a problem hiding this comment.
I'm not sure we want this check to happen. If users configure distribution version it's probably better to trust it and create that type of accessor. I think there could be cases where the distribution_version is correct but the info response can still return unexpected results. So I would just keep the behavior the same (if distribution_version is set, just force that type of accessor
There was a problem hiding this comment.
Sure I was just trying to add some extra validation. Will simplify the case
|
I also tried to run the DCO check and it failed |
Yeah some problem on my end i will figure it. |
Signed-off-by: Pallempati Saketh <pallempati.saketh@fmr.com>
…cessorStrategy - Force accessor type when distribution_version is explicitly configured - Default to OpenSearch when both API calls fail instead of throwing exceptions - Improve error resilience and respect user configuration preferences Signed-off-by: Pallempati Saketh <pallempati.saketh@fmr.com>
|
It's still failing DCO |
Sorry that I made a change which should be pushed by my org. Please give it some time. |
|
Hi @graytaylor0 |
graytaylor0
left a comment
graytaylor0
left a comment
There was a problem hiding this comment.
Thanks for cleaning this up!
|
Thanks for the approval @graytaylor0 |
2 participants
Description
Accurate Client Selection: The logic for choosing between an OpenSearchAccessor and an ElasticsearchAccessor was refined to correctly rely on the determined cluster distribution, especially after attempting to get version information.
Reliable Default to OpenSearch: If the cluster distribution cannot be auto-detected (i.e., API calls to both OpenSearch and Elasticsearch clients fail) and the distribution_version is not configured, the strategy now consistently defaults to OpenSearch behavior and uses an OpenSearchAccessor.
Enhanced Edge Case Handling: The code now better manages situations where an Elasticsearch client needs to be created dynamically, for instance, if an initial OpenSearch client call succeeds but indicates an Elasticsearch distribution.
Issues Resolved
Resolves #5673
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.