Skip to content

Fix serialization bug in SearchableSnapshotsStatsResponse #54864

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

Conversation

tlrx
Copy link
Member

@tlrx tlrx commented Apr 7, 2020

SearchableSnapshotsStatsResponse is missing the serialization of the list of SearchableSnapshotShardStats.

@tlrx tlrx added >bug :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs labels Apr 7, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (:Distributed/Snapshot/Restore)

import static org.elasticsearch.cluster.routing.TestShardRouting.newShardRouting;
import static org.hamcrest.CoreMatchers.equalTo;

public class SearchableSnapshotsStatsResponseTests extends ESTestCase {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't use AbstractBroadcastResponseTestCase here because it relies on XContent parsing. We can't use AbstractWireSerializingTestCase as it relies on object equality, which is not implemented by BroadcastResponse.

@tlrx
Copy link
Member Author

tlrx commented Apr 7, 2020

I'm not sure why CI did not catch this issue before.

@tlrx tlrx requested a review from DaveCTurner April 7, 2020 08:57
Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tlrx tlrx merged commit 0f2c03e into elastic:master Apr 7, 2020
@tlrx tlrx deleted the fix-missing-serialization-in-SearchableSnapshotsStatsResponse branch April 7, 2020 09:10
tlrx added a commit to tlrx/elasticsearch that referenced this pull request Apr 7, 2020
)

SearchableSnapshotsStatsResponse is missing the serialization of 
the list of SearchableSnapshotShardStats
tlrx added a commit that referenced this pull request Apr 7, 2020
This is a backport of #54803 for 7.x.

This pull request cherry picks the squashed commit from #54803 with the additional commits:

    6f50c92 which adjusts master code to 7.x
    a114549 to mute a failing ILM test (#54818)
    48cbca1 and 50186b2 that cleans up and fixes the previous test
    aae12bb that adds a missing feature flag (#54861)
    6f330e3 that adds missing serialization bits (#54864)
    bf72c02 that adjust the version in YAML tests
    a51955f that adds some plumbing for the transport client used in integration tests

Co-authored-by: David Turner <david.turner@elastic.co>
Co-authored-by: Yannick Welsch <yannick@welsch.lu>
Co-authored-by: Lee Hinman <dakrone@users.noreply.github.com>
Co-authored-by: Andrei Dan <andrei.dan@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants