Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix DLQ writer writing empty list #2931

Merged
merged 1 commit into from
Jun 26, 2023

Conversation

kkondaka
Copy link
Collaborator

Description

Fix DLQ writer writing empty list.

DQL writer is writing empty list like this sometimes. Upon investigation, found that bulkOperation is being tried even the operations count is zero. This is resulting in writing empty DLQ List when the bulk operation fails.

Issues Resolved

Check List

  • New functionality includes testing.
  • New functionality has been documented.
    • New functionality has javadoc added
  • [X ] 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.

Signed-off-by: Krishna Kondaka <krishkdk@dev-dsk-krishkdk-2c-bd29c437.us-west-2.amazon.com>
Copy link
Member

@graytaylor0 graytaylor0 left a comment

Choose a reason for hiding this comment

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

Should we have a unit test?

@@ -258,6 +258,10 @@ private void handleFailures(final AccumulatingBulkRequest<BulkOperationWrapper,

private BulkOperationRequestResponse handleRetry(final AccumulatingBulkRequest request, final BulkResponse response, int retryCount) throws InterruptedException {
final AccumulatingBulkRequest<BulkOperationWrapper, BulkRequest> bulkRequestForRetry = createBulkRequestForRetry(request, response);
if (bulkRequestForRetry.getOperationsCount() == 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you give some insight what is this getOperationsCount ?

Copy link
Collaborator Author

@kkondaka kkondaka Jun 26, 2023

Choose a reason for hiding this comment

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

It is the number of operations in the bulk request. If this operation fails (for any reason), we will retry. But if the request has 0 operations, then there is no need to retry and no reason to write to DLQ when we fail to write.

@kkondaka kkondaka merged commit 1dd8bd3 into opensearch-project:main Jun 26, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jul 10, 2023
Signed-off-by: Krishna Kondaka <krishkdk@dev-dsk-krishkdk-2c-bd29c437.us-west-2.amazon.com>
Co-authored-by: Krishna Kondaka <krishkdk@dev-dsk-krishkdk-2c-bd29c437.us-west-2.amazon.com>
(cherry picked from commit 1dd8bd3)
kkondaka added a commit that referenced this pull request Jul 10, 2023
Signed-off-by: Krishna Kondaka <krishkdk@dev-dsk-krishkdk-2c-bd29c437.us-west-2.amazon.com>
Co-authored-by: Krishna Kondaka <krishkdk@dev-dsk-krishkdk-2c-bd29c437.us-west-2.amazon.com>
(cherry picked from commit 1dd8bd3)

Co-authored-by: kkondaka <41027584+kkondaka@users.noreply.github.com>
MaGonzalMayedo pushed a commit to MaGonzalMayedo/data-prepper that referenced this pull request Jul 25, 2023
Signed-off-by: Krishna Kondaka <krishkdk@dev-dsk-krishkdk-2c-bd29c437.us-west-2.amazon.com>
Co-authored-by: Krishna Kondaka <krishkdk@dev-dsk-krishkdk-2c-bd29c437.us-west-2.amazon.com>
Signed-off-by: Marcos Gonzalez Mayedo <alemayed@amazon.com>
@kkondaka kkondaka deleted the s3-dlq-issue branch May 13, 2024 05:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants