Skip to content

Fix documentErrors metric incorrectly counting version conflict error…#6758

Merged
oeyh merged 1 commit into
opensearch-project:mainfrom
Keyur-S-Patel:fix/exclude-version-conflict-from-document-errors-5376
Apr 18, 2026
Merged

Fix documentErrors metric incorrectly counting version conflict error…#6758
oeyh merged 1 commit into
opensearch-project:mainfrom
Keyur-S-Patel:fix/exclude-version-conflict-from-document-errors-5376

Conversation

@Keyur-S-Patel

Copy link
Copy Markdown
Contributor

Description

In BulkRetryStrategy.handleFailures(), the documentErrorsCounter was being incremented for all item-level errors, including version conflict errors. Version conflict errors are not true document errors — they indicate the latest version of the document already exists in OpenSearch and are not sent to the DLQ. This caused the documentErrors metric to be inflated, creating a confusing user experience.

This change moves documentErrorsCounter.increment() and getDocumentStatusCounter().increment() into the else branch so they only fire for actual non-retryable failures, skipping version conflicts. This matches the existing behavior already present in createBulkRequestForRetry(). A unit test was added to verify that version conflicts only increment documentsVersionConflictErrors and not documentErrors.

Issues Resolved

Resolves #5376

Check List

  • New functionality includes testing.
  • New functionality has a documentation issue. Please link to it in this PR.
  • New functionality has javadoc added
  • Commits are signed with a real name per the DCO

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](https://github.com/opensearch-project/data-prepper/blob/main/CONTRIBUTING.md).

.withBulkOperation(bulkOperation)
.withBulkResponseItem(bulkItemResponse)
.build());
documentErrorsCounter.increment();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for making this change! there was a discussion here on having a condition to skip applying documentErrors only if the document_version_type is set to external. I think that in some cases there can be a true failure when not using external versioning. Can we add that check here?

graytaylor0
graytaylor0 previously approved these changes Apr 15, 2026
…pensearch-project#6601)

Events with invalid document_version values are correctly sent to the DLQ
but were still being added to the OpenSearch bulk request. Added continue
statements after the NumberFormatException and RuntimeException catch blocks
in doOutput() to skip the rest of the loop iteration for failed events.

Made version conflict handling in BulkRetryStrategy conditional on external
versioning. Version conflicts are only silently handled (skipping
documentErrors) when document_version_type is set to external or
external_gte. For internal versioning or when unset, version conflicts are
treated as document errors.

BulkRetryStrategy now accepts a boolean isExternalVersioning parameter
instead of importing VersionType directly, keeping the OpenSearch client
type out of its API. The VersionType check is done in OpenSearchSink via
the isExternalVersionType helper method.

Signed-off-by: Keyur-S-Patel <keyurpatel.opensource@gmail.com>
@Keyur-S-Patel Keyur-S-Patel force-pushed the fix/exclude-version-conflict-from-document-errors-5376 branch from 3754899 to 6d81b89 Compare April 16, 2026 01:17
@oeyh oeyh merged commit 4865dce into opensearch-project:main Apr 18, 2026
93 of 97 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Do not report documentVersionConflictErrors in documentErrors metric in the OpenSearch sink

3 participants