Fix retry_on_conflict raising errors on document update

[armando1793]

Closes #1432
Closes #1316

[cla-checker-service]

💚 CLA has been signed

[armando1793]

x Author of the following commits did not sign a Contributor Agreement:
b99fa9c

Please, read and sign the above mentioned agreement if you want to contribute to this project

my apologies. I thought I clicked submit but the form already timed out. I already signed the form and downloaded the finished copy. Is there a way to correct this after making the PR?

[sethmlarson]

Thanks for the PR. You're removing version, looking at Elasticsearch source code it looks like this error message is used when seq_no is set. Does this also match what you're seeing?

Potentially the fix should be:

        # Optimistic concurrency control
        if retry_on_conflict in (None, 0) and "seq_no" in self.meta and "primary_term" in self.meta:
            doc_meta["if_seq_no"] = self.meta["seq_no"]
            doc_meta["if_primary_term"] = self.meta["primary_term"]

and you'll need to update the PR with a test case as well.

[armando1793]

Hey @sethmlarson thanks for looking into my PR

The context for me is: I have two separate kafka consumers independently updating the same document. There's a race condition that happens where both consumers get the same document version and update it. When it happens I get the following

WARNING:elasticsearch:POST http://es-url:9200/index_name_v13/index/0000001243/_update?refresh=wait_for&retry_on_conflict=5&version=2 [status:400 request:0.003s]
{"message": "RequestError(400, 'action_request_validation_exception', \"Validation Failed: 1: can't provide both retry_on_conflict and a specific version;\")"}

From the request params and the error message I made a guess that it was because version was in the query the params. So when I override the version by doing the following on my own code, version disappears and the request pushes through

object.meta["version"] = None

INFO:elasticsearch:POST http://es-url:9200/index_name_v13/index/0000001258/_update?refresh=wait_for&retry_on_conflict=5 [status:200 request:0.669s]
{"message": "POS http://es-url:9200/index_name_v13/index/0000001258/_update?refresh=wait_for&retry_on_conflict=5 [status:200 request:0.669s]"}

I don't think changing the if_ meta fields fixes my particular issue. Please let me know what you think!

after looking deeper into the problem I realized that the elasticsearch-py we've been using is 6.x, and that in 7.x version is no longer a param for update. However retry_on_conflict seems to still be unusable for .update in the latest version of elasticsearch-dsl because of an issue similar to #1326. After running tests on my host machine, I think the potential fix you recommend works.

[sethmlarson]

CLA is still falling, did you sign with the same email address attached to your commits? If not it won't go to green.

[sethmlarson]

Also if you're interested in fixing 6.x you could open a PR against 6.x :)

[armando1793]

hey @sethmlarson yeah I'm going to fix that. I misunderstood the contribution doc

Yes I am interested in working on a fix for 6.x. I'll try to get on that when I can

[sethmlarson]

LGTM, thanks so much for this! I added an extra test case for retry_on_conflict=0 or None

[armando1793]

thank you, too, @sethmlarson it was a great learning experience. btw I'm not sure where the best place is to ask this: how do I make a bug fix for 6.x? I don't see a 6.x branch in this repo

[sethmlarson]

@armando1793 Great catch! I pushed a 6.x branch on where the last 6.x release was plus removing support for long-EOL Python versions, now you can create PRs against it :)