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

deleteDocuments results in: Validation Failed #2046

Closed
pheyse24 opened this issue Dec 17, 2021 · 3 comments
Closed

deleteDocuments results in: Validation Failed #2046

pheyse24 opened this issue Dec 17, 2021 · 3 comments

Comments

@pheyse24
Copy link
Contributor

We are using ES 7.15

Since we updated from Elastica v7.1.1 to v7.1.3 we are experiencing issues when trying to delete via \Elastica\Client::deleteDocuments.

The Documents we provide are gotten fresh from the index via \Elastica\Index::getDocument
Due to the changes in: #1973 the document now contains version Information, which was previously not included.

When deleting the documents via bulk:

Request included previously:
{"delete":{"_id":"66418906","_index":"$MY_INDEX"}}

Now includes:
{"delete":{"_id":"66418906","_index":"$MY_INDEX","version":1}}

This results in:
Validation Failed: 1: internal versioning can not be used for optimistic concurrency control. Please use `if_seq_no` and `if_primary_term` instead;

I am deleting the version from the document as a workaround now - but maybe you can come up with a fix or help me out on how to fix my issue.

@ruflin
Copy link
Owner

ruflin commented Dec 17, 2021

We likely need a patch for this, can't think of an easy workaround. @dsgrillo Could you chime in here as you provided the initial change?

Two paths forward I have in mind:

  • Provide an option for getDocuments to not include the version info
  • Wipe the version info in deleteDocuments.

Second approach seems preferrable as it is in the control of deleteDocuments to send correct requests. But it means touching each document.

@dsgrillo
Copy link
Contributor

The optimistic concurrency control changed between on ES6.7, where they've deprecated the version attributes in favor of if_seq_no and if_primary_term.

I think the second approach of "wiping the version info" is somehow better, as for what I've understood, it's already somewhat implemented (via whitelisting the attributes) - take a look at Elastica\Bulk\Action\DeleteDocument@_getMetadata() (and the other documents types).

I wouldn't blindly remove the version though, but instead swap it for if_seq_no and if_primary_term, as those fields are there to help preventing concurrent modification on documents.

@ruflin
Copy link
Owner

ruflin commented Dec 17, 2021

@dsgrillo @pheyse24 Could one of you open a PR with a potential change?

pheyse24 added a commit to pheyse24/Elastica that referenced this issue Dec 17, 2021
pheyse24 added a commit to pheyse24/Elastica that referenced this issue Dec 17, 2021
deguif added a commit that referenced this issue Dec 18, 2021
* Fixes Issue #2046

* Fixes Issue #2046

* Update CHANGELOG.md

Co-authored-by: François-Xavier de Guillebon <deguif@gmail.com>

Co-authored-by: François-Xavier de Guillebon <deguif@gmail.com>
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

No branches or pull requests

3 participants