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.

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

Added CCR rolling upgrade tests #36648

Merged
merged 18 commits into from Jan 8, 2019

Conversation

Projects
None yet
5 participants
@martijnvg
Copy link
Member

commented Dec 14, 2018

Tests index following and auto following in a rolling upgrade scenario.

Test are written as Java tests using low level client instead of yaml tests, because CCR needs assertBusy(...) snippets to be tested correctly and that is not possible in yaml tests.

@elasticmachine

This comment has been minimized.

Copy link
Collaborator

commented Dec 14, 2018

@martijnvg

This comment has been minimized.

Copy link
Member Author

commented Dec 14, 2018

This test is currently marked as WIP as it depends on a bug fix: #36647 and
sometime it fails with serialization errors that occur when a 6.6.0-SNAPSHOT node receives a shard changes reponse from a 7.0.0-SNAPSHOT node:

Caused by: java.lang.IllegalArgumentException: No version type match [-1]
        at org.elasticsearch.index.VersionType.fromValue(VersionType.java:368) ~[elasticsearch-7.0.0-SNAPSHOT.jar:7.0.0-SNAPSHOT]
        at org.elasticsearch.index.translog.Translog$Index.<init>(Translog.java:1151) ~[elasticsearch-7.0.0-SNAPSHOT.jar:7.0.0-SNAPSHOT]
        at org.elasticsearch.index.translog.Translog$Index.<init>(Translog.java:1120) ~[elasticsearch-7.0.0-SNAPSHOT.jar:7.0.0-SNAPSHOT]
        at org.elasticsearch.index.translog.Translog$Operation.readOperation(Translog.java:1072) ~[elasticsearch-7.0.0-SNAPSHOT.jar:7.0.0-SNAPSHOT]
        at org.elasticsearch.common.io.stream.StreamInput.readArray(StreamInput.java:734) ~[elasticsearch-7.0.0-SNAPSHOT.jar:7.0.0-SNAPSHOT]
        at org.elasticsearch.xpack.ccr.action.ShardChangesAction$Response.readFrom(ShardChangesAction.java:282) ~[?:?]

I'm not yet sure how this can happen, because the serialization logic in Translog.Index looks okay to me in both master and 6.x branches.

martijnvg added some commits Dec 14, 2018

@martijnvg

This comment has been minimized.

Copy link
Member Author

commented Dec 14, 2018

I've also noticed this serialization error in bulk shards operations api:

[2018-12-14T19:52:59,864][WARN ][o.e.x.c.a.ShardFollowNodeTask] [upgraded-node-0] shard follow task encounter non-retryable error
org.elasticsearch.transport.RemoteTransportException: [node-2][127.0.0.1:34723][indices:data/write/bulk_shard_operations[s]]
Caused by: java.lang.IllegalArgumentException: No version type match [-1]
        at org.elasticsearch.index.VersionType.fromValue(VersionType.java:368) ~[elasticsearch-7.0.0-SNAPSHOT.jar:7.0.0-SNAPSHOT]
        at org.elasticsearch.index.translog.Translog$Index.<init>(Translog.java:1151) ~[elasticsearch-7.0.0-SNAPSHOT.jar:7.0.0-SNAPSHOT]
        at org.elasticsearch.index.translog.Translog$Index.<init>(Translog.java:1120) ~[elasticsearch-7.0.0-SNAPSHOT.jar:7.0.0-SNAPSHOT]
        at org.elasticsearch.index.translog.Translog$Operation.readOperation(Translog.java:1072) ~[elasticsearch-7.0.0-SNAPSHOT.jar:7.0.0-SNAPSHOT]
        at org.elasticsearch.common.io.stream.StreamInput.readCollection(StreamInput.java:959) ~[elasticsearch-7.0.0-SNAPSHOT.jar:7.0.0-SNAPSHOT]
        at org.elasticsearch.common.io.stream.StreamInput.readList(StreamInput.java:941) ~[elasticsearch-7.0.0-SNAPSHOT.jar:7.0.0-SNAPSHOT]
        at org.elasticsearch.xpack.ccr.action.bulk.BulkShardOperationsRequest.readFrom(BulkShardOperationsRequest.java:54) ~[?:?]

@jasontedor jasontedor added v6.7.0 and removed v6.6.0 labels Dec 19, 2018

@dnhatn

This comment has been minimized.

Copy link
Contributor

commented Dec 19, 2018

@martijnvg I have merged #36676 which should fix the translog serialization issue.

@martijnvg martijnvg removed the WIP label Dec 20, 2018

martijnvg added some commits Dec 20, 2018

@martijnvg

