Skip to content

Commit aeed2c7

Browse files
authored
Improve exception messaging when encountering legacy version IDs (#20512)
When trying to upgrade an index that was originally created on Elasticsearch, we should throw a human-readable exception saying that it's not supported, rather than a cryptic message like "Version id 7090199 must contain OpenSearch mask". --------- Signed-off-by: Michael Froh <msfroh@apache.org>
1 parent ba6bddf commit aeed2c7

File tree

6 files changed

+99
-21
lines changed

6 files changed

+99
-21
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
3535
- Update streaming flag to use search request context ([#20530](https://github.com/opensearch-project/OpenSearch/pull/20530))
3636
- Move pull-based ingestion classes from experimental to publicAPI ([#20704](https://github.com/opensearch-project/OpenSearch/pull/20704))
3737
- Lazy init stored field reader in SourceLookup ([#20827](https://github.com/opensearch-project/OpenSearch/pull/20827))
38+
* Improved error message when trying to open an index originally created with Elasticsearch on OpenSearch ([#20512](https://github.com/opensearch-project/OpenSearch/pull/20512))
3839

3940
### Fixed
4041
- Relax index template pattern overlap check to use minimum-string heuristic, allowing distinguishable multi-wildcard patterns at the same priority ([#20702](https://github.com/opensearch-project/OpenSearch/pull/20702))

libs/core/src/main/java/org/opensearch/Version.java

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import org.opensearch.common.SuppressForbidden;
3636
import org.opensearch.common.annotation.PublicApi;
3737
import org.opensearch.core.Assertions;
38+
import org.opensearch.core.common.Strings;
3839
import org.opensearch.core.xcontent.ToXContentFragment;
3940
import org.opensearch.core.xcontent.XContentBuilder;
4041

@@ -56,6 +57,37 @@
5657
*/
5758
@PublicApi(since = "1.0.0")
5859
public class Version implements Comparable<Version>, ToXContentFragment {
60+
/**
61+
* Exception thrown when an unsupported version ID is encountered.
62+
*/
63+
public static class UnsupportedVersionException extends RuntimeException {
64+
private final String versionString;
65+
66+
private UnsupportedVersionException(int versionId) {
67+
super(String.format(Locale.ROOT, "Unsupported version [%s]", legacyFriendlyIdToString(versionId)));
68+
this.versionString = legacyFriendlyIdToString(versionId);
69+
}
70+
71+
public String getVersionString() {
72+
return versionString;
73+
}
74+
}
75+
76+
private static String legacyFriendlyIdToString(int versionId) {
77+
String prefix;
78+
if ((versionId & MASK) != 0) {
79+
versionId = versionId ^ MASK;
80+
prefix = "";
81+
} else {
82+
prefix = "ES ";
83+
}
84+
int major = (versionId / MAJOR_SHIFT) % VERSION_SHIFT;
85+
int minor = (versionId / MINOR_SHIFT) % VERSION_SHIFT;
86+
int revision = (versionId / REVISION_SHIFT) % VERSION_SHIFT;
87+
88+
return prefix + major + "." + minor + "." + revision;
89+
}
90+
5991
private static final int VERSION_SHIFT = 100; // Two digits per version part
6092
private static final int REVISION_SHIFT = VERSION_SHIFT;
6193
private static final int MINOR_SHIFT = VERSION_SHIFT * VERSION_SHIFT;
@@ -212,7 +244,7 @@ public class Version implements Comparable<Version>, ToXContentFragment {
212244

213245
public static Version fromId(int id) {
214246
if (id != 0 && (id & MASK) == 0) {
215-
throw new IllegalArgumentException("Version id " + id + " must contain OpenSearch mask");
247+
throw new UnsupportedVersionException(id);
216248
}
217249
final Version known = idToVersion.get(id);
218250
if (known != null) {
@@ -269,7 +301,7 @@ public static Version max(Version version1, Version version2) {
269301
* Returns the version given its string representation, current version if the argument is null or empty
270302
*/
271303
public static Version fromString(String version) {
272-
if (stringHasLength(version) == false) { // TODO replace with Strings.hasLength after refactoring Strings to core lib
304+
if (Strings.hasLength(version) == false) {
273305
return Version.CURRENT;
274306
}
275307
final Version cached = stringToVersion.get(version);
@@ -599,11 +631,4 @@ public static List<Version> getDeclaredVersions(final Class<?> versionClass) {
599631
return versions;
600632
}
601633

602-
/**
603-
* Check that the given String is neither <code>null</code> nor of length 0.
604-
* Note: Will return <code>true</code> for a String that purely consists of whitespace.
605-
*/
606-
public static boolean stringHasLength(String str) {
607-
return (str != null && str.length() > 0);
608-
}
609634
}

libs/core/src/main/java/org/opensearch/semver/SemverRange.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.opensearch.Version;
1212
import org.opensearch.common.Nullable;
1313
import org.opensearch.common.annotation.PublicApi;
14+
import org.opensearch.core.common.Strings;
1415
import org.opensearch.core.xcontent.ToXContentFragment;
1516
import org.opensearch.core.xcontent.XContentBuilder;
1617
import org.opensearch.semver.expr.Caret;
@@ -80,7 +81,7 @@ public static SemverRange fromString(final String range) {
8081

8182
RangeOperator rangeOperator = RangeOperator.fromRange(range);
8283
String version = range.replaceFirst(rangeOperator.asEscapedString(), "");
83-
if (!Version.stringHasLength(version)) {
84+
if (!Strings.hasLength(version)) {
8485
throw new IllegalArgumentException("Version cannot be empty");
8586
}
8687
return new SemverRange(Version.fromString(version), rangeOperator);

server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1302,6 +1302,7 @@ public Version getCreationVersion() {
13021302

13031303
/**
13041304
* Gets the ingestion source.
1305+
*
13051306
* @return ingestion source, or null if ingestion source is not enabled
13061307
*/
13071308
public IngestionSource getIngestionSource() {
@@ -2825,6 +2826,7 @@ public static Settings addHumanReadableSettings(Settings settings) {
28252826
}
28262827

28272828
private static final ToXContent.Params FORMAT_PARAMS;
2829+
28282830
static {
28292831
Map<String, String> params = new HashMap<>(2);
28302832
params.put("binary", "true");
@@ -2838,7 +2840,19 @@ public static Settings addHumanReadableSettings(Settings settings) {
28382840
* This looks for the presence of the {@link Version} object with key {@link IndexMetadata#SETTING_VERSION_CREATED}
28392841
*/
28402842
public static Version indexCreated(final Settings indexSettings) {
2841-
final Version indexVersion = SETTING_INDEX_VERSION_CREATED.get(indexSettings);
2843+
final Version indexVersion;
2844+
try {
2845+
indexVersion = SETTING_INDEX_VERSION_CREATED.get(indexSettings);
2846+
} catch (Version.UnsupportedVersionException e) {
2847+
final String message = String.format(
2848+
Locale.ROOT,
2849+
"index with UUID [%s] created on version [%s] is not supported by version [%s]",
2850+
indexSettings.get(IndexMetadata.SETTING_INDEX_UUID),
2851+
e.getVersionString(),
2852+
Version.CURRENT.toString()
2853+
);
2854+
throw new IllegalArgumentException(message, e);
2855+
}
28422856
if (indexVersion.equals(Version.V_EMPTY)) {
28432857
final String message = String.format(
28442858
Locale.ROOT,
@@ -2888,9 +2902,10 @@ public int getRoutingFactor() {
28882902

28892903
/**
28902904
* Returns the source shard ID to split the given target shard off
2891-
* @param shardId the id of the target shard to split into
2905+
*
2906+
* @param shardId the id of the target shard to split into
28922907
* @param sourceIndexMetadata the source index metadata
2893-
* @param numTargetShards the total number of shards in the target index
2908+
* @param numTargetShards the total number of shards in the target index
28942909
* @return a the source shard ID to split off from
28952910
*/
28962911
public static ShardId selectSplitShard(int shardId, IndexMetadata sourceIndexMetadata, int numTargetShards) {
@@ -2907,9 +2922,10 @@ public static ShardId selectSplitShard(int shardId, IndexMetadata sourceIndexMet
29072922

29082923
/**
29092924
* Returns the source shard ID to clone the given target shard off
2910-
* @param shardId the id of the target shard to clone into
2925+
*
2926+
* @param shardId the id of the target shard to clone into
29112927
* @param sourceIndexMetadata the source index metadata
2912-
* @param numTargetShards the total number of shards in the target index
2928+
* @param numTargetShards the total number of shards in the target index
29132929
* @return a the source shard ID to clone from
29142930
*/
29152931
public static ShardId selectCloneShard(int shardId, IndexMetadata sourceIndexMetadata, int numTargetShards) {
@@ -2954,9 +2970,10 @@ private static void assertSplitMetadata(int numSourceShards, int numTargetShards
29542970

29552971
/**
29562972
* Selects the source shards for a local shard recovery. This might either be a split or a shrink operation.
2957-
* @param shardId the target shard ID to select the source shards for
2973+
*
2974+
* @param shardId the target shard ID to select the source shards for
29582975
* @param sourceIndexMetadata the source metadata
2959-
* @param numTargetShards the number of target shards
2976+
* @param numTargetShards the number of target shards
29602977
*/
29612978
public static Set<ShardId> selectRecoverFromShards(int shardId, IndexMetadata sourceIndexMetadata, int numTargetShards) {
29622979
if (sourceIndexMetadata.getNumberOfShards() > numTargetShards) {
@@ -2970,9 +2987,10 @@ public static Set<ShardId> selectRecoverFromShards(int shardId, IndexMetadata so
29702987

29712988
/**
29722989
* Returns the source shard ids to shrink into the given shard id.
2973-
* @param shardId the id of the target shard to shrink to
2990+
*
2991+
* @param shardId the id of the target shard to shrink to
29742992
* @param sourceIndexMetadata the source index metadata
2975-
* @param numTargetShards the total number of shards in the target index
2993+
* @param numTargetShards the total number of shards in the target index
29762994
* @return a set of shard IDs to shrink into the given shard ID.
29772995
*/
29782996
public static Set<ShardId> selectShrinkShards(int shardId, IndexMetadata sourceIndexMetadata, int numTargetShards) {
@@ -3008,7 +3026,7 @@ public static Set<ShardId> selectShrinkShards(int shardId, IndexMetadata sourceI
30083026
* @param targetNumberOfShards the total number of shards in the target index
30093027
* @return the routing factor for and shrunk index with the given number of target shards.
30103028
* @throws IllegalArgumentException if the number of source shards is less than the number of target shards or if the source shards
3011-
* are not divisible by the number of target shards.
3029+
* are not divisible by the number of target shards.
30123030
*/
30133031
public static int getRoutingFactor(int sourceNumberOfShards, int targetNumberOfShards) {
30143032
final int factor;
@@ -3037,8 +3055,9 @@ public static int getRoutingFactor(int sourceNumberOfShards, int targetNumberOfS
30373055
* E.g.
30383056
* - For ".ds-logs-000002" it will return 2
30393057
* - For "&lt;logs-{now/d}-3&gt;" it'll return 3
3058+
*
30403059
* @throws IllegalArgumentException if the index doesn't contain a "-" separator or if the last token after the separator is not a
3041-
* number
3060+
* number
30423061
*/
30433062
public static int parseIndexNameCounter(String indexName) {
30443063
int numberIndex = indexName.lastIndexOf("-");

server/src/test/java/org/opensearch/VersionTests.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,4 +464,13 @@ public void testUnreleasedVersion() {
464464
Version VERSION_5_1_0_UNRELEASED = Version.fromString("5.1.0");
465465
VersionTests.assertUnknownVersion(VERSION_5_1_0_UNRELEASED);
466466
}
467+
468+
public void testLegacyVersion() {
469+
try {
470+
Version.fromId(7090199);
471+
fail("Legacy version should throw an exception");
472+
} catch (Version.UnsupportedVersionException e) {
473+
assertEquals("ES 7.9.1", e.getVersionString());
474+
}
475+
}
467476
}

server/src/test/java/org/opensearch/cluster/metadata/IndexMetadataTests.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -855,4 +855,27 @@ public void testPrimaryTermsMapDiffRoundTrip() throws IOException {
855855
assertEquals(1, applied.primaryTerm(0));
856856
}
857857
}
858+
859+
public void testLegacyCreatedVersion() {
860+
Index index = new Index("test-index", UUIDs.randomBase64UUID());
861+
final Settings settings = Settings.builder()
862+
.put(IndexMetadata.SETTING_VERSION_CREATED, "7090199")
863+
.put(IndexMetadata.SETTING_INDEX_UUID, index.getUUID())
864+
.build();
865+
try {
866+
IndexMetadata.builder(index.getName())
867+
.settings(settings)
868+
.numberOfShards(1)
869+
.numberOfReplicas(0)
870+
.creationDate(System.currentTimeMillis())
871+
.version(1)
872+
.system(false)
873+
.build();
874+
fail("Should not be able to create index with legacy created version");
875+
} catch (IllegalArgumentException e) {
876+
assertTrue(e.getCause() instanceof Version.UnsupportedVersionException);
877+
Version.UnsupportedVersionException versionException = (Version.UnsupportedVersionException) e.getCause();
878+
assertEquals("ES 7.9.1", versionException.getVersionString());
879+
}
880+
}
858881
}

0 commit comments

Comments
 (0)