Fix upgrade logic to check for major version bump. #8013
Conversation
|
That LGTM can we maybe have a test that has a zipped shard from a 0.20 version that get's upgraded? no need to do this here |
|
Ok, added a test. Thanks for the help with the integration. I also cleaned up the UpgradeTest a bit so the checks could be reused. I still want to add a final validation using the segments api that t he index is completely upgraded. |
| @@ -114,6 +115,7 @@ public static SearchRequest parseSearchRequest(RestRequest request) { | |||
| } | |||
|
|
|||
| searchRequest.types(Strings.splitStringByCommaToArray(request.param("type"))); | |||
| System.out.println("TYPES IN REQUEST: " + StringUtils.join(searchRequest.types(), ", ")); | |||
|
added minor comments - LGTM otherwise |
| import static org.hamcrest.Matchers.is; | ||
|
|
||
| @ElasticsearchIntegrationTest.ClusterScope(scope = ElasticsearchIntegrationTest.Scope.TEST, numDataNodes = 0, minNumDataNodes = 0, maxNumDataNodes = 0) | ||
| public class UpgradeReallyOldIndexTest extends ElasticsearchIntegrationTest { |
There was a problem hiding this comment.
Maybe specify how old in the class name instead of "really old"?
There was a problem hiding this comment.
Does this need scope TEST or SUITE?
There was a problem hiding this comment.
The scope is TEST, so that more tests can be added for "old" indexes. The name is purposefully general, so that any tests for indexes older than can be tested in normal backcompat tests can be tested (assuming this eventually is worked out to a general test class or something).
|
@javanna I think I addressed all your comments. |
|
@s1monw I get random failures sometimes, where flush failed so nothing to upgrade, or an extra flush seems to have happened so that there are no segments to be recovered. Can double check how I'm doing the flush in UpgradeTest? |
rjernst
force-pushed
the
fix/upgrade-logic
branch
from
8889ac9
to
4bcae0c
Compare
rjernst
force-pushed
the
fix/upgrade-logic
branch
from
4bcae0c
to
435b7f0
Compare
|
Ok, test is passing consistently now. |
| @@ -84,59 +88,109 @@ public void testUpgrade() throws Exception { | |||
| } | |||
| indexRandom(true, builder); | |||
| ensureGreen(indexName); | |||
| flushAndRefresh(); | |||
| if (globalCompatibilityVersion().before(Version.V_1_4_0_Beta1)) { | |||
There was a problem hiding this comment.
can you please leave a comment why we did it that way?
|
LGTM left two minor comments but you can squash and push once fixed |
Also improved upgrade tests, and added test against static ES 0.20 index which used Lucene 3.6. closes #8013
Also improved upgrade tests, and added test against static ES 0.20 index which used Lucene 3.6. closes #8013
clintongormley
changed the title
clintongormley
changed the title
Also improved upgrade tests, and added test against static ES 0.20 index which used Lucene 3.6. closes elastic#8013
I had not noticed this since I was only testing using bwc tests, which only go back to ES 1.1 (which is still lucene 4.x).