fix index refresh in test within 20_mix_typeless_typeful#39198
fix index refresh in test within 20_mix_typeless_typeful#39198talevy merged 11 commits intoelastic:masterfrom
Conversation
the test "Implicitly create a typeless ... typed template"
fails occasionally because the index operation hasn't
propogated to update the index mapping in time for the
following assertion about a dynamically mapped field "bar".
error failed with:
```
field [test-1.mappings.my_type.properties.bar] doesn't have a true value
Expected: not null
but: was null
```
refreshing the index should resolve this timing issue.
|
Pinging @elastic/es-search |
There was a problem hiding this comment.
Thanks @talevy for debugging this! To check I understand, it looks like the dynamic mapping update triggered by the index request isn't visible to the get_mapping call. Does an index refresh also ensure that mapping updates are available? As a side note, I came across this other test PR which ran into a similar issue: #38045
|
thanks for bringing that up Julie. In fact, I see no reason updated to add |
Yep, that is the cause.
It helps, but doesn't enforce it. Rather this is a timing issue with indexing and mapping updates. Mapping update visibility is asynchronous from the client's perspective (see #38873) so this hopefully will help. |
|
@jaymode I updated the code. mind re-reviewing? |
|
Thanks all for looking into this, I had missed that dynamic mapping updates were no longer acked! Since we are now adding the field to the mappings in the template, the refresh is irrelevant I think? Not including the mapping definition in the template was intentional, I wanted to test how Elasticsearch behaves when there is a typed template while an index and mappings are implicitly created via a typeless index call. I'm wondering whether there is something that we can do to force the mapping update to be visible to the client, eg. would running another put-mapping request give this guarantee? |
it does seem to be irrelevant
ah! I should have stuck with my gut. thanks.
I'll take a look at the code. that would obviously give a timing buffer, but I'm not sure whether the |
|
@jpountz The test is interested in how the direct index-mapping and the index-template-mapping or is it specifically interested in dynamic-mapping behavior |
|
ah nevermind. that is exactly what
|
|
I will update to put the same mapping as is done dynamically before getting the mapping. I think that is reasonable since the dynamic-mapping update just issues a regular put-mapping-request itself |
|
I muted the tests until we figure this out |
|
build failed due to: #29880 run elasticsearch-ci/2 |
|
@jpountz do you have any further thoughts? My change is competing with reality, and probably even an empty put-mapping-request would due the trick due to timing, but 🤷♂️ I checked the internal update-mapping-request and it looked like it passed in a type, so that is why the request here uses types |
awaits fix in elastic#39198
awaits fix in elastic#39198
|
I wonder if this should be moved from a rest yml test to an IT? This way we can add an awaitsBusy for the mapping update and not need the hacks of an empty put mapping or anything else. |
|
@jaymode yes. yes it should since there does not seem to be another way of correctly fixing this. I'll update shortly. thanks! |
|
If there is a simple way that we could wait for the mapping update to become visible, this would be my preference. In general I prefer REST tests over IT as I find them more realistic. |
@jpountz can you expand on this? If the IT runs against an external cluster and makes calls using an HTTP client, is it really less realistic? |
|
Apologies, I had assumed you meant a test with an internal cluster. |
|
I just had a quick discussion with @ywelsch and @DaveCTurner who told me that calling the cluster health API with |
|
Just got back to this. thanks for the update Adrien. I missed the notification... updated to use the cluster.health call to block. Much nicer than having to create new rest tests. |
the test "Implicitly create a typeless ... typed template"
fails occasionally because the index operation hasn't
propogated to update the index mapping in time for the
following assertion about a dynamically mapped field "bar".
error failed with:
```
field [test-1.mappings.my_type.properties.bar] doesn't have a true value
Expected: not null
but: was null
```
refreshing the index should resolve this timing issue.
the test "Implicitly create a typeless ... typed template"
fails occasionally because the index operation hasn't
propogated to update the index mapping in time for the
following assertion about a dynamically mapped field "bar".
error failed with:
```
field [test-1.mappings.my_type.properties.bar] doesn't have a true value
Expected: not null
but: was null
```
refreshing the index should resolve this timing issue.
* elastic/master: Add pre-upgrade check to test cluster routing allocation is enabled (elastic#39340) Update logstash-management.json to use typeless template (elastic#38653) Small simplifications to mapping validation. (elastic#39777) Update distribution build instructions to reflect file names with OS/architecture classifiers. (elastic#39762) Give jspawnhelper execute permissions in bundled JDK (elastic#39787) Maintain step order for ILM trace logging (elastic#39522) [ML-DataFrame] fix wire serialization issues in data frame response objects (elastic#39790) fix index refresh in test within 20_mix_typeless_typeful (elastic#39198) Combine overriddenOps and skippedOps in translog (elastic#39771)
…9804) the test "Implicitly create a typeless ... typed template" fails occasionally because the index operation hasn't propogated to update the index mapping in time for the following assertion about a dynamically mapped field "bar". error failed with: ``` field [test-1.mappings.my_type.properties.bar] doesn't have a true value Expected: not null but: was null ``` refreshing the index should resolve this timing issue.
…9803) the test "Implicitly create a typeless ... typed template" fails occasionally because the index operation hasn't propogated to update the index mapping in time for the following assertion about a dynamically mapped field "bar". error failed with: ``` field [test-1.mappings.my_type.properties.bar] doesn't have a true value Expected: not null but: was null ``` refreshing the index should resolve this timing issue.
the test "Implicitly create a typeless ... typed template"
fails occasionally because the index operation hasn't
propogated to update the index mapping in time for the
following assertion about a dynamically mapped field "bar".
error failed with:
refreshing the index should resolve this timing issue.