[BUG] Upsert in bulk might have some issue

[VachaShah]

What is the bug?

@kkondaka reported that they are facing some issue when using the upsert operation in bulk in data-prepper. https://github.com/opensearch-project/data-prepper/blob/main/data-prepper-plugins/opensearch/src/main/java/org/opensearch/dataprepper/plugins/sink/opensearch/OpenSearchSink.java#L312-L328

How can one reproduce the bug?

Steps to reproduce the behavior.

What is the expected behavior?

A clear and concise description of what you expected to happen.

What is your host/environment?

Operating system, version.

Do you have any screenshots?

If applicable, add screenshots to help explain your problem.

Do you have any additional context?

Add any other context about the problem.

[VachaShah]

@kkondaka Can you please add the stack trace here as well? Also, please fill in the required details in the issue like opensearch version. java client version etc.

@reta I see there was a fix for upsert in bulk in one of the previous versions. Any suggestions on why the operation might be failing?

I see that the test testBulkUpdateUpsert is running fine and I tried the code similar to that which works.

[reta]

@reta I see there was a fix for upsert in bulk in one of the previous versions. Any suggestions on why the operation might be failing?

This interesting, I will take a look shortly

[reta]

@VachaShah I think I have an idea why it may fail:

  final UpdateOperation.Builder<Object> updateOperationBuilder = (action.toLowerCase() == OpenSearchBulkActions.UPSERT.toString()) ?
              new UpdateOperation.Builder<>()
                  .index(indexName)
                  .document(filteredJsonNode)
                  .upsert(filteredJsonNode)
                  .versionType(versionType)
                  .version(version) :
              new UpdateOperation.Builder<>()
                  .index(indexName)
                  .document(filteredJsonNode)
                  .versionType(versionType)
                  .version(version);

...

       docId.ifPresent(updateOperationBuilder::id);

For upsert, the _id is required (since otherwise it is not possible to verify if document exists or not) but the last statement indicates that docId is optional and may not be present.

[VachaShah]

I thought so too but @kkondaka mentioned that for them id is always present. @kkondaka Can you please add more details here for the error you are seeing?

[kkondaka]

We have root caused the issue to a bug in our code. The upsert operation is working as expected. And yes, we always have "id" in these cases even though the code has it as optional. This issue can be closed.

[VachaShah]

Great! Thank you @kkondaka for adding the update. Closing this issue.