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.

Sign up for GitHub

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

Jump to bottom

[Tests] ShardSearchTransportRequestTests.testSerialization fails on 5.x #22808

Closed
cbuescher opened this issue Jan 26, 2017 · 2 comments · Fixed by #22820
Closed

[Tests] ShardSearchTransportRequestTests.testSerialization fails on 5.x #22808

cbuescher opened this issue Jan 26, 2017 · 2 comments · Fixed by #22820
Assignees
@cbuescher
Labels
>test-failure Triaged test failures from CI v5.3.0

Comments

@cbuescher
Copy link
Member

This fails reproducibly on 5.x (5d48757) as seen here:

https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+5.x+multijob-unix-compatibility/os=debian/474/consoleFull

Reproduce line:

gradle :core:test -Dtests.seed=CA62D2D3D29C115F -Dtests.class=org.elasticsearch.search.internal.ShardSearchTransportRequestTests -Dtests.method="testSerialization" -Dtests.security.manager=true -Dtests.locale=nl-BE -Dtests.timezone=Etc/GMT-14
@cbuescher cbuescher added help wanted adoptme >test-failure Triaged test failures from CI v5.3.0 labels Jan 26, 2017
@cbuescher
Copy link
Member Author

Also failing on master (f128b7a):

https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+master+multijob-unix-compatibility/os=centos/557/consoleFull

gradle :core:test -Dtests.seed=A6318C18632167B8 -Dtests.class=org.elasticsearch.search.internal.ShardSearchTransportRequestTests -Dtests.method="testSerialization" -Dtests.security.manager=true -Dtests.locale=is -Dtests.timezone=Pacific/Yap

@cbuescher cbuescher removed the help wanted adoptme label Jan 26, 2017
@cbuescher cbuescher self-assigned this Jan 26, 2017
@cbuescher
Copy link
Member Author

I traced this down to a different cacheKey in the deserialized ShardSearchTransportRequest. The reason seems to be that for the cacheKey computation we use ShardSearchLocalRequest#innerWriteTo which in turn deserialized the SearchSourceBuilder. While debugging I saw differences in the cacheKey bytes array where the source was rendered. Looking closer at the SearchSourceBuilder for this seed I found that the source().collapse().getInnerHit() renders to xContent in different ways in the order of the scriptField keys, because they are interenally stored as an unordered Set. Temporarily changing these to an ordered Set solved the problem for this particular seed.

cbuescher added a commit to cbuescher/elasticsearch that referenced this issue Jan 26, 2017
@cbuescher
Ensure fixed serialization order of InnerHitBuilder
147d059 
Usually the order in which we serialize sets and maps of things doesn't matter,
but since InnerHitBuilder is part of SearchSourceBuilder, which is in turn used
as a cache key in its bytes serialization, we need to ensure the order of all
these fields when writing them to an output stream.

This adds tests and makes sure we iterate over the scriptField set and the
childInnerHits map in a fixed order.

Closes elastic#22808
@cbuescher cbuescher mentioned this issue Jan 26, 2017
Merged
cbuescher added a commit that referenced this issue Jan 30, 2017
@cbuescher
Ensure fixed serialization order of InnerHitBuilder (#22820)
4e61313 
Usually the order in which we serialize sets and maps of things doesn't matter,
but since InnerHitBuilder is part of SearchSourceBuilder, which is in turn used
as a cache key in its bytes serialization, we need to ensure the order of all
these fields when writing them to an output stream.

This adds tests and makes sure we iterate over the scriptField set and the
childInnerHits map in a fixed order.

Closes #22808
cbuescher added a commit that referenced this issue Jan 30, 2017
@cbuescher
Ensure fixed serialization order of InnerHitBuilder (#22820)
1708be2 
Usually the order in which we serialize sets and maps of things doesn't matter,
but since InnerHitBuilder is part of SearchSourceBuilder, which is in turn used
as a cache key in its bytes serialization, we need to ensure the order of all
these fields when writing them to an output stream.

This adds tests and makes sure we iterate over the scriptField set and the
childInnerHits map in a fixed order.

Closes #22808
@cbuescher cbuescher mentioned this issue Feb 1, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cbuescher cbuescher

Labels
>test-failure Triaged test failures from CI v5.3.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Ensure fixed serialization order of InnerHitBuilder cbuescher/elasticsearch
1 participant
@cbuescher