This comment has been minimized.

Copy link
Member Author

commented Dec 20, 2018

run the gradle build tests 1

@martijnvg

This comment has been minimized.

Copy link
Member Author

commented Dec 20, 2018

run the gradle build tests 2

martijnvg added some commits Dec 21, 2018

@martijnvg

This comment has been minimized.

Copy link
Member Author

commented Dec 21, 2018

@dnhatn I've update this PR. Do you want to take another look?

martijnvg added some commits Dec 24, 2018

@dnhatn
Copy link
Contributor

left a comment

Thanks @martijnvg. I like these tests - they are easy to follow. I left some minor comments.


private static void putAutoFollowPattern(String patternName) throws IOException {
Request request = new Request("PUT", "/_ccr/auto_follow/" + patternName);
request.setJsonEntity("{\"leader_index_patterns\": [\"logs-*\"], \"remote_cluster\": \"local\"," +

This comment has been minimized.

Copy link
@dnhatn

dnhatn Jan 2, 2019

Contributor

It's easier to follow if we pass the pattern (that's logs-*) as a parameter.

}

private void assertFollowerGlobalCheckpoint(String followerIndex, int expectedFollowerCheckpoint) throws IOException {
Request statsRequest = new Request("GET", "/" + followerIndex + "/_ccr/stats");

This comment has been minimized.

Copy link
@dnhatn

dnhatn Jan 2, 2019

Contributor

I think a seq_no stats of the following index would give us a stronger confidence than the ccr stats.

}
}

private void assertNumberOfSuccessfulFollowedIndices(int expectedNumberOfSuccessfulFollowedIndices) throws IOException {

This comment has been minimized.

Copy link
@dnhatn

dnhatn Jan 2, 2019

Contributor

Maybe remove this method and assert the result of "getNumberOfSuccessfulFollowedIndices"?


pauseFollow("copy-" + leaderIndex2);
closeIndex("copy-" + leaderIndex2);
unfollow("copy-" + leaderIndex2);

This comment has been minimized.

Copy link
@dnhatn

dnhatn Jan 2, 2019

Contributor

Maybe do all pause, close and unfollow in a single method?

case MIXED:
if (SECOND_ROUND == false) {
index(leaderIndex, "2");
assertDocumentExists(leaderIndex, "2");

This comment has been minimized.

Copy link
@dnhatn

dnhatn Jan 2, 2019

Contributor

Maybe assert the existence of both doc1 and doc2? You can extend assertDocumentExists to accept varargs.

martijnvg added some commits Jan 7, 2019

@martijnvg

This comment has been minimized.

Copy link
Member Author

commented Jan 7, 2019

@dnhatn I've updated the PR.

@dnhatn

dnhatn approved these changes Jan 7, 2019

Copy link
Contributor

left a comment

LGTM. Thanks @martijnvg. I left minor nits but feel free to merge once CI is happy.

assertDocumentExists(leaderIndex, "4");
assertBusy(() -> {
assertFollowerGlobalCheckpoint(followerIndex, 3);
assertDocumentExists(followerIndex, "4");

This comment has been minimized.

Copy link
@dnhatn

dnhatn Jan 7, 2019

Contributor

nit: can we also verify doc1, doc2, and doc3?

assertDocumentExists(leaderIndex, "3");
assertBusy(() -> {
assertFollowerGlobalCheckpoint(followerIndex, 2);
assertDocumentExists(followerIndex, "3");

This comment has been minimized.

Copy link
@dnhatn

dnhatn Jan 7, 2019

Contributor

nit: can we also verify doc1, and doc2

martijnvg added some commits Jan 8, 2019

@martijnvg martijnvg merged commit c980cc1 into elastic:master Jan 8, 2019

7 checks passed

CLA Commit author is a member of Elasticsearch
Details
elasticsearch-ci-1 Build finished.
Details
elasticsearch-ci-2 Build finished.
Details
elasticsearch-ci/default-distro Build finished.
Details
elasticsearch-ci/docbldesx Build finished.
Details
elasticsearch-ci/oss-distro-docs Build finished.
Details
elasticsearch-ci/packaging-sample Build finished.
Details
@martijnvg

This comment has been minimized.

Copy link
Member Author

commented Jan 8, 2019

I will backport this later to 6.x, if I didn't encounter build failures related to this change.

martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request Jan 11, 2019

Added CCR rolling upgrade tests (elastic#36648)
Added CCR rolling upgrade tests.

martijnvg added a commit that referenced this pull request Jan 12, 2019

Backported CCR rolling upgrade tests (#37368)
These tests were added via #36648

@colings86 colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.