Skip to content

Commit

Permalink
Include read-only index versions in randomCompatbileVersion/randomCom…
Browse files Browse the repository at this point in the history
…patiblePreviousVersion (elastic#119013)

Read-only versions are now included in all index versions, see elastic#118793 . The next step is to broaden testing where possible to include index versions that cannot be written to. To do that, we change the behaviour of the existing randomCompatbileVersion and randomCompatiblePreviousVersion and methods to include read-only versions and we introduce corresponding variants of these methods that only return write compatible index versions.

As part of this change, we can also remove some of the `@UpdateForV9` annotations that relate to v7 index versions which randomCompatibleVersion no longer covered. That's now fixed and such tests can simply be restored.
  • Loading branch information
javanna authored Dec 19, 2024
1 parent 9cc6cd4 commit 8845cf7
Show file tree
Hide file tree
Showing 25 changed files with 36 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,6 @@ public XContentBuilder getMapping() throws IOException {

@Override
public IndexVersion randomSupportedVersion() {
return IndexVersionUtils.randomCompatibleVersion(random());
return IndexVersionUtils.randomCompatibleWriteVersion(random());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ protected void getGeoShapeMapping(XContentBuilder b) throws IOException {

@Override
protected IndexVersion randomSupportedVersion() {
return IndexVersionUtils.randomCompatibleVersion(random());
return IndexVersionUtils.randomCompatibleWriteVersion(random());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import org.elasticsearch.common.geo.GeometryNormalizer;
import org.elasticsearch.common.geo.GeometryParser;
import org.elasticsearch.common.geo.Orientation;
import org.elasticsearch.core.UpdateForV9;
import org.elasticsearch.geometry.Geometry;
import org.elasticsearch.geometry.GeometryCollection;
import org.elasticsearch.geometry.Line;
Expand Down Expand Up @@ -344,8 +343,6 @@ public void testParsePolygon() throws IOException, ParseException {
assertGeometryEquals(p, polygonGeoJson, false);
}

@UpdateForV9(owner = UpdateForV9.Owner.SEARCH_ANALYTICS)
@AwaitsFix(bugUrl = "this test is using pre 8.0.0 index versions so needs to be removed or updated")
public void testParse3DPolygon() throws IOException, ParseException {
XContentBuilder polygonGeoJson = XContentFactory.jsonBuilder()
.startObject()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import org.elasticsearch.common.geo.GeoPoint;
import org.elasticsearch.common.geo.GeometryNormalizer;
import org.elasticsearch.common.geo.Orientation;
import org.elasticsearch.core.UpdateForV9;
import org.elasticsearch.geometry.Geometry;
import org.elasticsearch.geometry.Line;
import org.elasticsearch.geometry.MultiLine;
Expand Down Expand Up @@ -303,8 +302,6 @@ public void testParseMixedDimensionPolyWithHole() throws IOException, ParseExcep
assertThat(e, hasToString(containsString("coordinate dimensions do not match")));
}

@UpdateForV9(owner = UpdateForV9.Owner.SEARCH_ANALYTICS)
@AwaitsFix(bugUrl = "this test is using pre 8.0.0 index versions so needs to be removed or updated")
public void testParseMixedDimensionPolyWithHoleStoredZ() throws IOException {
List<Coordinate> shellCoordinates = new ArrayList<>();
shellCoordinates.add(new Coordinate(100, 0));
Expand Down Expand Up @@ -338,8 +335,6 @@ public void testParseMixedDimensionPolyWithHoleStoredZ() throws IOException {
assertThat(e, hasToString(containsString("unable to add coordinate to CoordinateBuilder: coordinate dimensions do not match")));
}

@UpdateForV9(owner = UpdateForV9.Owner.SEARCH_ANALYTICS)
@AwaitsFix(bugUrl = "this test is using pre 8.0.0 index versions so needs to be removed or updated")
public void testParsePolyWithStoredZ() throws IOException {
List<Coordinate> shellCoordinates = new ArrayList<>();
shellCoordinates.add(new Coordinate(100, 0, 0));
Expand All @@ -363,8 +358,6 @@ public void testParsePolyWithStoredZ() throws IOException {
assertEquals(shapeBuilder.numDimensions(), 3);
}

@UpdateForV9(owner = UpdateForV9.Owner.SEARCH_ANALYTICS)
@AwaitsFix(bugUrl = "this test is using pre 8.0.0 index versions so needs to be removed or updated")
public void testParseOpenPolygon() throws IOException {
String openPolygon = "POLYGON ((100 5, 100 10, 90 10, 90 5))";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,13 @@
import org.apache.lucene.spatial.prefix.RecursivePrefixTreeStrategy;
import org.apache.lucene.spatial.prefix.tree.GeohashPrefixTree;
import org.apache.lucene.spatial.prefix.tree.QuadPrefixTree;
import org.apache.lucene.tests.util.LuceneTestCase.AwaitsFix;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.geo.GeoUtils;
import org.elasticsearch.common.geo.Orientation;
import org.elasticsearch.common.geo.ShapeRelation;
import org.elasticsearch.common.geo.SpatialStrategy;
import org.elasticsearch.core.CheckedConsumer;
import org.elasticsearch.core.UpdateForV9;
import org.elasticsearch.geometry.Point;
import org.elasticsearch.index.IndexVersion;
import org.elasticsearch.index.IndexVersions;
Expand Down Expand Up @@ -56,8 +54,6 @@
import static org.mockito.Mockito.when;

@SuppressWarnings("deprecation")
@UpdateForV9(owner = UpdateForV9.Owner.SEARCH_ANALYTICS)
@AwaitsFix(bugUrl = "this is testing legacy functionality so can likely be removed in 9.0")
public class LegacyGeoShapeFieldMapperTests extends MapperTestCase {

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@
*/
package org.elasticsearch.legacygeo.mapper;

import org.apache.lucene.tests.util.LuceneTestCase;
import org.elasticsearch.common.geo.SpatialStrategy;
import org.elasticsearch.core.UpdateForV9;
import org.elasticsearch.index.IndexVersion;
import org.elasticsearch.index.IndexVersions;
import org.elasticsearch.index.mapper.FieldTypeTestCase;
Expand All @@ -23,8 +21,6 @@
import java.util.List;
import java.util.Map;

@UpdateForV9(owner = UpdateForV9.Owner.SEARCH_ANALYTICS)
@LuceneTestCase.AwaitsFix(bugUrl = "this is testing legacy functionality so can likely be removed in 9.0")
public class LegacyGeoShapeFieldTypeTests extends FieldTypeTestCase {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ protected boolean forbidPrivateIndexSettings() {
}

public void testCreateCloneIndex() {
IndexVersion version = IndexVersionUtils.randomCompatibleVersion(random());
IndexVersion version = IndexVersionUtils.randomCompatibleWriteVersion(random());
int numPrimaryShards = randomIntBetween(1, 5);
prepareCreate("source").setSettings(
Settings.builder().put(indexSettings()).put("number_of_shards", numPrimaryShards).put("index.version.created", version)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -341,8 +341,8 @@ private static IndexMetadata indexMetadata(final Client client, final String ind
return clusterStateResponse.getState().metadata().index(index);
}

public void testCreateSplitIndex() throws Exception {
IndexVersion version = IndexVersionUtils.randomCompatibleVersion(random());
public void testCreateSplitIndex() {
IndexVersion version = IndexVersionUtils.randomCompatibleWriteVersion(random());
prepareCreate("source").setSettings(
Settings.builder().put(indexSettings()).put("number_of_shards", 1).put("index.version.created", version)
).get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public void testCanRecoverFromStoreWithoutPeerRecoveryRetentionLease() throws Ex
Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.put(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), true)
.put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersionUtils.randomCompatibleVersion(random()))
.put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersionUtils.randomCompatibleWriteVersion(random()))
)
);
ensureGreen(INDEX_NAME);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ protected boolean forbidPrivateIndexSettings() {
return false;
}

private final IndexVersion version = IndexVersionUtils.randomCompatibleVersion(random());
private final IndexVersion version = IndexVersionUtils.randomCompatibleWriteVersion(random());

private IndexRequestBuilder indexCity(String idx, String name, String... latLons) throws Exception {
XContentBuilder source = jsonBuilder().startObject().field("city", name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ protected boolean forbidPrivateIndexSettings() {
return false;
}

private final IndexVersion version = IndexVersionUtils.randomCompatibleVersion(random());
private final IndexVersion version = IndexVersionUtils.randomCompatibleWriteVersion(random());

static Map<String, Integer> expectedDocCountsForGeoHash = null;
static Map<String, Integer> multiValuedExpectedDocCountsForGeoHash = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -748,7 +748,7 @@ public void testDateWithoutOrigin() throws Exception {
}

public void testManyDocsLin() throws Exception {
IndexVersion version = IndexVersionUtils.randomCompatibleVersion(random());
IndexVersion version = IndexVersionUtils.randomCompatibleWriteVersion(random());
Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, version).build();
XContentBuilder xContentBuilder = jsonBuilder().startObject()
.startObject("_doc")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@ public XContentBuilder getMapping() throws IOException {

@Override
public IndexVersion randomSupportedVersion() {
return IndexVersionUtils.randomCompatibleVersion(random());
return IndexVersionUtils.randomCompatibleWriteVersion(random());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ protected boolean forbidPrivateIndexSettings() {

@Before
public void setupTestIndex() throws IOException {
IndexVersion version = IndexVersionUtils.randomCompatibleVersion(random());
IndexVersion version = IndexVersionUtils.randomCompatibleWriteVersion(random());
Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, version).build();
XContentBuilder xContentBuilder = XContentFactory.jsonBuilder()
.startObject()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ protected boolean forbidPrivateIndexSettings() {

@Override
protected void setupSuiteScopeCluster() throws Exception {
IndexVersion version = IndexVersionUtils.randomCompatibleVersion(random());
IndexVersion version = IndexVersionUtils.randomCompatibleWriteVersion(random());
Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, version).build();

assertAcked(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ protected boolean forbidPrivateIndexSettings() {
}

public void testDistanceSortingMVFields() throws Exception {
IndexVersion version = IndexVersionUtils.randomCompatibleVersion(random());
IndexVersion version = IndexVersionUtils.randomCompatibleWriteVersion(random());
Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, version).build();
XContentBuilder xContentBuilder = XContentFactory.jsonBuilder()
.startObject()
Expand Down Expand Up @@ -237,7 +237,7 @@ public void testDistanceSortingMVFields() throws Exception {
// Regression bug:
// https://github.com/elastic/elasticsearch/issues/2851
public void testDistanceSortingWithMissingGeoPoint() throws Exception {
IndexVersion version = IndexVersionUtils.randomCompatibleVersion(random());
IndexVersion version = IndexVersionUtils.randomCompatibleWriteVersion(random());
Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, version).build();
XContentBuilder xContentBuilder = XContentFactory.jsonBuilder()
.startObject()
Expand Down Expand Up @@ -299,7 +299,7 @@ public void testDistanceSortingWithMissingGeoPoint() throws Exception {
}

public void testDistanceSortingNestedFields() throws Exception {
IndexVersion version = IndexVersionUtils.randomCompatibleVersion(random());
IndexVersion version = IndexVersionUtils.randomCompatibleWriteVersion(random());
Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, version).build();
XContentBuilder xContentBuilder = XContentFactory.jsonBuilder()
.startObject()
Expand Down Expand Up @@ -551,7 +551,7 @@ public void testDistanceSortingNestedFields() throws Exception {
* Issue 3073
*/
public void testGeoDistanceFilter() throws IOException {
IndexVersion version = IndexVersionUtils.randomCompatibleVersion(random());
IndexVersion version = IndexVersionUtils.randomCompatibleWriteVersion(random());
Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, version).build();
double lat = 40.720611;
double lon = -73.998776;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public void testManyToManyGeoPoints() throws ExecutionException, InterruptedExce
* |___________________________
* 1 2 3 4 5 6 7
*/
IndexVersion version = randomBoolean() ? IndexVersion.current() : IndexVersionUtils.randomCompatibleVersion(random());
IndexVersion version = randomBoolean() ? IndexVersion.current() : IndexVersionUtils.randomCompatibleWriteVersion(random());
Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, version).build();
assertAcked(prepareCreate("index").setSettings(settings).setMapping(LOCATION_FIELD, "type=geo_point"));
XContentBuilder d1Builder = jsonBuilder();
Expand Down Expand Up @@ -152,7 +152,7 @@ public void testSingeToManyAvgMedian() throws ExecutionException, InterruptedExc
* d1 = (0, 1), (0, 4), (0, 10); so avg. distance is 5, median distance is 4
* d2 = (0, 1), (0, 5), (0, 6); so avg. distance is 4, median distance is 5
*/
IndexVersion version = randomBoolean() ? IndexVersion.current() : IndexVersionUtils.randomCompatibleVersion(random());
IndexVersion version = randomBoolean() ? IndexVersion.current() : IndexVersionUtils.randomCompatibleWriteVersion(random());
Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, version).build();
assertAcked(prepareCreate("index").setSettings(settings).setMapping(LOCATION_FIELD, "type=geo_point"));
XContentBuilder d1Builder = jsonBuilder();
Expand Down Expand Up @@ -225,7 +225,7 @@ public void testManyToManyGeoPointsWithDifferentFormats() throws ExecutionExcept
* |______________________
* 1 2 3 4 5 6
*/
IndexVersion version = randomBoolean() ? IndexVersion.current() : IndexVersionUtils.randomCompatibleVersion(random());
IndexVersion version = randomBoolean() ? IndexVersion.current() : IndexVersionUtils.randomCompatibleWriteVersion(random());
Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, version).build();
assertAcked(prepareCreate("index").setSettings(settings).setMapping(LOCATION_FIELD, "type=geo_point"));
XContentBuilder d1Builder = jsonBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public void testCustomSimilarity() {
.put("index.similarity.my_similarity.after_effect", "l")
.build()
);
service.verifyIndexMetadata(src, IndexVersions.MINIMUM_COMPATIBLE);
service.verifyIndexMetadata(src, IndexVersions.MINIMUM_READONLY_COMPATIBLE);
}

public void testIncompatibleVersion() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ protected void syncRetentionLeases(ShardId id, RetentionLeases leases, ActionLis
public void testTurnOffTranslogRetentionAfterAllShardStarted() throws Exception {
final Settings.Builder settings = Settings.builder().put(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), true);
if (randomBoolean()) {
settings.put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersionUtils.randomCompatibleVersion(random()));
settings.put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersionUtils.randomCompatibleWriteVersion(random()));
}
try (ReplicationGroup group = createGroup(between(1, 2), settings.build())) {
group.startAll();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ protected boolean forbidPrivateIndexSettings() {
}

public void testRestart() throws Exception {
IndexVersion version = IndexVersionUtils.randomPreviousCompatibleVersion(random(), IndexVersion.current());
IndexVersion version = IndexVersionUtils.randomPreviousCompatibleWriteVersion(random(), IndexVersion.current());
// create / delete an index with forbidden setting
prepareCreate("test").setSettings(settings(version).build()).get();
indicesAdmin().prepareDelete("test").get();
Expand All @@ -38,7 +38,7 @@ public void testRestart() throws Exception {
}

public void testRollingRestart() throws Exception {
IndexVersion version = IndexVersionUtils.randomPreviousCompatibleVersion(random(), IndexVersion.current());
IndexVersion version = IndexVersionUtils.randomPreviousCompatibleWriteVersion(random(), IndexVersion.current());
// create / delete an index with forbidden setting
prepareCreate("test").setSettings(settings(version).build()).get();
indicesAdmin().prepareDelete("test").get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,21 @@ public static IndexVersion getNextVersion(IndexVersion version) {

/** Returns a random {@code IndexVersion} that is compatible with {@link IndexVersion#current()} */
public static IndexVersion randomCompatibleVersion(Random random) {
return randomVersionBetween(random, IndexVersions.MINIMUM_READONLY_COMPATIBLE, IndexVersion.current());
}

/** Returns a random {@code IndexVersion} that is compatible with {@link IndexVersion#current()} and can be written to */
public static IndexVersion randomCompatibleWriteVersion(Random random) {
return randomVersionBetween(random, IndexVersions.MINIMUM_COMPATIBLE, IndexVersion.current());
}

/** Returns a random {@code IndexVersion} that is compatible with the previous version to {@code version} */
public static IndexVersion randomPreviousCompatibleVersion(Random random, IndexVersion version) {
return randomVersionBetween(random, IndexVersions.MINIMUM_READONLY_COMPATIBLE, getPreviousVersion(version));
}

/** Returns a random {@code IndexVersion} that is compatible with the previous version to {@code version} and can be written to */
public static IndexVersion randomPreviousCompatibleWriteVersion(Random random, IndexVersion version) {
return randomVersionBetween(random, IndexVersions.MINIMUM_COMPATIBLE, getPreviousVersion(version));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ protected void getGeoShapeMapping(XContentBuilder b) throws IOException {

@Override
protected IndexVersion randomSupportedVersion() {
return IndexVersionUtils.randomCompatibleVersion(random());
return IndexVersionUtils.randomCompatibleWriteVersion(random());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,6 @@ public XContentBuilder getMapping() throws IOException {

@Override
public IndexVersion randomSupportedVersion() {
return IndexVersionUtils.randomCompatibleVersion(random());
return IndexVersionUtils.randomCompatibleWriteVersion(random());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ protected void getGeoShapeMapping(XContentBuilder b) throws IOException {

@Override
protected IndexVersion randomSupportedVersion() {
return IndexVersionUtils.randomCompatibleVersion(random());
return IndexVersionUtils.randomCompatibleWriteVersion(random());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.geo.Orientation;
import org.elasticsearch.core.UpdateForV9;
import org.elasticsearch.geometry.Geometry;
import org.elasticsearch.geometry.utils.GeometryValidator;
import org.elasticsearch.geometry.utils.WellKnownBinary;
Expand Down Expand Up @@ -280,8 +279,6 @@ public void testInvalidCurrentVersion() {
);
}

@UpdateForV9(owner = UpdateForV9.Owner.SEARCH_ANALYTICS)
@AwaitsFix(bugUrl = "this is testing legacy functionality so can likely be removed in 9.0")
public void testGeoShapeLegacyMerge() throws Exception {
IndexVersion version = IndexVersionUtils.randomPreviousCompatibleVersion(random(), IndexVersions.V_8_0_0);
MapperService m = createMapperService(version, fieldMapping(b -> b.field("type", getFieldName())));
Expand Down

0 comments on commit 8845cf7

Please sign in to comment.