Skip to content

7.5.0 fails to index documents using low level client. #4289

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

Closed
niemyjski opened this issue Dec 18, 2019 · 2 comments
Closed

7.5.0 fails to index documents using low level client. #4289

niemyjski opened this issue Dec 18, 2019 · 2 comments

Comments

@niemyjski
Copy link
Contributor

niemyjski commented Dec 18, 2019

NEST/Elasticsearch.Net version: 7.5.0

Elasticsearch version: 7.4.2 & 7.5.0

Description of the problem including expected versus actual behavior:
We upgraded our clients this morning from 7.4.2 to 7.5.0 and we had two tests failing. This one occurs after we try to create an error index for documents that failed to index.

Please note this index is created in a previous call:

 var createResponse = await _client.Indices.CreateAsync(workItem.NewIndex + "-error", d => d.Map(md => md.Dynamic(false))).AnyContext();
            
string document = JsonConvert.SerializeObject(new {
                failure.Index,
                failure.Id,
                gr.Version,
                gr.Routing,
                gr.Source,
                failure.Cause,
                failure.Status,
                gr.Found,
            });
            var indexResponse = await _client.LowLevel.IndexAsync<VoidResponse>(workItem.NewIndex + "-error", PostData.String(document));
[405] PUT /employees-v2-error/_doc?pretty=true
 {
   "Cause": {
     "headers": {},
     "root_cause": null
   },
   "Found": true,
   "Id": "5dfa29b22da6ff00851bc366",
   "Index": "employees-v2",
   "Routing": null,
   "Source": {
     "age": 99,
     "companyId": "5dfa29b22da6ff00851bc365",
     "createdUtc": "2019-12-18T13:29:22.4686Z",
     "id": "5dfa29b22da6ff00851bc366",
     "isDeleted": false,
     "lastReview": "0001-01-01T00:00:00",
     "location": "37.0365416105075,-57.4688381317392",
     "nextReview": "0001-01-01T00:00:00+00:00",
     "phoneNumbers": [],
     "updatedUtc": "2019-12-18T13:29:22.4686Z",
     "yearsEmployed": 0
  },  
 "Status": 400,
  "Version": 1
 }
 {
   "error" : "Incorrect HTTP method for uri [/employees-v2-error/_doc?pretty=true] and method [PUT], allowed: [POST]",
   "status" : 405
 }

I tried going through the diff between the two versions but it was massive. I didn't see anything that really stood out of what would have changed in the low level client.

@niemyjski
Copy link
Contributor Author

niemyjski commented Dec 18, 2019

Stepping into this further with the old code this passes (on 7.4.2). Regardless looks like the index isn't being created due to a bad check with IsValid (in our code). I thought this had changed at some point where 404's were valid on exists requests.

            var existsResponse = await _client.Indices.ExistsAsync(errorIndex).AnyContext();
            _logger.LogTraceRequest(existsResponse);
            if (!existsResponse.IsValid || existsResponse.Exists)
                return true;

image

I updated the call to existsResponse.ApiCall.Success && existsResponse.Exists to ensure it's created. The test still fails with the exact same 405 error after ensuring the index exists.

Mpdreamz added a commit that referenced this issue Dec 18, 2019
We used to always send POST which is not right either but the server
accepted it. Now that we have a new format and transform to the old
format in the interim we only picked up PUT for all which is not correct
for index operations without an id.
@Mpdreamz
Copy link
Member

This is indeed a nasty new bug 😢

Opened #4290 to address this.

What happened is that the format for the rest specification files changed and we used to always pick up POST for all the index request. While still not technically okay, Elasticsearch accepted it.

With the switch we started picking up PUT for all which is really not okay so Elasticsearch rightfully starts complaining.

Mpdreamz added a commit that referenced this issue Dec 18, 2019
* Fix #4289 low level client index http method generation wrong

We used to always send POST which is not right either but the server
accepted it. Now that we have a new format and transform to the old
format in the interim we only picked up PUT for all which is not correct
for index operations without an id.

* add unit tests
Mpdreamz added a commit that referenced this issue Dec 18, 2019
* Fix #4289 low level client index http method generation wrong

We used to always send POST which is not right either but the server
accepted it. Now that we have a new format and transform to the old
format in the interim we only picked up PUT for all which is not correct
for index operations without an id.

* add unit tests

(cherry picked from commit 02d4076)
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

2 participants